Skip to content

Commit 11965f4

Browse files
ViczeiViczei
committed
refactor(declaration): collapse indicator 7 lookup to a single bounded query
Address revu-bot feedback on #3269: - Replace the per-declaration loop with an inner join on app_job_category plus ORDER BY year DESC LIMIT 1. The most recent submitted declaration containing indicator 7 is found in one round-trip, regardless of how many years of history the company has — fixes the N+1 pattern and removes the implicit unbounded scan. - Simplify fetchPreviousYearJobCategories unit tests: the mock now exposes two named chains (declaration probe, jobCategories fetch) whose terminals resolve on await, instead of tracking select() call counts.
1 parent b9d8e88 commit 11965f4

2 files changed

Lines changed: 71 additions & 108 deletions

File tree

‎packages/app/src/server/api/routers/__tests__/declarationHelpers.test.ts‎

Lines changed: 40 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,6 @@ describe("fetchAllCategories", () => {
199199
});
200200

201201
describe("fetchPreviousYearJobCategories", () => {
202-
type PreviousDeclarationRow = { id: string };
203202
type JobCategoryRow = {
204203
name: string;
205204
detail: string | null;
@@ -208,65 +207,61 @@ describe("fetchPreviousYearJobCategories", () => {
208207
};
209208

210209
/**
211-
* Builds a mocked Drizzle `tx` where:
212-
* - the first `select()` chain (declarations query) resolves to
213-
* `previousDeclarations` after `.orderBy()`
214-
* - each subsequent `select()` chain (jobCategories query per declaration)
215-
* resolves to the next entry in `jobsPerDeclaration` after `.where()`
210+
* Build a mocked `tx` with two select chains:
211+
* 1. declarations probe (innerJoin jobCategories, limit 1) → id or nothing
212+
* 2. jobCategories fetch for that id → rows
213+
*
214+
* Each chain resolves at its terminal call (`.limit()` / `.where()`), so
215+
* the mock is driven by the data the query is expected to return, not by
216+
* internal call-order counters.
216217
*/
217218
const makeTx = (
218-
previousDeclarations: PreviousDeclarationRow[],
219-
jobsPerDeclaration: JobCategoryRow[][],
219+
declarationId: string | null,
220+
jobs: JobCategoryRow[] = [],
220221
) => {
221-
let selectCallCount = 0;
222-
const queuedJobs = [...jobsPerDeclaration];
223-
224-
const mockSelect = vi.fn().mockImplementation(() => {
225-
selectCallCount++;
226-
if (selectCallCount === 1) {
227-
const mockOrderBy = vi.fn().mockResolvedValue(previousDeclarations);
228-
const mockWhere = vi.fn().mockReturnValue({ orderBy: mockOrderBy });
229-
const mockFrom = vi.fn().mockReturnValue({ where: mockWhere });
230-
return { from: mockFrom };
231-
}
232-
const mockWhere = vi.fn().mockResolvedValue(queuedJobs.shift() ?? []);
233-
const mockFrom = vi.fn().mockReturnValue({ where: mockWhere });
234-
return { from: mockFrom };
235-
});
236-
237-
return { select: mockSelect } as never;
222+
const declarationChain = {
223+
from: () => ({
224+
innerJoin: () => ({
225+
where: () => ({
226+
orderBy: () => ({
227+
limit: () =>
228+
Promise.resolve(declarationId ? [{ id: declarationId }] : []),
229+
}),
230+
}),
231+
}),
232+
}),
233+
};
234+
235+
const jobCategoriesChain = {
236+
from: () => ({
237+
where: () => Promise.resolve(jobs),
238+
}),
239+
};
240+
241+
const queue = [declarationChain, jobCategoriesChain];
242+
return { select: () => queue.shift() } as never;
238243
};
239244

240-
it("returns null when no submitted previous declaration exists", async () => {
245+
it("returns null when no previous declaration contains indicator 7", async () => {
241246
const { fetchPreviousYearJobCategories } = await import(
242247
"../declarationHelpers"
243248
);
244249

245-
const tx = makeTx([], []);
246-
247-
const result = await fetchPreviousYearJobCategories(tx, "123456789", 2026);
248-
249-
expect(result).toBeNull();
250-
});
251-
252-
it("returns null when no previous declaration contains job categories", async () => {
253-
const { fetchPreviousYearJobCategories } = await import(
254-
"../declarationHelpers"
250+
const result = await fetchPreviousYearJobCategories(
251+
makeTx(null),
252+
"123456789",
253+
2026,
255254
);
256255

257-
const tx = makeTx([{ id: "decl-2025" }, { id: "decl-2024" }], [[], []]);
258-
259-
const result = await fetchPreviousYearJobCategories(tx, "123456789", 2026);
260-
261256
expect(result).toBeNull();
262257
});
263258

264-
it("returns categories from the most recent previous declaration", async () => {
259+
it("returns categories from the most recent qualifying declaration", async () => {
265260
const { fetchPreviousYearJobCategories } = await import(
266261
"../declarationHelpers"
267262
);
268263

269-
const mostRecentJobs: JobCategoryRow[] = [
264+
const tx = makeTx("decl-2024", [
270265
{
271266
name: "Employés",
272267
detail: "Support",
@@ -279,9 +274,7 @@ describe("fetchPreviousYearJobCategories", () => {
279274
source: "convention-collective",
280275
categoryIndex: 0,
281276
},
282-
];
283-
284-
const tx = makeTx([{ id: "decl-2025" }], [mostRecentJobs]);
277+
]);
285278

286279
const result = await fetchPreviousYearJobCategories(tx, "123456789", 2026);
287280

@@ -294,48 +287,19 @@ describe("fetchPreviousYearJobCategories", () => {
294287
});
295288
});
296289

297-
it("falls back to an earlier year when N-1 has no job categories", async () => {
298-
const { fetchPreviousYearJobCategories } = await import(
299-
"../declarationHelpers"
300-
);
301-
302-
const olderJobs: JobCategoryRow[] = [
303-
{
304-
name: "Cadres",
305-
detail: "Managers",
306-
source: "autre",
307-
categoryIndex: 0,
308-
},
309-
];
310-
311-
const tx = makeTx(
312-
[{ id: "decl-2025" }, { id: "decl-2023" }],
313-
[[], olderJobs],
314-
);
315-
316-
const result = await fetchPreviousYearJobCategories(tx, "123456789", 2026);
317-
318-
expect(result).toEqual({
319-
source: "autre",
320-
categories: [{ name: "Cadres", detail: "Managers" }],
321-
});
322-
});
323-
324290
it("uses empty string for null detail", async () => {
325291
const { fetchPreviousYearJobCategories } = await import(
326292
"../declarationHelpers"
327293
);
328294

329-
const jobs: JobCategoryRow[] = [
295+
const tx = makeTx("decl-2024", [
330296
{
331297
name: "Cadres",
332298
detail: null,
333299
source: "autre",
334300
categoryIndex: 0,
335301
},
336-
];
337-
338-
const tx = makeTx([{ id: "decl-2025" }], [jobs]);
302+
]);
339303

340304
const result = await fetchPreviousYearJobCategories(tx, "123456789", 2026);
341305

‎packages/app/src/server/api/routers/declarationHelpers.ts‎

Lines changed: 31 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -97,48 +97,47 @@ export async function fetchPreviousYearJobCategories(
9797
siren: string,
9898
currentYear: number,
9999
) {
100-
// Walk previous submitted declarations from most recent down and pick the
101-
// first one that actually contains indicator 7 (i.e. has job categories).
102-
// A company may skip year N-1 or submit without indicator 7, in which case
103-
// we fall back to earlier years.
104-
const previousDeclarations = await tx
100+
// Find the most recent submitted declaration (strictly before currentYear)
101+
// that actually contains indicator 7 — i.e. has at least one row in
102+
// app_job_category. The inner join filters empty declarations out; ordering
103+
// by year desc + limit 1 keeps this to a single bounded round-trip, so a
104+
// company with no indicator 7 on N-1 falls back to N-2 / N-3 automatically.
105+
const [previousDeclaration] = await tx
105106
.select({ id: declarations.id })
106107
.from(declarations)
108+
.innerJoin(jobCategories, eq(jobCategories.declarationId, declarations.id))
107109
.where(
108110
and(
109111
eq(declarations.siren, siren),
110112
lt(declarations.year, currentYear),
111113
eq(declarations.status, "submitted"),
112114
),
113115
)
114-
.orderBy(desc(declarations.year));
115-
116-
for (const { id } of previousDeclarations) {
117-
const jobs = await tx
118-
.select({
119-
name: jobCategories.name,
120-
detail: jobCategories.detail,
121-
source: jobCategories.source,
122-
categoryIndex: jobCategories.categoryIndex,
123-
})
124-
.from(jobCategories)
125-
.where(eq(jobCategories.declarationId, id));
126-
127-
if (jobs.length === 0) continue;
128-
129-
const sorted = [...jobs].sort((a, b) => a.categoryIndex - b.categoryIndex);
130-
const source = sorted[0]?.source ?? "";
131-
132-
return {
133-
source,
134-
categories: sorted.map((j) => ({
135-
name: j.name,
136-
detail: j.detail ?? "",
137-
})),
138-
};
139-
}
116+
.orderBy(desc(declarations.year))
117+
.limit(1);
118+
119+
if (!previousDeclaration) return null;
120+
121+
const jobs = await tx
122+
.select({
123+
name: jobCategories.name,
124+
detail: jobCategories.detail,
125+
source: jobCategories.source,
126+
categoryIndex: jobCategories.categoryIndex,
127+
})
128+
.from(jobCategories)
129+
.where(eq(jobCategories.declarationId, previousDeclaration.id));
130+
131+
const sorted = [...jobs].sort((a, b) => a.categoryIndex - b.categoryIndex);
132+
const source = sorted[0]?.source ?? "";
140133

141-
return null;
134+
return {
135+
source,
136+
categories: sorted.map((j) => ({
137+
name: j.name,
138+
detail: j.detail ?? "",
139+
})),
140+
};
142141
}
143142

144143
type JobCategoryRow = typeof jobCategories.$inferSelect;

0 commit comments

Comments
 (0)