Skip to content

Sends user confirmation email#55

Merged
jbarnsley10 merged 12 commits intomainfrom
feat/DF-330-confirmation-email
Oct 9, 2025
Merged

Sends user confirmation email#55
jbarnsley10 merged 12 commits intomainfrom
feat/DF-330-confirmation-email

Conversation

@jbarnsley10
Copy link
Copy Markdown
Contributor

@jbarnsley10 jbarnsley10 commented Oct 8, 2025

Ticket DF-330

Processes submission message and sends user confirmation (if target email is supplied) in addition to the existing internal submission email

Comment thread src/service/notify.js
export async function sendNotifyEmail(formSubmissionMessage) {
export async function sendNotifyEmails(formSubmissionMessage) {
await sendInternalEmail(formSubmissionMessage)
await sendUserConfirmationEmail(formSubmissionMessage)
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.

pity notify doesn't have an atomic batch operation

Comment thread src/service/notify.js
Copy link
Copy Markdown
Contributor

@whitewaterdesign whitewaterdesign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple of questions, but looks good.

Copy link
Copy Markdown
Contributor

@whitewaterdesign whitewaterdesign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff 👍

Comment thread src/lib/manager.js
*/
export async function getFormMetadata(formId) {
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
if (!managerUrl) {
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.

Why is this eslint-disable-next-line needed? Shouldn't config.get('managerUrl') be string | undefined?

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.

I copied from previous method. Unfortunately the types from config.get() aren't correct (for all config items) so coerced the assignment. Let me know if you're happy with that or if you want me to do something more drastic with the config.get stuff

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.

Thanks for sorting that Jez 🎉

describe('user-confirmation', () => {
test('should handle general email content', () => {
const formName = 'My Form Name'
const submissionDate = new Date(2025, 10, 4, 14, 21, 35) // Novemeber date to prevent issues with BST during testing
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.

I'd prefer the isostring approach and a test for both inside and outside BST

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Oct 9, 2025

@jbarnsley10 jbarnsley10 merged commit 8af48a6 into main Oct 9, 2025
10 checks passed
@jbarnsley10 jbarnsley10 deleted the feat/DF-330-confirmation-email branch October 9, 2025 11:51
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

Successfully merging this pull request may close these issues.

3 participants