Skip to content

Commit ced2046

Browse files
committed
chore(admin): drop unused classifyNafSegment + harden SQL segment expr
Address the two minor findings from the post-implementation audit: - Remove `classifyNafSegment` + the `NafSegment` type. The router builds a SQL CASE instead of bucketing rows in TypeScript, so the helper was dead code; trimming it keeps the `~/modules/shared` barrel lean. Tests now assert the two constants still used by the router (DOMINANT_NAF_SECTIONS, OTHER_NAF_SEGMENT). - Replace `sql.raw(dominantList)` with `sql.join` in `buildSegmentExpression` so each dominant letter goes through Drizzle parameter binding. Safe today (constant is a compile-time `as const` array) but removes the `sql.raw` sink as defence in depth.
1 parent dad67be commit ced2046

4 files changed

Lines changed: 20 additions & 44 deletions

File tree

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,25 @@
11
import { describe, expect, it } from "vitest";
22

33
import {
4-
classifyNafSegment,
54
DOMINANT_NAF_SECTIONS,
5+
NAF_SECTION_CODES,
66
OTHER_NAF_SEGMENT,
77
} from "../nafSections";
88

9-
describe("classifyNafSegment", () => {
10-
it("returns the letter prefix for dominant sections", () => {
11-
for (const section of DOMINANT_NAF_SECTIONS) {
12-
expect(classifyNafSegment(`${section}01.11Z`)).toBe(section);
13-
}
14-
});
15-
16-
it("returns 'Autres' for non-dominant but valid sections", () => {
17-
// A, B, D, E, F, H, I, J, K, L, O, P, R, S, T, U are non-dominant.
18-
expect(classifyNafSegment("A01.11Z")).toBe(OTHER_NAF_SEGMENT);
19-
expect(classifyNafSegment("K64.19Z")).toBe(OTHER_NAF_SEGMENT);
20-
expect(classifyNafSegment("U99.00Z")).toBe(OTHER_NAF_SEGMENT);
9+
describe("DOMINANT_NAF_SECTIONS", () => {
10+
it("keeps the five manually curated sections", () => {
11+
expect([...DOMINANT_NAF_SECTIONS]).toEqual(["C", "G", "M", "N", "Q"]);
2112
});
2213

23-
it("normalises the letter case", () => {
24-
expect(classifyNafSegment("c10.11z")).toBe("C");
14+
it("only references valid NAF section letters", () => {
15+
for (const code of DOMINANT_NAF_SECTIONS) {
16+
expect(NAF_SECTION_CODES).toContain(code);
17+
}
2518
});
19+
});
2620

27-
it("returns null for null, empty, or invalid prefixes", () => {
28-
expect(classifyNafSegment(null)).toBeNull();
29-
expect(classifyNafSegment("")).toBeNull();
30-
expect(classifyNafSegment("99.00Z")).toBeNull(); // starts with digit
31-
expect(classifyNafSegment("Z99.00Z")).toBeNull(); // Z is not a NAF section
21+
describe("OTHER_NAF_SEGMENT", () => {
22+
it("uses the French 'Autres' label for the aggregated bucket", () => {
23+
expect(OTHER_NAF_SEGMENT).toBe("Autres");
3224
});
3325
});

packages/app/src/modules/shared/index.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,8 @@ export { CompanySizeFilter } from "./CompanySizeFilter";
22
export { FileUpload } from "./FileUpload";
33
export { getDsfrModal } from "./getDsfrModal";
44
export { NafSectorFilter } from "./NafSectorFilter";
5-
export type { NafSectionCode, NafSegment } from "./nafSections";
5+
export type { NafSectionCode } from "./nafSections";
66
export {
7-
classifyNafSegment,
87
DOMINANT_NAF_SECTIONS,
98
NAF_SECTION_CODES,
109
NAF_SECTIONS,

packages/app/src/modules/shared/nafSections.ts

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -78,22 +78,3 @@ export const DOMINANT_NAF_SECTIONS: readonly NafSectionCode[] = [
7878

7979
/** Label used for the aggregated series bucketing all non-dominant sections. */
8080
export const OTHER_NAF_SEGMENT = "Autres";
81-
82-
export type NafSegment = NafSectionCode | typeof OTHER_NAF_SEGMENT;
83-
84-
/**
85-
* Map a full NAF code (e.g. `"62.01Z"`) to its chart segment. Dominant
86-
* sections keep their letter; everything else collapses into `"Autres"`.
87-
* Returns `null` when the code is null / empty / unknown.
88-
*/
89-
export function classifyNafSegment(nafCode: string | null): NafSegment | null {
90-
if (!nafCode) return null;
91-
const prefix = nafCode.charAt(0).toUpperCase();
92-
if (DOMINANT_NAF_SECTIONS.includes(prefix as NafSectionCode)) {
93-
return prefix as NafSectionCode;
94-
}
95-
// Reject obviously invalid prefixes so upstream doesn't silently fold
96-
// corrupt data into "Autres".
97-
if (!NAF_SECTION_CODES.includes(prefix as NafSectionCode)) return null;
98-
return OTHER_NAF_SEGMENT;
99-
}

packages/app/src/server/api/routers/adminStats.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -239,11 +239,15 @@ function buildSegmentExpression(
239239
end`;
240240
}
241241
// NAF: collapse non-dominant sections into a single "Autres" segment.
242-
const dominantList = DOMINANT_NAF_SECTIONS.map((code) => `'${code}'`).join(
243-
", ",
242+
// Each dominant letter goes through parameter binding via `sql.join` —
243+
// prevents the helper from becoming an injection sink if the constant
244+
// is ever sourced from outside the module.
245+
const dominantParams = sql.join(
246+
DOMINANT_NAF_SECTIONS.map((code) => sql`${code}`),
247+
sql`, `,
244248
);
245249
return sql<string>`case
246-
when upper(left(${companies.nafCode}, 1)) in (${sql.raw(dominantList)})
250+
when upper(left(${companies.nafCode}, 1)) in (${dominantParams})
247251
then upper(left(${companies.nafCode}, 1))
248252
else ${OTHER_NAF_SEGMENT}
249253
end`;

0 commit comments

Comments
 (0)