From 4a68651edfa12d63672586e672e989b4154c4a35 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Mon, 19 Oct 2026 05:43:58 +1100 Subject: [PATCH 1/2] PM-6579: Resolve extension Work projects from challenge billing What was broken Extension deal details could still omit the shared Work project and its billing history after the earlier PM-6579 fix. Root cause The earlier fix added the Salesforce TC_Connect_Project_Id__c fallback, but an extension without a usable reference still needed a current project account match. Challenges already associate earlier billing accounts with the shared project, including after the project's current account changes. What was changed Preserve current-account and explicit-reference precedence. When neither resolves, use challenges billed to the opportunity's accounts to find a non-deleted project, with the existing updatedAt/id ordering. Retain the opportunity account and load the shared project's full challenge billing history. Update service documentation, the response DTO description, and README. Any added/updated tests Add eight cases covering absent, invalid and unresolved Salesforce references, multiple account/project candidates, deleted/missing projects, and lookup errors. Update existing precedence and no-match assertions. All four core extension regressions fail against the merged previous fix and pass with this change. pnpm lint and pnpm build pass. All 64 focused API tests and all 6 existing Sales modal rendering tests pass. The full unit suite has 502 passed, 3 skipped, and 15 failed; e2e has 73 passed, 29 skipped, and 8 failed. All failing test names match unchanged origin/dev at 47a0577; this change introduces no new failures. The existing failures concern metadata events, JWT handling, event fixtures, and permission checks. The screenshot account is unavailable in dev (404). --- README.md | 11 +- .../salesforce-opportunity-response.dto.ts | 2 +- .../salesforce-opportunity.service.spec.ts | 167 +++++++++++++++++- .../salesforce-opportunity.service.ts | 25 +++ 4 files changed, 198 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 6a0ed3c..4de0ab4 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,10 @@ Talent Manager JWT roles) returns the existing Salesforce fields plus: - `projectId`: string ID of a non-deleted project whose current `billingAccountId` matches the opportunity account. If none matches, the opportunity's explicit Salesforce `TC_Connect_Project_Id__c` resolves the shared Work project for - extensions. Returns `null` when neither lookup finds a non-deleted project. + extensions. If neither resolves, challenges billed to the opportunity's accounts + identify the shared project, including completed challenges whose account + differs from the project's current account. Returns `null` when none of these + lookups finds a non-deleted project. - `relatedBillingAccounts`: unique `{ id, name? }` summaries from every challenge associated with that project, sorted numerically. This includes inactive accounts and every challenge status. The current account appears in this array @@ -128,7 +131,11 @@ IDs and retains the opportunity's own billing account even when the shared proje uses a different account. It uses the same Work/Connect field as the Sales report, not the legacy Direct project ID or the Salesforce `Connect_Project__c` record ID; the Salesforce integration user must have read access to that field. Missing or -invalid references do not infer a project from opportunity names or customers. +invalid references still allow the challenge billing lookup; project relationships +are never inferred from opportunity names or customers. The challenge fallback +checks every matching opportunity account and every challenge status, ignores +challenges without a positive project ID, and applies the same project ordering +and deletion filter as the current-account lookup. Without an account, the project is `null` and history is `[]`. A billing/project/challenge lookup failure returns a sanitized 502, rather than claiming that no account/history exists. Historical metadata uses the existing diff --git a/src/api/salesforce/dto/salesforce-opportunity-response.dto.ts b/src/api/salesforce/dto/salesforce-opportunity-response.dto.ts index 574f408..9e80f5a 100644 --- a/src/api/salesforce/dto/salesforce-opportunity-response.dto.ts +++ b/src/api/salesforce/dto/salesforce-opportunity-response.dto.ts @@ -86,7 +86,7 @@ export class SalesforceOpportunityResponseDto { @ApiProperty({ description: - 'Non-deleted project matched by the current billing account, or by the opportunity Work/Connect project reference when no account matches; null when neither resolves.', + 'Non-deleted project matched by the current billing account, then the opportunity Work/Connect project reference, then challenges billed to the opportunity accounts; null when none resolves.', type: String, nullable: true, example: '12345', diff --git a/src/api/salesforce/salesforce-opportunity.service.spec.ts b/src/api/salesforce/salesforce-opportunity.service.spec.ts index c29424d..77a3492 100644 --- a/src/api/salesforce/salesforce-opportunity.service.spec.ts +++ b/src/api/salesforce/salesforce-opportunity.service.spec.ts @@ -174,7 +174,7 @@ describe('SalesforceOpportunityService', () => { expect(accountMetadata).not.toHaveBeenCalled(); }); - it('keeps the account when no non-deleted project currently uses it', async () => { + it('keeps the account when no project or challenge uses it', async () => { findAccounts.mockResolvedValue([{ id: '22', name: 'Current account' }]); const { service } = build(); await expect(service.getOpportunity(record.Id)).resolves.toMatchObject({ @@ -182,7 +182,15 @@ describe('SalesforceOpportunityService', () => { projectId: null, relatedBillingAccounts: [], }); - expect(findChallenges).not.toHaveBeenCalled(); + expect(findChallenges).toHaveBeenCalledTimes(1); + expect(findChallenges).toHaveBeenCalledWith({ + where: { + billingRecord: { billingAccountId: { in: ['22'] } }, + projectId: { gt: 0 }, + }, + select: { projectId: true }, + distinct: ['projectId'], + }); }); it('resolves all unique challenge accounts and retains IDs with missing metadata', async () => { @@ -293,6 +301,7 @@ describe('SalesforceOpportunityService', () => { where: { projectId: 123 }, select: { billingRecord: { select: { billingAccountId: true } } }, }); + expect(findChallenges).toHaveBeenCalledTimes(2); }, ); @@ -308,8 +317,156 @@ describe('SalesforceOpportunityService', () => { projectId: '456', }); expect(findProject).toHaveBeenCalledTimes(1); + expect(findChallenges).toHaveBeenCalledTimes(1); + expect(findChallenges).toHaveBeenCalledWith({ + where: { projectId: 456 }, + select: { billingRecord: { select: { billingAccountId: true } } }, + }); }); + it.each([undefined, null, '../123', '999'])( + 'resolves a billed extension to the original project when its Salesforce reference does not resolve (%j)', + async (reference) => { + const originalAccount = { id: '11', name: 'Original contract' }; + const extensionAccount = { id: '22', name: 'October extension' }; + const project = { id: BigInt(123), billingAccountId: BigInt(11) }; + const history = [ + { billingRecord: { billingAccountId: '11' } }, + { billingRecord: { billingAccountId: '22' } }, + ]; + findAccounts + .mockResolvedValueOnce([originalAccount]) + .mockResolvedValueOnce([extensionAccount]); + findProject.mockResolvedValueOnce(project).mockResolvedValueOnce(null); + if (reference === '999') findProject.mockResolvedValueOnce(null); + findProject.mockResolvedValueOnce(project); + findChallenges + .mockResolvedValueOnce(history) + .mockResolvedValueOnce([{ projectId: 123 }]) + .mockResolvedValueOnce(history); + accountMetadata.mockResolvedValue({ + '11': originalAccount, + '22': extensionAccount, + }); + const original = build(); + const extension = build([ + { + ...record, + Id: '006UN00000XamnuYAB', + StageName: 'Won - SOW Signed', + TC_Connect_Project_Id__c: reference, + }, + ]); + + const originalResponse = await original.service.getOpportunity(record.Id); + const extensionResponse = + await extension.service.getOpportunity('006UN00000XamnuYAB'); + + expect(extensionResponse).toMatchObject({ + billingAccount: extensionAccount, + projectId: originalResponse.projectId, + relatedBillingAccounts: [originalAccount, extensionAccount], + }); + expect(extensionResponse.projectId).toBe('123'); + // No status filter: completed challenges retain earlier billing accounts. + expect(findChallenges).toHaveBeenNthCalledWith(2, { + where: { + billingRecord: { billingAccountId: { in: ['22'] } }, + projectId: { gt: 0 }, + }, + select: { projectId: true }, + distinct: ['projectId'], + }); + expect(findProject).toHaveBeenLastCalledWith({ + where: { id: { in: [BigInt(123)] }, deletedAt: null }, + select: { id: true, billingAccountId: true }, + orderBy: [{ updatedAt: 'desc' }, { id: 'desc' }], + }); + expect(findChallenges).toHaveBeenLastCalledWith({ + where: { projectId: 123 }, + select: { billingRecord: { select: { billingAccountId: true } } }, + }); + }, + ); + + it('checks every opportunity account and orders matching historical projects consistently', async () => { + findAccounts.mockResolvedValue([ + { id: '99', name: 'Latest opportunity account' }, + { id: '22', name: 'Earlier opportunity account' }, + ]); + findChallenges.mockResolvedValueOnce([ + { projectId: 123 }, + { projectId: 456 }, + ]); + findProject.mockResolvedValueOnce(null).mockResolvedValueOnce({ + id: BigInt(456), + billingAccountId: BigInt(11), + }); + const { service } = build(); + + await expect(service.getOpportunity(record.Id)).resolves.toMatchObject({ + billingAccount: { id: '99', name: 'Latest opportunity account' }, + projectId: '456', + }); + expect(findChallenges).toHaveBeenNthCalledWith(1, { + where: { + billingRecord: { billingAccountId: { in: ['99', '22'] } }, + projectId: { gt: 0 }, + }, + select: { projectId: true }, + distinct: ['projectId'], + }); + expect(findProject).toHaveBeenLastCalledWith({ + where: { id: { in: [BigInt(123), BigInt(456)] }, deletedAt: null }, + select: { id: true, billingAccountId: true }, + orderBy: [{ updatedAt: 'desc' }, { id: 'desc' }], + }); + }); + + it('does not link a missing or deleted historical project', async () => { + findAccounts.mockResolvedValue([{ id: '22', name: 'Extension' }]); + findChallenges.mockResolvedValueOnce([{ projectId: 123 }]); + const { service } = build(); + + await expect(service.getOpportunity(record.Id)).resolves.toMatchObject({ + billingAccount: { id: '22', name: 'Extension' }, + projectId: null, + relatedBillingAccounts: [], + }); + expect(findProject).toHaveBeenLastCalledWith({ + where: { id: { in: [BigInt(123)] }, deletedAt: null }, + select: { id: true, billingAccountId: true }, + orderBy: [{ updatedAt: 'desc' }, { id: 'desc' }], + }); + expect(findChallenges).toHaveBeenCalledTimes(1); + expect(accountMetadata).not.toHaveBeenCalled(); + }); + + it.each(['challenge', 'project'])( + 'reports a failed historical %s lookup instead of hiding the Work link', + async (lookup) => { + findAccounts.mockResolvedValue([{ id: '22', name: 'Extension' }]); + if (lookup === 'challenge') { + findChallenges.mockRejectedValueOnce( + new Error('database connection failed'), + ); + } else { + findChallenges.mockResolvedValueOnce([{ projectId: 123 }]); + findProject + .mockResolvedValueOnce(null) + .mockRejectedValueOnce(new Error('database connection failed')); + } + const { service } = build(); + + await expect(service.getOpportunity(record.Id)).rejects.toThrow( + new BadGatewayException( + 'Unable to load opportunity project billing history.', + ), + ); + expect(accountMetadata).not.toHaveBeenCalled(); + }, + ); + it.each([ undefined, null, @@ -337,7 +494,8 @@ describe('SalesforceOpportunityService', () => { relatedBillingAccounts: [], }); expect(findProject).toHaveBeenCalledTimes(1); - expect(findChallenges).not.toHaveBeenCalled(); + expect(findChallenges).toHaveBeenCalledTimes(1); + expect(accountMetadata).not.toHaveBeenCalled(); }, ); @@ -354,7 +512,8 @@ describe('SalesforceOpportunityService', () => { where: { id: BigInt(123), deletedAt: null }, select: { id: true, billingAccountId: true }, }); - expect(findChallenges).not.toHaveBeenCalled(); + expect(findChallenges).toHaveBeenCalledTimes(1); + expect(accountMetadata).not.toHaveBeenCalled(); }); it('reports a failed project reference lookup instead of hiding the Work link', async () => { diff --git a/src/api/salesforce/salesforce-opportunity.service.ts b/src/api/salesforce/salesforce-opportunity.service.ts index 9e7674d..ab528fc 100644 --- a/src/api/salesforce/salesforce-opportunity.service.ts +++ b/src/api/salesforce/salesforce-opportunity.service.ts @@ -75,6 +75,8 @@ export class SalesforceOpportunityService { * Resolves an opportunity's current account and the project's lifetime accounts. * For multiple matches, uses the most recently updated non-deleted project; * if none matches, falls back to the opportunity's explicit Work project ID. + * If that does not resolve, finds projects through challenges billed to the + * opportunity's accounts, using the same project ordering and deletion filter. * The opportunity's account is retained even if the shared project uses another. * Without a matching project account, uses the highest account ID from the listing. * Challenge status and account active status do not restrict history. @@ -133,6 +135,29 @@ export class SalesforceOpportunityService { select: { id: true, billingAccountId: true }, }); } + if (!project) { + const billedChallenges = + await getChallengesPrismaClient().challenge.findMany({ + where: { + billingRecord: { + billingAccountId: { in: accounts.map((account) => account.id) }, + }, + projectId: { gt: 0 }, + }, + select: { projectId: true }, + distinct: ['projectId'], + }); + const projectIds = billedChallenges.flatMap((challenge) => + challenge.projectId === null ? [] : [BigInt(challenge.projectId)], + ); + if (projectIds.length) { + project = await this.prisma.project.findFirst({ + where: { id: { in: projectIds }, deletedAt: null }, + select: { id: true, billingAccountId: true }, + orderBy: [{ updatedAt: 'desc' }, { id: 'desc' }], + }); + } + } const billingAccount = accounts.find( (account) => account.id === project?.billingAccountId?.toString(), From a6d82a886f74ce5392087a5d8c5faec5b0512967 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Mon, 19 Oct 2026 06:27:36 +1100 Subject: [PATCH 2/2] PM-6598: Accept AMPE when saving and publishing showcases What was broken Work showcase creation, editing, and publishing rejected the AMPE SMU option with "Invalid SMU." Root cause The Work forms use AMPE, but shared project metadata validation still accepted APME and normalized APMEA to that outdated code. What was changed Use AMPE as the supported value and normalize legacy APME/APMEA values to AMPE. The shared mapping also keeps Salesforce opportunity imports aligned with Work. Update the showcase metadata documentation. Any added/updated tests Add 12 cases covering current SMU options, legacy aliases, draft/published showcase creation and updates, and Salesforce imports. Confirm the regression tests fail before the fix. Validation: pnpm lint and pnpm build passed. The targeted showcase, project, metadata, and Salesforce suites passed all 185 tests, including PostgreSQL persistence tests. Full pnpm test --runInBand and pnpm test:e2e --runInBand reproduced the same 15 unit and 8 end-to-end failures on untouched dev; no new failures were introduced. --- docs/showcase-win.md | 7 +- .../project-showcase-post.service.spec.ts | 72 +++++++++++++++++++ .../salesforce-opportunity.service.spec.ts | 13 ++++ .../utils/showcase-metadata.utils.spec.ts | 17 ++++- src/shared/utils/showcase-metadata.utils.ts | 13 ++-- 5 files changed, 112 insertions(+), 10 deletions(-) diff --git a/docs/showcase-win.md b/docs/showcase-win.md index fcbd081..1666971 100644 --- a/docs/showcase-win.md +++ b/docs/showcase-win.md @@ -10,8 +10,11 @@ The generated Prisma package is updated with the new fields. `Project.details` stores `customer`, `smu`, `smuOther`, and `dealCloseDate`. Project create/update validates supplied values while accepting projects with no showcase metadata. Strings are trimmed; Customer and custom SMU have a 255-character -limit. SMU accepts APMEA, Europe, Americas1, Americas2, or Others. Others requires -`smuOther`; a standard SMU clears a previous custom value. Dates must be valid +limit. SMU accepts AMPE, EURP, AMR1, AMR2, or Others, matching the Work forms. +Legacy values remain accepted and are normalized on save: APME/APMEA to AMPE, +Europe to EURP, Americas1 to AMR1, and Americas2 to AMR2. The same mapping applies +to imported Salesforce opportunities. Others requires `smuOther`; a standard +SMU clears a previous custom value. Dates must be valid `YYYY-MM-DD` calendar dates (timestamps and impossible dates are rejected). Post create/update accepts these four fields at the top level. They are merged diff --git a/src/api/project-showcase-post/project-showcase-post.service.spec.ts b/src/api/project-showcase-post/project-showcase-post.service.spec.ts index d10785b..d568171 100644 --- a/src/api/project-showcase-post/project-showcase-post.service.spec.ts +++ b/src/api/project-showcase-post/project-showcase-post.service.spec.ts @@ -351,6 +351,43 @@ describe('ProjectShowcasePostService', () => { ); }); + it.each(['DRAFT', 'PUBLISHED'] as const)( + 'creates an AMPE showcase with status %s', + async (status) => { + prismaMock.projectShowcasePost.create.mockResolvedValue( + buildPostRecord({ status }), + ); + + const response = await service.createPost( + '1001', + { + type: 'Private POD Delivery', + title: 'AMPE showcase', + content: 'Solution', + smu: 'AMPE', + status, + }, + user, + ); + + expect(prismaMock.project.update).toHaveBeenLastCalledWith({ + where: { id: BigInt(1001) }, + data: expect.objectContaining({ + details: { + customer: 'Example', + smu: 'AMPE', + smuOther: '', + dealCloseDate: '2026-09-16', + }, + }), + }); + expect(prismaMock.projectShowcasePost.create).toHaveBeenCalledWith( + expect.objectContaining({ data: expect.objectContaining({ status }) }), + ); + expect(response.status).toBe(status); + }, + ); + it('creates a new project showcase post with media assets', async () => { prismaMock.projectShowcasePost.create.mockResolvedValue( buildPostRecord({ @@ -507,6 +544,41 @@ describe('ProjectShowcasePostService', () => { expect(response.title).toBe('Updated title'); }); + it.each(['DRAFT', 'PUBLISHED'] as const)( + 'saves an existing showcase with AMPE and status %s', + async (status) => { + prismaMock.projectShowcasePost.findFirst.mockResolvedValue( + buildPostRecord(), + ); + prismaMock.projectShowcasePost.update.mockResolvedValue( + buildPostRecord({ status }), + ); + + const response = await service.updatePost( + '1001', + '10', + { smu: 'AMPE', status }, + user, + ); + + expect(prismaMock.project.update).toHaveBeenLastCalledWith({ + where: { id: BigInt(1001) }, + data: expect.objectContaining({ + details: { + customer: 'Example', + smu: 'AMPE', + smuOther: '', + dealCloseDate: '2026-09-16', + }, + }), + }); + expect(prismaMock.projectShowcasePost.update).toHaveBeenCalledWith( + expect.objectContaining({ data: expect.objectContaining({ status }) }), + ); + expect(response.status).toBe(status); + }, + ); + it('updates a post with media assets', async () => { prismaMock.projectShowcasePost.findFirst.mockResolvedValue( buildPostRecord(), diff --git a/src/api/salesforce/salesforce-opportunity.service.spec.ts b/src/api/salesforce/salesforce-opportunity.service.spec.ts index c29424d..6e61484 100644 --- a/src/api/salesforce/salesforce-opportunity.service.spec.ts +++ b/src/api/salesforce/salesforce-opportunity.service.spec.ts @@ -101,6 +101,19 @@ describe('SalesforceOpportunityService', () => { ).resolves.toMatchObject({ smu: 'AMR2', reportingSmu: 'Americas2' }); }); + it.each(['AMPE', 'APME', 'APMEA'])( + 'maps Reporting SMU %s to the current AMPE option', + async (reportingSmu) => { + const { service } = build([ + { ...record, Reporting_SMU__c: reportingSmu }, + ]); + + await expect( + service.getOpportunity('006UN00000XamntYAB'), + ).resolves.toMatchObject({ smu: 'AMPE', reportingSmu }); + }, + ); + it('surfaces an unrecognized Reporting SMU as a custom value', async () => { const { service } = build([{ ...record, Reporting_SMU__c: 'INTERNAL' }]); diff --git a/src/shared/utils/showcase-metadata.utils.spec.ts b/src/shared/utils/showcase-metadata.utils.spec.ts index 210e660..e4628bd 100644 --- a/src/shared/utils/showcase-metadata.utils.spec.ts +++ b/src/shared/utils/showcase-metadata.utils.spec.ts @@ -52,8 +52,23 @@ describe('shared showcase project metadata', () => { ).toBe(''); }); + it.each(['AMPE', 'EURP', 'AMR1', 'AMR2'])( + 'accepts the current Work SMU option %s', + (smu) => { + expect( + normalizeShowcaseProjectMetadata({ ...metadata, smu }, true), + ).toEqual({ + customer: 'Customer', + smu, + smuOther: '', + dealCloseDate: '2024-02-29', + }); + }, + ); + it.each([ - ['APMEA', 'APME'], + ['APME', 'AMPE'], + ['APMEA', 'AMPE'], ['Europe', 'EURP'], ['Americas1', 'AMR1'], ['Americas2', 'AMR2'], diff --git a/src/shared/utils/showcase-metadata.utils.ts b/src/shared/utils/showcase-metadata.utils.ts index 8b5dafc..6ab2fb2 100644 --- a/src/shared/utils/showcase-metadata.utils.ts +++ b/src/shared/utils/showcase-metadata.utils.ts @@ -1,21 +1,20 @@ import { BadRequestException } from '@nestjs/common'; /** - * Supported SMU options. - * - * These match the Salesforce `Opportunity.Reporting_SMU__c` codes so an - * imported opportunity maps onto a project without translation. + * Supported SMU options, matching the Work project and showcase forms. + * Legacy project labels and Salesforce codes are normalized to these values. */ -export const SMU_VALUES = ['APME', 'EURP', 'AMR1', 'AMR2', 'Others']; +export const SMU_VALUES = ['AMPE', 'EURP', 'AMR1', 'AMR2', 'Others']; /** - * SMU labels used before the Salesforce naming alignment. + * Legacy SMU labels, including the earlier APME code for AMPE. * * Projects saved with the old labels are upgraded in place the next time their * details are written, so historical records stay valid. */ export const LEGACY_SMU_VALUES: Readonly> = { - APMEA: 'APME', + APME: 'AMPE', + APMEA: 'AMPE', Europe: 'EURP', Americas1: 'AMR1', Americas2: 'AMR2',