Support send template

[norbertszivos]

Hi @spovich @jlberglund @kshnurov,

I created a support for send template, because we would like to use it. It looks like somebody was faster than me, but I did this a little bit different way. And I would like to start a conversation about how can we support the template_name template_content async ip_pool send_at. I think to store these things in the Message class will not be the right place, but it would be great that I can setup it when I call the mail method. That's why I created a new Template class for it. But how about that rename it and call SendOptions or something similar and there we can reach these things.

And I was thinking about the async setup as well. So my problem the present setup that I can choose, it will send all the emails via async or not, but it would be better if I can choose these emails will send with async and those ones with normal mode in the mail method. Maybe I'm not right, but we could discuss it.

[kshnurov]

Hi @Sysqa, thanks for the PR!

Is there any supposed functionality for Template class? If not (I don't see any) - there's no need for it and 2-3 simple methods inside Message would be more than enough.

Also I don't like 3 additional methods (mandrill_deliver, send_template and send), looks very ugly for me - it's not saving any lines, adds extra complexity and ambiguity (can you say what's the difference between mandrill_deliver and send without looking into the code?). I think it's better to leave everything inside deliver! method.

If we add template?, template_name and template_content methods, the resulting code can look like this:

def deliver!(mail)
  mandrill_api = Mandrill::API.new(MandrillDm.configuration.api_key)
  message = Message.new(mail)

  @response = if message.template?
    mandrill_api.messages.send_template(
      message.template_name,
      message.template_content,
      message.to_json,
      MandrillDm.configuration.async
    )
  else
    mandrill_api.messages.send(
      message.to_json,
      MandrillDm.configuration.async
    )
  end
end

[norbertszivos]

Hi @kshnurov,

I tried to explain above why I'm using a different class and asking your idea about that.

Basically I can agree with your idea that leave inside everything in the deliver method, but the rubocop always complaining: the row is too long, the condition is too deep, the method is too complex. So that's why this 'ugly' solution. Actually what is the main rule that I should follow?

[spovich]

@Sysqa I agree with @kshnurov I'm not opposed to selectively disabling rubocop rules where it makes sense.

[norbertszivos]

right good to know, no problem

[kshnurov]

@Sysqa, once again, is there any supposed functionality for Template class? My idea is that a class that does nothing shouldn't exist.

[norbertszivos]

@kshnurov sure I'll try to explain the idea. If you check the madrill api how to look like the api request then you can see something like this:

    "template_name": "example template_name",
    "template_content": [
        {
            "name": "example name",
            "content": "example content"
        }
    ],
    "message": { ... }
    "async": false,
    "ip_pool": "Main Pool",
    "send_at": "example send_at"

So we implemented the message part of this thing. I thought that the template things are not belong to the message class. As I suggested or actually asked your idea that how about we will create a new class (or rename my template class) call it sendoptions or something like that where we can store, handle the template_name template_content async ip_pool send_at. Basically that is why I asked before do something else, because I do not want to do unnecessary work if you will not use it. I hope this a better answer your question.

[kshnurov]

@Sysqa oh, now I got it. We already have a Configuration class, we can use it. It'll allow to add per-mailer and per-mail configuration.

[norbertszivos]

@kshnurov right Could you explain how? I'm not sure that everything is clear about it.

[kshnurov]

@Sysqa it's hidden inside mandrill_dm.rb https://github.com/spovich/mandrill_dm/blob/master/lib/mandrill_dm.rb Put it into a separate file, add everything as you want and send a pull.

[norbertszivos]

@kshnurov that's ok I found it. What is not clear that how can I use different config with different emails?

[kshnurov]

The same way you're doing it with template_name/template_content. Parse what's passed to mail() and use it to initialize Configure class inside deliver! (instead of initializing it once in mandrill_dm.rb).

сб, 30 янв. 2016 г. в 23:36, Sysqa notifications@github.com:

@kshnurov https://github.com/kshnurov that's ok I found it. What is not clear that how can I use different config with different emails?

— Reply to this email directly or view it on GitHub https://github.com/spovich/mandrill_dm/pull/30#issuecomment-177295460.

Шнуров Кирилл

[norbertszivos]

@kshnurov gotcha, but it's a bit weird for me...

[kshnurov]

Templates support added in https://github.com/spovich/mandrill_dm/pull/26