From c1bf73e4eb51a952575604bd16acdb0e0002064d Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Fri, 10 Apr 2026 13:39:10 +1000 Subject: [PATCH 01/10] PM-4800: align engagement report country with profile What was broken The Engagement Data report could export the wrong country or null for some members because it did not follow the same location precedence as the Profile app. Root cause The engagement member enrichment SQL only resolved country from competition-country data and the denormalized member country field, so it ignored home-country data that the Profile app prefers. What was changed Updated the engagement member enrichment SQL to resolve country names through the lookups country table with homeCountryCode first, then competitionCountryCode, then the stored member country fallback. Updated the engagement report service documentation to reflect the profile-aligned country precedence. Any added/updated tests Added src/reports/topcoder/topcoder-reports.sql.spec.ts to lock the Engagement Data country precedence in focused SQL regression coverage. --- .../topcoder/engagement-data-members.sql | 21 +++++++++++++++---- .../topcoder/topcoder-reports.service.ts | 3 ++- .../topcoder/topcoder-reports.sql.spec.ts | 13 ++++++++++++ 3 files changed, 32 insertions(+), 5 deletions(-) create mode 100644 src/reports/topcoder/topcoder-reports.sql.spec.ts diff --git a/sql/reports/topcoder/engagement-data-members.sql b/sql/reports/topcoder/engagement-data-members.sql index 383e7a5..019f970 100644 --- a/sql/reports/topcoder/engagement-data-members.sql +++ b/sql/reports/topcoder/engagement-data-members.sql @@ -4,8 +4,15 @@ SELECT NULLIF(BTRIM(m."firstName"), '') AS first_name, NULLIF(BTRIM(m."lastName"), '') AS last_name, NULLIF(BTRIM(m.email), '') AS email, - COALESCE(NULLIF(BTRIM(c.country_name), ''), NULLIF(BTRIM(m.country), '')) - AS country, + COALESCE( + home_code.name, + home_id.name, + comp_code.name, + comp_id.name, + NULLIF(BTRIM(m."homeCountryCode"), ''), + NULLIF(BTRIM(m."competitionCountryCode"), ''), + NULLIF(BTRIM(m.country), '') + ) AS country, preferred_address.street_addr_1, preferred_address.street_addr_2, preferred_address.city, @@ -13,8 +20,14 @@ SELECT preferred_address.zip, preferred_phone.phone_number FROM members.member m -LEFT JOIN identity.country c - ON c.iso_alpha3_code = m."competitionCountryCode" +LEFT JOIN lookups."Country" AS home_code + ON UPPER(home_code."countryCode") = UPPER(m."homeCountryCode") +LEFT JOIN lookups."Country" AS home_id + ON UPPER(home_id.id) = UPPER(m."homeCountryCode") +LEFT JOIN lookups."Country" AS comp_code + ON UPPER(comp_code."countryCode") = UPPER(m."competitionCountryCode") +LEFT JOIN lookups."Country" AS comp_id + ON UPPER(comp_id.id) = UPPER(m."competitionCountryCode") LEFT JOIN LATERAL ( SELECT NULLIF(BTRIM(a."streetAddr1"), '') AS street_addr_1, diff --git a/src/reports/topcoder/topcoder-reports.service.ts b/src/reports/topcoder/topcoder-reports.service.ts index 81beb45..79e0bf0 100644 --- a/src/reports/topcoder/topcoder-reports.service.ts +++ b/src/reports/topcoder/topcoder-reports.service.ts @@ -645,7 +645,8 @@ export class TopcoderReportsService implements OnModuleDestroy { * * The base member list comes from the engagements database, while member * profile/contact fields and project names are resolved directly from the - * main reports database so the export stays DB-only. + * main reports database so the export stays DB-only. Country resolution + * follows the same home-country-first fallback order used by the profile UI. * * @returns One row per member with the engagement experience summary fields. * @throws Error when the engagements database URL is not configured. diff --git a/src/reports/topcoder/topcoder-reports.sql.spec.ts b/src/reports/topcoder/topcoder-reports.sql.spec.ts new file mode 100644 index 0000000..2a0f2bf --- /dev/null +++ b/src/reports/topcoder/topcoder-reports.sql.spec.ts @@ -0,0 +1,13 @@ +import { SqlLoaderService } from "src/common/sql-loader.service"; + +describe("Topcoder report SQL", () => { + const sqlLoader = new SqlLoaderService(); + + it("uses profile-style country precedence for engagement data members", () => { + const sql = sqlLoader.load("reports/topcoder/engagement-data-members.sql"); + + expect(sql).toMatch( + /COALESCE\(\s*home_code\.name,\s*home_id\.name,\s*comp_code\.name,\s*comp_id\.name,\s*NULLIF\(BTRIM\(m\."homeCountryCode"\), ''\),\s*NULLIF\(BTRIM\(m\."competitionCountryCode"\), ''\),\s*NULLIF\(BTRIM\(m\.country\), ''\)\s*\)\s+AS country/, + ); + }); +}); From 614605668fcd321073a7f856b2f876fed7fdb15c Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Fri, 10 Apr 2026 15:48:57 +1000 Subject: [PATCH 02/10] PM-4800: stop engagement report from using stale country fallback What was broken The Engagement Data report could still show the wrong country or null for some members after the first QA follow-up. Members whose profile location resolved from home or competition country codes could still export the stale legacy country value instead. Root cause The previous fix expanded the SQL lookup coverage, but it still collapsed country resolution into a single SQL field that fell back to members.member.country. That legacy field can be stale and does not match how the profile UI resolves member location. What was changed Split engagement member enrichment into separate home-country and competition-country resolution fields plus the raw profile country codes. Resolved the final export country in the report service with the same home-country-first precedence the profile UI uses, and removed the stale members.member.country fallback. Expanded SQL coverage to resolve country names through both lookups.Country and identity.country so legacy alpha-2, alpha-3, and lookup-id values still map correctly. Updated the report documentation to match the current country sources. Any added/updated tests Updated src/reports/topcoder/topcoder-reports.service.spec.ts to cover the profile-style country precedence for the report output. Updated src/reports/topcoder/topcoder-reports.sql.spec.ts to lock the SQL against falling back to members.member.country. --- README.md | 2 +- .../topcoder/engagement-data-members.sql | 51 +++++++++++++------ .../member/member-search.controller.ts | 16 +++++- .../member/member-search.service.spec.ts | 12 +++-- .../topcoder/topcoder-reports.service.spec.ts | 16 ++++-- .../topcoder/topcoder-reports.service.ts | 28 ++++++++-- .../topcoder/topcoder-reports.sql.spec.ts | 8 ++- 7 files changed, 101 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 9f2673e..d53e594 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,7 @@ ENGAGEMENTS_DB_URL="postgresql://user:password@localhost:5432/engagements" # The same report also reads member/profile/project data from the main # DATABASE_URL connection, including members.member, members.memberAddress, -# members.memberPhone, identity.country, and projects.projects. +# members.memberPhone, identity.country, lookups.Country, and projects.projects. # Old tc-payments database URL (used by member-tax CSV export script) OLD_PAYMENTS_DATABASE_URL="postgresql://user:password@localhost:5432/tc_payments?schema=public" diff --git a/sql/reports/topcoder/engagement-data-members.sql b/sql/reports/topcoder/engagement-data-members.sql index 019f970..c057ec1 100644 --- a/sql/reports/topcoder/engagement-data-members.sql +++ b/sql/reports/topcoder/engagement-data-members.sql @@ -5,14 +5,21 @@ SELECT NULLIF(BTRIM(m."lastName"), '') AS last_name, NULLIF(BTRIM(m.email), '') AS email, COALESCE( - home_code.name, - home_id.name, - comp_code.name, - comp_id.name, - NULLIF(BTRIM(m."homeCountryCode"), ''), - NULLIF(BTRIM(m."competitionCountryCode"), ''), - NULLIF(BTRIM(m.country), '') - ) AS country, + NULLIF(BTRIM(home_lookup_code.name), ''), + NULLIF(BTRIM(home_lookup_id.name), ''), + NULLIF(BTRIM(home_identity_alpha3.country_name), ''), + NULLIF(BTRIM(home_identity_code.country_name), ''), + NULLIF(BTRIM(home_identity_alpha2.country_name), '') + ) AS home_country, + COALESCE( + NULLIF(BTRIM(comp_lookup_code.name), ''), + NULLIF(BTRIM(comp_lookup_id.name), ''), + NULLIF(BTRIM(comp_identity_alpha3.country_name), ''), + NULLIF(BTRIM(comp_identity_code.country_name), ''), + NULLIF(BTRIM(comp_identity_alpha2.country_name), '') + ) AS competition_country, + NULLIF(BTRIM(m."homeCountryCode"), '') AS home_country_code, + NULLIF(BTRIM(m."competitionCountryCode"), '') AS competition_country_code, preferred_address.street_addr_1, preferred_address.street_addr_2, preferred_address.city, @@ -20,14 +27,26 @@ SELECT preferred_address.zip, preferred_phone.phone_number FROM members.member m -LEFT JOIN lookups."Country" AS home_code - ON UPPER(home_code."countryCode") = UPPER(m."homeCountryCode") -LEFT JOIN lookups."Country" AS home_id - ON UPPER(home_id.id) = UPPER(m."homeCountryCode") -LEFT JOIN lookups."Country" AS comp_code - ON UPPER(comp_code."countryCode") = UPPER(m."competitionCountryCode") -LEFT JOIN lookups."Country" AS comp_id - ON UPPER(comp_id.id) = UPPER(m."competitionCountryCode") +LEFT JOIN lookups."Country" AS home_lookup_code + ON UPPER(home_lookup_code."countryCode") = UPPER(BTRIM(m."homeCountryCode")) +LEFT JOIN lookups."Country" AS home_lookup_id + ON UPPER(home_lookup_id.id) = UPPER(BTRIM(m."homeCountryCode")) +LEFT JOIN identity.country AS home_identity_alpha3 + ON UPPER(home_identity_alpha3.iso_alpha3_code) = UPPER(BTRIM(m."homeCountryCode")) +LEFT JOIN identity.country AS home_identity_code + ON UPPER(home_identity_code.country_code) = UPPER(BTRIM(m."homeCountryCode")) +LEFT JOIN identity.country AS home_identity_alpha2 + ON UPPER(home_identity_alpha2.iso_alpha2_code) = UPPER(BTRIM(m."homeCountryCode")) +LEFT JOIN lookups."Country" AS comp_lookup_code + ON UPPER(comp_lookup_code."countryCode") = UPPER(BTRIM(m."competitionCountryCode")) +LEFT JOIN lookups."Country" AS comp_lookup_id + ON UPPER(comp_lookup_id.id) = UPPER(BTRIM(m."competitionCountryCode")) +LEFT JOIN identity.country AS comp_identity_alpha3 + ON UPPER(comp_identity_alpha3.iso_alpha3_code) = UPPER(BTRIM(m."competitionCountryCode")) +LEFT JOIN identity.country AS comp_identity_code + ON UPPER(comp_identity_code.country_code) = UPPER(BTRIM(m."competitionCountryCode")) +LEFT JOIN identity.country AS comp_identity_alpha2 + ON UPPER(comp_identity_alpha2.iso_alpha2_code) = UPPER(BTRIM(m."competitionCountryCode")) LEFT JOIN LATERAL ( SELECT NULLIF(BTRIM(a."streetAddr1"), '') AS street_addr_1, diff --git a/src/reports/member/member-search.controller.ts b/src/reports/member/member-search.controller.ts index 6c42b0c..5702a98 100644 --- a/src/reports/member/member-search.controller.ts +++ b/src/reports/member/member-search.controller.ts @@ -1,5 +1,17 @@ -import { Body, Controller, HttpCode, HttpStatus, Post, UseGuards } from "@nestjs/common"; -import { ApiBearerAuth, ApiOperation, ApiResponse, ApiTags } from "@nestjs/swagger"; +import { + Body, + Controller, + HttpCode, + HttpStatus, + Post, + UseGuards, +} from "@nestjs/common"; +import { + ApiBearerAuth, + ApiOperation, + ApiResponse, + ApiTags, +} from "@nestjs/swagger"; import { MemberSearchBodyDto } from "./dto/member-search.dto"; import { MemberSearchResponseDto } from "./dto/member-search-response.dto"; import { MemberSearchService } from "./member-search.service"; diff --git a/src/reports/member/member-search.service.spec.ts b/src/reports/member/member-search.service.spec.ts index 13dca55..faf35c6 100644 --- a/src/reports/member/member-search.service.spec.ts +++ b/src/reports/member/member-search.service.spec.ts @@ -56,7 +56,9 @@ describe("MemberSearchService", () => { expect(dataSql).toContain("WITH recently_active AS"); expect(dataSql).not.toContain("requested_skills AS"); - expect(dataSql).toContain('ORDER BY "matchIndex" DESC NULLS LAST, m.handle ASC'); + expect(dataSql).toContain( + 'ORDER BY "matchIndex" DESC NULLS LAST, m.handle ASC', + ); expect(countSql).toContain("SELECT COUNT(*)::integer AS total"); @@ -85,7 +87,9 @@ describe("MemberSearchService", () => { }); it("uses filter params for count query but excludes pagination params", async () => { - mockDbService.query.mockResolvedValueOnce([]).mockResolvedValueOnce([{ total: 0 }]); + mockDbService.query + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ total: 0 }]); await service.search({ country: "us", @@ -99,7 +103,9 @@ describe("MemberSearchService", () => { const dataParams = mockDbService.query.mock.calls[0][1] as unknown[]; const countParams = mockDbService.query.mock.calls[1][1] as unknown[]; - expect(dataSql).toContain('ORDER BY m.handle ASC, "matchIndex" DESC NULLS LAST'); + expect(dataSql).toContain( + 'ORDER BY m.handle ASC, "matchIndex" DESC NULLS LAST', + ); expect(dataParams).toEqual(["us", 5, 5]); expect(countParams).toEqual(["us"]); }); diff --git a/src/reports/topcoder/topcoder-reports.service.spec.ts b/src/reports/topcoder/topcoder-reports.service.spec.ts index b5a871c..442ee44 100644 --- a/src/reports/topcoder/topcoder-reports.service.spec.ts +++ b/src/reports/topcoder/topcoder-reports.service.spec.ts @@ -21,7 +21,10 @@ describe("TopcoderReportsService", () => { first_name: "Ada", last_name: "Lovelace", email: "ada@example.com", - country: "United States", + home_country: null, + competition_country: null, + home_country_code: "JPN", + competition_country_code: null, street_addr_1: "1 Main St", street_addr_2: null, city: "New York", @@ -35,7 +38,10 @@ describe("TopcoderReportsService", () => { first_name: null, last_name: null, email: null, - country: "Canada", + home_country: null, + competition_country: "Sri Lanka", + home_country_code: null, + competition_country_code: "LKA", street_addr_1: null, street_addr_2: null, city: null, @@ -114,13 +120,13 @@ describe("TopcoderReportsService", () => { jest.clearAllMocks(); }); - it("builds engagement data rows with DB-backed enrichment, fallbacks, and project names", async () => { + it("builds engagement data rows with profile-style country resolution, fallbacks, and project names", async () => { await expect(service.getEngagementData()).resolves.toEqual([ { handle: "assigned_user", firstName: "Ada", lastName: "Lovelace", - country: "United States", + country: "Japan", emailId: "ada@example.com", phoneNumber: "+1 555 0101", address: "1 Main St, New York, NY, 10001", @@ -131,7 +137,7 @@ describe("TopcoderReportsService", () => { handle: "applicant_user", firstName: "Grace", lastName: "Hopper", - country: "Canada", + country: "Sri Lanka", emailId: "applicant@example.com", phoneNumber: "222-222-2222", address: "Applicant Address", diff --git a/src/reports/topcoder/topcoder-reports.service.ts b/src/reports/topcoder/topcoder-reports.service.ts index 79e0bf0..eb884db 100644 --- a/src/reports/topcoder/topcoder-reports.service.ts +++ b/src/reports/topcoder/topcoder-reports.service.ts @@ -134,7 +134,10 @@ type EngagementMemberRow = { first_name: string | null; last_name: string | null; email: string | null; - country: string | null; + home_country: string | null; + competition_country: string | null; + home_country_code: string | null; + competition_country_code: string | null; street_addr_1: string | null; street_addr_2: string | null; city: string | null; @@ -646,7 +649,8 @@ export class TopcoderReportsService implements OnModuleDestroy { * The base member list comes from the engagements database, while member * profile/contact fields and project names are resolved directly from the * main reports database so the export stays DB-only. Country resolution - * follows the same home-country-first fallback order used by the profile UI. + * follows the same home-country-first profile fields used by the profile UI + * and avoids the stale legacy members.member.country fallback. * * @returns One row per member with the engagement experience summary fields. * @throws Error when the engagements database URL is not configured. @@ -702,7 +706,7 @@ export class TopcoderReportsService implements OnModuleDestroy { this.toOptionalString(member?.first_name) ?? parsedName.firstName, lastName: this.toOptionalString(member?.last_name) ?? parsedName.lastName, - country: this.toOptionalString(member?.country), + country: this.resolveEngagementMemberCountry(member), emailId: this.toOptionalString(member?.email) ?? row.application_email ?? null, phoneNumber: @@ -945,6 +949,24 @@ export class TopcoderReportsService implements OnModuleDestroy { return membersById; } + /** + * Resolves the report country using the same home-country-first fields the + * profile UI uses for member location display. + * + * @param member DB-backed member enrichment row for the engagement report. + * @returns Resolved country name or null when the profile has no known country. + */ + private resolveEngagementMemberCountry( + member?: EngagementMemberRow, + ): string | null { + return ( + this.toOptionalString(member?.home_country) ?? + alpha3ToCountryName(member?.home_country_code) ?? + this.toOptionalString(member?.competition_country) ?? + alpha3ToCountryName(member?.competition_country_code) + ); + } + /** * Resolves project names for the assigned project ids included in the report. * diff --git a/src/reports/topcoder/topcoder-reports.sql.spec.ts b/src/reports/topcoder/topcoder-reports.sql.spec.ts index 2a0f2bf..1c7df50 100644 --- a/src/reports/topcoder/topcoder-reports.sql.spec.ts +++ b/src/reports/topcoder/topcoder-reports.sql.spec.ts @@ -3,11 +3,15 @@ import { SqlLoaderService } from "src/common/sql-loader.service"; describe("Topcoder report SQL", () => { const sqlLoader = new SqlLoaderService(); - it("uses profile-style country precedence for engagement data members", () => { + it("resolves engagement data countries from profile location codes instead of the stale legacy field", () => { const sql = sqlLoader.load("reports/topcoder/engagement-data-members.sql"); expect(sql).toMatch( - /COALESCE\(\s*home_code\.name,\s*home_id\.name,\s*comp_code\.name,\s*comp_id\.name,\s*NULLIF\(BTRIM\(m\."homeCountryCode"\), ''\),\s*NULLIF\(BTRIM\(m\."competitionCountryCode"\), ''\),\s*NULLIF\(BTRIM\(m\.country\), ''\)\s*\)\s+AS country/, + /COALESCE\(\s*NULLIF\(BTRIM\(home_lookup_code\.name\), ''\),\s*NULLIF\(BTRIM\(home_lookup_id\.name\), ''\),\s*NULLIF\(BTRIM\(home_identity_alpha3\.country_name\), ''\),\s*NULLIF\(BTRIM\(home_identity_code\.country_name\), ''\),\s*NULLIF\(BTRIM\(home_identity_alpha2\.country_name\), ''\)\s*\)\s+AS home_country/, ); + expect(sql).toMatch( + /COALESCE\(\s*NULLIF\(BTRIM\(comp_lookup_code\.name\), ''\),\s*NULLIF\(BTRIM\(comp_lookup_id\.name\), ''\),\s*NULLIF\(BTRIM\(comp_identity_alpha3\.country_name\), ''\),\s*NULLIF\(BTRIM\(comp_identity_code\.country_name\), ''\),\s*NULLIF\(BTRIM\(comp_identity_alpha2\.country_name\), ''\)\s*\)\s+AS competition_country/, + ); + expect(sql).not.toMatch(/NULLIF\(BTRIM\(m\.country\), ''\)/); }); }); From 358d3c95b8a8b86113108d6acf886cced416e154 Mon Sep 17 00:00:00 2001 From: himaniraghav3 Date: Tue, 21 Apr 2026 16:56:37 +0530 Subject: [PATCH 03/10] PM-4886 Allow multiple countries in member search --- .circleci/config.yml | 1 + src/reports/member/dto/member-search.dto.ts | 12 ++++++++---- .../member/member-search.controller.spec.ts | 2 +- src/reports/member/member-search.service.spec.ts | 6 +++--- src/reports/member/member-search.service.ts | 16 ++++++++++++---- 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 10c4841..6a40302 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -69,6 +69,7 @@ workflows: - PM-4490 - PM-4491-fix - PM-3497_talent-search + - PM-4886 # Production builds are exectuted only on tagged commits to the # master branch. diff --git a/src/reports/member/dto/member-search.dto.ts b/src/reports/member/dto/member-search.dto.ts index e0f36fa..1e730ae 100644 --- a/src/reports/member/dto/member-search.dto.ts +++ b/src/reports/member/dto/member-search.dto.ts @@ -8,6 +8,7 @@ import { IsOptional, IsString, IsUUID, + ArrayNotEmpty, Max, Min, ValidateNested, @@ -77,12 +78,15 @@ export class MemberSearchBodyDto { @ApiPropertyOptional({ description: - "Filter by country name or code as stored in the member location (case-insensitive).", - example: "Australia", + "Filter by multiple country names or country codes (case-insensitive).", + type: [String], + example: ["US", "Australia"], }) @IsOptional() - @IsString() - country?: string; + @IsArray() + @ArrayNotEmpty() + @IsString({ each: true }) + countries?: string[]; @ApiPropertyOptional({ description: diff --git a/src/reports/member/member-search.controller.spec.ts b/src/reports/member/member-search.controller.spec.ts index 1a79ef4..0c1b3f0 100644 --- a/src/reports/member/member-search.controller.spec.ts +++ b/src/reports/member/member-search.controller.spec.ts @@ -33,7 +33,7 @@ describe("MemberSearchController", () => { it("delegates search requests to the service and returns response", async () => { const body = { - country: "Australia", + countries: ["Australia"], sortBy: "handle" as const, sortOrder: "asc" as const, page: 2, diff --git a/src/reports/member/member-search.service.spec.ts b/src/reports/member/member-search.service.spec.ts index faf35c6..0d43800 100644 --- a/src/reports/member/member-search.service.spec.ts +++ b/src/reports/member/member-search.service.spec.ts @@ -92,7 +92,7 @@ describe("MemberSearchService", () => { .mockResolvedValueOnce([{ total: 0 }]); await service.search({ - country: "us", + countries: ["us"], page: 2, limit: 5, sortBy: "handle", @@ -106,8 +106,8 @@ describe("MemberSearchService", () => { expect(dataSql).toContain( 'ORDER BY m.handle ASC, "matchIndex" DESC NULLS LAST', ); - expect(dataParams).toEqual(["us", 5, 5]); - expect(countParams).toEqual(["us"]); + expect(dataParams).toEqual([["us"], 5, 5]); + expect(countParams).toEqual([["us"]]); }); it("deduplicates skills and keeps last wins value when building skill query", async () => { diff --git a/src/reports/member/member-search.service.ts b/src/reports/member/member-search.service.ts index 2470f8a..72f45fc 100644 --- a/src/reports/member/member-search.service.ts +++ b/src/reports/member/member-search.service.ts @@ -31,7 +31,7 @@ export class MemberSearchService { openToWork, recentlyActive, verifiedProfile, - country, + countries, sortBy = "matchIndex", sortOrder = "desc", page = 1, @@ -201,10 +201,18 @@ member_address AS ( ); } - if (country) { - const pCountry = p(country); + const normalizedCountries = Array.isArray(countries) + ? countries.map((value) => String(value).trim()).filter(Boolean) + : []; + + if (normalizedCountries.length > 0) { + const pCountries = p(normalizedCountries); where.push( - `(LOWER(m."homeCountryCode") = LOWER(${pCountry}) OR LOWER(m."competitionCountryCode") = LOWER(${pCountry}) OR LOWER(m.country) = LOWER(${pCountry}))`, + `( + LOWER(m."homeCountryCode") = ANY(SELECT LOWER(c) FROM unnest(${pCountries}::text[]) AS c) + OR LOWER(m."competitionCountryCode") = ANY(SELECT LOWER(c) FROM unnest(${pCountries}::text[]) AS c) + OR LOWER(m.country) = ANY(SELECT LOWER(c) FROM unnest(${pCountries}::text[]) AS c) + )`, ); } From 08076111b694a93cdb6a4619906b34570b497695 Mon Sep 17 00:00:00 2001 From: himaniraghav3 Date: Wed, 22 Apr 2026 00:46:46 +0530 Subject: [PATCH 04/10] PM-4886 Fix PR feedback --- src/reports/member/dto/member-search.dto.ts | 2 -- .../member/member-search.service.spec.ts | 17 +++++++++++++++++ src/reports/member/member-search.service.ts | 14 ++++++++++---- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/reports/member/dto/member-search.dto.ts b/src/reports/member/dto/member-search.dto.ts index 1e730ae..d84e299 100644 --- a/src/reports/member/dto/member-search.dto.ts +++ b/src/reports/member/dto/member-search.dto.ts @@ -8,7 +8,6 @@ import { IsOptional, IsString, IsUUID, - ArrayNotEmpty, Max, Min, ValidateNested, @@ -84,7 +83,6 @@ export class MemberSearchBodyDto { }) @IsOptional() @IsArray() - @ArrayNotEmpty() @IsString({ each: true }) countries?: string[]; diff --git a/src/reports/member/member-search.service.spec.ts b/src/reports/member/member-search.service.spec.ts index 0d43800..f33239c 100644 --- a/src/reports/member/member-search.service.spec.ts +++ b/src/reports/member/member-search.service.spec.ts @@ -106,10 +106,27 @@ describe("MemberSearchService", () => { expect(dataSql).toContain( 'ORDER BY m.handle ASC, "matchIndex" DESC NULLS LAST', ); + expect(dataSql).toContain( + 'LOWER(m."homeCountryCode") = ANY($1::text[])', + ); expect(dataParams).toEqual([["us"], 5, 5]); expect(countParams).toEqual([["us"]]); }); + it("treats empty countries as no country filter", async () => { + mockDbService.query + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ total: 0 }]); + + await service.search({ countries: [] }); + + const dataSql = mockDbService.query.mock.calls[0][0] as string; + const countParams = mockDbService.query.mock.calls[1][1] as unknown[]; + + expect(dataSql).not.toContain('LOWER(m."homeCountryCode") = ANY('); + expect(countParams).toEqual([]); + }); + it("deduplicates skills and keeps last wins value when building skill query", async () => { const skillA = "550e8400-e29b-41d4-a716-446655440000"; const skillB = "550e8400-e29b-41d4-a716-446655440001"; diff --git a/src/reports/member/member-search.service.ts b/src/reports/member/member-search.service.ts index 72f45fc..b37807b 100644 --- a/src/reports/member/member-search.service.ts +++ b/src/reports/member/member-search.service.ts @@ -202,16 +202,22 @@ member_address AS ( } const normalizedCountries = Array.isArray(countries) - ? countries.map((value) => String(value).trim()).filter(Boolean) + ? [ + ...new Set( + countries + .map((value) => String(value).trim().toLowerCase()) + .filter(Boolean), + ), + ] : []; if (normalizedCountries.length > 0) { const pCountries = p(normalizedCountries); where.push( `( - LOWER(m."homeCountryCode") = ANY(SELECT LOWER(c) FROM unnest(${pCountries}::text[]) AS c) - OR LOWER(m."competitionCountryCode") = ANY(SELECT LOWER(c) FROM unnest(${pCountries}::text[]) AS c) - OR LOWER(m.country) = ANY(SELECT LOWER(c) FROM unnest(${pCountries}::text[]) AS c) + LOWER(m."homeCountryCode") = ANY(${pCountries}::text[]) + OR LOWER(m."competitionCountryCode") = ANY(${pCountries}::text[]) + OR LOWER(m.country) = ANY(${pCountries}::text[]) )`, ); } From cde4808805a6b0937f19c0607c3bf92cf0acf308 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Wed, 22 Apr 2026 15:27:09 +1000 Subject: [PATCH 05/10] SFDC report to output better info about engagements --- sql/reports/sfdc/payments.sql | 103 +++++++++++------- src/reports/sfdc/sfdc-reports.dto.ts | 30 +++-- src/reports/sfdc/sfdc-reports.service.spec.ts | 50 +++++++-- src/reports/sfdc/test-helpers/mock-data.ts | 28 ++--- 4 files changed, 144 insertions(+), 67 deletions(-) diff --git a/sql/reports/sfdc/payments.sql b/sql/reports/sfdc/payments.sql index b854c89..aa1c451 100644 --- a/sql/reports/sfdc/payments.sql +++ b/sql/reports/sfdc/payments.sql @@ -1,44 +1,71 @@ +WITH resolved_payment_references AS ( + SELECT + p.payment_id, + p.created_at, + p.billing_account, + p.payment_status, + p.challenge_fee, + p.total_amount, + w.external_id, + w.winner_id, + w.category, + CASE + WHEN w.category IS DISTINCT FROM 'ENGAGEMENT_PAYMENT' THEN w.external_id + END AS challenge_filter_id, + CASE + WHEN w.category = 'ENGAGEMENT_PAYMENT' THEN e.title + ELSE c.name + END AS challenge_name, + c.status AS challenge_status, + m.handle, + m."userId", + m."firstName", + m."lastName" + FROM finance.payment p + LEFT JOIN finance.winnings w + ON w.winning_id = p.winnings_id + LEFT JOIN challenges."Challenge" c + ON c.id = w.external_id + AND w.category IS DISTINCT FROM 'ENGAGEMENT_PAYMENT' + LEFT JOIN engagements."EngagementAssignment" ea + ON ea.id = w.external_id + AND w.category = 'ENGAGEMENT_PAYMENT' + LEFT JOIN engagements."Engagement" e + ON e.id = ea."engagementId" + LEFT JOIN members.member m + ON m."userId" = w.winner_id::bigint +) SELECT - p.payment_id as "paymentId", - p.created_at AT TIME ZONE 'America/New_York' as "paymentDate", - p.billing_account as "billingAccountId", - p.payment_status as "paymentStatus", - p.challenge_fee as "challengeFee", - p.total_amount as "paymentAmount", -- Looker cost_transaction.member_payments - w.external_id as "challengeId", - w.category, - (w.category = 'TASK_PAYMENT') AS "isTask", - c.name AS "challengeName", - c.status AS "challengeStatus", - m.handle AS "winnerHandle", - m."userId" as "winnerId", - m."firstName" as "winnerFirstName", - m."lastName" as "winnerLastName" -FROM finance.payment p -LEFT JOIN finance.winnings w - ON w.winning_id = p.winnings_id -LEFT JOIN challenges."Challenge" c - ON c.id = w.external_id -LEFT JOIN members.member m - ON m."userId" = w.winner_id::bigint + payment_id as "paymentId", + created_at AT TIME ZONE 'America/New_York' as "paymentDate", + billing_account as "billingAccountId", + payment_status as "paymentStatus", + challenge_fee as "challengeFee", + total_amount as "paymentAmount", -- Looker cost_transaction.member_payments + external_id as "challengeId", + category, + (category = 'TASK_PAYMENT') AS "isTask", + challenge_name AS "challengeName", + challenge_status AS "challengeStatus", + handle AS "winnerHandle", + "userId" as "winnerId", + "firstName" as "winnerFirstName", + "lastName" as "winnerLastName" +FROM resolved_payment_references WHERE - ($1::text[] IS NULL OR p.billing_account = ANY($1::text[])) - AND ($2::text[] IS NULL OR p.billing_account <> ANY($2::text[])) - AND ($3::text[] IS NULL OR c.id = ANY($3::text[])) - AND ($4::text[] IS NULL OR w.winner_id::text IN ( + ($1::text[] IS NULL OR billing_account = ANY($1::text[])) + AND ($2::text[] IS NULL OR billing_account <> ANY($2::text[])) + AND ($3::text[] IS NULL OR challenge_filter_id = ANY($3::text[])) + AND ($4::text[] IS NULL OR winner_id::text IN ( SELECT m2."userId"::text FROM members.member m2 WHERE m2.handle = ANY($4::text[]) )) - AND ($5::text IS NULL OR w.external_id IN ( - SELECT c2.id - FROM challenges."Challenge" c2 - WHERE c2.name ILIKE '%' || $5 || '%' - )) - AND p.created_at >= COALESCE($6::timestamptz, (NOW() AT TIME ZONE 'UTC') - INTERVAL '45 days') - AND ($7::timestamptz IS NULL OR p.created_at <= $7::timestamptz) - AND ($8::numeric IS NULL OR p.total_amount >= $8::numeric) - AND ($9::numeric IS NULL OR p.total_amount <= $9::numeric) - AND ($10::text[] IS NULL OR c.status::text = ANY($10::text[])) - AND ($11::text[] IS NULL OR p.payment_status::text = ANY($11::text[])) -ORDER BY p.created_at DESC + AND ($5::text IS NULL OR challenge_name ILIKE '%' || $5 || '%') + AND created_at >= COALESCE($6::timestamptz, (NOW() AT TIME ZONE 'UTC') - INTERVAL '45 days') + AND ($7::timestamptz IS NULL OR created_at <= $7::timestamptz) + AND ($8::numeric IS NULL OR total_amount >= $8::numeric) + AND ($9::numeric IS NULL OR total_amount <= $9::numeric) + AND ($10::text[] IS NULL OR challenge_status::text = ANY($10::text[])) + AND ($11::text[] IS NULL OR payment_status::text = ANY($11::text[])) +ORDER BY created_at DESC diff --git a/src/reports/sfdc/sfdc-reports.dto.ts b/src/reports/sfdc/sfdc-reports.dto.ts index 5c44c44..700146d 100644 --- a/src/reports/sfdc/sfdc-reports.dto.ts +++ b/src/reports/sfdc/sfdc-reports.dto.ts @@ -206,8 +206,9 @@ export class PaymentsReportQueryDto { @ApiProperty({ required: false, - description: "Challenge name to search for", - example: "Task Payment for member", + description: + "Display name search across challenge names and engagement titles.", + example: "Customer Support Engagement", }) @IsOptional() @IsString() @@ -216,7 +217,8 @@ export class PaymentsReportQueryDto { @ApiProperty({ required: false, - description: "List of challenge IDs", + description: + "List of challenge IDs for challenge-backed payments only. ENGAGEMENT_PAYMENT rows are not matched by engagement assignment ID through this filter.", example: ["e74c3e37-73c9-474e-a838-a38dd4738906"], }) @IsOptional() @@ -276,7 +278,8 @@ export class PaymentsReportQueryDto { @ApiProperty({ required: false, - description: "List of challenge statuses to filter payments", + description: + "List of challenge statuses for challenge-backed payments only. Engagement payment rows have no challenge status and are excluded when this filter is supplied.", example: ["COMPLETED", "ACTIVE"], }) @IsOptional() @@ -300,7 +303,17 @@ export class PaymentsReportQueryDto { export class PaymentsReportResponse { billingAccountId: string; - challengeName: string; + @ApiProperty({ + description: + 'Resolved display name from challenges."Challenge".name for challenge payments or engagements."Engagement".title for engagement payments. Null when the external reference cannot be resolved.', + nullable: true, + type: String, + }) + challengeName: string | null; + @ApiProperty({ + description: + "Reference from finance.winnings.external_id. This is a challenge ID for challenge payments and an engagement assignment ID for ENGAGEMENT_PAYMENT rows.", + }) challengeId: string; @ApiProperty({ description: "Winnings category from finance.winnings.category", @@ -317,9 +330,12 @@ export class PaymentsReportResponse { challengeFee: number; paymentAmount: number; @ApiProperty({ - description: "Challenge status from challenges.Challenge.status", + description: + 'Challenge status from challenges."Challenge".status. Null for ENGAGEMENT_PAYMENT rows and challenge-backed rows whose external reference cannot be resolved.', + nullable: true, + type: String, }) - challengeStatus: string; + challengeStatus: string | null; } export class TaasJobsReportQueryDto { diff --git a/src/reports/sfdc/sfdc-reports.service.spec.ts b/src/reports/sfdc/sfdc-reports.service.spec.ts index c40f856..84e4b45 100644 --- a/src/reports/sfdc/sfdc-reports.service.spec.ts +++ b/src/reports/sfdc/sfdc-reports.service.spec.ts @@ -1,5 +1,7 @@ import { BadRequestException } from "@nestjs/common"; import { Test, TestingModule } from "@nestjs/testing"; +import { readFileSync } from "fs"; +import { join } from "path"; import { SqlLoaderService } from "src/common/sql-loader.service"; import { DbService } from "../../db/db.service"; import { @@ -300,7 +302,21 @@ describe("SfdcReportsService - getPaymentsReport", () => { ); }); - it("runs a basic query successfully", async () => { + it("uses external winning references for challenge-backed payment filters", () => { + const paymentsSql = readFileSync( + join(__dirname, "../../../sql/reports/sfdc/payments.sql"), + "utf8", + ); + + expect(paymentsSql).toContain( + "WHEN w.category IS DISTINCT FROM 'ENGAGEMENT_PAYMENT' THEN w.external_id", + ); + expect(paymentsSql).toContain( + "($3::text[] IS NULL OR challenge_filter_id = ANY($3::text[]))", + ); + }); + + it("returns mixed challenge, engagement, and unresolved challenge payments successfully", async () => { const result = await service.getPaymentsReport( mockPaymentQueryDto.billingAccount, ); @@ -319,6 +335,24 @@ describe("SfdcReportsService - getPaymentsReport", () => { undefined, ]); expect(result).toEqual(normalizedPaymentData); + expect(result).toEqual([ + expect.objectContaining({ + category: "CHALLENGE_PAYMENT", + challengeName: "Sample Challenge 1", + challengeStatus: "Completed", + }), + expect.objectContaining({ + category: "ENGAGEMENT_PAYMENT", + challengeName: "Customer Support Engagement", + challengeStatus: null, + }), + expect.objectContaining({ + category: "CHALLENGE_PAYMENT", + challengeId: "6bc7a37d-37ad-4c52-a2f0-71fa2c84e6e3", + challengeName: null, + challengeStatus: null, + }), + ]); }); it("splits include/exclude billing account filters", async () => { @@ -349,7 +383,7 @@ describe("SfdcReportsService - getPaymentsReport", () => { ["90000000"], ["e74c3e37-73c9-474e-a838-a38dd4738906"], ["user_01", "user_02"], - "Task Payment for member", + "Customer Support Engagement", "2023-01-01T00:00:00.000Z", "2023-03-01T00:00:00.000Z", 100, @@ -377,12 +411,13 @@ describe("SfdcReportsService - getPaymentsReport", () => { ]); }); - it("handles payment status filter", async () => { - await service.getPaymentsReport(mockPaymentQueryDto.paymentStatus); + it("passes orphaned challenge-backed external IDs to the challengeIds filter", async () => { + await service.getPaymentsReport(mockPaymentQueryDto.orphanedChallenge); expect(mockDbService.query).toHaveBeenCalledWith(mockSqlQuery, [ undefined, undefined, + ["6bc7a37d-37ad-4c52-a2f0-71fa2c84e6e3"], undefined, undefined, undefined, @@ -391,12 +426,11 @@ describe("SfdcReportsService - getPaymentsReport", () => { undefined, undefined, undefined, - ["ON_HOLD"], ]); }); - it("handles null challengeStatus for cancellable payments", async () => { - await service.getPaymentsReport(mockPaymentQueryDto.nullChallengeStatus); + it("handles payment status filter", async () => { + await service.getPaymentsReport(mockPaymentQueryDto.paymentStatus); expect(mockDbService.query).toHaveBeenCalledWith(mockSqlQuery, [ undefined, @@ -408,8 +442,8 @@ describe("SfdcReportsService - getPaymentsReport", () => { undefined, undefined, undefined, - [null as unknown as string], undefined, + ["ON_HOLD"], ]); }); diff --git a/src/reports/sfdc/test-helpers/mock-data.ts b/src/reports/sfdc/test-helpers/mock-data.ts index 1875c7b..af269ef 100644 --- a/src/reports/sfdc/test-helpers/mock-data.ts +++ b/src/reports/sfdc/test-helpers/mock-data.ts @@ -83,7 +83,7 @@ export const mockPaymentData: PaymentsReportResponse[] = [ challengeFee: 150.75, paymentAmount: 500, challengeId: "e74c3e37-73c9-474e-a838-a38dd4738906", - category: "Challenge Prizes", + category: "CHALLENGE_PAYMENT", isTask: false, challengeName: "Sample Challenge 1", challengeStatus: "COMPLETED", @@ -99,11 +99,11 @@ export const mockPaymentData: PaymentsReportResponse[] = [ paymentStatus: "Owed", challengeFee: 90.25, paymentAmount: 350, - challengeId: "8d4a1ce9-6a58-4bf2-8c3b-c57a8b3a3f92", - category: "Task Payments", - isTask: true, - challengeName: "Task Payment for member", - challengeStatus: "ACTIVE", + challengeId: "915739d5-d6c8-4699-9e58-bbb2118f8c4e", + category: "ENGAGEMENT_PAYMENT", + isTask: false, + challengeName: "Customer Support Engagement", + challengeStatus: null, winnerHandle: "user_02", winnerId: "654321", winnerFirstName: "Jane", @@ -117,11 +117,11 @@ export const mockPaymentData: PaymentsReportResponse[] = [ challengeFee: 120, paymentAmount: 425.5, challengeId: "6bc7a37d-37ad-4c52-a2f0-71fa2c84e6e3", - category: "Marathon Match", + category: "CHALLENGE_PAYMENT", isTask: false, - challengeName: "Algo Payment", - challengeStatus: null as unknown as string, - winnerHandle: "user_cancel", + challengeName: null, + challengeStatus: null, + winnerHandle: "user_orphaned_challenge", winnerId: "789012", winnerFirstName: "Alex", winnerLastName: "Johnson", @@ -139,8 +139,8 @@ export const mockPaymentQueryDto: Record = { challengeStatus: { challengeStatus: ["COMPLETED"], }, - nullChallengeStatus: { - challengeStatus: [null as unknown as string], + orphanedChallenge: { + challengeIds: ["6bc7a37d-37ad-4c52-a2f0-71fa2c84e6e3"], }, paymentStatus: { status: ["ON_HOLD"], @@ -149,7 +149,7 @@ export const mockPaymentQueryDto: Record = { billingAccountIds: ["80001012", "!90000000"], challengeIds: ["e74c3e37-73c9-474e-a838-a38dd4738906"], handles: ["user_01", "user_02"], - challengeName: "Task Payment for member", + challengeName: "Customer Support Engagement", startDate: "2023-01-01T00:00:00.000Z", endDate: "2023-03-01T00:00:00.000Z", minPaymentAmount: 100, @@ -168,7 +168,7 @@ export const normalizedChallengeData = mockChallengeData.map((challenge) => ({ export const normalizedPaymentData = mockPaymentData.map((payment) => ({ ...payment, - challengeStatus: normalizeChallengeStatus(payment.challengeStatus) as string, + challengeStatus: normalizeChallengeStatus(payment.challengeStatus), })); export const mockBaFeesData: BaFeesReportResponse[] = [ From 18b829f15e81f275fc956fae5521b546acf7ce81 Mon Sep 17 00:00:00 2001 From: himaniraghav3 Date: Wed, 22 Apr 2026 18:08:09 +0530 Subject: [PATCH 06/10] return all members until filter is sent --- .../member/member-search.service.spec.ts | 20 +++++++++++++++++++ src/reports/member/member-search.service.ts | 6 +++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/reports/member/member-search.service.spec.ts b/src/reports/member/member-search.service.spec.ts index f33239c..b1847ff 100644 --- a/src/reports/member/member-search.service.spec.ts +++ b/src/reports/member/member-search.service.spec.ts @@ -127,6 +127,26 @@ describe("MemberSearchService", () => { expect(countParams).toEqual([]); }); + it("does not apply boolean filters when explicitly false", async () => { + mockDbService.query + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ total: 0 }]); + + await service.search({ + openToWork: false, + recentlyActive: false, + verifiedProfile: false, + }); + + const dataSql = mockDbService.query.mock.calls[0][0] as string; + + expect(dataSql).not.toContain('m."availableForGigs" = true'); + expect(dataSql).not.toContain( + 'EXISTS (SELECT 1 FROM recently_active ra WHERE ra.user_id = m."userId")', + ); + expect(dataSql).not.toContain("COALESCE(m.verified, false) = true"); + }); + it("deduplicates skills and keeps last wins value when building skill query", async () => { const skillA = "550e8400-e29b-41d4-a716-446655440000"; const skillB = "550e8400-e29b-41d4-a716-446655440001"; diff --git a/src/reports/member/member-search.service.ts b/src/reports/member/member-search.service.ts index b37807b..e3ea7c6 100644 --- a/src/reports/member/member-search.service.ts +++ b/src/reports/member/member-search.service.ts @@ -185,17 +185,17 @@ member_address AS ( // ------------------------------------------------- dynamic WHERE const where: string[] = [`m.status = 'ACTIVE'`]; - if (typeof openToWork === "boolean") { + if (openToWork === true) { where.push(`m."availableForGigs" = true`); } - if (typeof recentlyActive === "boolean") { + if (recentlyActive === true) { where.push( `EXISTS (SELECT 1 FROM recently_active ra WHERE ra.user_id = m."userId")`, ); } - if (typeof verifiedProfile === "boolean") { + if (verifiedProfile === true) { where.push( `(COALESCE(m.verified, false) = true OR EXISTS (SELECT 1 FROM verified_via_trolley vt WHERE vt.user_id = m."userId"))`, ); From 1993e6da3eb75c209bae28789c41231bf77a5a53 Mon Sep 17 00:00:00 2001 From: himaniraghav3 Date: Wed, 22 Apr 2026 19:07:47 +0530 Subject: [PATCH 07/10] return country names --- src/reports/member/member-search.service.ts | 23 ++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/reports/member/member-search.service.ts b/src/reports/member/member-search.service.ts index e3ea7c6..774037f 100644 --- a/src/reports/member/member-search.service.ts +++ b/src/reports/member/member-search.service.ts @@ -1,4 +1,5 @@ import { Injectable, NotFoundException } from "@nestjs/common"; +import { alpha3ToCountryName } from "../../common/country.util"; import { DbService } from "../../db/db.service"; import { MemberSearchBodyDto } from "./dto/member-search.dto"; import { @@ -20,6 +21,26 @@ type RawMemberRow = { matchIndex: number; }; +function formatLocation(location: string): string { + const normalizedLocation = String(location || "").trim(); + if (!normalizedLocation) { + return ""; + } + + const parts = normalizedLocation.split(/\s+/); + const lastPart = parts[parts.length - 1]; + const mappedCountryName = alpha3ToCountryName(lastPart); + if (!mappedCountryName) { + return normalizedLocation; + } + + if (parts.length === 1) { + return mappedCountryName; + } + + return `${parts.slice(0, -1).join(" ")} ${mappedCountryName}`; +} + @Injectable() export class MemberSearchService { constructor(private readonly db: DbService) {} @@ -288,7 +309,7 @@ WHERE ${whereClause}`; isRecentlyActive: row.isRecentlyActive ?? false, isVerified: row.isVerified ?? false, openToWork: row.openToWork ?? false, - location: row.location || "", + location: formatLocation(row.location), matchedSkills: row.matchedSkills ?? [], matchIndex: row.matchIndex ?? 0, })); From 94f9fbbb58bebeba53c61598dc01b28e7a646ef7 Mon Sep 17 00:00:00 2001 From: himaniraghav3 Date: Wed, 22 Apr 2026 19:18:24 +0530 Subject: [PATCH 08/10] Add comma separator --- src/reports/member/member-search.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/reports/member/member-search.service.ts b/src/reports/member/member-search.service.ts index 774037f..1d9e38c 100644 --- a/src/reports/member/member-search.service.ts +++ b/src/reports/member/member-search.service.ts @@ -38,7 +38,7 @@ function formatLocation(location: string): string { return mappedCountryName; } - return `${parts.slice(0, -1).join(" ")} ${mappedCountryName}`; + return `${parts.slice(0, -1).join(" ")}, ${mappedCountryName}`; } @Injectable() From 34cc4cad8ab5474ff90d254327c833104a6ff124 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Fri, 24 Apr 2026 08:25:59 +1000 Subject: [PATCH 09/10] Better handling of engagements ID in SFDC payments report --- TESTING.md | 10 +++-- sql/reports/sfdc/payments.sql | 27 ++++++++----- src/reports/report-directory.data.ts | 10 ++++- .../sfdc/sfdc-reports.controller.spec.ts | 2 + src/reports/sfdc/sfdc-reports.controller.ts | 3 +- src/reports/sfdc/sfdc-reports.dto.spec.ts | 23 +++++++++++ src/reports/sfdc/sfdc-reports.dto.ts | 14 ++++++- src/reports/sfdc/sfdc-reports.service.spec.ts | 40 ++++++++++++++++++- src/reports/sfdc/sfdc-reports.service.ts | 1 + src/reports/sfdc/test-helpers/mock-data.ts | 4 ++ 10 files changed, 115 insertions(+), 19 deletions(-) diff --git a/TESTING.md b/TESTING.md index 39a0327..c6a2fab 100644 --- a/TESTING.md +++ b/TESTING.md @@ -60,6 +60,7 @@ it('passes all filters in correct order', async () => { await service.getPaymentsReport({ billingAccountIds: ['12345'], challengeIds: ['uuid1'], + engagementIds: ['engagement-uuid1'], handles: ['user1'], challengeName: 'Task', startDate: '2023-01-01', @@ -73,12 +74,13 @@ it('passes all filters in correct order', async () => { ['12345'], // include billing accounts undefined, // exclude billing accounts ['uuid1'], // challenge IDs - ['user1'], // handles - 'Task', // challenge name + ['engagement-uuid1'], // engagement IDs + ['user1'], // handles + 'Task', // challenge name '2023-01-01', // start date '2023-12-31', // end date - 100, // min amount - 1000, // max amount + 100, // min amount + 1000, // max amount ['COMPLETED'] // challenge status ]); }); diff --git a/sql/reports/sfdc/payments.sql b/sql/reports/sfdc/payments.sql index aa1c451..a3ccc6e 100644 --- a/sql/reports/sfdc/payments.sql +++ b/sql/reports/sfdc/payments.sql @@ -12,6 +12,9 @@ WITH resolved_payment_references AS ( CASE WHEN w.category IS DISTINCT FROM 'ENGAGEMENT_PAYMENT' THEN w.external_id END AS challenge_filter_id, + CASE + WHEN w.category = 'ENGAGEMENT_PAYMENT' THEN ea."engagementId" + END AS engagement_filter_id, CASE WHEN w.category = 'ENGAGEMENT_PAYMENT' THEN e.title ELSE c.name @@ -55,17 +58,21 @@ FROM resolved_payment_references WHERE ($1::text[] IS NULL OR billing_account = ANY($1::text[])) AND ($2::text[] IS NULL OR billing_account <> ANY($2::text[])) - AND ($3::text[] IS NULL OR challenge_filter_id = ANY($3::text[])) - AND ($4::text[] IS NULL OR winner_id::text IN ( + AND ( + ($3::text[] IS NULL AND $4::text[] IS NULL) + OR ($3::text[] IS NOT NULL AND challenge_filter_id = ANY($3::text[])) + OR ($4::text[] IS NOT NULL AND engagement_filter_id = ANY($4::text[])) + ) + AND ($5::text[] IS NULL OR winner_id::text IN ( SELECT m2."userId"::text FROM members.member m2 - WHERE m2.handle = ANY($4::text[]) + WHERE m2.handle = ANY($5::text[]) )) - AND ($5::text IS NULL OR challenge_name ILIKE '%' || $5 || '%') - AND created_at >= COALESCE($6::timestamptz, (NOW() AT TIME ZONE 'UTC') - INTERVAL '45 days') - AND ($7::timestamptz IS NULL OR created_at <= $7::timestamptz) - AND ($8::numeric IS NULL OR total_amount >= $8::numeric) - AND ($9::numeric IS NULL OR total_amount <= $9::numeric) - AND ($10::text[] IS NULL OR challenge_status::text = ANY($10::text[])) - AND ($11::text[] IS NULL OR payment_status::text = ANY($11::text[])) + AND ($6::text IS NULL OR challenge_name ILIKE '%' || $6 || '%') + AND created_at >= COALESCE($7::timestamptz, (NOW() AT TIME ZONE 'UTC') - INTERVAL '45 days') + AND ($8::timestamptz IS NULL OR created_at <= $8::timestamptz) + AND ($9::numeric IS NULL OR total_amount >= $9::numeric) + AND ($10::numeric IS NULL OR total_amount <= $10::numeric) + AND ($11::text[] IS NULL OR challenge_status::text = ANY($11::text[])) + AND ($12::text[] IS NULL OR payment_status::text = ANY($12::text[])) ORDER BY created_at DESC diff --git a/src/reports/report-directory.data.ts b/src/reports/report-directory.data.ts index 6542ac1..88cf6d8 100644 --- a/src/reports/report-directory.data.ts +++ b/src/reports/report-directory.data.ts @@ -225,7 +225,14 @@ const challengeNameParam: ReportParameter = { const challengeIdsParam: ReportParameter = { name: "challengeIds", type: "string[]", - description: "List of challenge IDs", + description: "List of challenge IDs for challenge-backed payments", + location: "query", +}; + +const engagementIdsParam: ReportParameter = { + name: "engagementIds", + type: "string[]", + description: "List of engagement IDs for engagement-backed payments", location: "query", }; @@ -270,6 +277,7 @@ const paymentsFilters = [ billingAccountIdsParam, challengeNameParam, challengeIdsParam, + engagementIdsParam, paymentsStartDateParam, paymentsEndDateParam, handlesParam, diff --git a/src/reports/sfdc/sfdc-reports.controller.spec.ts b/src/reports/sfdc/sfdc-reports.controller.spec.ts index 54e3935..2ae4c69 100644 --- a/src/reports/sfdc/sfdc-reports.controller.spec.ts +++ b/src/reports/sfdc/sfdc-reports.controller.spec.ts @@ -168,6 +168,7 @@ describe("SfdcReportsController", () => { const dto = plainToInstance(PaymentsReportQueryDto, { billingAccountIds: "80001012,80002012", challengeIds: "e74c3e37-73c9-474e-a838-a38dd4738906", + engagementIds: "3cf4ec0b-47e5-4d96-b4c3-ef6af5b0f954", handles: "user_01", challengeStatus: "COMPLETED", status: "ON_HOLD", @@ -179,6 +180,7 @@ describe("SfdcReportsController", () => { expect.objectContaining({ billingAccountIds: ["80001012", "80002012"], challengeIds: ["e74c3e37-73c9-474e-a838-a38dd4738906"], + engagementIds: ["3cf4ec0b-47e5-4d96-b4c3-ef6af5b0f954"], handles: ["user_01"], challengeStatus: ["COMPLETED"], status: ["ON_HOLD"], diff --git a/src/reports/sfdc/sfdc-reports.controller.ts b/src/reports/sfdc/sfdc-reports.controller.ts index 4884f88..d922967 100644 --- a/src/reports/sfdc/sfdc-reports.controller.ts +++ b/src/reports/sfdc/sfdc-reports.controller.ts @@ -60,7 +60,8 @@ export class SfdcReportsController { @ApiBearerAuth() @ApiOperation({ summary: "SFDC Payments report", - description: "", + description: + "Retrieve SFDC payments with billing account, challenge, engagement, handle, payment status, and challenge status filters.", }) @ApiResponse({ status: 200, diff --git a/src/reports/sfdc/sfdc-reports.dto.spec.ts b/src/reports/sfdc/sfdc-reports.dto.spec.ts index 43be0da..8274e30 100644 --- a/src/reports/sfdc/sfdc-reports.dto.spec.ts +++ b/src/reports/sfdc/sfdc-reports.dto.spec.ts @@ -232,6 +232,7 @@ describe("PaymentsReportQueryDto validation", () => { const { errors } = await validatePaymentDto({ billingAccountIds: ["80001012", "!90000000"], challengeIds: ["e74c3e37-73c9-474e-a838-a38dd4738906"], + engagementIds: ["3cf4ec0b-47e5-4d96-b4c3-ef6af5b0f954"], handles: ["user_01", "user_02"], challengeName: "Task Payment for member", startDate: "2023-01-01T00:00:00.000Z", @@ -297,6 +298,28 @@ describe("PaymentsReportQueryDto validation", () => { expect(dto.challengeIds).toEqual(["uuid1"]); }); + it("accepts valid engagementIds", async () => { + const { errors } = await validatePaymentDto({ + engagementIds: ["uuid1", "uuid2"], + }); + expect(errors).toHaveLength(0); + }); + + it("drops empty engagementIds entries during transform", async () => { + const { dto, errors } = await validatePaymentDto({ engagementIds: [""] }); + expect(errors).toHaveLength(0); + expect(dto.engagementIds).toEqual([]); + }); + + it("transforms single engagementId into array", async () => { + const { dto, errors } = await validatePaymentDto({ + // @ts-expect-error intentional single value for transform check + engagementIds: "uuid1", + }); + expect(errors).toHaveLength(0); + expect(dto.engagementIds).toEqual(["uuid1"]); + }); + it("accepts valid handles", async () => { const { errors } = await validatePaymentDto({ handles: ["user1", "user2"], diff --git a/src/reports/sfdc/sfdc-reports.dto.ts b/src/reports/sfdc/sfdc-reports.dto.ts index 700146d..fa90ad7 100644 --- a/src/reports/sfdc/sfdc-reports.dto.ts +++ b/src/reports/sfdc/sfdc-reports.dto.ts @@ -218,7 +218,7 @@ export class PaymentsReportQueryDto { @ApiProperty({ required: false, description: - "List of challenge IDs for challenge-backed payments only. ENGAGEMENT_PAYMENT rows are not matched by engagement assignment ID through this filter.", + "List of challenge IDs for challenge-backed payments only. Use engagementIds to filter ENGAGEMENT_PAYMENT rows by engagement.", example: ["e74c3e37-73c9-474e-a838-a38dd4738906"], }) @IsOptional() @@ -227,6 +227,18 @@ export class PaymentsReportQueryDto { @Transform(transformArray) challengeIds?: string[]; + @ApiProperty({ + required: false, + description: + 'List of engagement IDs for ENGAGEMENT_PAYMENT rows only. This matches engagements."EngagementAssignment"."engagementId", not finance.winnings.external_id.', + example: ["3cf4ec0b-47e5-4d96-b4c3-ef6af5b0f954"], + }) + @IsOptional() + @IsString({ each: true }) + @IsNotEmpty({ each: true }) + @Transform(transformArray) + engagementIds?: string[]; + @ApiProperty({ required: false, description: "Start date for the report query in ISO 8601 format", diff --git a/src/reports/sfdc/sfdc-reports.service.spec.ts b/src/reports/sfdc/sfdc-reports.service.spec.ts index 84e4b45..1e86c0e 100644 --- a/src/reports/sfdc/sfdc-reports.service.spec.ts +++ b/src/reports/sfdc/sfdc-reports.service.spec.ts @@ -302,7 +302,7 @@ describe("SfdcReportsService - getPaymentsReport", () => { ); }); - it("uses external winning references for challenge-backed payment filters", () => { + it("uses dedicated challenge and engagement filter IDs in the payments SQL", () => { const paymentsSql = readFileSync( join(__dirname, "../../../sql/reports/sfdc/payments.sql"), "utf8", @@ -312,7 +312,16 @@ describe("SfdcReportsService - getPaymentsReport", () => { "WHEN w.category IS DISTINCT FROM 'ENGAGEMENT_PAYMENT' THEN w.external_id", ); expect(paymentsSql).toContain( - "($3::text[] IS NULL OR challenge_filter_id = ANY($3::text[]))", + "WHEN w.category = 'ENGAGEMENT_PAYMENT' THEN ea.\"engagementId\"", + ); + expect(paymentsSql).toContain( + "($3::text[] IS NULL AND $4::text[] IS NULL)", + ); + expect(paymentsSql).toContain( + "($3::text[] IS NOT NULL AND challenge_filter_id = ANY($3::text[]))", + ); + expect(paymentsSql).toContain( + "($4::text[] IS NOT NULL AND engagement_filter_id = ANY($4::text[]))", ); }); @@ -333,6 +342,7 @@ describe("SfdcReportsService - getPaymentsReport", () => { undefined, undefined, undefined, + undefined, ]); expect(result).toEqual(normalizedPaymentData); expect(result).toEqual([ @@ -372,6 +382,7 @@ describe("SfdcReportsService - getPaymentsReport", () => { undefined, undefined, undefined, + undefined, ]); }); @@ -382,6 +393,7 @@ describe("SfdcReportsService - getPaymentsReport", () => { ["80001012"], ["90000000"], ["e74c3e37-73c9-474e-a838-a38dd4738906"], + ["3cf4ec0b-47e5-4d96-b4c3-ef6af5b0f954"], ["user_01", "user_02"], "Customer Support Engagement", "2023-01-01T00:00:00.000Z", @@ -393,6 +405,25 @@ describe("SfdcReportsService - getPaymentsReport", () => { ]); }); + it("passes engagement-backed payment filters to the engagementIds parameter", async () => { + await service.getPaymentsReport(mockPaymentQueryDto.engagement); + + expect(mockDbService.query).toHaveBeenCalledWith(mockSqlQuery, [ + undefined, + undefined, + undefined, + ["3cf4ec0b-47e5-4d96-b4c3-ef6af5b0f954"], + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + ]); + }); + it("handles challengeStatus filter for cancel payment checks", async () => { await service.getPaymentsReport(mockPaymentQueryDto.challengeStatus); @@ -406,6 +437,7 @@ describe("SfdcReportsService - getPaymentsReport", () => { undefined, undefined, undefined, + undefined, ["COMPLETED"], undefined, ]); @@ -426,6 +458,7 @@ describe("SfdcReportsService - getPaymentsReport", () => { undefined, undefined, undefined, + undefined, ]); }); @@ -443,6 +476,7 @@ describe("SfdcReportsService - getPaymentsReport", () => { undefined, undefined, undefined, + undefined, ["ON_HOLD"], ]); }); @@ -484,6 +518,7 @@ describe("SfdcReportsService - getPaymentsReport", () => { undefined, undefined, undefined, + undefined, ]); }); @@ -502,6 +537,7 @@ describe("SfdcReportsService - getPaymentsReport", () => { undefined, undefined, undefined, + undefined, ]); }); }); diff --git a/src/reports/sfdc/sfdc-reports.service.ts b/src/reports/sfdc/sfdc-reports.service.ts index 59cd80b..56f42e5 100644 --- a/src/reports/sfdc/sfdc-reports.service.ts +++ b/src/reports/sfdc/sfdc-reports.service.ts @@ -115,6 +115,7 @@ export class SfdcReportsService { billingAccountIds.length ? billingAccountIds : undefined, excludeBillingAccountIds.length ? excludeBillingAccountIds : undefined, filters.challengeIds, + filters.engagementIds, filters.handles, filters.challengeName, filters.startDate, diff --git a/src/reports/sfdc/test-helpers/mock-data.ts b/src/reports/sfdc/test-helpers/mock-data.ts index af269ef..4b9bfd5 100644 --- a/src/reports/sfdc/test-helpers/mock-data.ts +++ b/src/reports/sfdc/test-helpers/mock-data.ts @@ -136,6 +136,9 @@ export const mockPaymentQueryDto: Record = { withExclusions: { billingAccountIds: ["80001012", "!90000000"], }, + engagement: { + engagementIds: ["3cf4ec0b-47e5-4d96-b4c3-ef6af5b0f954"], + }, challengeStatus: { challengeStatus: ["COMPLETED"], }, @@ -148,6 +151,7 @@ export const mockPaymentQueryDto: Record = { full: { billingAccountIds: ["80001012", "!90000000"], challengeIds: ["e74c3e37-73c9-474e-a838-a38dd4738906"], + engagementIds: ["3cf4ec0b-47e5-4d96-b4c3-ef6af5b0f954"], handles: ["user_01", "user_02"], challengeName: "Customer Support Engagement", startDate: "2023-01-01T00:00:00.000Z", From 65c971d200ac08705a5d12eeaf0735f74026bb26 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Fri, 24 Apr 2026 09:28:40 +1000 Subject: [PATCH 10/10] Additional tweaks for payment report --- sql/reports/sfdc/payments.sql | 8 ++++++-- src/reports/member/member-search.service.spec.ts | 4 +--- src/reports/sfdc/sfdc-reports.dto.ts | 2 +- src/reports/sfdc/sfdc-reports.service.spec.ts | 8 +++++++- src/reports/sfdc/sfdc-reports.service.ts | 5 ++++- src/reports/sfdc/test-helpers/mock-data.ts | 5 ++++- 6 files changed, 23 insertions(+), 9 deletions(-) diff --git a/sql/reports/sfdc/payments.sql b/sql/reports/sfdc/payments.sql index a3ccc6e..a3a0987 100644 --- a/sql/reports/sfdc/payments.sql +++ b/sql/reports/sfdc/payments.sql @@ -20,6 +20,10 @@ WITH resolved_payment_references AS ( ELSE c.name END AS challenge_name, c.status AS challenge_status, + CASE + WHEN w.category = 'ENGAGEMENT_PAYMENT' THEN 'COMPLETED' + ELSE c.status + END AS reported_challenge_status, m.handle, m."userId", m."firstName", @@ -49,7 +53,7 @@ SELECT category, (category = 'TASK_PAYMENT') AS "isTask", challenge_name AS "challengeName", - challenge_status AS "challengeStatus", + reported_challenge_status AS "challengeStatus", handle AS "winnerHandle", "userId" as "winnerId", "firstName" as "winnerFirstName", @@ -73,6 +77,6 @@ WHERE AND ($8::timestamptz IS NULL OR created_at <= $8::timestamptz) AND ($9::numeric IS NULL OR total_amount >= $9::numeric) AND ($10::numeric IS NULL OR total_amount <= $10::numeric) - AND ($11::text[] IS NULL OR challenge_status::text = ANY($11::text[])) + AND ($11::text[] IS NULL OR reported_challenge_status::text = ANY($11::text[])) AND ($12::text[] IS NULL OR payment_status::text = ANY($12::text[])) ORDER BY created_at DESC diff --git a/src/reports/member/member-search.service.spec.ts b/src/reports/member/member-search.service.spec.ts index b1847ff..d83ae52 100644 --- a/src/reports/member/member-search.service.spec.ts +++ b/src/reports/member/member-search.service.spec.ts @@ -106,9 +106,7 @@ describe("MemberSearchService", () => { expect(dataSql).toContain( 'ORDER BY m.handle ASC, "matchIndex" DESC NULLS LAST', ); - expect(dataSql).toContain( - 'LOWER(m."homeCountryCode") = ANY($1::text[])', - ); + expect(dataSql).toContain('LOWER(m."homeCountryCode") = ANY($1::text[])'); expect(dataParams).toEqual([["us"], 5, 5]); expect(countParams).toEqual([["us"]]); }); diff --git a/src/reports/sfdc/sfdc-reports.dto.ts b/src/reports/sfdc/sfdc-reports.dto.ts index fa90ad7..3f1d908 100644 --- a/src/reports/sfdc/sfdc-reports.dto.ts +++ b/src/reports/sfdc/sfdc-reports.dto.ts @@ -343,7 +343,7 @@ export class PaymentsReportResponse { paymentAmount: number; @ApiProperty({ description: - 'Challenge status from challenges."Challenge".status. Null for ENGAGEMENT_PAYMENT rows and challenge-backed rows whose external reference cannot be resolved.', + "Normalized challenge status label for challenge-backed payments. ENGAGEMENT_PAYMENT rows are always reported as Completed; challenge-backed rows whose external reference cannot be resolved remain null.", nullable: true, type: String, }) diff --git a/src/reports/sfdc/sfdc-reports.service.spec.ts b/src/reports/sfdc/sfdc-reports.service.spec.ts index 1e86c0e..f54f947 100644 --- a/src/reports/sfdc/sfdc-reports.service.spec.ts +++ b/src/reports/sfdc/sfdc-reports.service.spec.ts @@ -314,6 +314,12 @@ describe("SfdcReportsService - getPaymentsReport", () => { expect(paymentsSql).toContain( "WHEN w.category = 'ENGAGEMENT_PAYMENT' THEN ea.\"engagementId\"", ); + expect(paymentsSql).toContain( + "WHEN w.category = 'ENGAGEMENT_PAYMENT' THEN 'COMPLETED'", + ); + expect(paymentsSql).toContain( + "reported_challenge_status::text = ANY($11::text[])", + ); expect(paymentsSql).toContain( "($3::text[] IS NULL AND $4::text[] IS NULL)", ); @@ -354,7 +360,7 @@ describe("SfdcReportsService - getPaymentsReport", () => { expect.objectContaining({ category: "ENGAGEMENT_PAYMENT", challengeName: "Customer Support Engagement", - challengeStatus: null, + challengeStatus: "Completed", }), expect.objectContaining({ category: "CHALLENGE_PAYMENT", diff --git a/src/reports/sfdc/sfdc-reports.service.ts b/src/reports/sfdc/sfdc-reports.service.ts index 56f42e5..51e2a5d 100644 --- a/src/reports/sfdc/sfdc-reports.service.ts +++ b/src/reports/sfdc/sfdc-reports.service.ts @@ -130,7 +130,10 @@ export class SfdcReportsService { return payments.map((payment) => ({ ...payment, - challengeStatus: normalizeChallengeStatus(payment.challengeStatus), + challengeStatus: + payment.category === "ENGAGEMENT_PAYMENT" + ? "Completed" + : normalizeChallengeStatus(payment.challengeStatus), })); } diff --git a/src/reports/sfdc/test-helpers/mock-data.ts b/src/reports/sfdc/test-helpers/mock-data.ts index 4b9bfd5..6cf3aeb 100644 --- a/src/reports/sfdc/test-helpers/mock-data.ts +++ b/src/reports/sfdc/test-helpers/mock-data.ts @@ -172,7 +172,10 @@ export const normalizedChallengeData = mockChallengeData.map((challenge) => ({ export const normalizedPaymentData = mockPaymentData.map((payment) => ({ ...payment, - challengeStatus: normalizeChallengeStatus(payment.challengeStatus), + challengeStatus: + payment.category === "ENGAGEMENT_PAYMENT" + ? "Completed" + : normalizeChallengeStatus(payment.challengeStatus), })); export const mockBaFeesData: BaFeesReportResponse[] = [