Skip to content

fix(commitments): le formulaire ne demandait jamais la date de premiere echeance - #296

Open
thierryvm wants to merge 3 commits into
mainfrom
fix/echeances-date-premiere-echeance
Open

fix(commitments): le formulaire ne demandait jamais la date de premiere echeance#296
thierryvm wants to merge 3 commits into
mainfrom
fix/echeances-date-premiere-echeance

Conversation

@thierryvm

@thierryvm thierryvm commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Le défaut

Signalé par @Thierry sur le plan réel « SPF impôt » : 2 407,93 € sur 11 mensualités depuis le 15/05/2026, 4 payées. La carte annonçait « 4/11 échéances de 220 € · dernière en Mai 2027 » — deux mois trop tard, et un montant faux.

Diagnostic — les trois hypothèses de départ sont réfutées

L'arithmétique n'a jamais été en cause. installmentPeriods fait bien first + i · cycle avec i de 0 à total − 1 ; aucun new Date(y, m, d) ni setMonth() sur ce chemin ; formatMonth décale correctement.

La ligne en base portait start_month = 7 — son mois de création (21/07/2026) — et non 5. CommitmentsClient câblait startYear/startMonth sur la période affichée et paymentDay à 1, et le formulaire d'édition ne les renvoyait jamais. La date saisie par l'utilisateur n'avait aucun chemin vers la base. first + (total − 1) depuis une ancre que personne n'a choisie donne mai 2027. L'écart de +2 mois est l'écart entre mai et juillet.

Le reste dû (1 527,93 €) et le compteur 4/11 étaient justes : ils lisent le calendrier, pas la documentation.

Ce que fait cette PR

1. ADR-021 D3 corrigé — l'ancre est la PREMIÈRE échéance. D3 disait « la prochaine » ; rien ne l'implémentait. Le grand livre compte les cochages depuis l'ancre et + remplit la plus ancienne échéance non payée du calendrier — deux comportements qui n'ont de sens que si l'ancre ouvre l'historique payable. Le motif du choix est écrit dans l'ADR, pas seulement la conclusion.

2. Le formulaire demande enfin la date. Première échéance (mois + année en deux contrôles, pas input[type=month] que Safari iOS dégrade), jour de prélèvement optionnel, périodicité. Et il rend la conséquence avant validation : « Échéancier : 15 mai 2026 → 15 mars 2027 ».

3. La dernière échéance est dérivée, en Decimal. total − (n − 1) × mensualité. En flottant natif, 2407.93 - 10 * 220 vaut 207.92999999999984. La carte lit « 10 × 220 € + 207,93 € » au lieu de prétendre 11 échéances identiques. L'en-tête de justification de schedule.ts — « un compte fixe d'échéances identiques » — est réécrit : il ne tenait plus dès qu'on dérive un solde résiduel.

4. La troisième dérivation disparaît. engagements-lisses.ts portait sa propre copie de start + (total − 1) · step avec un helper ordinal() privé, sans filet : rien n'aurait échoué si la définition avait changé. Elle consomme endOrdinal(). Les six recopies de (total % 12) + 1 hors engagements (charges, prévisions) sont signalées et non touchées — autre concern.

5. Le jour ne s'affiche que s'il a été saisi. La colonne est not null default 1 et l'ancien formulaire y écrivait 1 en dur : afficher ce 1 montrerait une date inventée à toutes les lignes existantes. Un 1er réellement choisi s'affiche donc en mois + année — le côté bénin assumé du compromis.

Aucune migration, aucune correction de données

La ligne de production reste fausse après cette PR, délibérément. Seul son propriétaire sait que la vraie première échéance est le 15/05/2026 ; un script qui devinerait serait une correction silencieuse de données financières. La correction se fait depuis le formulaire d'édition, désormais capable de la porter.

Vérification

Toutes les portes vertes, plus un rendu réel : npm run dev contre la stack Supabase locale, compte éphémère, cas SPF saisi via le formulaire et 4 échéances cochées via +.

Surface Rendu mesuré
Aperçu formulaire Échéancier : 15 mai 2026 → 15 mars 2027
Carte FR 4/11 échéances : 10 × 220 € + 207,93 € · dernière : 15 mars 2027Reste 1 527,93 €
Carte EN 4/11 instalments: 10 × €220 + €207.93 · last: March 15, 2027
Tableau de bord 7 échéances restantes · fin : 15 mars 20271 527,93 €

lint 0 erreur · lint:use-server ✅ · typecheck 0 · test 140 fichiers / 1871 cas, 100 % · build ✅ · dev démarre et sert (.next purgé entre les deux).

Tests ajoutés : le cas SPF de bout en bout, plan à une seule échéance, dernière échéance de montant différent, passage d'année (dont ancre décembre), fin de mois 31 → 28/29 février et 30 → février, plan entièrement soldé, plan sur-spécifié (mensualités > total → reste borné à 0, jamais négatif), et un verrou d'équivalence entre endOrdinal() et la dernière période planifiée sur les quatre périodicités.

Hors périmètre — deux constats mesurés pendant la vérification

  • La barre de progression est cassée en navigateur, avant cette PR. Mesuré au DOM : aria-valuenow="36" et style="width:36%" bien présents, mais largeur calculée 1054 px sur une piste de 1054 px. La CSP (style-src 'self' 'nonce-…' 'unsafe-inline') bloque l'attribut style — un nonce annule 'unsafe-inline', et sans style-src-attr les attributs retombent sur style-src. La barre lit donc toujours 100 % pour un utilisateur voyant, alors que le lecteur d'écran reçoit la bonne valeur. Code non touché ici ; correctif = PR dédiée (CSP).
  • Avertissement d'hydratation sur / et /login — pages non touchées, donc antérieur.

🤖 Generated with Claude Code

Summary by Sourcery

Aligner le domaine des engagements et l’interface utilisateur sur un premier versement comme ancre, dériver précisément les derniers versements irréguliers, et mettre à jour le formulaire d’engagements ainsi que la documentation pour exposer et prévisualiser le calendrier complet.

Nouvelles fonctionnalités :

  • Ajouter des champs de date du premier versement, de jour de paiement facultatif et de fréquence au formulaire d’engagements, avec une fenêtre de prévisualisation du calendrier en temps réel.
  • Introduire un formatage des dates de versement qui affiche soit des dates complètes, soit mois/année, selon qu’un jour de paiement a été explicitement choisi ou non.

Corrections de bugs :

  • S’assurer que les engagements nouvellement créés utilisent la date du premier versement saisie par l’utilisateur au lieu d’être silencieusement ancrés sur le mois de création.
  • Corriger les calculs de date de fin et de solde restant en basant les fenêtres de calendrier sur le premier versement et en utilisant une arithmétique Decimal précise pour le dernier versement et le solde résiduel.
  • Éviter d’afficher des jours de paiement inventés pour les engagements existants lorsque le jour n’a jamais été explicitement choisi.

Améliorations :

  • Étendre le domaine de planification des engagements avec des helpers d’ordinaux partagés, des dates calendaires explicites, un calcul de prochaine échéance, et une détection des derniers versements irréguliers consommés à la fois par le cockpit et l’interface des engagements.
  • Refactoriser les calculs de lissage du cockpit pour qu’ils s’appuient sur les ordinaux de calendrier exportés et les helpers de cadence au lieu de les re-dériver localement.
  • Clarifier et amender ADR-021 et la spécification de l’epic afin de documenter que l’ancre est le premier versement et que le montant du dernier versement est dérivé du total.

Tests :

  • Ajouter des tests de calendrier complets couvrant le cas de taxe SPF signalé, les plans à versement unique, les frontières d’année, le clampage de fin de mois, les plans sur-spécifiés, et l’équivalence entre endOrdinal et la dernière période planifiée.
  • Étendre les tests de CommitmentsClient et du formateur i18n pour couvrir les nouveaux champs du formulaire, la prévisualisation de fenêtre, le comportement du jour de paiement, et la cohérence du formatage des dates de versement.
Original summary in English

Summary by Sourcery

Align commitments domain and UI on a first-installment anchor, derive irregular final installments precisely, and update the commitments form and documentation to expose and preview the full schedule.

New Features:

  • Add first-installment date, optional payment day, and frequency fields to the commitments form with a live schedule window preview.
  • Introduce installment date formatting that shows either full dates or month/year depending on whether a payment day was explicitly chosen.

Bug Fixes:

  • Ensure newly created commitments use the user-entered first installment date instead of silently anchoring on the creation month.
  • Fix end-date and remaining-balance calculations by basing schedule windows on the first installment and by using precise Decimal arithmetic for the final installment and residual balance.
  • Avoid displaying invented payment days for existing commitments where the day was never explicitly chosen.

Enhancements:

  • Extend the commitments scheduling domain with shared ordinal helpers, explicit calendar dates, next-due computation, and irregular-final-installment detection consumed by both cockpit and commitments UI.
  • Refactor cockpit smoothing calculations to rely on exported schedule ordinals and cadence helpers instead of re-deriving them locally.
  • Clarify and amend ADR-021 and the epic spec to document that the anchor is the first installment and that the last installment amount is derived from the total.

Tests:

  • Add comprehensive schedule tests covering the reported SPF tax case, single-installment plans, year boundaries, end-of-month clamping, over-specified plans, and equivalence between endOrdinal and the last scheduled period.
  • Extend CommitmentsClient and i18n formatter tests to cover the new form fields, window preview, payment day behavior, and installment date formatting consistency.

Reported on the real « SPF impôt » plan: 2 407,93 € over 11 monthly
instalments from 15/05/2026, 4 paid. The card announced « 11 échéances de
220 € · dernière en Mai 2027 » — two months late, and a wrong amount.

The schedule arithmetic was never at fault. Read from production, the row
carried `start_month = 7` (its creation month, 21/07/2026), not 5:
`CommitmentsClient` hard-coded `startYear/startMonth` to the viewed period
and `paymentDay` to 1, and the edit form never sent them back. The typed
date had no way of reaching the database, so `first + (total - 1)` landed
on May 2027 from an anchor nobody chose.

