Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 35 additions & 33 deletions src/server/utils/data/get-opportunity-where.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> {
const dealFilter: Record<string, unknown> = {};
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<Opportunity> {
const dealFilter = getDealWhere(filter);

return {
...getTypeWhere(filter, appointment?.excludeAccompanying),
...getAppointmentDateWhere(appointment),
Expand All @@ -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<Opportunity>;
}
70 changes: 70 additions & 0 deletions src/test/server/utils/data/get-opportunity-where.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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" } },
});
});
});