diff --git a/src/server/utils/data/get-opportunity-where.ts b/src/server/utils/data/get-opportunity-where.ts index 5215fd1b..ebed45de 100644 --- a/src/server/utils/data/get-opportunity-where.ts +++ b/src/server/utils/data/get-opportunity-where.ts @@ -101,12 +101,46 @@ function getTypeWhere( : {}; } +// language, district, activity and skill all constrain the same `deal` +// relation, so they must accumulate onto one shared `deal` key rather than +// each writing their own top-level spread — two spreads with the same key +// have the second replace the first, silently dropping the earlier +// constraint. Mirrors get-volunteer-where.ts's identical dealFilter. +function getDealWhere( + filter: QuerystringOpportunityFiltering["filter"], +): Record { + const dealFilter: Record = {}; + if (filter?.language) { + dealFilter.dealLanguage = { + language: { id: normalizeStringArrayInput(filter.language) }, + }; + } + if (filter?.district) { + dealFilter.dealDistrict = { + district: { id: normalizeStringArrayInput(filter.district) }, + }; + } + if (filter?.activity) { + dealFilter.dealActivity = { + activity: { id: normalizeStringArrayInput(filter.activity) }, + }; + } + if (filter?.skill) { + dealFilter.dealSkill = { + skill: { id: normalizeStringArrayInput(filter.skill) }, + }; + } + return dealFilter; +} + // SECURITY (#666): filters run on unmasked DB columns, so a non-privileged // caller can infer PII masked in the response by probing which rows match. export function getOpportunityWhere( filter: QuerystringOpportunityFiltering["filter"], appointment?: OpportunityAppointmentFilter, ): FindOptionsWhere { + const dealFilter = getDealWhere(filter); + return { ...getTypeWhere(filter, appointment?.excludeAccompanying), ...getAppointmentDateWhere(appointment), @@ -120,38 +154,6 @@ export function getOpportunityWhere( title: ILike(`%${filter.search}%`), } : {}), - ...(filter?.language - ? { - deal: { - dealLanguage: { - language: { - id: normalizeStringArrayInput(filter.language), - }, - }, - }, - } - : {}), - ...(filter?.district - ? { - deal: { - dealDistrict: { - district: { - id: normalizeStringArrayInput(filter.district), - }, - }, - }, - } - : {}), - ...(filter?.activity - ? { - deal: { - dealActivity: { - activity: { - id: normalizeStringArrayInput(filter.activity), - }, - }, - }, - } - : {}), + ...(Object.keys(dealFilter).length ? { deal: dealFilter } : {}), } as FindOptionsWhere; } diff --git a/src/test/server/utils/data/get-opportunity-where.test.ts b/src/test/server/utils/data/get-opportunity-where.test.ts index 4e0241b7..dff06dd8 100644 --- a/src/test/server/utils/data/get-opportunity-where.test.ts +++ b/src/test/server/utils/data/get-opportunity-where.test.ts @@ -9,6 +9,7 @@ import { Not, } from "typeorm"; import { describe, expect, it } from "vitest"; +import { QuerystringOpportunityFiltering } from "../../../../server/types"; import { getOpportunityWhere } from "../../../../server/utils/data/get-opportunity-where"; import { berlinDayBoundaries } from "../../../../services/jobs/german-holidays"; @@ -140,4 +141,73 @@ describe("getOpportunityWhere", () => { }); }); }); + + it("keeps every deal constraint when several are combined", () => { + const where = getOpportunityWhere({ + type: "", + status: "", + language: "1", + district: "2", + activity: "3", + skill: "4", + }); + + expect(where.deal).toEqual({ + dealLanguage: { language: { id: "1" } }, + dealDistrict: { district: { id: "2" } }, + dealActivity: { activity: { id: "3" } }, + dealSkill: { skill: { id: "4" } }, + }); + }); + + // Multiple selections arrive as an array at runtime (?language=3&language=4) + // and become In([...]), which TypeORM ORs. The querystring type declares + // every filter as `string`, hence the cast, see the + // `// TODO: what about arrays?` above QuerystringOpportunityFiltering. + it("ORs multiple values within a single filter", () => { + const where = getOpportunityWhere({ + type: "", + status: "", + language: ["3", "4"], + } as unknown as QuerystringOpportunityFiltering["filter"]); + + expect(where.deal).toEqual({ + dealLanguage: { language: { id: In(["3", "4"]) } }, + }); + }); + + it("applies only the language constraint when nothing else is selected", () => { + const where = getOpportunityWhere({ type: "", status: "", language: "3" }); + + expect(where.deal).toEqual({ + dealLanguage: { language: { id: "3" } }, + }); + }); + + // be#888's own repro: activity alone matched 1 row; activity plus a + // non-matching language still matched that same 1, because the language + // spread silently overwrote the activity spread on the shared `deal` key. + // Asserting both constraints survive together is the unit-level proof that + // no longer happens. + it("keeps both activity and language when combined, matching be#888's repro", () => { + const activityOnly = getOpportunityWhere({ + type: "", + status: "", + activity: "3", + }); + expect(activityOnly.deal).toEqual({ + dealActivity: { activity: { id: "3" } }, + }); + + const activityPlusLanguage = getOpportunityWhere({ + type: "", + status: "", + activity: "3", + language: "9", + }); + expect(activityPlusLanguage.deal).toEqual({ + dealActivity: { activity: { id: "3" } }, + dealLanguage: { language: { id: "9" } }, + }); + }); });