- ADR-021 D3 said the anchor was the NEXT instalment; nothing implemented
  it. The ledger counts ticks from the anchor forward and `+` fills the
  oldest unpaid SCHEDULED period - both only make sense if the anchor is
  the FIRST instalment. D3 is corrected, with the reasoning written down.
- The form now asks for the first instalment, the payment day (optional)
  and the cadence, and previews the resulting window before submission.
- The final instalment is derived - `total - (n - 1) x instalment` - in
  Decimal: `2407.93 - 10 * 220` is 207.92999999999984 in float. The card
  reads « 10 x 220 € + 207,93 € » instead of claiming 11 identical ones.
- `engagements-lisses.ts` carried a third, unguarded copy of the window's
  upper bound; it now consumes `endOrdinal()`.
- The payment day renders only when explicitly chosen: the column is
  `not null default 1`, so showing the stored 1 would invent a date on
  every pre-existing row.

No migration, no data correction: the anchor of existing rows is only
knowable by their owner, and is now fixable from the edit form.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@vercel

vercel Bot commented Aug 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
ankora Ready Ready Preview Aug 2, 2026 8:02pm

@github-actions github-actions Bot added status:review-needed Ready for review type:fix Bug fix labels Aug 2, 2026
@sourcery-ai

sourcery-ai Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Guide du·de la relecteur·rice

Cette PR corrige la façon dont les échéanciers de engagements sont ancrés et affichés en traitant l’ancre comme la date de la première échéance, en propageant cette date à travers le formulaire et le domaine, en calculant les montants de la dernière échéance avec Decimal, et en alignant le rendu du tableau de bord/cartes ainsi que la logique du cockpit avec les nouvelles sémantiques d’échéancier corrigées.

Diagramme de séquence pour le nouveau formulaire d’engagement et l’aperçu de l’échéancier

sequenceDiagram
  actor User
  participant CommitmentsClient
  participant CommitmentsDomain as CommitmentsDomain(schedule.ts)
  participant ServerActions as createCommitmentAction

  User->>CommitmentsClient: Type startMonth, startYear, paymentDay, installmentsTotal
  CommitmentsClient->>CommitmentsDomain: firstInstallmentDate(draft)
  CommitmentsClient->>CommitmentsDomain: endInstallmentDate(draft)
  CommitmentsDomain-->>CommitmentsClient: first / last InstallmentDate
  CommitmentsClient-->>User: Render window preview (formatInstallmentDate)

  User->>CommitmentsClient: Submit form
  CommitmentsClient->>ServerActions: createCommitmentAction(payload with startYear, startMonth, paymentDay, frequency)
  ServerActions-->>CommitmentsClient: ok
  CommitmentsClient-->>User: Show success, list card uses endInstallmentDate/lastInstallmentAmount
Loading

Modifications au niveau des fichiers

Changement Détails Fichiers
Traiter l’ancre de l’engagement comme la première échéance et exporter depuis le domaine les helpers partagés d’échéancier/ordinal, y compris les dérivations de dates et de montants.
  • Refactorer schedule.ts pour définir l’ancre comme la première période d’échéance et dériver la dernière échéance via endOrdinal et installmentOrdinal.
  • Introduire les types et helpers InstallmentDate (installmentDate, firstInstallmentDate, endInstallmentDate, hasExplicitPaymentDay, nextDuePeriod) avec limitation en fin de mois et sémantique de jour de paiement.
  • Basculer les calculs monétaires du domaine des engagements vers Decimal via le helper money, en ajoutant lastInstallmentAmount, installmentAmountAt, hasIrregularFinalInstallment, et en rendant remainingBalance borné et exact.
src/lib/domain/commitments/schedule.ts
src/lib/domain/commitments/index.ts
src/lib/domain/commitments/__tests__/schedule.test.ts
Mettre à jour le formulaire d’engagements pour collecter et valider la date de la première échéance, le jour de paiement optionnel et la fréquence, et prévisualiser la fenêtre d’échéancier résultante avant la soumission.
  • Ajouter un état contrôlé pour startMonth, startYear, paymentDay et frequency dans CommitmentsClient avec un comportement de réinitialisation lié à la période courante et aux valeurs par défaut.
  • Valider l’année/mois d’ancrage et le jour de paiement dans handleSubmit, implémenter la convention PAYMENT_DAY_UNSET (1 comme valeur par défaut signifiant « jamais choisi ») et envoyer ces champs directement aux actions de création/mise à jour, y compris frequency pour les engagements non ponctuels.
  • Rendre de nouveaux champs de formulaire pour le mois/année de la première échéance, le jour de paiement et la fréquence, avec labels, aides et options de sélection i18n, ainsi qu’un aperçu en direct draftWindow de la fenêtre d’échéancier en utilisant les helpers de domaine et formatInstallmentDate.
  • Ajuster les tests pour vérifier que la date de première échéance et le jour de paiement saisis atteignent bien l’action, que l’aperçu affiche la bonne fenêtre et que le pré-remplissage du jour de paiement respecte la sémantique « jamais choisi ».
