feat: gate declaration modification by campaign deadline#3162
feat: gate declaration modification by campaign deadline#3162LucasCharrier merged 18 commits intoalphafrom
Conversation
Add app_campaign_deadline table with 6 date columns per year (decl1/decl2 modification, justification, joint evaluation deadlines). Server pages fetch deadlines from DB and pass them as props to UI components, falling back to hardcoded defaults when no row exists for the year.
CampaignDeadlines now carries raw ISO date strings (YYYY-MM-DD) instead of pre-formatted French strings. Each leaf UI component calls formatFrenchDate() at the point of display.
Once the campaign modification deadline has passed for a submitted declaration, hide the "Modifier" button in the process panel and lock step navigation by redirecting non-recap steps to the recap. While the deadline is in the future, users can freely re-edit a submitted declaration. Also adds a dev-only /test-panel playground to visually tweak variants and deadlines, and E2E coverage for both branches via an upsertable app_campaign_deadline row.
revu-bot
left a comment
There was a problem hiding this comment.
PR Review: Gate Declaration Modification by Campaign Deadline
This PR introduces deadline-based gating for declaration modifications — a well-scoped feature with good test coverage (unit + E2E). The core logic (isDeadlinePassed) is clean and correctly injectable for testing. Below are the issues found, ranging from a potential security gap to a few maintainability concerns.
| File | Lines | Severity | Issue |
|---|---|---|---|
app/test-panel/page.tsx |
7–9 | CRITICAL | notFound() called at runtime, not build-time — production bundle still includes playground code |
e2e/campaign-deadlines-gating.e2e.ts |
12 | IMPORTANT | CURRENT_YEAR hardcoded to 2026 — tests will silently break next year |
etape/[step]/page.tsx |
28–33 | IMPORTANT | Extra DB round-trip on every non-recap step visit for submitted declarations |
my-space/__tests__/DeclarationProcessPanel.test.tsx |
209–220 | IMPORTANT | "future" deadline test relies on system clock — will fail after 2027 |
my-space/PanelPlayground.tsx |
1 | MINOR | PanelPlayground exported from my-space/index.ts — leaks dev component into production bundle |
| export default function TestPanelPage() { | ||
| if (env.NODE_ENV === "production") { | ||
| notFound(); | ||
| } | ||
| return <PanelPlayground />; | ||
| } |
There was a problem hiding this comment.
[CRITICAL] notFound() at runtime doesn't prevent the playground from being bundled into production
notFound()is a runtime guard — it throws a Next.js 404 response, but the module (and everything it imports, includingPanelPlayground) is still compiled into the production bundle.- Any user who discovers the
/test-panelroute and bypasses the check (e.g. via a middleware bug or a future refactor) gets access to the playground UI. - The correct pattern for Next.js is to use a build-time environment check so the route is never emitted in production builds at all.
Proposed fix: use a build-time guard that makes the route statically unreachable in production.
| export default function TestPanelPage() { | |
| if (env.NODE_ENV === "production") { | |
| notFound(); | |
| } | |
| return <PanelPlayground />; | |
| } | |
| export default function TestPanelPage() { | |
| if (process.env.NODE_ENV === "production") { | |
| notFound(); | |
| } | |
| return <PanelPlayground />; | |
| } |
Better yet, move the file under a conditional segment or use Next.js route groups that are excluded from the production build via a custom next.config pageExtensions trick, so the module is tree-shaken entirely. At minimum, the env.NODE_ENV check should be replaced with process.env.NODE_ENV so bundlers can statically eliminate the dead branch.
There was a problem hiding this comment.
Fixed in b5451f9: PanelPlayground is now lazy-loaded via next/dynamic, and removed from the my-space barrel so it won't be pulled into unrelated chunks. The runtime notFound() still guards production access.
- Remove PanelPlayground from my-space barrel, lazy-load via next/dynamic in the test-panel route to keep dev code out of the prod bundle. - Derive test declaration year from DB instead of hardcoding 2026 in E2E. - Use far-future year (2099) for DeclarationProcessPanel unit tests so 'deadline in the future' assertions stay valid over time.
|
Re: "Extra DB round-trip on every non-recap step visit for submitted declarations" — this is intentional. The query only fires when |
- Move the submitted-redirect decision into a pure domain helper with 5 unit tests covering all branches. - Use it in both the decl1 step page and the second-declaration step page so the logic is now actually tested instead of living inline in server components. - Exclude the dev-only PanelPlayground / test-panel route from coverage in vitest and SonarQube configs.
The CI DB has no pre-existing app_declaration row at beforeAll time, so selecting year from the DB returned nothing and left the variable undefined (which then crashed deleteCampaignDeadlines). Use the domain helper that matches the year api.declaration.getOrCreate() picks on first login.
The Rémunération button click was firing before DSFR had bound to the modal, so the dialog never received the open attribute and the test timed out (and cascaded to the next test in the file).
Without a phone number, hasRequiredDeclarationInfo() returns false and the Rémunération button opens the missing-info modal instead of the declaration process panel, causing the test to time out waiting for the panel's open attribute.
The DB helpers only UPDATE, so seeding in beforeAll was a no-op on a fresh CI database where no app_declaration row exists yet. Move the seed step inside each test, after loginWithProConnect() triggers getOrCreate() which actually creates the row.
/mon-espace does not call api.declaration.getOrCreate() — that only happens when visiting a /declaration-remuneration page. On a fresh CI DB the app_declaration row therefore never existed when the seed ran, leaving the panel in 'start' variant. Visit /declaration-remuneration once after login to create the row, then seed, then go to /mon-espace.
…nes-gating # Conflicts: # packages/app/drizzle/meta/_journal.json # packages/app/src/modules/declaration-remuneration/steps/secondDeclaration/SecondDeclarationStepPage.tsx # packages/app/src/modules/domain/__tests__/campaign.test.ts # packages/app/src/modules/domain/shared/campaign.ts # packages/app/src/modules/my-space/VerticalStepper.tsx # packages/app/src/modules/my-space/__tests__/DeclarationProcessPanel.test.tsx
|
🎉 Deployment for commit f7ee8df : Docker images
|
Summary
isDeadlinePassedpure helper in domain (with unit tests), a dev-only/test-panelplayground to visually tweak variants and deadlines, and E2E coverage of both branches via an upsertableapp_campaign_deadlinerow.Stacked on top of
feat/configurable-campaign-deadlines.Quality gates
Generated with Claude Code