From eab75adbef9cc5a7fb9d0a14b6b13b72e8c297c4 Mon Sep 17 00:00:00 2001 From: Liam Lloyd Date: Tue, 13 Jun 2023 17:59:55 -0700 Subject: [PATCH] Send notification emails to archive stewards We want to send notification emails to users when they become archive stewards. This commit updates the create and update endpoints of directives to send such emails. --- src/directive/queries/update_directive.sql | 12 +- src/directive/service/create.test.ts | 39 ++++ src/directive/service/create.ts | 7 + src/directive/service/update.test.ts | 80 ++++++++ src/directive/service/update.ts | 33 ++-- src/email/index.ts | 7 +- .../get_archive_stewardship_details.sql | 25 +++ src/email/service.test.ts | 172 ++++++++++++++++-- src/email/service.ts | 88 ++++++--- src/fixtures/create_test_profile_items.sql | 5 + src/legacy_contact/service/update.ts | 17 +- 11 files changed, 421 insertions(+), 64 deletions(-) create mode 100644 src/email/queries/get_archive_stewardship_details.sql create mode 100644 src/fixtures/create_test_profile_items.sql diff --git a/src/directive/queries/update_directive.sql b/src/directive/queries/update_directive.sql index 4e3299b1..2edd9170 100644 --- a/src/directive/queries/update_directive.sql +++ b/src/directive/queries/update_directive.sql @@ -1,3 +1,11 @@ +WITH original_directive AS ( + SELECT + steward_account_id + FROM + directive + WHERE + directive_id = :directiveId +) UPDATE directive SET @@ -37,4 +45,6 @@ RETURNING accountId = steward_account_id ) "steward", note, - execution_dt "executionDt"; + execution_dt "executionDt", + steward_account_id != (SELECT steward_account_id FROM original_directive) "stewardChanged"; + diff --git a/src/directive/service/create.test.ts b/src/directive/service/create.test.ts index 5ef90aa3..c65e2dc2 100644 --- a/src/directive/service/create.test.ts +++ b/src/directive/service/create.test.ts @@ -1,9 +1,19 @@ import { NotFound, BadRequest, InternalServerError } from "http-errors"; import { db } from "../../database"; +import { sendArchiveStewardNotification } from "../../email"; +import { logger } from "../../log"; import { directiveService } from "./index"; import type { Directive, DirectiveTrigger } from "../model"; jest.mock("../../database"); +jest.mock("../../email", () => ({ + sendArchiveStewardNotification: jest.fn(), +})); +jest.mock("../../log", () => ({ + logger: { + error: jest.fn(), + }, +})); const loadFixtures = async (): Promise => { await db.sql("fixtures.create_test_accounts"); @@ -24,9 +34,15 @@ describe("createDirective", () => { }); afterEach(async () => { await clearDatabase(); + jest.clearAllMocks(); }); test("should successfully create a directive and trigger", async () => { + ( + sendArchiveStewardNotification as jest.MockedFunction< + typeof sendArchiveStewardNotification + > + ).mockResolvedValueOnce(undefined); await directiveService.createDirective({ emailFromAuthToken: "test@permanent.org", archiveId: "1", @@ -69,6 +85,29 @@ describe("createDirective", () => { { directiveId: directiveResult.rows[0]?.directiveId } ); expect(triggerResult.rows.length).toBe(1); + expect(sendArchiveStewardNotification).toHaveBeenCalledWith( + directiveResult.rows[0]?.directiveId + ); + }); + + test("should log error if notification email fails", async () => { + const testError = new Error("out of cheese error - redo from start"); + ( + sendArchiveStewardNotification as jest.MockedFunction< + typeof sendArchiveStewardNotification + > + ).mockRejectedValueOnce(testError); + await directiveService.createDirective({ + emailFromAuthToken: "test@permanent.org", + archiveId: "1", + stewardEmail: "test@permanent.org", + type: "transfer", + trigger: { + type: "admin", + }, + }); + + expect(logger.error).toHaveBeenCalledWith(testError); }); test("should error if authenticated account doesn't own the archive", async () => { diff --git a/src/directive/service/create.ts b/src/directive/service/create.ts index f0ff36bd..596596e5 100644 --- a/src/directive/service/create.ts +++ b/src/directive/service/create.ts @@ -10,6 +10,7 @@ import { isMissingStewardAccountError, getInvalidValueFromInvalidEnumMessage, } from "../../database_util"; +import { sendArchiveStewardNotification } from "../../email"; import { logger } from "../../log"; import { confirmArchiveOwnership } from "./utils"; @@ -80,5 +81,11 @@ export const createDirective = async ( return directive; }); + await sendArchiveStewardNotification(directiveToReturn.directiveId).catch( + (err) => { + logger.error(err); + } + ); + return directiveToReturn; }; diff --git a/src/directive/service/update.test.ts b/src/directive/service/update.test.ts index b8edbf56..73f8047a 100644 --- a/src/directive/service/update.test.ts +++ b/src/directive/service/update.test.ts @@ -1,9 +1,19 @@ import { NotFound, BadRequest, InternalServerError } from "http-errors"; import { db } from "../../database"; +import { sendArchiveStewardNotification } from "../../email"; +import { logger } from "../../log"; import { directiveService } from "./index"; import type { Directive, DirectiveTrigger } from "../model"; jest.mock("../../database"); +jest.mock("../../email", () => ({ + sendArchiveStewardNotification: jest.fn(), +})); +jest.mock("../../log", () => ({ + logger: { + error: jest.fn(), + }, +})); const testDirectiveId = "39b2a5fa-3508-4030-91b6-21dc6ec7a1ab"; const testNote = "test note"; @@ -29,9 +39,15 @@ describe("updateDirective", () => { }); afterEach(async () => { await clearDatabase(); + jest.clearAllMocks(); }); test("should successfully update steward account and note", async () => { + ( + sendArchiveStewardNotification as jest.MockedFunction< + typeof sendArchiveStewardNotification + > + ).mockResolvedValueOnce(undefined); await directiveService.updateDirective(testDirectiveId, { emailFromAuthToken: "test@permanent.org", stewardEmail: "test+2@permanent.org", @@ -88,6 +104,70 @@ describe("updateDirective", () => { ); expect(triggerResult.rows.length).toBe(1); expect(triggerResult.rows[0]?.type).toBe("admin"); + expect(sendArchiveStewardNotification).toHaveBeenCalledWith( + testDirectiveId + ); + }); + + test("should successfully update and note only", async () => { + await directiveService.updateDirective(testDirectiveId, { + emailFromAuthToken: "test@permanent.org", + note: testNote, + }); + + const directiveResult = await db.query( + `SELECT + directive_id "directiveId", + archive_id "archiveId", + type, + created_dt "createdDt", + updated_dt "updatedDt", + ( + SELECT + jsonb_build_object( + 'email', + primaryEmail, + 'name', + fullName + ) + FROM + account + WHERE + accountId = steward_account_id + ) "steward", + note, + execution_dt "executionDt" + FROM + directive + WHERE + archive_id = :archiveId`, + { archiveId: 1 } + ); + expect(directiveResult.rows.length).toBe(1); + expect(directiveResult.rows[0]?.steward?.email).toBe( + "test+1@permanent.org" + ); + expect(directiveResult.rows[0]?.note).toBe(testNote); + expect(directiveResult.rows[0]?.type).toBe("transfer"); + + expect(sendArchiveStewardNotification).toHaveBeenCalledTimes(0); + }); + + test("should log error if notification email fails", async () => { + const testError = new Error("out of cheese error - redo from start"); + ( + sendArchiveStewardNotification as jest.MockedFunction< + typeof sendArchiveStewardNotification + > + ).mockRejectedValueOnce(testError); + await directiveService.updateDirective(testDirectiveId, { + emailFromAuthToken: "test@permanent.org", + stewardEmail: "test+2@permanent.org", + note: testNote, + }); + + expect(sendArchiveStewardNotification).toHaveBeenCalledTimes(1); + expect(logger.error).toHaveBeenCalledWith(testError); }); test("should error if authenticated account doesn't own the directive", async () => { diff --git a/src/directive/service/update.ts b/src/directive/service/update.ts index 91fd92e5..94bbaccd 100644 --- a/src/directive/service/update.ts +++ b/src/directive/service/update.ts @@ -5,6 +5,7 @@ import { isMissingStewardAccountError, getInvalidValueFromInvalidEnumMessage, } from "../../database_util"; +import { sendArchiveStewardNotification } from "../../email"; import { logger } from "../../log"; import type { UpdateDirectiveRequest, @@ -27,18 +28,19 @@ export const updateDirective = async ( throw new createError.NotFound("Directive not found"); } - const directiveToReturn = await db.transaction(async (transactionDb) => { - const directive = await (async (): Promise => { + const directiveData = await db.transaction(async (transactionDb) => { + const directive = await (async (): Promise< + Directive & { stewardChanged: boolean } + > => { try { - const directiveResult = await transactionDb.sql( - "directive.queries.update_directive", - { - directiveId, - type: requestBody.type, - stewardEmail: requestBody.stewardEmail, - note: requestBody.note, - } - ); + const directiveResult = await transactionDb.sql< + Directive & { stewardChanged: boolean } + >("directive.queries.update_directive", { + directiveId, + type: requestBody.type, + stewardEmail: requestBody.stewardEmail, + note: requestBody.note, + }); if (directiveResult.rows[0] === undefined) { throw new createError.BadRequest( "Directives cannot be updated after they've been executed" @@ -95,5 +97,14 @@ export const updateDirective = async ( return directive; }); + const { stewardChanged, ...directiveToReturn } = directiveData; + if (stewardChanged) { + await sendArchiveStewardNotification(directiveToReturn.directiveId).catch( + (err) => { + logger.error(err); + } + ); + } + return directiveToReturn; }; diff --git a/src/email/index.ts b/src/email/index.ts index 603bdf80..dc831cfa 100644 --- a/src/email/index.ts +++ b/src/email/index.ts @@ -1,3 +1,6 @@ -import { sendLegacyContactNotification } from "./service"; +import { + sendLegacyContactNotification, + sendArchiveStewardNotification, +} from "./service"; -export { sendLegacyContactNotification }; +export { sendLegacyContactNotification, sendArchiveStewardNotification }; diff --git a/src/email/queries/get_archive_stewardship_details.sql b/src/email/queries/get_archive_stewardship_details.sql new file mode 100644 index 00000000..f87787e9 --- /dev/null +++ b/src/email/queries/get_archive_stewardship_details.sql @@ -0,0 +1,25 @@ +SELECT + steward_account.fullName AS "stewardName", + steward_account.primaryEmail AS "stewardEmail", + owner_account.fullName AS "ownerName", + profile_item.string1 AS "archiveName" +FROM + directive +JOIN + account AS steward_account + ON steward_account.accountId = directive.steward_account_id +JOIN + account_archive + ON account_archive.archiveId = directive.archive_id + AND account_archive.accessRole = 'access.role.owner' + AND account_archive.status = 'status.generic.ok' +JOIN + account AS owner_account + ON owner_account.accountId = account_archive.accountId +JOIN + profile_item + ON profile_item.archiveId = directive.archive_id + AND profile_item.fieldNameUI = 'profile.basic' + AND profile_item.status != 'status.generic.deleted' +WHERE + directive.directive_id = :directiveId diff --git a/src/email/service.test.ts b/src/email/service.test.ts index 09cdeb5b..24a952c4 100644 --- a/src/email/service.test.ts +++ b/src/email/service.test.ts @@ -2,7 +2,12 @@ import type { MessagesSendSuccessResponse, MessagesSendRejectResponse, } from "@mailchimp/mailchimp_transactional"; -import { sendLegacyContactNotification } from "./service"; +import type { AxiosError } from "axios"; +import { + sendLegacyContactNotification, + sendArchiveStewardNotification, + sendEmail, +} from "./service"; import { MailchimpTransactional } from "../mailchimp"; import { db } from "../database"; @@ -16,14 +21,21 @@ jest.mock("../mailchimp", () => ({ })); const testLegacyContactId = "0cb0738c-5607-42d0-8014-8666a8d6ba13"; +const testDirectiveId = "39b2a5fa-3508-4030-91b6-21dc6ec7a1ab"; const loadFixtures = async (): Promise => { await db.sql("fixtures.create_test_accounts"); await db.sql("fixtures.create_test_legacy_contacts"); + await db.sql("fixtures.create_test_archive"); + await db.sql("fixtures.create_test_account_archive"); + await db.sql("fixtures.create_test_directive"); + await db.sql("fixtures.create_test_profile_items"); }; const clearDatabase = async (): Promise => { - await db.query("TRUNCATE account, legacy_contact CASCADE"); + await db.query( + "TRUNCATE account, legacy_contact, archive, directive, account_archive, profile_item CASCADE" + ); }; describe("sendLegacyContactNotification", () => { @@ -87,21 +99,38 @@ describe("sendLegacyContactNotification", () => { expect(MailchimpTransactional.messages.sendTemplate).not.toHaveBeenCalled(); }); +}); - test("should throw an error if no email is sent", async () => { - const mockResponse: MessagesSendSuccessResponse[] = []; +describe("sendArchiveNotification", () => { + beforeEach(async () => { + await clearDatabase(); + await loadFixtures(); + }); + + afterEach(async () => { + await clearDatabase(); + jest.clearAllMocks(); + }); + + test("should send archive steward notification successfully", async () => { + const mockResponse = [ + { + status: "sent", + _id: "test", + email: "test+1@permanent.org", + reject_reason: null, + } as MessagesSendSuccessResponse, + ]; ( MailchimpTransactional.messages.sendTemplate as jest.MockedFunction< typeof MailchimpTransactional.messages.sendTemplate > ).mockResolvedValueOnce(mockResponse); - await expect( - sendLegacyContactNotification(testLegacyContactId) - ).rejects.toThrow("no email sent"); + await sendArchiveStewardNotification(testDirectiveId); expect(MailchimpTransactional.messages.sendTemplate).toHaveBeenCalledWith({ - template_name: "legacy-contact-added", + template_name: "archive-steward-added", template_content: [], message: { from_email: "support@permanent.org", @@ -111,24 +140,44 @@ describe("sendLegacyContactNotification", () => { merge: true, merge_language: "mailchimp", from_name: "Jack Rando", - to: [{ email: "contact@permanent.org", name: "John Rando" }], - subject: "*|FROM_FULLNAME|* wants you to be their Legacy Contact", + to: [{ email: "test+1@permanent.org", name: "John Rando" }], + subject: "*|FROM_FULLNAME|* wants you to be their Archive Steward", global_merge_vars: [ { name: "from_fullname", content: "Jack Rando" }, { name: "to_fullname", content: "John Rando" }, + { name: "from_archive_name", content: "Jack Rando" }, ], }, }); }); - test("should throw an error if the Mailchimp API response indicates email not sent", async () => { + test("should throw an error if archive stewardship details are not found in the database", async () => { + await db.query( + "DELETE FROM directive WHERE directive_id = :testDirectiveId", + { testDirectiveId } + ); + + await expect( + sendArchiveStewardNotification(testDirectiveId) + ).rejects.toThrow(`Directive ${testDirectiveId} not found`); + + expect(MailchimpTransactional.messages.sendTemplate).not.toHaveBeenCalled(); + }); +}); + +describe("sendEmail", () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + test("should successfully call Mailchimp", async () => { const mockResponse = [ { - status: "rejected", - reject_reason: "invalid", - email: "contact@permanent.org", + status: "sent", _id: "test", - } as MessagesSendRejectResponse, + email: "contact@permanent.org", + reject_reason: null, + } as MessagesSendSuccessResponse, ]; ( MailchimpTransactional.messages.sendTemplate as jest.MockedFunction< @@ -136,9 +185,16 @@ describe("sendLegacyContactNotification", () => { > ).mockResolvedValueOnce(mockResponse); - await expect( - sendLegacyContactNotification(testLegacyContactId) - ).rejects.toThrow("Email not sent. Status: rejected; Reason: invalid"); + await sendEmail( + "legacy-contact-added", + "Jack Rando", + [{ email: "contact@permanent.org", name: "John Rando" }], + "*|FROM_FULLNAME|* wants you to be their Legacy Contact", + [ + { name: "from_fullname", content: "Jack Rando" }, + { name: "to_fullname", content: "John Rando" }, + ] + ); expect(MailchimpTransactional.messages.sendTemplate).toHaveBeenCalledWith({ template_name: "legacy-contact-added", @@ -160,4 +216,84 @@ describe("sendLegacyContactNotification", () => { }, }); }); + + test("should throw an error if no email is sent", async () => { + const mockResponse: MessagesSendSuccessResponse[] = []; + ( + MailchimpTransactional.messages.sendTemplate as jest.MockedFunction< + typeof MailchimpTransactional.messages.sendTemplate + > + ).mockResolvedValueOnce(mockResponse); + + await expect( + sendEmail( + "legacy-contact-added", + "Jack Rando", + [{ email: "contact@permanent.org", name: "John Rando" }], + "*|FROM_FULLNAME|* wants you to be their Legacy Contact", + [ + { name: "from_fullname", content: "Jack Rando" }, + { name: "to_fullname", content: "John Rando" }, + ] + ) + ).rejects.toThrow("no email sent"); + }); + + test("should throw an error if the Mailchimp API response indicates email not sent", async () => { + const mockResponse = [ + { + status: "rejected", + reject_reason: "invalid", + email: "contact@permanent.org", + _id: "test", + } as MessagesSendRejectResponse, + ]; + ( + MailchimpTransactional.messages.sendTemplate as jest.MockedFunction< + typeof MailchimpTransactional.messages.sendTemplate + > + ).mockResolvedValueOnce(mockResponse); + + await expect( + sendEmail( + "legacy-contact-added", + "Jack Rando", + [{ email: "contact@permanent.org", name: "John Rando" }], + "*|FROM_FULLNAME|* wants you to be their Legacy Contact", + [ + { name: "from_fullname", content: "Jack Rando" }, + { name: "to_fullname", content: "John Rando" }, + ] + ) + ).rejects.toThrow("Email not sent. Status: rejected; Reason: invalid"); + }); + + test("should throw an error if the Mailchimp API response is an axios error", async () => { + const mockResponse = { + config: {}, + isAxiosError: true, + toJSON: () => {}, + response: { + status: 500, + }, + } as AxiosError; + ( + MailchimpTransactional.messages.sendTemplate as jest.MockedFunction< + typeof MailchimpTransactional.messages.sendTemplate + > + ).mockResolvedValueOnce(mockResponse); + + await expect( + sendEmail( + "legacy-contact-added", + "Jack Rando", + [{ email: "contact@permanent.org", name: "John Rando" }], + "*|FROM_FULLNAME|* wants you to be their Legacy Contact", + [ + { name: "from_fullname", content: "Jack Rando" }, + { name: "to_fullname", content: "John Rando" }, + ] + ) + ).rejects.toThrow("Error calling Mailchimp. Status: 500"); + }); }); diff --git a/src/email/service.ts b/src/email/service.ts index 3f271ee1..cbffe058 100644 --- a/src/email/service.ts +++ b/src/email/service.ts @@ -11,36 +11,29 @@ const defaultMessage = { merge_language: "mailchimp" as const, }; -export const sendLegacyContactNotification = async ( - legacyContactId: string +export const sendEmail = async ( + templateName: string, + fromName: string, + toData: { email: string; name: string }[], + subject: string, + mergeVariables: { name: string; content: string }[] ): Promise => { - const legacyContactDetailsResult = await db.sql<{ - accountName: string; - legacyContactName: string; - legacyContactEmail: string; - }>("email.queries.get_legacy_contact_details", { legacyContactId }); - if (legacyContactDetailsResult.rows[0] === undefined) { - throw new Error(`Legacy contact ${legacyContactId} not found`); - } - const { accountName, legacyContactName, legacyContactEmail } = - legacyContactDetailsResult.rows[0]; const response = await MailchimpTransactional.messages.sendTemplate({ - template_name: "legacy-contact-added", + template_name: templateName, template_content: [], message: { ...defaultMessage, - from_name: accountName, - to: [{ email: legacyContactEmail, name: legacyContactName }], - subject: "*|FROM_FULLNAME|* wants you to be their Legacy Contact", - global_merge_vars: [ - { name: "from_fullname", content: accountName }, - { name: "to_fullname", content: legacyContactName }, - ], + from_name: fromName, + to: toData, + subject, + global_merge_vars: mergeVariables, }, }); if ("isAxiosError" in response) { - throw response; + throw new Error( + `Error calling Mailchimp. Status: ${response.response?.status ?? ""}` + ); } else if (!response[0]) { throw new Error("no email sent"); } else if (response[0].status !== "sent") { @@ -51,3 +44,56 @@ export const sendLegacyContactNotification = async ( ); } }; + +export const sendLegacyContactNotification = async ( + legacyContactId: string +): Promise => { + const legacyContactDetailsResult = await db.sql<{ + accountName: string; + legacyContactName: string; + legacyContactEmail: string; + }>("email.queries.get_legacy_contact_details", { legacyContactId }); + if (legacyContactDetailsResult.rows[0] === undefined) { + throw new Error(`Legacy contact ${legacyContactId} not found`); + } + const { accountName, legacyContactName, legacyContactEmail } = + legacyContactDetailsResult.rows[0]; + + await sendEmail( + "legacy-contact-added", + accountName, + [{ email: legacyContactEmail, name: legacyContactName }], + "*|FROM_FULLNAME|* wants you to be their Legacy Contact", + [ + { name: "from_fullname", content: accountName }, + { name: "to_fullname", content: legacyContactName }, + ] + ); +}; + +export const sendArchiveStewardNotification = async ( + directiveId: string +): Promise => { + const archiveStewardshipDetailsResult = await db.sql<{ + stewardName: string; + stewardEmail: string; + ownerName: string; + archiveName: string; + }>("email.queries.get_archive_stewardship_details", { directiveId }); + if (archiveStewardshipDetailsResult.rows[0] === undefined) { + throw new Error(`Directive ${directiveId} not found`); + } + const { stewardName, stewardEmail, ownerName, archiveName } = + archiveStewardshipDetailsResult.rows[0]; + await sendEmail( + "archive-steward-added", + ownerName, + [{ email: stewardEmail, name: stewardName }], + "*|FROM_FULLNAME|* wants you to be their Archive Steward", + [ + { name: "from_fullname", content: ownerName }, + { name: "to_fullname", content: stewardName }, + { name: "from_archive_name", content: archiveName }, + ] + ); +}; diff --git a/src/fixtures/create_test_profile_items.sql b/src/fixtures/create_test_profile_items.sql new file mode 100644 index 00000000..9489de3c --- /dev/null +++ b/src/fixtures/create_test_profile_items.sql @@ -0,0 +1,5 @@ +INSERT INTO + profile_item (archiveId, fieldNameUi, string1, status, type) +VALUES + (1, 'profile.basic', 'Jack Rando', 'status.generic.ok', 'type.profile_item.basic'); + diff --git a/src/legacy_contact/service/update.ts b/src/legacy_contact/service/update.ts index 93cd6ed8..786459ee 100644 --- a/src/legacy_contact/service/update.ts +++ b/src/legacy_contact/service/update.ts @@ -8,7 +8,7 @@ export const updateLegacyContact = async ( legacyContactId: string, requestBody: UpdateLegacyContactRequest ): Promise => { - const legacyContact = await db + const legacyContactResult = await db .sql( "legacy_contact.queries.update_legacy_contact", { @@ -25,22 +25,17 @@ export const updateLegacyContact = async ( ); }); - if (legacyContact.rows[0] === undefined) { + if (legacyContactResult.rows[0] === undefined) { throw new createError.NotFound("Legacy contact not found"); } - if (legacyContact.rows[0].emailChanged) { + const { emailChanged, ...legacyContact } = legacyContactResult.rows[0]; + + if (emailChanged) { await sendLegacyContactNotification(legacyContactId).catch((err) => { logger.error(err); }); } - return { - legacyContactId: legacyContact.rows[0].legacyContactId, - accountId: legacyContact.rows[0].accountId, - name: legacyContact.rows[0].name, - email: legacyContact.rows[0].email, - createdDt: legacyContact.rows[0].createdDt, - updatedDt: legacyContact.rows[0].updatedDt, - }; + return legacyContact; };