src/app/[locale]/app/commitments/CommitmentsClient.tsx
src/app/[locale]/app/commitments/__tests__/CommitmentsClient.test.tsx
Modifier les résumés d’engagements et le rendu des cartes du tableau de bord pour utiliser les dates d’échéance et un message de dernière échéance irrégulière au lieu de supposer des échéances identiques ou un ancrage sur le mois de création.
  • Remplacer l’utilisation de endPeriod et des startMonth/startYear bruts par firstInstallmentDate et endInstallmentDate plus formatInstallmentDate à la fois dans le résumé de la liste d’engagements et dans la carte de tableau de bord EngagementsCard.
  • Utiliser hasIrregularFinalInstallment et lastInstallmentAmount pour choisir entre un texte standard « N échéances de X » et un libellé « (N-1) × X + dernière » dans les résumés de cartes.
  • S’assurer que les engagements ponctuels s’affichent en utilisant la date de la première échéance plutôt que les startMonth/startYear stockés.
  • Ajouter le formateur formatInstallmentDate et des tests pour rendre soit mois+année, soit la date complète selon qu’un jour de paiement est explicite ou non, avec des chaînes de dates ISO remplies (date seule) pour éviter les décalages de fuseau horaire.
src/app/[locale]/app/commitments/CommitmentsClient.tsx
src/components/dashboard/EngagementsCard.tsx
src/lib/i18n/formatters.ts
src/lib/i18n/__tests__/formatters.test.ts
Aligner la logique de lissage du cockpit avec les helpers partagés d’échéancier/ordinal et mettre à jour la documentation (ADR et spécification) pour refléter les sémantiques d’ancre corrigées et le comportement de dérivation de la dernière échéance.
  • Refactorer engagements-lisses.ts pour consommer endOrdinal, periodOrdinal et monthsPerCycle depuis le domaine des engagements plutôt que de re-dériver localement les bornes d’échéancier et les longueurs de cycle.
  • Ajuster le calcul de lissage pour diviser par monthsPerCycle plutôt que par une map locale CYCLE_MONTHS.
  • Mettre à jour ADR-021 pour documenter que l’ancre est la première échéance, expliquer la logique et décrire l’absence de migration ainsi que le comportement d’affichage du jour de paiement.
  • Modifier la spécification de l’epic pour changer D3 de « solde restant + échéances restantes » à « montant total + échéances totales + date de la première échéance » et noter que la dernière échéance est désormais dérivée ; marquer les échéanciers arbitrairement irréguliers comme hors périmètre.
src/lib/domain/cockpit/engagements-lisses.ts
docs/adr/ADR-021-engagements-dans-le-cockpit.md
docs/plans/epic-dettes-echeanciers-spec.md

Conseils et commandes

Interagir avec Sourcery

  • Déclencher une nouvelle revue : Commentez @sourcery-ai review sur la pull request.
  • Poursuivre les discussions : Répondez directement aux commentaires de revue de Sourcery.
  • Générer une issue GitHub à partir d’un commentaire de revue : Demandez à Sourcery de créer une issue à partir d’un commentaire de revue en y répondant. Vous pouvez aussi répondre à un commentaire de revue avec @sourcery-ai issue pour créer une issue à partir de celui-ci.
  • Générer un titre de pull request : Écrivez @sourcery-ai n’importe où dans le titre de la pull request pour générer un titre à tout moment. Vous pouvez aussi commenter @sourcery-ai title sur la pull request pour (re)générer le titre à tout moment.
  • Générer un résumé de pull request : Écrivez @sourcery-ai summary n’importe où dans le corps de la pull request pour générer un résumé de PR à tout moment exactement à l’endroit souhaité. Vous pouvez aussi commenter @sourcery-ai summary sur la pull request pour (re)générer le résumé à tout moment.
  • Générer le guide du·de la relecteur·rice : Commentez @sourcery-ai guide sur la pull request pour (re)générer le guide du·de la relecteur·rice à tout moment.
  • Résoudre tous les commentaires Sourcery : Commentez @sourcery-ai resolve sur la pull request pour marquer tous les commentaires Sourcery comme résolus. Utile si vous avez déjà traité tous les commentaires et ne souhaitez plus les voir.
  • Ignorer toutes les revues Sourcery : Commentez @sourcery-ai dismiss sur la pull request pour ignorer toutes les revues Sourcery existantes. Particulièrement utile si vous voulez repartir de zéro avec une nouvelle revue – n’oubliez pas de commenter @sourcery-ai review pour déclencher une nouvelle revue !

Personnaliser votre expérience

Accédez à votre tableau de bord pour :

  • Activer ou désactiver des fonctionnalités de revue comme le résumé de pull request généré par Sourcery, le guide du·de la relecteur·rice et d’autres.
  • Changer la langue de revue.
  • Ajouter, supprimer ou modifier des instructions de revue personnalisées.
  • Ajuster d’autres paramètres de revue.

Obtenir de l’aide

Original review guide in English

Reviewer's Guide

This PR fixes how commitments schedules are anchored and displayed by treating the anchor as the first installment date, wiring that date through the form and domain, deriving final installment amounts with Decimal, and aligning dashboard/card rendering and cockpit logic with the corrected schedule semantics.

Sequence diagram for new commitment form and schedule preview

