-
Notifications
You must be signed in to change notification settings - Fork 1
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
AQTS230 Save failed emails to MailDeliveryFailure table #2311
Conversation
03a80a4
to
6a0ab77
Compare
fcd2f85
to
ddc005c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few small comments, but otherwise looks good.
app/mailers/application_mailer.rb
Outdated
raise | ||
end | ||
|
||
def notify_email(headers) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this method here? It looks like a convenience method for calling view_mail
with the template ID already provided, but it's not used anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was originally going to add the mailer_action_method and mailer_class to the header but that was done automatically. However It's still used in the application mailer spec on line 15 to set some dummy info.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should have a method that's only used by the tests, I'd suggest the tests can be updated to call view_mail
directly like our other mailers to:
view_mail(GOVUK_NOTIFY_TEMPLATE_ID, to: "[email protected]", subject: "Some subject")
52bce21
to
bc18745
Compare
ticket
MailDeliveryFailure model has been merged into the codebase, this pr will provide the functionality to create entries for the table by recording errors when sending emails.