DigitalState / Platform-Communication-Bundle

OroPlatform bundle that provide multi-channel/Omni-Channel Message generation with query builder: SMS, Email, Inbox, IVR, Twitter, Facebook, etc -- DEVELOPMENT REPO
Other
3 stars 4 forks source link

Communication Batch Processing & personalization Improvements #7

Open hb7777 opened 7 years ago

hb7777 commented 7 years ago

I've modified the transport send() method to receive a collection of message objects. The reason is to minimize the number of calls made to the third-party SDK, since most SDK mailers allow batch processing (passing multiple users list with a common template with placeholders to be substituted at the SDK location for a personalized message.)

To that end.

`The Ds\Bundle\CommunicationBundle\Channel\Channel interface send method was changed to
/**
 * Send message
 *
 * @param \Ds\Bundle\TransportBundle\Collection\MessageCollection $messageCollection
 * @return \Ds\Bundle\CommunicationBundle\Channel\Channel
 */
public function send(MessageCollection $messageCollection);

The Ds\Bundle\TransportBundle\Transport\Transport interface send method was changed to 
/**
 * Send a message
 *
 * @param \Ds\Bundle\TransportBundle\Collection\MessageCollection $messageCollection
 * @param \Ds\Bundle\TransportBundle\Entity\Profile $profile
 * @param \Ds\Bundle\CommunicationBundle\Entity\Template $template
           I've added this param, since all messageCollection will use the same template. Passing the template in the
           messageCollection will consume memory for nothing, the template will be identical for each message
 * @return \Ds\Bundle\TransportBundle\Transport\Transport
 * @throws \LogicException
 * @throws \UnexpectedValueException
 */
public function send(MessageCollection $messageCollection, Profile $profile = null, Template $template = null);`

All classes implementing those interfaces were changed.

Next the communicationBundle Manager classes were changed to passed a collection of message and a template.

The class Ds\Bundle\TransportBundle\Model\Message was modified to add the property field data, of type json_array, to contain user specific data from the Oro\Bundle\UserBundle\Entity\User entity not passed once in the transport classes, that could be useful for a personalized message.

This is an example of the a json structure to be transformed into a specific SDK personalization structure. Each third-party have their own format structure, so this should tie to an sdk transport bundle.

 `{
   "userData": {"field":"firstName,middleName,lastName" },
   "merge_vars":{
         "rcpt":"userData.field.email",
         "vars":[
               {"name":"firstName","content":"userData.field.firstName" },
               {"name":"middleName","content":"userData.field.middleName" },
               {"name":"lastName","content":"userData.field.lastName"}
         ]
   }}`

Third-Party API data transformation from the json above

`[merge_vars] => Array
(
      [rcpt] => jcostello@ville.montreal.qc.ca
      [vars] => Array
      (
           [0] => Array  
           (
                [name] => firstName
                [content] => John
           )
           [1] => Array
           (
                [name] => middleName
                [content] => P
           )
           [2] => Array
           (
                [name] => lastName
                [content] => Costello
           )
      )
)`

The transformation of the json into the SDK specific array structure was done in the communicationManager class. This should be done in the SDK transport bundle since it specific to that transport, but to do that the Oro\Bundle\UserBundle\Entity\User entity need to be part of the Ds\Bundle\TransportBundle\Model\Message entity.

Created the class Ds\Bundle\TransportBundle\Collection\MessageCollection that extends ArrayCollection to contain all Ds\Bundle\TransportBundle\Model\Message classes passed to the transport specific class.

`namespace Ds\Bundle\TransportBundle\Collection;
 use Doctrine\Common\Collections\ArrayCollection;

 class MessageCollection extends ArrayCollection
 {

 }`
sadortun commented 7 years ago

@hb7777 Had you look at PR #4, i already some big upgrades to the bundle in there.

  1. Queue is handled by a Command
  2. I was thinking to have the send() return a Promise() so if the long running operation could complete asynchronously.
  3. You need to be able to handle results for each sent() message, which failed ? or succeed ?
  4. This will still cause an issue, when not all messages can be sent at once due to ressources limitation, or process timeout. (example, you send 50 000 messages) 4.1. You need to keep track of which messages get delivered, otherwise you could send a message twice, or not at all
hb7777 commented 7 years ago

@sadortun

The point 3. You need to be able to handle results for each sent() message, which failed ? or succeed ?

Is crucial, but from my test with the Mandrill mailer, we do get back, a detail returned object, detailing for each user sent to them the status of the mail (sent, queued, rejected, etc..) and also a unique ID to do follow up inquiries like (mail was open, etc..), The batch processing only works if we can track the outcome per individual user.

hb7777 commented 7 years ago

@sadortun I had a look at PR #4 seems interesting.

hb7777 commented 7 years ago

@sadortun was personalization when sending (email, sms) also addressed in that PR ?

sadortun commented 7 years ago

@hb7777 yes, but since SMS are not in our roadmap for a few more months, we have not made a sms transport, but it should be fairly easy to add. We currently support 12 providers in a other project, and we can reuse the code here

This is our current Email.sparkpost implementation

https://github.com/SDDProductions/SDDSparkPostTransportBundle

sadortun commented 7 years ago

@hb7777 there is a few weeks of work around this PR, there is a few internal improvement to do, but it otherwise production ready.

I would be happy if you could do a code review, I am sure there is a few things to improve. Currently it working well in this state, but there is many pending improvements that will come in a few weeks since we currently have more immediare priorities.