sequenceDiagram
  actor User
  participant CommitmentsClient
  participant CommitmentsDomain as CommitmentsDomain(schedule.ts)
  participant ServerActions as createCommitmentAction

  User->>CommitmentsClient: Type startMonth, startYear, paymentDay, installmentsTotal
  CommitmentsClient->>CommitmentsDomain: firstInstallmentDate(draft)
  CommitmentsClient->>CommitmentsDomain: endInstallmentDate(draft)
  CommitmentsDomain-->>CommitmentsClient: first / last InstallmentDate
  CommitmentsClient-->>User: Render window preview (formatInstallmentDate)

  User->>CommitmentsClient: Submit form
  CommitmentsClient->>ServerActions: createCommitmentAction(payload with startYear, startMonth, paymentDay, frequency)
  ServerActions-->>CommitmentsClient: ok
  CommitmentsClient-->>User: Show success, list card uses endInstallmentDate/lastInstallmentAmount
Loading

File-Level Changes

Change Details Files
Treat the commitment anchor as the first installment and export shared schedule/ordinal helpers from the domain, including date and amount derivations.
  • Rework schedule.ts to define the anchor as the first installment period and derive the last installment via endOrdinal and installmentOrdinal.
  • Introduce InstallmentDate types and helpers (installmentDate, firstInstallmentDate, endInstallmentDate, hasExplicitPaymentDay, nextDuePeriod) with end-of-month clamping and payment-day semantics.
  • Switch monetary computations in the commitments domain to Decimal via the money helper, adding lastInstallmentAmount, installmentAmountAt, hasIrregularFinalInstallment, and tightening remainingBalance to be clamped and exact.
src/lib/domain/commitments/schedule.ts
src/lib/domain/commitments/index.ts
src/lib/domain/commitments/__tests__/schedule.test.ts
Update the commitments form to collect and validate the first installment date, optional payment day, and frequency, and preview the resulting schedule window before submission.
  • Add controlled state for startMonth, startYear, paymentDay, and frequency to CommitmentsClient with reset behavior tied to the current period and defaults.
  • Validate anchor year/month and payment day in handleSubmit, implement the PAYMENT_DAY_UNSET convention (1 as default meaning "never chosen"), and send these fields directly to create/update actions including frequency for non-one-off commitments.
  • Render new form inputs for first installment month/year, payment day, and frequency with i18n labels, hints, and select options, plus a live draftWindow preview of the schedule span using domain helpers and formatInstallmentDate.
  • Adjust tests to assert that the typed first installment date and payment day reach the action, that the preview shows the correct window, and that payment day prefill respects the "never chosen" semantics.
src/app/[locale]/app/commitments/CommitmentsClient.tsx
src/app/[locale]/app/commitments/__tests__/CommitmentsClient.test.tsx
Change commitments summaries and dashboard card rendering to use installment dates and irregular-final-installment messaging instead of assuming identical installments or anchoring on creation month.
  • Replace use of endPeriod and raw startMonth/startYear with firstInstallmentDate and endInstallmentDate plus formatInstallmentDate in both the commitments list summary and the dashboard EngagementsCard.
  • Use hasIrregularFinalInstallment and lastInstallmentAmount to choose between regular "N installments of X" text and "(N-1) × X + last" wording in card summaries.
  • Ensure one-off commitments render using the first installment date rather than stored startMonth/year.
  • Add formatInstallmentDate formatter and tests to render either month+year or full date based on whether a payment day is explicit, with ISO-padded date-only strings to avoid timezone shifts.
src/app/[locale]/app/commitments/CommitmentsClient.tsx
src/components/dashboard/EngagementsCard.tsx
src/lib/i18n/formatters.ts
src/lib/i18n/__tests__/formatters.test.ts
Align cockpit smoothing logic with the shared schedule/ordinal helpers and update documentation (ADR and spec) to reflect the corrected anchor semantics and derived final installment behavior.
  • Refactor engagements-lisses.ts to consume endOrdinal, periodOrdinal, and monthsPerCycle from the commitments domain instead of re-deriving schedule bounds and cycle lengths locally.
  • Adjust smoothing computation to divide by monthsPerCycle rather than a local CYCLE_MONTHS map.
  • Update ADR-021 to document that the anchor is the first installment, explain the rationale, and describe the no-migration, payment-day-display behavior.
  • Amend the epic spec to change D3 from "remaining balance + remaining installments" to "total amount + total installments + first installment date" and note that the last installment is now derived; mark arbitrary irregular schedules as out of scope.
src/lib/domain/cockpit/engagements-lisses.ts
docs/adr/ADR-021-engagements-dans-le-cockpit.md
docs/plans/epic-dettes-echeanciers-spec.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - j'ai trouvé 1 problème et laissé quelques retours globaux :

  • La convention PAYMENT_DAY_UNSET = 1 est actuellement dupliquée entre l'UI et le domaine (via hasExplicitPaymentDay) ; envisage de centraliser ce sentinelle dans le domaine des commitments et de le consommer depuis le client pour éviter toute divergence si la valeur par défaut change.
  • La logique de validation année/mois/jour pour la fenêtre de brouillon et le submit handler dans CommitmentsClient est répétée avec des bornes en dur ; extraire un helper partagé ou s'appuyer sur un validateur au niveau du domaine rendrait les futures modifications des plages valides moins sujettes aux erreurs.
