diff --git a/lib/services/campaign-grouping-service.test.ts b/lib/services/campaign-grouping-service.test.ts index 991c288..dcb868c 100644 --- a/lib/services/campaign-grouping-service.test.ts +++ b/lib/services/campaign-grouping-service.test.ts @@ -339,7 +339,7 @@ describe('groupReportIntoCampaign', () => { expect(callsContaining('UPDATE campaigns')).toHaveLength(0); }); - it('re-analyzing a report already linked to a campaign that a sibling report re-matches does not increment report_count a second time', async () => { + it('Test G (CR-02 guard against over-firing): same-campaign re-match remains a pure no-op — the origin decrement must NOT fire when the match resolves back to the report\'s own campaign', async () => { // Own report is already linked to campaign-1. A SIBLING report (not this // report's own excluded row) also belongs to campaign-1 and matches via // Tier 3 (sender + subject + client). This is the CAMP-02/CR-01 gap: @@ -464,4 +464,95 @@ describe('groupReportIntoCampaign', () => { }); expect(callsContaining('INSERT INTO campaigns')).toHaveLength(1); }); + + // =========================================================================== + // CR-02 (gap closure, 18-05 Task 2): decrement the origin campaign's + // report_count on cross-campaign migration, AND on the signal-diverged + // create-new fall-through added by Task 1 (CR-03) above — both are the + // same "abandoning an origin campaign" situation, just landing on an + // existing sibling's campaign vs. a brand-new one. Tests E and H below + // FAILED against the pre-Task-2 code (confirmed during RED). + // =========================================================================== + + it('Test E (CR-02 migration decrements origin): cross-campaign migration decrements the origin and increments the destination exactly once each', async () => { + stage({ + ownReport: [{ ...REPORT_ROW, campaign_id: 'campaign-A' }], + ownMessage: [], + tier3: [{ campaign_id: 'campaign-B', title: 'Invoice Alert' }], + }); + + const result = await groupReportIntoCampaign('report-e'); + + expect(result).toEqual({ + campaignId: 'campaign-B', + groupMethod: 'sender_subject_client', + created: false, + }); + + const updateCampaignCalls = callsContaining('UPDATE campaigns'); + expect(updateCampaignCalls).toHaveLength(2); + const incrementCall = updateCampaignCalls.find((c) => c.sql.includes('report_count + 1')); + const decrementCall = updateCampaignCalls.find((c) => c.sql.includes('report_count - 1')); + expect(incrementCall?.params).toEqual(['campaign-B']); + expect(decrementCall?.params).toEqual(['campaign-A']); + + const updateReportCalls = callsContaining('UPDATE reports SET campaign_id'); + expect(updateReportCalls).toHaveLength(1); + expect(updateReportCalls[0].params).toEqual(['campaign-B', 'report-e']); + }); + + it('Test F (D-08 upgrade preserved, no origin to decrement): a never-grouped report upgrading to a sibling campaign increments the destination only', async () => { + stage({ + ownReport: [REPORT_ROW], // campaign_id null/undefined — never grouped before + ownMessage: [{ id: 'own-msg-id', message_id: 'shared-message-id-upgrade' }], + tier1: [{ campaign_id: 'campaign-B' }], + }); + + const result = await groupReportIntoCampaign('report-f'); + + expect(result).toEqual({ + campaignId: 'campaign-B', + groupMethod: 'message_id', + created: false, + }); + + const updateCampaignCalls = callsContaining('UPDATE campaigns'); + expect(updateCampaignCalls).toHaveLength(1); + expect(updateCampaignCalls[0].sql).toContain('report_count + 1'); + expect(updateCampaignCalls[0].params).toEqual(['campaign-B']); + + const updateReportCalls = callsContaining('UPDATE reports SET campaign_id'); + expect(updateReportCalls).toHaveLength(1); + expect(updateReportCalls[0].params).toEqual(['campaign-B', 'report-f']); + }); + + it('Test H (plan-checker-flagged divergence case): signal-diverged create-new also decrements the abandoned origin campaign exactly once', async () => { + stage({ + ownReport: [{ ...REPORT_ROW, campaign_id: 'campaign-A' }], + ownMessage: [], + tier3: [], + ownCampaign: [ + { campaign_key: 'sender_subject_client:5:old subject:10', group_method: 'sender_subject_client' }, + ], + insertCampaign: [{ id: 'diverged-new-campaign-h' }], + }); + + const result = await groupReportIntoCampaign('report-h'); + + expect(result).toEqual({ + campaignId: 'diverged-new-campaign-h', + groupMethod: 'sender_subject_client', + created: true, + }); + + expect(callsContaining('INSERT INTO campaigns')).toHaveLength(1); + const updateCampaignCalls = callsContaining('UPDATE campaigns'); + expect(updateCampaignCalls).toHaveLength(1); + expect(updateCampaignCalls[0].sql).toContain('report_count - 1'); + expect(updateCampaignCalls[0].params).toEqual(['campaign-A']); + + const updateReportCalls = callsContaining('UPDATE reports SET campaign_id'); + expect(updateReportCalls).toHaveLength(1); + expect(updateReportCalls[0].params).toEqual(['diverged-new-campaign-h', 'report-h']); + }); }); diff --git a/lib/services/campaign-grouping-service.ts b/lib/services/campaign-grouping-service.ts index 84ea7b9..e11a59d 100644 --- a/lib/services/campaign-grouping-service.ts +++ b/lib/services/campaign-grouping-service.ts @@ -20,6 +20,7 @@ * Tier 3 (sender + normalized subject + client + 24h window). */ +import type { PoolClient } from 'pg'; import { postgresClient } from './postgres-client'; // ============================================================================= @@ -118,6 +119,24 @@ function computeTier3Key( return `sender_subject_client:${requesterContactId}:${normalizedSubject}:${companyId}`; } +/** + * CR-02: decrements a now-abandoned origin campaign's report_count. Shared + * by both places a report can be abandoned from its origin campaign — the + * sibling-migration branch (moving to a DIFFERENT existing campaign) and the + * signal-diverged create-new fall-through (moving to a brand-new campaign) — + * so neither is left with a stale, inflated count. Decrement only; never + * deletes the origin row even if its count reaches 0 (out of scope — see + * CR-02 scope note at the call sites). + */ +async function decrementOriginCampaign(client: PoolClient, originCampaignId: string): Promise { + await client.query( + `UPDATE campaigns + SET report_count = report_count - 1, updated_at = NOW() + WHERE id = $1`, + [originCampaignId] + ); +} + /** * Shared entry point called by the webhook path, the cron sweep, and the * on-demand `/analyze` route. Looks up the report's own signal (message, @@ -363,6 +382,17 @@ export async function groupReportIntoCampaign( `UPDATE reports SET campaign_id = $1, updated_at = NOW() WHERE id = $2`, [matchCampaignId, reportId] ); + // CR-02: if the report had a DIFFERENT prior campaign (the + // same-campaign case returned above; a never-grouped report has no + // origin — D-08), decrement it so it doesn't keep a permanently + // stale report_count for a report it no longer holds. Scope: + // decrement only, never delete the origin row even if its count + // reaches 0 (deleting introduces FK-cascade/detail-route risk out of + // scope for this gap fix; an empty campaign is a strictly lesser + // problem than a stale count). + if (ownReport.campaign_id) { + await decrementOriginCampaign(client, ownReport.campaign_id); + } return { campaignId: matchCampaignId, groupMethod: matchGroupMethod, created: false }; } @@ -397,10 +427,14 @@ export async function groupReportIntoCampaign( created: false, }; } - // Own campaign row is gone, or the report's signal has genuinely - // diverged from its stored key — fall through to create-new below. - // (CR-02: decrementing this now-abandoned origin campaign's - // report_count is handled in Task 2 of the 18-05 gap-closure plan.) + // CR-02: own campaign row is gone, or the report's signal has + // genuinely diverged from its stored key — this report is about to + // be abandoned from its origin campaign in favor of a brand-new one + // below (the same class of abandonment as the sibling-migration + // branch above, just landing on a new row instead of an existing + // sibling's). Decrement now so the origin doesn't show a stale + // count. Falls through to create-new unchanged. + await decrementOriginCampaign(client, ownReport.campaign_id); } // No match on any tier — create a new campaign. Prefer the strongest