fix(18-05): decrement origin campaign report_count on migration + diverged create-new (CR-02)
Cross-campaign migration (a report moves from campaign A to a different existing campaign B on re-analyze) never decremented A's report_count, leaving it permanently stale. The same abandonment happens on the signal-diverged create-new fall-through added for CR-03 (Task 1) — landing on a brand-new campaign instead of an existing sibling's, but the same class of staleness. Add a shared decrementOriginCampaign() helper and call it in both locations, guarded so it only fires when there is a real origin (ownReport.campaign_id non-null) and the match doesn't resolve back to the report's own campaign (the existing same-campaign no-op guard is unchanged). D-08 sibling upgrades (never-grouped report -> sibling campaign) increment the destination only, with no origin to decrement. - Test E: cross-campaign migration decrements origin, increments destination - Test F: D-08 upgrade with no prior campaign increments destination only - Test G (renamed from existing 18-04 test): same-campaign re-match remains zero mutations - Test H: signal-diverged create-new also decrements the abandoned origin (plan-checker-flagged case) Tests E and H failed against the pre-fix code (confirmed during RED).
This commit is contained in:
parent
633b48c3b2
commit
6945591f76
2 changed files with 130 additions and 5 deletions
|
|
@ -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']);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<void> {
|
||||
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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue