renz45 / mandrill_mailer

A small gem for sending Mandrill template emails
260 stars 84 forks source link

unintuitive recipient_vars data type #111

Closed nguyen-brian closed 8 years ago

nguyen-brian commented 8 years ago

This is half-question, half-suggestion -- is there any reason why the recipient_vars expects an array where each element is a hash with one key? Why not simply make it a flat hash? Example from the docs:

[{'someone@email.com' => {'INVITEE_NAME' => 'Roger'}}, {'another@email.com' => {'INVITEE_NAME' => 'Tommy'}}]

can be easily simplified to:

{'someone@email.com' => {'INVITEE_NAME' => 'Roger'}, 'another@email.com' => {'INVITEE_NAME' => 'Tommy'}}

renz45 commented 8 years ago

Hi Brian, That's a good question, I believe it has to do with how mandrill expects the data to be formatted. We could abstract that in the gem to allow for the nicer syntax, but at this point it would have to be included in a major version bump, since it would affect a lot of people using it.

Also - I would guess part of the reason they did this was to not be filtering duplicate emails by the nature of how the data is structured (since same hash keys would overwrite each other). It prevents odd bugs in that respect. If you do send duplicate emails, then it's the fault of the user (us) rather then mandrill, which makes things easier to debug. Rather then wondering where one of the emails disappeared to. :)

The data in this gem tries to match the documentation pretty closely for ease of looking up what does what: https://mandrillapp.com/api/docs/messages.JSON.html#method=send-template

nguyen-brian commented 8 years ago

Thanks for the response (and all your hard work on the gem!). I respectfully disagree with you regarding the duplicate emails argument. Looking through the Mandrill API there's nothing to indicate that having an email address show up multiple times in the per-recipient metadata will result in any sort of defined behavior; the examples they have are pretty minimal and only illustrate an email address appearing once. So we're only guessing at their intentions. My guess is that it's an artifact of how you accomplish this using the SMTP method, i.e. using multiple X-MC-Metadata headers, and they didn't put much thought into making it nice for the JSON API.

I respect that such a change would require a major version bump, but I still think it's a valid change that would make peoples' lives easier. Perhaps it's possible to retain some kind of backwards compatibility as well.

renz45 commented 8 years ago

I understand where you're coming from, it would be nice to have a better format, but I'm going to have to veto this. If we start changing how data is handled in this gem and make it different from the official api, we take on the burden of creating a new set of docs and fragmenting existing docs related to interacting with Mandrill. Seeing as how complex the data is for this api, I'm not willing to take that on. This especially since in my current line of work, I rarely if ever, use mandrill anymore (I no longer work on the codeschool.com site which is what handles all our transactional emails). I'm a big fan in general of keeping things compatible with existing documentation.

Originally, I thought that adding a helper class to help with formatting data might be the way to go in order to preserve backwards compatibility, but I'm not sure this gem should be providing much in that respect. Use cases in a per app basis will likely implement their own helpers to format data. The original intent of this gem was to provide an ActionMailer'ish interface for using the Mandril apil, while allowing the sending of test emails with mock data so front enders/designers could test emails with realistic data without contacting a dev.

nguyen-brian commented 8 years ago

No worries, it's a weird quirk of their API, and I totally understand not wanting to A) fragment the documentation, and B) taking on the burden of hiding away their weird idiosyncrasies when it should be Mandrill's job in the first place. Thanks again for the consideration and the thoughtful responses.