kshnurov / mandrill_dm

A basic Mandrill delivery method for Rails.
MIT License
46 stars 45 forks source link

Mandrill templates #26

Closed genaromadrid closed 8 years ago

genaromadrid commented 8 years ago

@spovich, now everything is ready. Let me know if you see something that needs to be changed.

I leave you the link of mandrill templates again for reference.

https://mandrillapp.com/api/docs/messages.JSON.html#method=send-template

Cheers

genaromadrid commented 8 years ago

Well, something weird is happening, in my travis the tests are passing https://travis-ci.org/genaromadrid/mandrill_dm/builds/104152311

spovich commented 8 years ago

@genaromadrid you will need to update the upstream location in your .git/config for your repo, then rebase things. Also, it would be good to squash this down to only a few commits.

genaromadrid commented 8 years ago

@spovich, @kshnurov i update the PR with your comments, squash it down to a fewer commits and use the mandrill syntax for template_content.

Let me know it's ok

kshnurov commented 8 years ago

@genaromadrid I don't see a big difference, you're still building something in template_content instead of passing it as it is. Please stick with 1 simple deliver! method inside delivery_method.rb.

genaromadrid commented 8 years ago

I build the template_content because it's the only way to get the contents from a rails view. Please see my Readme: If you put a :html in the content param, the contents of the view get inserted in the content. For what I have find, there is no easy way to fetch the contents of the rails view in the mailer.

genaromadrid commented 8 years ago

@kshnurov about the deliver! method, you're right... I'll put the logic of template_content in messages.rb

kshnurov commented 8 years ago

@genaromadrid

I build the template_content because it's the only way to get the contents from a rails view

It's not. Use render_to_string

genaromadrid commented 8 years ago

@spovich oh, of course, i'm sorry, i got confused with the gem mandrill_mailer which does not extend from ActionMailer. But anyways...

If i do everything in the deliver! method rubocop cries because the method has too many lines. I just made a few commits. Let me know what you think.

Cheers

kshnurov commented 8 years ago

@genaromadrid please keep one deliver! method.

genaromadrid commented 8 years ago

@kshnurov if i leave one deliver! method, Rubocop cries with:

Here is the build: https://travis-ci.org/genaromadrid/mandrill_dm/jobs/106512208

kshnurov commented 8 years ago

@genaromadrid just move @response assignment inside if/else for the first one and add # rubocop:disable Metrics/MethodLength after method name to ignore the second one.

genaromadrid commented 8 years ago

@kshnurov perfect, i also had to add # rubocop:disable Metrics/AbcSize and fix some rubocops that were failing.

kshnurov commented 8 years ago

@genaromadrid good to go! Thank you and sorry for the delay.

genaromadrid commented 8 years ago

Thanks @kshnurov, it was nice to collaborate with you guys. Let me know if i can help with anything!

Cheers