Skip to content

TAN-7939 - Make final 4 emails editable #14036

Open
jamesspeake wants to merge 11 commits into
masterfrom
TAN-7939-email-confirmation-as-campaign
Open

TAN-7939 - Make final 4 emails editable #14036
jamesspeake wants to merge 11 commits into
masterfrom
TAN-7939-email-confirmation-as-campaign

Conversation

@jamesspeake

@jamesspeake jamesspeake commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

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).

image image image

Changelog

Changed

  • Made confirmation emails editable & trackable
  • Made password reset email editable & trackable
  • Made user blocked email editable & trackable

@notion-workspace

Copy link
Copy Markdown

@jamesspeake jamesspeake changed the title [TAN-7939] Moved email confirmation into campaign emails TAN-7939 - Make confirmation emails editable Jun 5, 2026
@cl-dev-bot

cl-dev-bot commented Jun 7, 2026

Copy link
Copy Markdown
Collaborator
Messages
📖 Changelog provided 🎉
📖 Notion issue: TAN-7939
📖

Run the e2e tests

📖 Check translation progress

Generated by 🚫 dangerJS against ddbec71

Comment on lines +16 to +17
# Sent synchronously: the user is waiting for this email to reset their password.
reset_password_service.send_email user, token

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.

Not sure why this was being sent asynchronously before

@jamesspeake jamesspeake changed the title TAN-7939 - Make confirmation emails editable TAN-7939 - Make final 4 emails editable Jun 8, 2026
@jamesspeake jamesspeake marked this pull request as ready for review June 8, 2026 12:17

@sebastienhoorens sebastienhoorens left a comment

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.

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!

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.

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

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.

It might be good to use the HTML matchers.

@luucvanderzee luucvanderzee left a comment

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.

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 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants