Skip to content
This repository was archived by the owner on Dec 3, 2019. It is now read-only.

feature/51-send-email-with-sendgrid-template - #92

Merged
rickr merged 4 commits into
masterfrom
feature/51-send-email-with-sendgrid-template
Jul 23, 2017
Merged

rickr merged 4 commits into
masterfrom
feature/51-send-email-with-sendgrid-template

Conversation

@hpjaj

@hpjaj hpjaj commented Jul 18, 2017 •

Copy link
Copy Markdown
Contributor

Description of changes

Creates a new method for the SendGridClient that sends a transactional email using sendgrid-ruby. This method takes a transactional_template_id and a hash of values that can be substituted into the template.

Issue Resolved

Fixes #51

@hpjaj

hpjaj commented Jul 18, 2017

Copy link
Copy Markdown
Contributor Author

@rickr - I was having issues with the substitutions portion during testing:

"substitutions" => {
"-user_count-" => options[:user_count]
}

We will have to investigate this further.

@hpjaj
hpjaj requested a review from rickr July 18, 2017 03:29
@hpjaj hpjaj changed the title Method to send a transaction email with a template via Sendgrid feature/51-send-email-with-sendgrid-template Jul 18, 2017
@sendgrid.client.mail._("send").post(request_body: mail)
rescue => e
Rails.logger.error "Failed to send email to #{options[:to]} re: #{options[:subject]} due to: #{e}"
Rails.logger.error e.backtrace.join("\n")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we reraise the error here so we know if it fails?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rickr - Done 419f7f8

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we just need a solution for this:

@rickr - I was having issues with the substitutions portion during testing:

"substitutions" => {
"-user_count-" => options[:user_count]
}

We will have to investigate this further.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved the user_count issue: 6d5668d

@hpjaj

hpjaj commented Jul 21, 2017 •

Copy link
Copy Markdown
Contributor Author

@rickr - this one is ready. Can you 'wont fix' the code climate issue'?

Comment thread app/lib/send_grid_client.rb Outdated
],
"subject" => options[:subject],
"substitutions" => {
"-user_count-" => options[:user_count]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this requires a string can you add a .to_s to this line to make sure we always pass a string to the API?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: 4bf02cb

hpjaj added 4 commits July 22, 2017 08:51
I was having issues with the substitutions portion during testing:

"substitutions" => {
  "-user_count-" => options[:user_count]
}

We will have to investigate this further.
The value for user_count needs to be a string, not an integer.
@hpjaj
hpjaj force-pushed the feature/51-send-email-with-sendgrid-template branch from 6d5668d to 4bf02cb Compare July 22, 2017 14:53
@hpjaj

hpjaj commented Jul 22, 2017

Copy link
Copy Markdown
Contributor Author

@rickr - this one is ready

@rickr
rickr merged commit b0e828a into master Jul 23, 2017
@rickr rickr removed the ready label Jul 23, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants