Extend Gem with additional Template and Recipient Functionality #23

yez · 2015-09-11T23:05:07Z

@rbin and anyone else at SendGrid

Wrote RSpec 3 compatible spec suite
Added Travis integration for Continuous Integration
README updated to remove Eddie Zane (He informed me he has left SendGrid)
POROs introduced to ease with Template Calling
- Recipient
  - Responsible for holding user data and optionally keeping track
    of template substitutions.
  - Has ability to add itself to an SMTPAPI
- Template
  - Responsible for keeping track of template_id and managing Recipients
  - Has ability to add itself to an SMTPAPI
- TemplateMailer
  - Responsible for composing requests given a Template and 1 to many Recipients
  - Handles common error cases
  - Invokes the existing Mail and Client objects to handle actual delivery

Two main models: Client and Mail now have specs covering their public api

Spec suite for existing logic

This will add filters to the smtpapi gem: https://github.com/sendgrid/smtpapi-ruby The template_id will be passed in like other attributes to mail Example: ``` mail = SendGrid::Mail.new do |m| m.to = '[email protected]' m.from = '[email protected]' m.subject = 'Hello world!' m.text = 'I heard you like pineapple.' m.template_id = 1234 end ```

Add support for templates

Add travis config for automagic builds

To help encapsulate the substitution logic for templates and how they correlate with email recipients, this model has been introduced. The benefit is now something like this is possible: ```ruby template = Template.new(TEMPLATE_ID) users = User.find(['[email protected]', '[email protected]']) users.each do |user| recipient = Recipient.new(user) recipient.add_substitution(:name, user.name) recipient.add_substitution(:location, user.location) template.add_recipient(recipient) end ``` The next step will be to integrate the `Recipient` and `Template` with `Mail`.

Recipient introduced

Wrong variable names for x-smtpapi header

…-for-recipient Fail on initialization instead of silently on output

This will add an accessor for the template without an initialization through a template_id

Bug with substitution calling in template and change to association

This is the final piece of abstracting recipients, templates and the sending of mail. The use of this would look like: ```ruby users = User.where(email: ['[email protected]', '[email protected]']) recipients = [] users.each do |user| recipient = SendGrid::Recipient.new(user.email) recipient.add_substitution('first_name', user.first_name) recipient.add_substitution('city', user.city) recipients << recipient end template = SendGrid::Template.new('MY_TEMPLATE_ID') client = SendGrid::Client.new(api_user: my_user, api_key: my_key) mail_defaults = { from: '[email protected]', html: '<h1>I like email</h1>', text: 'I like email' subject: 'Email is great', } mailer = TemplateMailer.new(client, template, recipients) mailer.mail(mail_defaults) ```

Template Mailer

mbernier · 2015-09-12T16:52:52Z

Thanks @yez, you rock. We very much appreciate the effort and will work to get this reviewed and merged soon!

yez · 2015-09-29T04:08:04Z

@thinkingserious looks like #24 was recently merged in and this PR is now conflict ridden. Was this PR able to be reviewed at all to see which parts SendGrid would like to include?

thinkingserious · 2015-09-29T14:48:49Z

Hi @yez,

We needed to merge #24 before we tackled your PR.

We are hoping to review this one before the end of the week, thanks again!

With Best Regards,

Elmer

thinkingserious · 2015-09-29T21:56:55Z

Hi @yez,

Now that I've spent some playing with this code and a possible merge with the new code, here are my thoughts:

I think the code that you based this PR on changed significantly enough that I can't easily resolve the merge conflicts.
I was able to get the code working by manually integrating your changes, but it got messy quickly (probably because I'm just getting familiar both with your and Eddie's code) and it feels like we need to rethink how to implement attachments considering the new changes to the library.
I tried another approach that involved no extra classes, just new modules in the Mail Class, but I suspect you had good reason to extract out the template code into it's own class. I'm interested to hear your thoughts on this.

To move forward I suggest either:

You try your hand at submitting a new pull request based on the newly updated library, since you best understand this code.
I can take some bits and pieces of your code with the end result of a working integration with our template engine, but it may not hold true to your original implementation.

Again, we greatly appreciate your contribution and want to figure out the best approach of achieving the goal of getting the template engine functionality added to the library.

Please let me know your thoughts.

Thanks in advance!

yez · 2015-10-01T23:47:11Z

I am going to go ahead and close this PR and then try and salvage the template pieces that will fit with the new code architecture. When that is done, I will open a new PR with the changes.

thinkingserious · 2015-10-02T01:27:04Z

That's awesome, thanks!

Add TROUBLESHOOTING.md

Jake Yesbeck and others added 26 commits August 25, 2015 14:43

Spec suite for existing logic

b691ed6

Two main models: Client and Mail now have specs covering their public api

Merge pull request #1 from apartmentlist/feature/spec-suite

a579727

Spec suite for existing logic

Gitignore update

38a8de1

Merge pull request #2 from apartmentlist/feature/templating

e1b5358

Add support for templates

Add travis config for automagic builds

b86a810

Merge pull request #3 from apartmentlist/chore/add-travis

1e89c82

Add travis config for automagic builds

Merge pull request #4 from apartmentlist/feature/recipient

72f179b

Recipient introduced

Wrong variable names for x-smtpapi header

e6b9ae9

Fail on initialization instead of silently on output

f81cc75

Merge pull request #5 from apartmentlist/bug/actual-template-calling

9b0c975

Wrong variable names for x-smtpapi header

Merge pull request #6 from apartmentlist/feature/update-failure-logic…

5f4e852

…-for-recipient Fail on initialization instead of silently on output

Bug with substitution calling in template and change to association

1e19538

This will add an accessor for the template without an initialization through a template_id

Merge pull request #7 from apartmentlist/bug/actual-template-calling

eeedd4d

Bug with substitution calling in template and change to association

Spec fixes

e555986

Make sure substitutions are not constantly overwritten

6bc1d11

Make :files computed entirely from a method

8aec056

Explanitory comment about payload mapping

93dfe8a

Explanitory comment about how to use template mailer

6fd9c7b

Remove redundant empty check

f7a26cc

Merge pull request #8 from apartmentlist/feature/template-mailer

3fe5c0c

Template Mailer

Update README to remove employee who no longer works at SendGrid

bb6dc0e

Update README with new Template pattern

8e51c3e

Make payload varname mapping explicit

15ce2f5

thinkingserious mentioned this pull request Sep 29, 2015

Template Engine support #21

Closed

yez closed this Oct 1, 2015

yez mentioned this pull request Oct 13, 2015

Extend Gem with additional Template and Recipient Functionality (round 2) #28

Merged

jamietanna pushed a commit to jamietanna/sendgrid-ruby that referenced this pull request Oct 13, 2018

Merge pull request sendgrid#23 from colto/chore/troubleshooting

7a291dc

Add TROUBLESHOOTING.md

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend Gem with additional Template and Recipient Functionality #23

Extend Gem with additional Template and Recipient Functionality #23

yez commented Sep 11, 2015

mbernier commented Sep 12, 2015

yez commented Sep 29, 2015

thinkingserious commented Sep 29, 2015

thinkingserious commented Sep 29, 2015

yez commented Oct 1, 2015

thinkingserious commented Oct 2, 2015

Extend Gem with additional Template and Recipient Functionality #23

Extend Gem with additional Template and Recipient Functionality #23

Conversation

yez commented Sep 11, 2015

mbernier commented Sep 12, 2015

yez commented Sep 29, 2015

thinkingserious commented Sep 29, 2015

thinkingserious commented Sep 29, 2015

yez commented Oct 1, 2015

thinkingserious commented Oct 2, 2015