Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 35 additions & 20 deletions apps/report-job/src/__tests__/runReport.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function harness(
}

describe("runReport", () => {
it("publishes a winner message and DMs the winner their descriptions", async () => {
it("publishes a shoutout naming every recipient and DMs each of them their descriptions", async () => {
const noms = [
nomination("U_A", "Kicked off migration", "n1"),
nomination("U_A", "Owned the rollout", "n2"),
Expand All @@ -130,48 +130,63 @@ describe("runReport", () => {

await runReport(event(), h.deps);

// PENDING row created with counts and per-winner delivery entries.
// PENDING row created with counts and one delivery entry per recipient
// (everyone with ≥1 nomination — see ADR-007).
expect(h.reports.putPendingExecution).toHaveBeenCalledTimes(1);
const pending = h.reports.putPendingExecution.mock
.calls[0]![0] as ReportExecutionItem;
expect(pending.status).toBe("PENDING");
expect(pending.winnerSlackIds).toEqual(["U_A"]);
expect(pending.winnerSlackIds).toEqual(["U_A", "U_B"]);
expect(pending.countsBySlackId).toEqual({ U_A: 2, U_B: 1 });
expect(pending.dmDeliveries).toEqual([
{ recipientSlackId: "U_A", status: "PENDING", attempts: 0 },
{ recipientSlackId: "U_B", status: "PENDING", attempts: 0 },
]);

// Public post + PUBLISHED update.
// Public post names every recipient. No descriptions, no counts.
expect(h.slack.postMessage).toHaveBeenCalled();
const publicPost = h.slack.postMessage.mock.calls[0]![0] as {
channel: string;
text: string;
};
expect(publicPost.channel).toBe(RECOG);
expect(publicPost.text).toContain("<@U_A>");
expect(publicPost.text).toContain("<@U_B>");
expect(publicPost.text).not.toContain("Kicked off migration");
expect(h.reports.markPublished).toHaveBeenCalledTimes(1);

// Winner DM with all their descriptions and no nominator identifiers.
const dmCall = h.slack.postMessage.mock.calls[1]![0] as {
// One DM per recipient, iterated in sorted Slack-ID order. Each DM
// contains only the descriptions written about that recipient and no
// nominator identifiers.
const dmA = h.slack.postMessage.mock.calls[1]![0] as {
channel: string;
text: string;
};
expect(dmCall.channel).toBe("D-U_A");
expect(dmCall.text).toContain("Kicked off migration");
expect(dmCall.text).toContain("Owned the rollout");
expect(dmCall.text).not.toContain("NOM-");
expect(dmA.channel).toBe("D-U_A");
expect(dmA.text).toContain("Kicked off migration");
expect(dmA.text).toContain("Owned the rollout");
expect(dmA.text).not.toContain("Helped review");
expect(dmA.text).not.toContain("NOM-");

// DM state persisted as SENT.
expect(h.reports.updateDmDelivery).toHaveBeenCalledTimes(1);
const dmDelivery = h.reports.updateDmDelivery.mock.calls[0]![0] as {
delivery: WinnerDmDelivery;
const dmB = h.slack.postMessage.mock.calls[2]![0] as {
channel: string;
text: string;
};
expect(dmDelivery.delivery.status).toBe("SENT");
expect(dmDelivery.delivery.attempts).toBe(1);
expect(dmB.channel).toBe("D-U_B");
expect(dmB.text).toContain("Helped review");
expect(dmB.text).not.toContain("Kicked off migration");
expect(dmB.text).not.toContain("NOM-");

// Both delivery rows persisted as SENT.
expect(h.reports.updateDmDelivery).toHaveBeenCalledTimes(2);
const deliveries = h.reports.updateDmDelivery.mock.calls.map(
(c) => (c[0] as { delivery: WinnerDmDelivery }).delivery,
);
expect(deliveries.every((d) => d.status === "SENT")).toBe(true);
expect(deliveries.every((d) => d.attempts === 1)).toBe(true);
});

it("publishes all tied winners", async () => {
it("names every recipient regardless of count (count does not filter who is published or DMed)", async () => {
const noms = [
nomination("U_A", "one", "n1"),
nomination("U_A", "two", "n2"),
Expand All @@ -184,13 +199,13 @@ describe("runReport", () => {

const pending = h.reports.putPendingExecution.mock
.calls[0]![0] as ReportExecutionItem;
expect(pending.winnerSlackIds.sort()).toEqual(["U_A", "U_B"]);
expect(pending.winnerSlackIds).toEqual(["U_A", "U_B"]);

const publicPost = h.slack.postMessage.mock.calls[0]![0] as { text: string };
expect(publicPost.text).toContain("<@U_A>");
expect(publicPost.text).toContain("<@U_B>");

// Each winner receives their own DM.
// One DM per recipient.
expect(h.slack.openDm).toHaveBeenCalledTimes(2);
expect(h.reports.updateDmDelivery).toHaveBeenCalledTimes(2);
});
Expand Down Expand Up @@ -255,7 +270,7 @@ describe("runReport", () => {
expect(delivery.attempts).toBe(2);
});

it("does not retry winners whose DM was already SENT", async () => {
it("does not retry recipients whose DM was already SENT", async () => {
const noms = [
nomination("U_A", "one", "n1"),
nomination("U_B", "two", "n2"),
Expand Down
85 changes: 44 additions & 41 deletions apps/report-job/src/runReport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ import type { BiweeklyReportRequestedV1 } from "@nominate/contracts";
import type {
NominationItem,
ReportExecutionItem,
Winner,
WinnerDmDelivery,
} from "@nominate/domain";
import { tallyWinners } from "@nominate/domain";
import type { Logger } from "@nominate/observability";
import type {
NominationRepository,
Expand All @@ -18,11 +16,12 @@ import {
type SlackClient,
} from "@nominate/slack";

// docs/04 §Report Lambda + docs/10 §Aggregation, §Public message, §Winner DMs.
// Idempotency is anchored to the REPORT_EXECUTION row: PENDING → PUBLISHED →
// per-winner DMs. A retry that lands after PUBLISHED must never re-post the
// public message; a DM failure must not roll back publication (docs/02
// §Publication rules).
// docs/04 §Report Lambda + docs/10 §Aggregation, §Public message, §Recipient DMs.
// docs/adr/ADR-007: everyone with ≥1 nomination this period is named publicly
// and DMed their descriptions — no top-count filtering. Idempotency is anchored
// to the REPORT_EXECUTION row: PENDING → PUBLISHED → per-recipient DMs. A retry
// that lands after PUBLISHED must never re-post the public message; a DM
// failure must not roll back publication (docs/02 §Publication rules).

export interface RunReportDeps {
nominations: NominationRepository;
Expand Down Expand Up @@ -64,27 +63,29 @@ export async function runReport(
periodEndEpochMs,
});

const winners = tallyWinners(nominations);
const countsBySlackId: Record<string, number> = {};
for (const nomination of nominations) {
countsBySlackId[nomination.recipientSlackId] =
(countsBySlackId[nomination.recipientSlackId] ?? 0) + 1;
}
// Every teammate with ≥1 nomination is a recipient. Sorted for deterministic
// ordering so retries and admin re-runs produce identical output.
const recipientSlackIds = Object.keys(countsBySlackId).sort();

const existing = await deps.reports.getExecution({
workspaceId: event.workspaceId,
periodStart: event.periodStart,
});

// Admin-triggered "force" runs (docs/03 §Admin surface) rewrite the execution
// row to a fresh PENDING state so the public post and winner DMs re-fire.
// row to a fresh PENDING state so the public post and recipient DMs re-fire.
// Scheduled EventBridge invocations must never set this flag.
const forceRepublish = event.forceRepublish === true;

const execution = await ensurePendingExecution({
event,
existing: forceRepublish ? null : existing,
winners,
recipientSlackIds,
countsBySlackId,
forceRepublish,
deps,
Expand All @@ -93,18 +94,18 @@ export async function runReport(
const published = await ensurePublished({
event,
execution,
winners,
recipientSlackIds,
forceRepublish,
deps,
log,
});

if (winners.length === 0) return;
if (recipientSlackIds.length === 0) return;

await deliverWinnerDms({
await deliverRecipientDms({
event,
execution: published,
winners,
recipientSlackIds,
nominations,
deps,
log,
Expand All @@ -114,7 +115,7 @@ export async function runReport(
interface PendingContext {
event: BiweeklyReportRequestedV1;
existing: ReportExecutionItem | null;
winners: Winner[];
recipientSlackIds: readonly string[];
countsBySlackId: Record<string, number>;
forceRepublish: boolean;
deps: RunReportDeps;
Expand All @@ -123,23 +124,25 @@ interface PendingContext {
async function ensurePendingExecution({
event,
existing,
winners,
recipientSlackIds,
countsBySlackId,
forceRepublish,
deps,
}: PendingContext): Promise<ReportExecutionItem> {
if (existing) return existing;
// winnerSlackIds field carries every recipient now; the field name is a
// rename target (see ADR-007) but the storage semantics are unchanged.
const pending: ReportExecutionItem = {
entityType: "REPORT_EXECUTION",
workspaceId: event.workspaceId,
periodStart: event.periodStart,
periodEnd: event.periodEnd,
status: "PENDING",
winnerSlackIds: winners.map((w) => w.recipientSlackId),
winnerSlackIds: [...recipientSlackIds],
countsBySlackId,
dmDeliveries: winners.map(
(w): WinnerDmDelivery => ({
recipientSlackId: w.recipientSlackId,
dmDeliveries: recipientSlackIds.map(
(recipientSlackId): WinnerDmDelivery => ({
recipientSlackId,
status: "PENDING",
attempts: 0,
}),
Expand All @@ -159,7 +162,7 @@ async function ensurePendingExecution({
interface PublishContext {
event: BiweeklyReportRequestedV1;
execution: ReportExecutionItem;
winners: Winner[];
recipientSlackIds: readonly string[];
forceRepublish: boolean;
deps: RunReportDeps;
log: Logger;
Expand All @@ -168,7 +171,7 @@ interface PublishContext {
async function ensurePublished({
event,
execution,
winners,
recipientSlackIds,
forceRepublish,
deps,
log,
Expand All @@ -179,12 +182,12 @@ async function ensurePublished({
}

const message =
winners.length === 0
recipientSlackIds.length === 0
? buildEmptyPeriodMessage(event.periodEnd)
: buildReportMessage({
periodStart: event.periodStart,
periodEnd: event.periodEnd,
winners,
recipientSlackIds,
});
const posted = await deps.slack.postMessage({
channel: deps.recognitionChannelId,
Expand All @@ -201,7 +204,7 @@ async function ensurePublished({
});
log.info("report_published", {
outcome: "PUBLISHED",
winnerCount: winners.length,
recipientCount: recipientSlackIds.length,
});
return {
...execution,
Expand All @@ -214,23 +217,23 @@ async function ensurePublished({
interface DeliverContext {
event: BiweeklyReportRequestedV1;
execution: ReportExecutionItem;
winners: Winner[];
recipientSlackIds: readonly string[];
nominations: NominationItem[];
deps: RunReportDeps;
log: Logger;
}

async function deliverWinnerDms({
async function deliverRecipientDms({
event,
execution,
winners,
recipientSlackIds,
nominations,
deps,
log,
}: DeliverContext): Promise<void> {
const descriptionsBySlackId = new Map<string, string[]>();
for (const winner of winners) {
descriptionsBySlackId.set(winner.recipientSlackId, []);
for (const recipientSlackId of recipientSlackIds) {
descriptionsBySlackId.set(recipientSlackId, []);
}
for (const nomination of nominations) {
const bucket = descriptionsBySlackId.get(nomination.recipientSlackId);
Expand All @@ -242,23 +245,23 @@ async function deliverWinnerDms({
deliveriesById.set(entry.recipientSlackId, entry);
}

for (const winner of winners) {
for (const recipientSlackId of recipientSlackIds) {
const current =
deliveriesById.get(winner.recipientSlackId) ??
deliveriesById.get(recipientSlackId) ??
({
recipientSlackId: winner.recipientSlackId,
recipientSlackId,
status: "PENDING",
attempts: 0,
} as WinnerDmDelivery);
if (current.status === "SENT" || current.status === "FAILED_TERMINAL") {
continue;
}

const descriptions = descriptionsBySlackId.get(winner.recipientSlackId) ?? [];
const descriptions = descriptionsBySlackId.get(recipientSlackId) ?? [];
const attemptedAt = new Date(deps.now()).toISOString();
try {
const { channel } = await deps.slack.openDm({
userSlackId: winner.recipientSlackId,
userSlackId: recipientSlackId,
workspaceId: event.workspaceId,
});
const dm = buildWinnerDm({
Expand All @@ -276,32 +279,32 @@ async function deliverWinnerDms({
workspaceId: event.workspaceId,
periodStart: event.periodStart,
delivery: {
recipientSlackId: winner.recipientSlackId,
recipientSlackId,
status: "SENT",
attempts: current.attempts + 1,
lastAttemptAt: attemptedAt,
},
});
log.info("winner_dm_sent", {
log.info("recipient_dm_sent", {
outcome: "SENT",
recipient: winner.recipientSlackId,
recipient: recipientSlackId,
});
} catch (err) {
const errorCategory = err instanceof Error ? err.name : "unknown";
await deps.reports.updateDmDelivery({
workspaceId: event.workspaceId,
periodStart: event.periodStart,
delivery: {
recipientSlackId: winner.recipientSlackId,
recipientSlackId,
status: "FAILED_RETRYABLE",
attempts: current.attempts + 1,
lastAttemptAt: attemptedAt,
lastError: errorCategory,
},
});
log.warn("winner_dm_failed", {
log.warn("recipient_dm_failed", {
outcome: "FAILED_RETRYABLE",
recipient: winner.recipientSlackId,
recipient: recipientSlackId,
errorCategory,
});
}
Expand Down
Loading
Loading