Prompt pour les agents IA
Merci de prendre en compte les commentaires de cette revue de code :

## Commentaires généraux
- La convention `PAYMENT_DAY_UNSET = 1` est actuellement dupliquée entre l'UI et le domaine (via `hasExplicitPaymentDay`) ; envisage de centraliser ce sentinelle dans le domaine des commitments et de le consommer depuis le client pour éviter toute divergence si la valeur par défaut change.
- La logique de validation année/mois/jour pour la fenêtre de brouillon et le submit handler dans `CommitmentsClient` est répétée avec des bornes en dur ; extraire un helper partagé ou s'appuyer sur un validateur au niveau du domaine rendrait les futures modifications des plages valides moins sujettes aux erreurs.

## Commentaires individuels

### Commentaire 1
<location path="src/app/[locale]/app/commitments/__tests__/CommitmentsClient.test.tsx" line_range="286-295" />
<code_context>
+  it('sends the FIRST-instalment date the user typed, not the viewed period', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Ajoute des tests pour un mois/année de début et un jour de paiement invalides afin de couvrir les nouvelles branches de validation côté client.

Le test actuel ne couvre que le chemin de succès. Pour exercer les nouveaux garde-fous, ajoute des tests qui soumettent le formulaire avec :

- Une année invalide (par ex. `1999` ou `"abcd"`) et vérifie que `createMock` n'est pas appelé.
- Un mois invalide (par ex. `0` ou `13`) et vérifie que `createMock` n'est pas appelé.
- Un jour de paiement invalide (par ex. `0`, `32` ou non numérique) et vérifie que `createMock` n'est pas appelé.

Cela garantit que le formulaire n'est pas soumis lorsque ces validations côté client échouent.

Implémentation suggérée :

```typescript
  // The regression that started all this: the form used to hard-code the anchor
  // to the CREATION month, so a plan whose first instalment was two months
  // earlier ended two months late. The typed date must reach the action.

  it('does not submit when the start year is invalid', async () => {
    createMock.mockResolvedValue({ ok: true });
    renderPage([], { currentPeriod: { year: 2026, month: 7 } });

    fireEvent.click(screen.getByTestId('commitments-add-toggle'));
    fireEvent.change(screen.getByLabelText('Libellé'), { target: { value: 'SPF impôt' } });
    fireEvent.change(screen.getByLabelText(/Montant total dû/), { target: { value: '2407.93' } });
    fireEvent.change(screen.getByLabelText(/Montant par échéance/), { target: { value: '220' } });
    fireEvent.change(screen.getByLabelText(/Nombre total d'échéances/), {
      target: { value: '11' },
    });
    // Valid month so that only the year is invalid
    fireEvent.change(screen.getByTestId('commitment-start-month'), { target: { value: '5' } });
    // Invalid year (too old)
    fireEvent.change(screen.getByTestId('commitment-start-year'), { target: { value: '1999' } });

    await act(async () => {
      fireEvent.submit(screen.getByRole('button', { name: /^ajouter$/i }).closest('form')!);
    });

    expect(createMock).not.toHaveBeenCalled();
  });

  it('does not submit when the start month is invalid', async () => {
    createMock.mockResolvedValue({ ok: true });
    renderPage([], { currentPeriod: { year: 2026, month: 7 } });

    fireEvent.click(screen.getByTestId('commitments-add-toggle'));
    fireEvent.change(screen.getByLabelText('Libellé'), { target: { value: 'SPF impôt' } });
    fireEvent.change(screen.getByLabelText(/Montant total dû/), { target: { value: '2407.93' } });
    fireEvent.change(screen.getByLabelText(/Montant par échéance/), { target: { value: '220' } });
    fireEvent.change(screen.getByLabelText(/Nombre total d'échéances/), {
      target: { value: '11' },
    });
    fireEvent.change(screen.getByTestId('commitment-start-year'), { target: { value: '2026' } });
    // Invalid month (13)
    fireEvent.change(screen.getByTestId('commitment-start-month'), { target: { value: '13' } });

    await act(async () => {
      fireEvent.submit(screen.getByRole('button', { name: /^ajouter$/i }).closest('form')!);
    });

    expect(createMock).not.toHaveBeenCalled();
  });

  it('does not submit when the payment day is invalid', async () => {
    createMock.mockResolvedValue({ ok: true });
    renderPage([], { currentPeriod: { year: 2026, month: 7 } });

    fireEvent.click(screen.getByTestId('commitments-add-toggle'));
    fireEvent.change(screen.getByLabelText('Libellé'), { target: { value: 'SPF impôt' } });
    fireEvent.change(screen.getByLabelText(/Montant total dû/), { target: { value: '2407.93' } });
    fireEvent.change(screen.getByLabelText(/Montant par échéance/), { target: { value: '220' } });
    fireEvent.change(screen.getByLabelText(/Nombre total d'échéances/), {
      target: { value: '11' },
    });
    fireEvent.change(screen.getByTestId('commitment-start-year'), { target: { value: '2026' } });
    fireEvent.change(screen.getByTestId('commitment-start-month'), { target: { value: '5' } });
    // Invalid payment day (32)
    fireEvent.change(screen.getByTestId('commitment-payment-day'), { target: { value: '32' } });

    await act(async () => {
      fireEvent.submit(screen.getByRole('button', { name: /^ajouter$/i }).closest('form')!);
    });

    expect(createMock).not.toHaveBeenCalled();
  });

  it('sends the FIRST-instalment date the user typed, not the viewed period', async () => {
    createMock.mockResolvedValue({ ok: true });
    renderPage([], { currentPeriod: { year: 2026, month: 7 } });
    fireEvent.click(screen.getByTestId('commitments-add-toggle'));
    fireEvent.change(screen.getByLabelText('Libellé'), { target: { value: 'SPF impôt' } });
    fireEvent.change(screen.getByLabelText(/Montant total dû/), { target: { value: '2407.93' } });
    fireEvent.change(screen.getByLabelText(/Montant par échéance/), { target: { value: '220' } });
    fireEvent.change(screen.getByLabelText(/Nombre total d'échéances/), {
      target: { value: '11' },
    });
    fireEvent.change(screen.getByTestId('commitment-start-month'), { target: { value: '5' } });

```

- Ajuste les sélecteurs pour les champs année et jour de paiement s'ils diffèrent des `data-testid="commitment-start-year"` et `data-testid="commitment-payment-day"` supposés (par exemple s'ils utilisent `getByLabelText` au lieu de `getByTestId`).
- Si la validation côté client distingue les valeurs numériques hors plage des valeurs non numériques, il peut être utile d'ajouter un test supplémentaire pour un jour de paiement non numérique (par ex. `"abcd"`), en miroir du test `does not submit when the payment day is invalid`.
- Assure-toi que `createMock` est correctement importé/mocké dans ce fichier de test afin que `expect(createMock).not.toHaveBeenCalled()` fonctionne ; si le mock est réinitialisé entre les tests (par ex. `beforeEach(createMock.mockReset)`), il faut conserver ce comportement.
</issue_to_address>

