From a2bbf453d252ec75d0ce04054e05891d41e7d742 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Fri, 9 Oct 2026 18:33:19 +1100 Subject: [PATCH] PM-5449: Send manual phase change emails per recipient What was broken When a challenge phase was manually closed or reopened from the review app, the phase change email listed every notified member in the To field, so each member could see the other members' email addresses. Autopilot phase change emails were not affected because autopilot already publishes one email event per recipient. Root cause Manual phase changes go through PATCH /challenges/:id/phases/:phaseId, which sends mail through helper.sendPhaseChangeNotification. That helper put the whole recipient list into a single external.action.email bus event. The email service then sends that event as one SendGrid message with all recipients in To. PM-5763 changed ChallengePhaseService to call the helper once per recipient, but the helper still merged any multi-recipient list into one shared email. What was changed helper.sendPhaseChangeNotification now publishes a separate external.action.email event for each recipient, with a single address in recipients. This applies the privacy guarantee at the point where the email event is built, so callers cannot expose addresses. A failed publish for one recipient is logged and the remaining recipients are still notified. The helper's doc comment was updated to describe this. Any added/updated tests Added test/unit/phase-change-notification-helper.test.js. It loads the helper against a stubbed bus client and checks two things: a multi-recipient call publishes one email event per recipient, each with exactly one address, and a failed publish for one recipient does not block the others. Both tests fail against the previous helper and pass with this change. Co-Authored-By: Claude Opus 5.5 --- src/common/helper.ts | 27 ++++-- .../phase-change-notification-helper.test.js | 88 +++++++++++++++++++ 2 files changed, 106 insertions(+), 9 deletions(-) create mode 100644 test/unit/phase-change-notification-helper.test.js diff --git a/src/common/helper.ts b/src/common/helper.ts index 7254e65..f7e75b7 100644 --- a/src/common/helper.ts +++ b/src/common/helper.ts @@ -1973,7 +1973,10 @@ function buildPhaseChangeEmailData({ } /** - * Send phase change notification + * Send phase change notification. + * Publishes one email event per recipient so member email addresses are never + * shared in a single To field. A failed publish is logged and does not stop the + * remaining recipients from being notified. * @param {String} type the notification type * @param {Array} recipients the array of recipients emails * @param {Object} data the data @@ -2000,14 +2003,20 @@ async function sendPhaseChangeNotification(type, recipients, data) { return; } - await postBusEvent("external.action.email", { - from: config.EMAIL_FROM, - replyTo: config.EMAIL_FROM, - recipients: safeRecipients, - data: data, - sendgrid_template_id: settings.sendgridTemplateId, - version: "v3", - }); + for (const recipient of safeRecipients) { + try { + await postBusEvent("external.action.email", { + from: config.EMAIL_FROM, + replyTo: config.EMAIL_FROM, + recipients: [recipient], + data: data, + sendgrid_template_id: settings.sendgridTemplateId, + version: "v3", + }); + } catch (e) { + logger.debug(`Failed to post notification ${type} to a recipient: ${e.message}`); + } + } } catch (e) { logger.debug(`Failed to post notification ${type}: ${e.message}`); } diff --git a/test/unit/phase-change-notification-helper.test.js b/test/unit/phase-change-notification-helper.test.js new file mode 100644 index 0000000..62e2824 --- /dev/null +++ b/test/unit/phase-change-notification-helper.test.js @@ -0,0 +1,88 @@ +require("../../app-bootstrap"); + +const { expect } = require("chai"); +const constants = require("../../app-constants"); + +const helperPath = require.resolve("../../src/common/helper"); +const busApiPath = require.resolve("topcoder-bus-api-wrapper"); + +describe("phase change notification helper", () => { + const settings = constants.PhaseChangeNotificationSettings.PHASE_CHANGE; + const originalTemplateId = settings.sendgridTemplateId; + const originalHelperModule = require.cache[helperPath]; + const originalBusApiModule = require.cache[busApiPath]; + let postedEvents; + let failingRecipients; + let helper; + + beforeEach(() => { + postedEvents = []; + failingRecipients = new Set(); + settings.sendgridTemplateId = "phase-change-template"; + + // Load a fresh helper bound to a stub bus client so published events can be inspected. + require.cache[busApiPath] = { + id: busApiPath, + filename: busApiPath, + loaded: true, + exports: () => ({ + postEvent: async (message) => { + if (failingRecipients.has(message.payload.recipients[0])) { + throw new Error("bus unavailable"); + } + postedEvents.push(message); + }, + }), + }; + delete require.cache[helperPath]; + helper = require(helperPath); + }); + + afterEach(() => { + settings.sendgridTemplateId = originalTemplateId; + require.cache[helperPath] = originalHelperModule; + if (originalBusApiModule) { + require.cache[busApiPath] = originalBusApiModule; + } else { + delete require.cache[busApiPath]; + } + }); + + it("publishes a separate email event for each recipient", async () => { + const data = { challengeName: "Design challenge", phase_change: "Registration Closed" }; + + await helper.sendPhaseChangeNotification( + "PHASE_CHANGE", + ["alice@example.com", "", "bob@example.com"], + data, + ); + + expect(postedEvents).to.have.length(2); + expect(postedEvents.map((event) => event.topic)).to.deep.equal([ + "external.action.email", + "external.action.email", + ]); + expect(postedEvents.map((event) => event.payload.recipients)).to.deep.equal([ + ["alice@example.com"], + ["bob@example.com"], + ]); + postedEvents.forEach((event) => { + expect(event.payload.data).to.deep.equal(data); + expect(event.payload.sendgrid_template_id).to.equal("phase-change-template"); + }); + }); + + it("continues notifying remaining recipients when one publish fails", async () => { + failingRecipients.add("alice@example.com"); + + await helper.sendPhaseChangeNotification( + "PHASE_CHANGE", + ["alice@example.com", "bob@example.com"], + {}, + ); + + expect(postedEvents.map((event) => event.payload.recipients)).to.deep.equal([ + ["bob@example.com"], + ]); + }); +});