-
Notifications
You must be signed in to change notification settings - Fork 17
feat(mail): add outgoing email, group email templates, and assignment emails #1439
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
Open
thomasbeaudry
wants to merge
21
commits into
main
Choose a base branch
from
split/mailer
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
a53b04b
feat(mail): add outgoing email, group email templates, and assignment…
thomasbeaudry 068886e
Merge branch 'main' into split/mailer
thomasbeaudry a42d229
test(e2e): locate the assignment submit button by role
thomasbeaudry 0e16f66
fix(mail): validate the SES region before using it as a request host
thomasbeaudry 1d30847
Merge remote-tracking branch 'origin/main' into split/mailer
thomasbeaudry 8a8f7c3
Merge remote-tracking branch 'origin/main' into split/mailer
thomasbeaudry 975c38d
fix(mail): correct isMailEnabled for HTTP transport, pass language th…
thomasbeaudry a37546d
Merge remote-tracking branch 'origin/main' into split/mailer
thomasbeaudry 820c246
fix(web): allowlist mail provider names and extract template tag from…
thomasbeaudry 578f924
refactor(mail): address review feedback on #1439
thomasbeaudry 0eff0ea
fix(web): extract template tag from JSX in GroupEmailTemplates
thomasbeaudry 2b0b688
test(api): update users.controller spec for language pass-through
thomasbeaudry c119677
test(e2e): remove broken form-retention test
thomasbeaudry 49b09be
Merge remote-tracking branch 'origin/split/mailer' into split/mailer
thomasbeaudry 34eed92
Merge remote-tracking branch 'origin/split/mailer' into split/mailer
thomasbeaudry f1ff963
fix(web): carry the new group revision after saving email templates
thomasbeaudry ef40df7
fix(testing): target the delete button on the row under test
thomasbeaudry 8148961
fix(mail): encrypt the SMTP password and address the second review
thomasbeaudry 4d10b20
fix(schemas): drop the type import left behind by moving the template…
thomasbeaudry ee7d589
Merge remote-tracking branch 'origin/main' into split/mailer
thomasbeaudry f511b87
Merge remote-tracking branch 'origin/split/mailer' into split/mailer
thomasbeaudry File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
157 changes: 157 additions & 0 deletions
157
apps/api/src/assignments/__tests__/assignments.controller.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,157 @@ | ||
| import type { RequestUser } from '@douglasneuroinformatics/libnest'; | ||
| import { MockFactory } from '@douglasneuroinformatics/libnest/testing'; | ||
| import type { MockedInstance } from '@douglasneuroinformatics/libnest/testing'; | ||
| import { Test } from '@nestjs/testing'; | ||
| import type { Language } from '@opendatacapture/schemas/core'; | ||
| import { DEFAULT_ASSIGNMENT_EMAIL_TEMPLATE } from '@opendatacapture/schemas/mail'; | ||
| import { beforeEach, describe, expect, it } from 'vitest'; | ||
|
|
||
| import { AuditLogger } from '@/audit/audit.logger'; | ||
| import type { AppAbility } from '@/auth/auth.types'; | ||
| import { GroupsService } from '@/groups/groups.service'; | ||
| import { MailService } from '@/mail/mail.service'; | ||
|
|
||
| import { AssignmentsController } from '../assignments.controller'; | ||
| import { AssignmentsService } from '../assignments.service'; | ||
|
|
||
| const currentUser = { ability: {} as AppAbility, id: 'user-1' } as RequestUser; | ||
|
|
||
| const assignment = { | ||
| expiresAt: new Date('2026-08-01T12:00:00.000Z'), | ||
| groupId: 'group-1', | ||
| id: 'assignment-1', | ||
| url: 'https://gateway.example.org/assignments/abc' | ||
| }; | ||
|
|
||
| const customTemplate = { | ||
| body: { en: 'Custom body {{url}}', fr: 'Corps personnalisé {{url}}' }, | ||
| id: 'tpl-1', | ||
| name: 'Custom', | ||
| subject: { en: 'Custom subject', fr: 'Objet personnalisé' } | ||
| }; | ||
|
|
||
| describe('AssignmentsController', () => { | ||
| let assignmentsController: AssignmentsController; | ||
| let assignmentsService: MockedInstance<AssignmentsService>; | ||
| let auditLogger: MockedInstance<AuditLogger>; | ||
| let groupsService: MockedInstance<GroupsService>; | ||
| let mailService: MockedInstance<MailService>; | ||
|
|
||
| beforeEach(async () => { | ||
| const moduleRef = await Test.createTestingModule({ | ||
| controllers: [AssignmentsController], | ||
| providers: [ | ||
| MockFactory.createForService(AssignmentsService), | ||
| MockFactory.createForService(AuditLogger), | ||
| MockFactory.createForService(GroupsService), | ||
| MockFactory.createForService(MailService) | ||
| ] | ||
| }).compile(); | ||
| assignmentsController = moduleRef.get(AssignmentsController); | ||
| assignmentsService = moduleRef.get(AssignmentsService); | ||
| auditLogger = moduleRef.get(AuditLogger); | ||
| groupsService = moduleRef.get(GroupsService); | ||
| mailService = moduleRef.get(MailService); | ||
| }); | ||
|
|
||
| it('should be defined', () => { | ||
| expect(assignmentsController).toBeDefined(); | ||
| }); | ||
|
|
||
| describe('sendEmail', () => { | ||
| const sendEmail = (body: { language: Language; recipient: string; templateId?: null | string }) => | ||
| assignmentsController.sendEmail('assignment-1', body, currentUser); | ||
|
|
||
| it('uses the built-in default template when the assignment has no group', async () => { | ||
| assignmentsService.findById.mockResolvedValueOnce({ ...assignment, groupId: null }); | ||
| await sendEmail({ language: 'en', recipient: 'p@x.org' }); | ||
| expect(groupsService.findById).not.toHaveBeenCalled(); | ||
| expect(mailService.sendAssignmentEmail.mock.lastCall?.[0]).toMatchObject({ | ||
| body: DEFAULT_ASSIGNMENT_EMAIL_TEMPLATE.body.en, | ||
| recipient: 'p@x.org', | ||
| subject: DEFAULT_ASSIGNMENT_EMAIL_TEMPLATE.subject.en, | ||
| url: `${assignment.url}?lang=en` | ||
| }); | ||
| }); | ||
|
|
||
| it('formats the expiry with a time rather than a bare UTC date', async () => { | ||
| assignmentsService.findById.mockResolvedValueOnce({ ...assignment, groupId: null }); | ||
| await sendEmail({ language: 'en', recipient: 'p@x.org' }); | ||
| const { expiresAt } = mailService.sendAssignmentEmail.mock.lastCall?.[0] as { expiresAt: string }; | ||
| expect(expiresAt).not.toBe('2026-08-01'); | ||
| expect(expiresAt).toMatch(/\d{1,2}:\d{2}/); | ||
| }); | ||
|
|
||
| it("uses the group's active template when no template id is given", async () => { | ||
| assignmentsService.findById.mockResolvedValueOnce(assignment); | ||
| groupsService.findById.mockResolvedValueOnce({ | ||
| activeAssignmentEmailTemplateId: 'tpl-1', | ||
| emailTemplates: [customTemplate] | ||
| }); | ||
| await sendEmail({ language: 'en', recipient: 'p@x.org' }); | ||
| expect(mailService.sendAssignmentEmail.mock.lastCall?.[0]).toMatchObject({ | ||
| body: customTemplate.body.en, | ||
| subject: customTemplate.subject.en | ||
| }); | ||
| }); | ||
|
|
||
| it('uses the explicitly requested template over the active one', async () => { | ||
| assignmentsService.findById.mockResolvedValueOnce(assignment); | ||
| groupsService.findById.mockResolvedValueOnce({ | ||
| activeAssignmentEmailTemplateId: 'tpl-other', | ||
| emailTemplates: [customTemplate] | ||
| }); | ||
| await sendEmail({ language: 'en', recipient: 'p@x.org', templateId: 'tpl-1' }); | ||
| expect(mailService.sendAssignmentEmail.mock.lastCall?.[0]).toMatchObject({ | ||
| body: customTemplate.body.en, | ||
| subject: customTemplate.subject.en | ||
| }); | ||
| }); | ||
|
|
||
| it('uses the default template when the template id is null, even if an active template is set', async () => { | ||
| assignmentsService.findById.mockResolvedValueOnce(assignment); | ||
| groupsService.findById.mockResolvedValueOnce({ | ||
| activeAssignmentEmailTemplateId: 'tpl-1', | ||
| emailTemplates: [customTemplate] | ||
| }); | ||
| await sendEmail({ language: 'en', recipient: 'p@x.org', templateId: null }); | ||
| expect(mailService.sendAssignmentEmail.mock.lastCall?.[0]).toMatchObject({ | ||
| body: DEFAULT_ASSIGNMENT_EMAIL_TEMPLATE.body.en, | ||
| subject: DEFAULT_ASSIGNMENT_EMAIL_TEMPLATE.subject.en | ||
| }); | ||
| }); | ||
|
|
||
| it('sends the French content when language is fr', async () => { | ||
| assignmentsService.findById.mockResolvedValueOnce(assignment); | ||
| groupsService.findById.mockResolvedValueOnce({ | ||
| activeAssignmentEmailTemplateId: 'tpl-1', | ||
| emailTemplates: [customTemplate] | ||
| }); | ||
| await sendEmail({ language: 'fr', recipient: 'p@x.org' }); | ||
| expect(mailService.sendAssignmentEmail.mock.lastCall?.[0]).toMatchObject({ | ||
| body: customTemplate.body.fr, | ||
| subject: customTemplate.subject.fr | ||
| }); | ||
| }); | ||
|
|
||
| it('records an audit log entry against the assignment group', async () => { | ||
| assignmentsService.findById.mockResolvedValueOnce(assignment); | ||
| groupsService.findById.mockResolvedValueOnce({ emailTemplates: [] }); | ||
| await sendEmail({ language: 'en', recipient: 'p@x.org' }); | ||
| expect(auditLogger.log).toHaveBeenCalledWith('SEND_EMAIL', 'ASSIGNMENT', { | ||
| groupId: 'group-1', | ||
| userId: 'user-1' | ||
| }); | ||
| }); | ||
|
|
||
| it('returns the delivery result from the mail service', async () => { | ||
| assignmentsService.findById.mockResolvedValueOnce({ ...assignment, groupId: null }); | ||
| mailService.sendAssignmentEmail.mockResolvedValueOnce({ | ||
| message: 'rendered', | ||
| recipient: 'p@x.org', | ||
| status: 'SENT' | ||
| }); | ||
| await expect(sendEmail({ language: 'en', recipient: 'p@x.org' })).resolves.toMatchObject({ status: 'SENT' }); | ||
| }); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,16 @@ | ||
| import { forwardRef, Module } from '@nestjs/common'; | ||
|
|
||
| import { GatewayModule } from '@/gateway/gateway.module'; | ||
| import { GroupsModule } from '@/groups/groups.module'; | ||
| import { MailModule } from '@/mail/mail.module'; | ||
|
|
||
| import { AssignmentsController } from './assignments.controller'; | ||
| import { AssignmentsService } from './assignments.service'; | ||
|
|
||
| @Module({ | ||
| controllers: [AssignmentsController], | ||
| exports: [AssignmentsService], | ||
| imports: [forwardRef(() => GatewayModule)], | ||
| imports: [forwardRef(() => GatewayModule), GroupsModule, MailModule], | ||
| providers: [AssignmentsService] | ||
| }) | ||
| export class AssignmentsModule {} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| import { ValidationSchema } from '@douglasneuroinformatics/libnest'; | ||
| import { ApiProperty } from '@nestjs/swagger'; | ||
| import type { Language } from '@opendatacapture/schemas/core'; | ||
| import { $SendAssignmentEmailData } from '@opendatacapture/schemas/mail'; | ||
| import type { SendAssignmentEmailData } from '@opendatacapture/schemas/mail'; | ||
|
|
||
| @ValidationSchema($SendAssignmentEmailData) | ||
| export class SendAssignmentEmailDto implements SendAssignmentEmailData { | ||
| @ApiProperty({ description: 'The language to send the email in', enum: ['en', 'fr'] }) | ||
| language: Language; | ||
|
|
||
| @ApiProperty({ description: "The participant's email address" }) | ||
| recipient: string; | ||
|
|
||
| @ApiProperty({ | ||
| description: 'Template id to use; null for the built-in default, omitted to use the group active template', | ||
| required: false | ||
| }) | ||
| templateId?: null | string; | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Two things on this endpoint:
Recipient is unconstrained.
RouteAccess({ action: 'update', subject: 'Assignment' })means any user who can update an assignment can make the instance's SMTP identity deliver to an arbitrary address, unthrottled. The content is template-constrained so it isn't a general open relay, but it is a free "send mail from <institution>" primitive, and the assignment URL it carries is a live credential. A rate limit (per user and per recipient) would be cheap insurance.expiresAtis formatted in UTC.new Date(assignment.expiresAt).toISOString().slice(0, 10)renders the date in UTC, so an assignment expiring at 2026-08-01T02:00Z reads as "expires 2026-08-01" to a recipient in Montréal for whom it already expired on July 31 local. Given the participant acts on this date, formatting in the instance's timezone (or including the time) would avoid off-by-one-day support tickets.