Sourcery est gratuit pour l'open source - si nos revues te plaisent, pense à les partager ✨
Aide-moi à être plus utile ! Clique sur 👍 ou 👎 sur chaque commentaire et j'utiliserai ces retours pour améliorer tes revues.
Original comment in English

Hey - I've found 1 issue, and left some high level feedback:

  • The PAYMENT_DAY_UNSET = 1 convention is currently duplicated between the UI and the domain (via hasExplicitPaymentDay); consider centralizing this sentinel in the commitments domain and consuming it from the client to avoid drift if the default ever changes.
  • Year/month/day validation logic for the draft window and submit handler in CommitmentsClient is repeated with hard-coded bounds; extracting a shared helper or relying on a domain-level validator would make future changes to valid ranges less error-prone.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `PAYMENT_DAY_UNSET = 1` convention is currently duplicated between the UI and the domain (via `hasExplicitPaymentDay`); consider centralizing this sentinel in the commitments domain and consuming it from the client to avoid drift if the default ever changes.
- Year/month/day validation logic for the draft window and submit handler in `CommitmentsClient` is repeated with hard-coded bounds; extracting a shared helper or relying on a domain-level validator would make future changes to valid ranges less error-prone.

## Individual Comments

### Comment 1
<location path="src/app/[locale]/app/commitments/__tests__/CommitmentsClient.test.tsx" line_range="286-295" />
<code_context>
+  it('sends the FIRST-instalment date the user typed, not the viewed period', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for invalid start month/year and payment day to cover the new client-side validation branches.

The current test only covers the successful path. To exercise the new guards, please add tests that submit the form with:

- An invalid year (e.g. `1999` or `"abcd"`) and assert `createMock` is not called.
- An invalid month (e.g. `0` or `13`) and assert `createMock` is not called.
- An invalid payment day (e.g. `0`, `32`, or non‑numeric) and assert `createMock` is not called.

This ensures the form does not submit when these client-side validations fail.

Suggested implementation:

```typescript
  // The regression that started all this: the form used to hard-code the anchor
  // to the CREATION month, so a plan whose first instalment was two months
  // earlier ended two months late. The typed date must reach the action.

  it('does not submit when the start year is invalid', async () => {
    createMock.mockResolvedValue({ ok: true });
    renderPage([], { currentPeriod: { year: 2026, month: 7 } });

    fireEvent.click(screen.getByTestId('commitments-add-toggle'));
    fireEvent.change(screen.getByLabelText('Libellé'), { target: { value: 'SPF impôt' } });
    fireEvent.change(screen.getByLabelText(/Montant total dû/), { target: { value: '2407.93' } });
    fireEvent.change(screen.getByLabelText(/Montant par échéance/), { target: { value: '220' } });
    fireEvent.change(screen.getByLabelText(/Nombre total d'échéances/), {
      target: { value: '11' },
    });
    // Valid month so that only the year is invalid
    fireEvent.change(screen.getByTestId('commitment-start-month'), { target: { value: '5' } });
    // Invalid year (too old)
    fireEvent.change(screen.getByTestId('commitment-start-year'), { target: { value: '1999' } });

    await act(async () => {
      fireEvent.submit(screen.getByRole('button', { name: /^ajouter$/i }).closest('form')!);
    });

    expect(createMock).not.toHaveBeenCalled();
  });

  it('does not submit when the start month is invalid', async () => {
    createMock.mockResolvedValue({ ok: true });
    renderPage([], { currentPeriod: { year: 2026, month: 7 } });

    fireEvent.click(screen.getByTestId('commitments-add-toggle'));
    fireEvent.change(screen.getByLabelText('Libellé'), { target: { value: 'SPF impôt' } });
    fireEvent.change(screen.getByLabelText(/Montant total dû/), { target: { value: '2407.93' } });
    fireEvent.change(screen.getByLabelText(/Montant par échéance/), { target: { value: '220' } });
    fireEvent.change(screen.getByLabelText(/Nombre total d'échéances/), {
      target: { value: '11' },
    });
    fireEvent.change(screen.getByTestId('commitment-start-year'), { target: { value: '2026' } });
    // Invalid month (13)
    fireEvent.change(screen.getByTestId('commitment-start-month'), { target: { value: '13' } });

    await act(async () => {
      fireEvent.submit(screen.getByRole('button', { name: /^ajouter$/i }).closest('form')!);
    });

    expect(createMock).not.toHaveBeenCalled();
  });

  it('does not submit when the payment day is invalid', async () => {
    createMock.mockResolvedValue({ ok: true });
    renderPage([], { currentPeriod: { year: 2026, month: 7 } });

    fireEvent.click(screen.getByTestId('commitments-add-toggle'));
    fireEvent.change(screen.getByLabelText('Libellé'), { target: { value: 'SPF impôt' } });
    fireEvent.change(screen.getByLabelText(/Montant total dû/), { target: { value: '2407.93' } });
    fireEvent.change(screen.getByLabelText(/Montant par échéance/), { target: { value: '220' } });
    fireEvent.change(screen.getByLabelText(/Nombre total d'échéances/), {
      target: { value: '11' },
    });
    fireEvent.change(screen.getByTestId('commitment-start-year'), { target: { value: '2026' } });
    fireEvent.change(screen.getByTestId('commitment-start-month'), { target: { value: '5' } });
    // Invalid payment day (32)
    fireEvent.change(screen.getByTestId('commitment-payment-day'), { target: { value: '32' } });

    await act(async () => {
      fireEvent.submit(screen.getByRole('button', { name: /^ajouter$/i }).closest('form')!);
    });

    expect(createMock).not.toHaveBeenCalled();
  });

  it('sends the FIRST-instalment date the user typed, not the viewed period', async () => {
    createMock.mockResolvedValue({ ok: true });
    renderPage([], { currentPeriod: { year: 2026, month: 7 } });
    fireEvent.click(screen.getByTestId('commitments-add-toggle'));
    fireEvent.change(screen.getByLabelText('Libellé'), { target: { value: 'SPF impôt' } });
    fireEvent.change(screen.getByLabelText(/Montant total dû/), { target: { value: '2407.93' } });
    fireEvent.change(screen.getByLabelText(/Montant par échéance/), { target: { value: '220' } });
    fireEvent.change(screen.getByLabelText(/Nombre total d'échéances/), {
      target: { value: '11' },
    });
    fireEvent.change(screen.getByTestId('commitment-start-month'), { target: { value: '5' } });

```

- Adjust the selectors for the year and payment day inputs if they differ from the assumed `data-testid="commitment-start-year"` and `data-testid="commitment-payment-day"` (for example, if they use `getByLabelText` instead of `getByTestId`).
- If the client-side validation distinguishes between “out-of-range numeric” and “non-numeric” values, you may want to add an extra test for a non-numeric payment day (e.g. `"abcd"`) mirroring the `does not submit when the payment day is invalid` test.
- Ensure `createMock` is correctly imported/mocked in this test file so that `expect(createMock).not.toHaveBeenCalled()` works; if the mock is reset between tests (e.g. `beforeEach(createMock.mockReset)`), that should remain in place.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

`20260719000001_commitments.sql:28` reads "Anchor = the NEXT instalment
(D3)". It stays: an applied migration on a shared production database is
not retouched for a comment. Saying so in the ADR keeps a future reader
from mistaking a historical trace for an unresolved contradiction.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sourcery was right: the validation branches added with the date fields had
no test. Six cases now exercise them — years below 2000, above 2100,
non-numeric and empty; payment days of 0 and 32 — plus the case that an
empty payment day is ACCEPTED, since it is optional and stores the
"unset" 1.

Two of the suggested cases were dropped after they failed against correct
behaviour, which is the useful part of writing them:

- A non-numeric payment day cannot exist. `type="number"` blanks the
  field, and a blank day legitimately means "not chosen".
- An invalid month cannot come from the UI. It is a controlled <select>
  of exactly 1..12; pushing an absent value at it leaves a valid option
  selected (measured: React re-renders it back to 1). A test there could
  only have asserted that a valid month is accepted while looking like a
  guard. What replaces it asserts the property that actually holds.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:review-needed Ready for review type:fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant