TAN-7939 - Make final 4 emails editable #14036
Conversation
|
| # Sent synchronously: the user is waiting for this email to reset their password. | ||
| reset_password_service.send_email user, token |
There was a problem hiding this comment.
Not sure why this was being sent asynchronously before
sebastienhoorens
left a comment
There was a problem hiding this comment.
Great to have all emails in the emails being emails campaigns now! I have a few comments, but none blocking.
I think there is some cleanup we can do now? For example, there is an ApplicationMailer that these emails were subclasses of: now it can be deleted and merged into the application mailer of the email campaigns?
| confirmation = user.email_confirmation | ||
| confirmation.reset_code! | ||
| EmailConfirmationMailer.with(user: user).send_code.deliver_now | ||
| campaign = EmailCampaigns::Campaigns::EmailConfirmation.first_or_create! |
There was a problem hiding this comment.
There is an "AssureCampaigns" service and rake task (running on each release I think), so I think it would be more consistent to assume the rake task will provide it?
| require 'rails_helper' | ||
|
|
||
| RSpec.describe EmailCampaigns::EmailConfirmationMailer do | ||
| describe 'campaign_mail' do |
There was a problem hiding this comment.
It might be good to use the HTML matchers.
There was a problem hiding this comment.
Went through the code, didn't see anything that looks crazy. Also have to say that I am far from an expert on email campaigns. I did deploy to epic and test if the emails there get sent correctly (w/ mailcatcher): https://7939.epic.govocal.com/en/
All seems to work 👍
Following conversations about making SMS trackable, I thought I'd ask Claude to move the final 4 emails that we did not tackle in the tandem last year, into the email campaigns engine to make them editable and also trackable - so in the future we could expose to admins the delivery status of these emails (if they have been sent etc).
Changelog
Changed