ebizmarts / magento2-mandrill

Mandrill integration for Magento 2.
https://mandrill.com/
36 stars 36 forks source link

Plug into TransportInterfaceFactory instead of Transport #42

Closed Silarn closed 7 years ago

Silarn commented 7 years ago

Fix for #37 and potential fix for #39

In order to send an email properly, the message data model needs to be passed into the constructor for the Transport model. Since neither the transport model or the transport builder provides public access to the saved message object, the only place we can intercept this data is in the Factory class.

This code replaces the old sendMessage plugin with a TransportInterfaceFactory plugin which replaces the standard Transport model with the Mandrill Transport model if Mandrill is enabled and correctly passes the Message model into the constructor from the factory $data parameter.

Silarn commented 7 years ago

Had to fix a typo in the Test

Silarn commented 7 years ago

I guess a 'callable' isn't the same as a \Closure. Hopefully this last commit will fix things.

centerax commented 7 years ago

theres also a coding standard validation on the same file that is failing, minor: https://travis-ci.org/ebizmarts/magento2-mandrill/jobs/256182407#L777

Silarn commented 7 years ago

Odd. I swear I added that newline in a previous commit.

I guess it's impossible to mock a \Closure - but I wanted to test the parameter passed to it. This change should make it work.

Silarn commented 7 years ago

Okay, everything is good now. The test verifies that the function switches between each transport model depending on the isMandrillEnabled check as well as verifying that the message data is being passed to both constructors.