Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testability of Template Based Email #77

Closed
v-kumar opened this issue Apr 16, 2020 · 10 comments
Closed

Testability of Template Based Email #77

v-kumar opened this issue Apr 16, 2020 · 10 comments

Comments

@v-kumar
Copy link

v-kumar commented Apr 16, 2020

Great gem.

We are migrating a lot of mails from conventional rails mailers to Sendgrid templates.

One thing we are missing is testability. We use personalizations with dynamic templates and there is no clear way to get the final list of recipients.

The gem does the bulk of its work in deliver!. Is there a way the final sendgrid_mail.to_json can be exposed in the delivery? Maybe as additional mail header?

Or may be the preparation of request can be extracted into its own method that an email Observer can use to verify?

def prepare_request(mail)
  sendgrid_mail = Mail.new.tap do |m|
    m.from = to_email(mail.from)
    m.reply_to = to_email(mail.reply_to)
    m.subject = mail.subject || ""
  end

  add_personalizations(sendgrid_mail, mail)
  add_api_key(sendgrid_mail, mail)
  add_content(sendgrid_mail, mail)
  add_send_options(sendgrid_mail, mail)
  add_mail_settings(sendgrid_mail, mail)
  add_tracking_settings(sendgrid_mail, mail)

  sendgrid_mail
end

def deliver!(mail)
  response = perform_send_request(prepare_request(mail))

  settings[:return_response] ? response : self
end
@tyrauber
Copy link
Collaborator

Thanks @v-kumar! Glad the gem is useful for you.

Unless I am misunderstanding something, the email isn't sent until you use mail.deliver!

@mail = mail(to: 'example@email.com',
  subject: 'email subject',
  body: 'email body',
  delivery_method_options: {
    api_key: 'SENDGRID_API_KEY'
  }
)

You should be able to inspect that the email is configured properly by reviewing the mail object, including getting the final list of recipients. @mail.to_json should give you all the attributes that will be sent to sendgrid on .deliver!

But you could also be able to review and cancel sending through an observer:

# In initializer
ActionMailer::Base.register_interceptor(EmailInterceptor)
ActionMailer::Base.register_observer(EmailInterceptor)

# In lib
class EmailInterceptor
  def self.delivering_email(message)
    # log email
    Rails.logger.info message.to_json

   # some conditions to not send the email
    if message.to.nil? || message.to.empty?
      message.perform_deliveries = false
    end
  end

  def self.delivered_email(message)
    message
  end
end

I believe the gem provides the functionality you require as is. If this is not the case, or I am misunderstanding the request, please feel free to reopen the issue. Thanks

@v-kumar
Copy link
Author

v-kumar commented Apr 16, 2020

There is a disconnect between the Mail object and the actual mail recipients because of SendGrid Mail3 API.

personalizations = [
  { 
    to: [{ email: recipient1 }, { email: recipient2 }],
    subject: "Subject Test A"
  }, 
  { 
    to: [{ email: recipient3 }, { email: recipient4 }],
    subject: "Subject Test B"
  }
]
mail(template_id: 'blah', personalizations: personalizations)

In the above case where you completely rely on Mail3 API, mail.to and mail.subject qill be nil while the actual recipients are completely different.

If we need to migrate our tests to verify subject/recipients, there's no way to get to the true recipients/subjects.

@v-kumar
Copy link
Author

v-kumar commented Apr 16, 2020

The way In would like to migrate our existing ActionMailer based tests is to

  1. use VCR to mock Sendgrid Mail3 response
  2. verify envelope using an interceptor. However, my issue is about getting the real envelope that is in sendgrid_mail.to_json vs what is in Mail (or lack of the real envelope details in Mail).

@tyrauber
Copy link
Collaborator

You can inspect the personalizations attribute on mail, no? @mail.pesonalizations mail.to and mail.subject may be nil, but if personalizations were supplied, they will exist on the mail object.

I don't think you want to use VCR because that would record actual requests to the send grid api. You could stub or mock the sendgrid library and capture results. But I think the far easier method would be to just test the mail object.

#pseudo code

let(:mail_queue)  {  ActionMailer::Base.deliveries }
let(:mail) do
  Mail.new()
end
let(:personalizations) do
  [
    {
      to: [
        {email: 'sally1@example.com', subject: 'Hi Sally 1'}
      ]
    }
  ]
it "should send personalizations" do
  expect(mail_queue.length).to eq 2
  expect(mail_queue.first.personalization).to include "has been published" 
  expect(mail_queue.first['personalizations'][0]).to eq(personalizations)
end

But you could always Stub out SendGrid API as outlined in the gems test suite and actually deliver! the messages and inspect the results. A mail intercepter / observer is not required.

@v-kumar
Copy link
Author

v-kumar commented Apr 16, 2020

I am not asking to test how personalizations is set.

I want to test the actual envelope recipients.

If I use both mail.to and personalizations, the actual recipients for each email would be both [mail.to + personalizations.map(&:to)].flaten

My main point is that, the Sendgrid Mail3 API does not fit squarely with Mail + ActionMailer.

That makes testing for mail difficult.

I want to test the whole flow.. that email got delivered to SendGrid. VCR makes it testable.

Between the API and this gem, there is a mismatch between mail.to/mail.subject vs actual mail.to/mail.subject.

i want the gem to expose that final request body (the request_body indeed is the final source of truth here) in a neat way so that we can test it frictionless.

@tyrauber
Copy link
Collaborator

This gem is merely a wrapper to the Sengrid Ruby gem. mail.json is the only request_body the gem has access to as the actual API interaction is done through the Sendgrid Ruby gem.

If you want to see the actual request_body before it goes to the SendGrid API, without a real request, you'd need to stub out the SendGrid ruby gem method that formats the request prior to sending.

If you wanted to send an actual request, with a real sendgrid api_key and valid email address, you can record the response body with VCR on mail.deliver!.

Perhaps I am still misunderstanding, but mail.json is the last request body this gem has before it is sent through the SendGrid ruby client to the SendGrid API.

@v-kumar
Copy link
Author

v-kumar commented Apr 16, 2020

Yep. And that (mail.json) is the only thing I want access to.

This way, I can at least asset if the subjects/recipients were sent properly.

Maybe it can be set as mail.headers['X-Sendgrid-Request'].

Or maybe it can be sent in delivery method response, like this:

settings[:return_response] ? { request: request_body,  response: response } : self

Or maybe the sendgrid_request prep could be a dedicated method, that can be used as mail.delivery_method.sendgrid_request(mail).

Any of the above would help us test envelope information and dynamic_template_data per personalization.

@tyrauber
Copy link
Collaborator

@v-kumar My apologies. I see it now. to_json is not available on mail, because the actual creation of the request payload only happens on deliver! And an interceptor won't work because it completely avoids the deliver! method.

But, I worked up another option. Check out the branch feature/settings_perform_send_request. I added a new mail settings perform_send_request that will disable the api request. When used in combination with return_response = true it will allow you to test the mail object, as it would exist prior to sending to the SendGrid API. Example spec

@v-kumar
Copy link
Author

v-kumar commented Apr 16, 2020

This is much better. Thanks,

You were very responsive throughout the day. Additional thanks for that.

@v-kumar v-kumar closed this as completed Apr 16, 2020
@tyrauber
Copy link
Collaborator

No problem. Sorry for being dense. Would you mind checking out the branch and confirming it will work for your use case? Thanks for working through the issue with me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants