feat(charges): CadenceField unified cadence picker (THI-301) - #228
Merged
Conversation
…06-02) onto current main
…dit drawer (THI-301) Replaces the 3 separate fields (frequency select / always-visible anchor month / day number input) with one controlled cluster, in BOTH entry points: - Native <select>s (iOS a11y locked decision; ankora-form-control-16 guards the Safari auto-zoom), 1..30 + explicit 'Dernier jour du mois' option mapping to paymentDay=31 (the domain year-aware clamp already turns 31 into 30/29/28 — end-of-month by design, no leap-year trap). - Anchor month HIDDEN for monthly charges (pure noise removed) and shown as 'À partir de [mars]' otherwise; everything stays editable at all times. - Human summary line: 'Prélevé le 15 : mars, juin, sept., déc.' derived via paymentMonthsFromFrequency — the confusing frequency arithmetic explained in plain words (@Thierry verbatim: frequencies are confusing). - Zero schema/domain change; Server Actions consume the same {frequency, dueMonth, paymentDay} as before. i18n: app.charges.cadence.* x5 locales (labels reuse the existing wording). Design + plan: docs/plans/THI-301-cadencefield-{design,plan}.md (plan-reviewer APPROVED 2026-06-02, executed per @Thierry's Fable-5 mandate with reality re-verification). Tests: 9 CadenceField + 2 adapted; full suite 1484 green.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
status:review-needed
Contributor
Guide du relecteurIntroduit un nouveau composant unifié CadenceField pour la sélection de la cadence de facturation et l’intègre à la fois dans le formulaire de création et le tiroir d’édition, avec les mises à jour i18n, tests et documentation, tout en laissant inchangées la logique métier et les actions côté serveur. Diagramme de séquence pour la mise à jour de l’état du formulaire parent par CadenceField via onChangesequenceDiagram
actor User
participant CadenceField
participant ChargesClient
User->>CadenceField: change frequency/day/month selects
CadenceField->>ChargesClient: onChange(next)
ChargesClient->>ChargesClient: setFrequency(next.frequency)
ChargesClient->>ChargesClient: setDueMonth(String(next.dueMonth))
ChargesClient->>ChargesClient: setPaymentDay(String(next.paymentDay))
Modifications au niveau des fichiers
Conseils et commandesInteragir avec Sourcery
Personnaliser votre expérienceAccédez à votre dashboard pour :
Obtenir de l’aide
Original review guide in EnglishReviewer's GuideIntroduces a new unified CadenceField component for charge cadence selection and wires it into both the create form and edit drawer, along with i18n, tests, and documentation updates, while keeping domain logic and server actions unchanged. Sequence diagram for CadenceField onChange updating parent form statesequenceDiagram
actor User
participant CadenceField
participant ChargesClient
User->>CadenceField: change frequency/day/month selects
CadenceField->>ChargesClient: onChange(next)
ChargesClient->>ChargesClient: setFrequency(next.frequency)
ChargesClient->>ChargesClient: setDueMonth(String(next.dueMonth))
ChargesClient->>ChargesClient: setPaymentDay(String(next.paymentDay))
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - j’ai identifié 6 points, et laissé quelques retours plus globaux :
- La liste des fréquences prises en charge est maintenant écrite en dur à la fois dans
CadenceFieldet dans les écrans de charges (FREQUENCIESdansChargesClient/ChargeEditDrawer), ce qui risque de diverger si une nouvelle fréquence est ajoutée. Il vaudrait mieux centraliser cet enum dans un domaine/module partagé pour que tous les callsites dérivent d’une source unique. - La chaîne Tailwind
selectClassdansCadenceFieldencode tout le contrat de stylage d’un form-control ; si d’autres inputs/selects utilisent le même pattern, vous pourriez extraire une constante ou utilitaire partagé afin de garder les changements de style cohérents dans toute l’app. - Dans les callsites de
CadenceField, vous forcezpaymentDayavecNumber(paymentDay) || 1, ce qui retombe silencieusement sur le jour 1 quand le champ est vide. Si ce n’est pas un choix UX intentionnel, il serait sans doute plus sûr soit de le laisser àundefinedtant que l’utilisateur n’a pas choisi de valeur, soit de gérer l’erreur à la validation plutôt que de mettre une valeur par défaut.
Prompt pour les agents IA
Veuillez traiter les commentaires de cette revue de code :
## Commentaires généraux
- La liste des fréquences prises en charge est maintenant écrite en dur à la fois dans `CadenceField` et dans les écrans de charges (`FREQUENCIES` dans `ChargesClient`/`ChargeEditDrawer`), ce qui risque de diverger si une nouvelle fréquence est ajoutée. Il vaudrait mieux centraliser cet enum dans un domaine/module partagé pour que tous les callsites dérivent d’une source unique.
- La chaîne Tailwind `selectClass` dans `CadenceField` encode tout le contrat de stylage d’un form-control ; si d’autres inputs/selects utilisent le même pattern, vous pourriez extraire une constante ou utilitaire partagé afin de garder les changements de style cohérents dans toute l’app.
- Dans les callsites de `CadenceField`, vous forcez `paymentDay` avec `Number(paymentDay) || 1`, ce qui retombe silencieusement sur le jour 1 quand le champ est vide. Si ce n’est pas un choix UX intentionnel, il serait sans doute plus sûr soit de le laisser à `undefined` tant que l’utilisateur n’a pas choisi de valeur, soit de gérer l’erreur à la validation plutôt que de mettre une valeur par défaut.
## Commentaires individuels
### Commentaire 1
<location path="src/app/[locale]/app/charges/__tests__/CadenceField.test.tsx" line_range="20" />
<code_context>
+const monthly: CadenceValue = { frequency: 'monthly', dueMonth: 1, paymentDay: 15 };
+const quarterly: CadenceValue = { frequency: 'quarterly', dueMonth: 3, paymentDay: 15 };
+
+describe('<CadenceField />', () => {
+ it('hides the anchor-month select when monthly', () => {
+ renderField(monthly);
</code_context>
<issue_to_address>
**suggestion (testing):** Ajoutez une couverture de test pour la prop `disabled` afin de vérifier que tous les contrôles sont bien désactivés quand on la passe.
Comme `disabled` est propagé à chaque `<select>`, merci d’ajouter un test qui rend `CadenceField` avec `disabled={true}` et vérifie que `t-frequency`, `t-day` et `t-month` (pour une configuration non mensuelle) ont tous l’attribut `disabled`. Cela permettra de détecter les régressions lorsqu’un contrôle arrête de propager l’état `disabled`.
Suggested implementation:
```typescript
<NextIntlClientProvider locale="fr-BE" messages={messages} timeZone="Europe/Brussels">
<CadenceField idPrefix="t" value={value} onChange={onChange} />
</NextIntlClientProvider>,
);
return onChange;
}
function renderDisabledField(value: CadenceValue) {
const onChange = jest.fn();
render(
<NextIntlClientProvider locale="fr-BE" messages={messages} timeZone="Europe/Brussels">
<CadenceField idPrefix="t" value={value} onChange={onChange} disabled />
</NextIntlClientProvider>,
);
return onChange;
}
const monthly: CadenceValue = { frequency: 'monthly', dueMonth: 1, paymentDay: 15 };
```
```typescript
it('shows an editable anchor-month select when non-monthly', () => {
renderField(quarterly);
expect(screen.getByTestId('t-month')).toBeInTheDocument();
});
it('disables all selects when disabled is true for a non-monthly cadence', () => {
renderDisabledField(quarterly);
expect(screen.getByTestId('t-frequency')).toBeDisabled();
expect(screen.getByTestId('t-day')).toBeDisabled();
expect(screen.getByTestId('t-month')).toBeDisabled();
});
```
</issue_to_address>
### Commentaire 2
<location path="src/app/[locale]/app/charges/__tests__/CadenceField.test.tsx" line_range="38-46" />
<code_context>
+ expect(screen.getByTestId('t-summary')).toHaveTextContent('Prélevé le 15 de chaque mois');
+ });
+
+ it('renders the recurring summary with computed months (quarterly anchored March)', () => {
+ renderField(quarterly);
+ // paymentMonthsFromFrequency('quarterly', 3) → [3,6,9,12] → mars, juin, sept., déc.
</code_context>
<issue_to_address>
**suggestion (testing):** Envisagez un test pour le résumé "dernier jour" aussi dans le cas récurrent non mensuel.
Actuellement, on vérifie seulement le libellé "dernier jour" pour le mensuel (`paymentDay = 31`) et le résumé récurrent générique pour le trimestriel, mais pas la combinaison `frequency !== 'monthly'` avec `paymentDay === 31`. Merci d’ajouter un cas trimestriel (par exemple `frequency: 'quarterly', dueMonth: 3, paymentDay: 31`) qui vérifie à la fois la formulation "dernier jour" et la liste de mois attendue, pour valider que `daySummary` est bien réutilisé dans la branche récurrente.
```suggestion
it('renders the recurring summary with computed months (quarterly anchored March)', () => {
renderField(quarterly);
// paymentMonthsFromFrequency('quarterly', 3) → [3,6,9,12] → mars, juin, sept., déc.
expect(screen.getByTestId('t-summary')).toHaveTextContent(/mars/i);
expect(screen.getByTestId('t-summary')).toHaveTextContent(/juin/i);
expect(screen.getByTestId('t-summary')).toHaveTextContent(/déc/i);
});
it('renders the recurring summary with "dernier jour" wording for non-monthly last-day payments', () => {
renderField({ frequency: 'quarterly', dueMonth: 3, paymentDay: 31 });
const summary = screen.getByTestId('t-summary');
// Reuses the same "dernier jour" daySummary wording in the recurring branch
expect(summary).toHaveTextContent(/dernier jour/i);
// paymentMonthsFromFrequency('quarterly', 3) → [3,6,9,12] → mars, juin, sept., déc.
expect(summary).toHaveTextContent(/mars/i);
expect(summary).toHaveTextContent(/juin/i);
expect(summary).toHaveTextContent(/sept/i);
expect(summary).toHaveTextContent(/déc/i);
});
it('exposes a "Dernier jour du mois" option that emits paymentDay=31', () => {
```
</issue_to_address>
### Commentaire 3
<location path="src/app/[locale]/app/charges/__tests__/ChargesClient.test.tsx" line_range="196-205" />
<code_context>
expect(screen.getByLabelText(/Montant/)).toBeInTheDocument();
expect(screen.getByLabelText('Fréquence')).toBeInTheDocument();
- expect(screen.getByLabelText('Mois de référence')).toBeInTheDocument();
+ // THI-301: the anchor-month select is intentionally HIDDEN for monthly
+ // (the default) — the CadenceField summary line proves the cluster is
+ // mounted instead.
+ expect(screen.getByTestId('create-charge-summary')).toBeInTheDocument();
expect(screen.getByLabelText(/jour du mois/i)).toBeInTheDocument();
expect(screen.getByRole('button', { name: /^ajouter$/i })).toBeInTheDocument();
</code_context>
<issue_to_address>
**suggestion (testing):** Renforcez le test de flux de création en vérifiant le contenu du résumé CadenceField, pas seulement sa présence.
Cette modification vérifie seulement que `create-charge-summary` est rendu, pas que la sémantique de cadence est correcte. Pour que ce test continue à couvrir le scénario de création par défaut, merci de vérifier aussi que le texte du résumé reflète la copie par défaut attendue (par exemple, mensuel au jour N) et que le select du mois d’ancrage n’est pas présent dans le DOM. De cette façon, le test prouve toujours que le formulaire est câblé sur la bonne cadence par défaut, et pas seulement que le composant est monté.
```suggestion
expect(screen.getByLabelText('Libellé')).toBeInTheDocument();
expect(screen.getByLabelText(/Montant/)).toBeInTheDocument();
expect(screen.getByLabelText('Fréquence')).toBeInTheDocument();
// THI-301: the anchor-month select is intentionally HIDDEN for monthly
// (the default) — the CadenceField summary line proves the cluster is
// mounted instead.
const createChargeSummary = screen.getByTestId('create-charge-summary');
expect(createChargeSummary).toBeInTheDocument();
// Verify that the default cadence semantics are correctly wired:
// monthly collection on a day-of-month (default create scenario).
expect(createChargeSummary).toHaveTextContent(/mensuel(le)?/i);
expect(createChargeSummary).toHaveTextContent(/mois/i);
// The anchor-month select should not be rendered for the default monthly cadence.
expect(screen.queryByLabelText('Mois de référence')).not.toBeInTheDocument();
expect(screen.getByLabelText(/jour du mois/i)).toBeInTheDocument();
expect(screen.getByRole('button', { name: /^ajouter$/i })).toBeInTheDocument();
});
```
</issue_to_address>
### Commentaire 4
<location path="src/app/[locale]/app/charges/__tests__/ChargesClient.test.tsx" line_range="280-281" />
<code_context>
expect(screen.getByTestId('charge-edit-label')).toHaveValue('Loyer appartement');
expect(screen.getByTestId('charge-edit-amount')).toHaveValue(1200);
- expect(screen.getByTestId('charge-edit-payment-day')).toHaveValue(5);
+ // THI-301: native <select> in CadenceField → string value.
+ expect(screen.getByTestId('edit-charge-day')).toHaveValue('5');
});
</code_context>
<issue_to_address>
**suggestion (testing):** Envisagez d’ajouter un test de flux d’édition qui couvre une cadence non mensuelle pour s’assurer que le jour et le mois d’ancrage sont correctement préremplis.
Ce test ne couvre toujours que le cas mensuel et vérifie maintenant la valeur chaîne issue du `<select>`. Avec le nouveau `CadenceField` dépendant de `frequency`, `dueMonth` et `paymentDay` de la charge existante, merci d’ajouter un test d’intégration pour une charge non mensuelle (par exemple trimestrielle/semestrielle/annuelle) qui ouvre le drawer d’édition et vérifie que `edit-charge-day` et `edit-charge-month` (ou équivalents) sont pré-remplis avec les valeurs attendues. Cela permettra de vérifier que les données de cadence héritées sont correctement mappées dans le nouveau picker pour toutes les cadences.
Suggested implementation:
```typescript
await screen.findByTestId('charge-edit-drawer');
expect(screen.getByTestId('charge-edit-label')).toHaveValue('Loyer appartement');
expect(screen.getByTestId('charge-edit-amount')).toHaveValue(1200);
// THI-301: native <select> in CadenceField → string value.
expect(screen.getByTestId('edit-charge-day')).toHaveValue('5');
});
it('pre-fills cadence fields when editing a non-monthly charge', async () => {
/**
* THI-301:
* Ensure legacy non-monthly cadence values (frequency, dueMonth, paymentDay)
* are correctly mapped into CadenceField when opening the edit drawer.
*
* This test uses an annual charge as a representative non-monthly cadence.
* Adjust the seeded charge below if your fixtures use different labels
* or cadence shapes.
*/
const charges = [
{
id: 'annual-charge-id',
label: 'Assurance habitation',
amount: 35000,
// Legacy cadence fields feeding CadenceField:
frequency: 'ANNUAL',
paymentDay: 15,
dueMonth: 3, // e.g. March
},
];
renderChargesClientWithCharges(charges);
// Open the edit drawer for the non-monthly charge.
// Reuse the same interaction pattern as the monthly edit-flow test.
await userEvent.click(
screen.getByRole('button', { name: /modifier\s+assurance habitation/i }),
);
await screen.findByTestId('charge-edit-drawer');
// Label / amount sanity check.
expect(screen.getByTestId('charge-edit-label')).toHaveValue('Assurance habitation');
expect(screen.getByTestId('charge-edit-amount')).toHaveValue(350);
// CadenceField should be pre-populated from paymentDay / dueMonth.
expect(screen.getByTestId('edit-charge-day')).toHaveValue('15');
expect(screen.getByTestId('edit-charge-month')).toHaveValue('3');
});
it('calls updateChargeAction with the modified amount on Save', async () => {
```
1. Assurez-vous que `renderChargesClientWithCharges` existe et accepte une liste de charges comme ci‑dessus. Si votre suite de tests utilise un autre helper ou un wrapper de provider, adaptez l’appel en conséquence (par exemple `renderChargesClient({ charges })` ou similaire).
2. Alignez la forme de la charge initialisée avec votre véritable API/fixtures :
- Si votre domaine utilise `due_month` / `payment_day` ou des objets `cadence` au lieu de propriétés de haut niveau `frequency`, `paymentDay`, `dueMonth`, adaptez les noms de propriétés dans le tableau `charges`.
- Si l’UI affiche les montants en euros (par exemple 350 au lieu de 35000 centimes), ajustez l’assertion sur `amount` pour correspondre à vos conventions existantes.
3. Remplacez le sélecteur `getByRole('button', { name: /modifier\s+assurance habitation/i })` par le sélecteur exact du bouton d’édition utilisé dans le test de flux d’édition mensuelle existant (pour rester cohérent avec la manière dont vous ouvrez le drawer d’édition).
4. Vérifiez que les test IDs `edit-charge-day` et `edit-charge-month` correspondent bien aux attributs `data-testid` réels dans le formulaire d’édition CadenceField ; si ce n’est pas le cas, mettez à jour les sélecteurs `getByTestId` en conséquence.
</issue_to_address>
### Commentaire 5
<location path="docs/plans/THI-301-cadencefield-plan.md" line_range="543" />
<code_context>
+
+### Task 7 : QA agents + DoD final
+
+- [ ] **Step 1 : Agents QA ciblés (voie lourde, par ce que le diff touche)**
+
+- `ui-auditor` (nouveaux selects natifs, labels, contraste tokens, mobile-first)
</code_context>
<issue_to_address>
**issue (typo):** Corriger "par ce que" en "parce que".
Formulation correcte en français : « parce que » en un seul mot.
```suggestion
- [ ] **Step 1 : Agents QA ciblés (voie lourde, parce que le diff touche)**
```
</issue_to_address>
### Commentaire 6
<location path="docs/plans/THI-301-cadencefield-design.md" line_range="65-72" />
<code_context>
+
+```
+MENSUEL TRIMESTRIEL
+ Frequence ( Mensuel v ) Frequence ( Trimestriel v )
+ Preleve le [ 15 v ] de chaque mois Preleve le [15 v] a partir de [mars v]
+ (... 1..28, 'Dernier jour') (jour: 1..28, 'Dernier jour')
+ -> 'le 15 de chaque mois' -> 'le 15 : mars, juin, sept, dec'
</code_context>
<issue_to_address>
**nitpick (typo):** Ajouter les accents manquants dans l’exemple ASCII (Fréquence, Prélevé, à partir de, déc.).
Dans l’exemple, ces mots devraient être accentués pour rester cohérents avec le reste de la doc : « Frequence » → « Fréquence », « Preleve » → « Prélevé », « a partir de » → « à partir de », et « dec » → « déc. ».
```suggestion
```
MENSUEL TRIMESTRIEL
Fréquence ( Mensuel v ) Fréquence ( Trimestriel v )
Prélevé le [ 15 v ] de chaque mois Prélevé le [15 v] à partir de [mars v]
(... 1..28, 'Dernier jour') (jour: 1..28, 'Dernier jour')
-> 'le 15 de chaque mois' -> 'le 15 : mars, juin, sept, déc.'
(changer ancre/freq = tout bouge)
```
```
</issue_to_address>Sourcery est gratuit pour l’open source – si nos revues vous sont utiles, pensez à les partager ✨
Original comment in English
Hey - I've found 6 issues, and left some high level feedback:
- The list of supported frequencies is now hard-coded in both
CadenceFieldand the charges screens (FREQUENCIESinChargesClient/ChargeEditDrawer), which risks drift if a new frequency is added; consider centralizing this enum in a shared domain/module so all callsites derive from a single source. - The
selectClassTailwind string inCadenceFieldencodes a full form-control styling contract; if other inputs/selects use the same pattern, you might want to extract a shared constant or utility to keep styling changes consistent across the app. - In the
CadenceFieldcallsites you coercepaymentDaywithNumber(paymentDay) || 1, which silently falls back to day 1 when the field is empty; if this is not intentional UX, it might be safer to either keep itundefineduntil the user chooses a value or handle the error at validation time instead of defaulting.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The list of supported frequencies is now hard-coded in both `CadenceField` and the charges screens (`FREQUENCIES` in `ChargesClient`/`ChargeEditDrawer`), which risks drift if a new frequency is added; consider centralizing this enum in a shared domain/module so all callsites derive from a single source.
- The `selectClass` Tailwind string in `CadenceField` encodes a full form-control styling contract; if other inputs/selects use the same pattern, you might want to extract a shared constant or utility to keep styling changes consistent across the app.
- In the `CadenceField` callsites you coerce `paymentDay` with `Number(paymentDay) || 1`, which silently falls back to day 1 when the field is empty; if this is not intentional UX, it might be safer to either keep it `undefined` until the user chooses a value or handle the error at validation time instead of defaulting.
## Individual Comments
### Comment 1
<location path="src/app/[locale]/app/charges/__tests__/CadenceField.test.tsx" line_range="20" />
<code_context>
+const monthly: CadenceValue = { frequency: 'monthly', dueMonth: 1, paymentDay: 15 };
+const quarterly: CadenceValue = { frequency: 'quarterly', dueMonth: 3, paymentDay: 15 };
+
+describe('<CadenceField />', () => {
+ it('hides the anchor-month select when monthly', () => {
+ renderField(monthly);
</code_context>
<issue_to_address>
**suggestion (testing):** Add coverage for the `disabled` prop to ensure all controls are properly disabled when requested.
Since `disabled` is passed through to each `<select>`, please add a test that renders `CadenceField` with `disabled={true}` and asserts that `t-frequency`, `t-day`, and `t-month` (for a non-monthly config) all have the `disabled` attribute. This will help catch regressions where any control stops forwarding the `disabled` state.
Suggested implementation:
```typescript
<NextIntlClientProvider locale="fr-BE" messages={messages} timeZone="Europe/Brussels">
<CadenceField idPrefix="t" value={value} onChange={onChange} />
</NextIntlClientProvider>,
);
return onChange;
}
function renderDisabledField(value: CadenceValue) {
const onChange = jest.fn();
render(
<NextIntlClientProvider locale="fr-BE" messages={messages} timeZone="Europe/Brussels">
<CadenceField idPrefix="t" value={value} onChange={onChange} disabled />
</NextIntlClientProvider>,
);
return onChange;
}
const monthly: CadenceValue = { frequency: 'monthly', dueMonth: 1, paymentDay: 15 };
```
```typescript
it('shows an editable anchor-month select when non-monthly', () => {
renderField(quarterly);
expect(screen.getByTestId('t-month')).toBeInTheDocument();
});
it('disables all selects when disabled is true for a non-monthly cadence', () => {
renderDisabledField(quarterly);
expect(screen.getByTestId('t-frequency')).toBeDisabled();
expect(screen.getByTestId('t-day')).toBeDisabled();
expect(screen.getByTestId('t-month')).toBeDisabled();
});
```
</issue_to_address>
### Comment 2
<location path="src/app/[locale]/app/charges/__tests__/CadenceField.test.tsx" line_range="38-46" />
<code_context>
+ expect(screen.getByTestId('t-summary')).toHaveTextContent('Prélevé le 15 de chaque mois');
+ });
+
+ it('renders the recurring summary with computed months (quarterly anchored March)', () => {
+ renderField(quarterly);
+ // paymentMonthsFromFrequency('quarterly', 3) → [3,6,9,12] → mars, juin, sept., déc.
</code_context>
<issue_to_address>
**suggestion (testing):** Consider a test for the "dernier jour" summary in the non-monthly (recurring) case as well.
Right now we only assert the "dernier jour" wording for monthly (`paymentDay = 31`) and the generic recurring summary for quarterly, but not the combination of `frequency !== 'monthly'` with `paymentDay === 31`. Please add a quarterly case (e.g. `frequency: 'quarterly', dueMonth: 3, paymentDay: 31`) that checks both the "dernier jour" phrasing and the expected months list, to validate that `daySummary` is correctly reused in the recurring branch.
```suggestion
it('renders the recurring summary with computed months (quarterly anchored March)', () => {
renderField(quarterly);
// paymentMonthsFromFrequency('quarterly', 3) → [3,6,9,12] → mars, juin, sept., déc.
expect(screen.getByTestId('t-summary')).toHaveTextContent(/mars/i);
expect(screen.getByTestId('t-summary')).toHaveTextContent(/juin/i);
expect(screen.getByTestId('t-summary')).toHaveTextContent(/déc/i);
});
it('renders the recurring summary with "dernier jour" wording for non-monthly last-day payments', () => {
renderField({ frequency: 'quarterly', dueMonth: 3, paymentDay: 31 });
const summary = screen.getByTestId('t-summary');
// Reuses the same "dernier jour" daySummary wording in the recurring branch
expect(summary).toHaveTextContent(/dernier jour/i);
// paymentMonthsFromFrequency('quarterly', 3) → [3,6,9,12] → mars, juin, sept., déc.
expect(summary).toHaveTextContent(/mars/i);
expect(summary).toHaveTextContent(/juin/i);
expect(summary).toHaveTextContent(/sept/i);
expect(summary).toHaveTextContent(/déc/i);
});
it('exposes a "Dernier jour du mois" option that emits paymentDay=31', () => {
```
</issue_to_address>
### Comment 3
<location path="src/app/[locale]/app/charges/__tests__/ChargesClient.test.tsx" line_range="196-205" />
<code_context>
expect(screen.getByLabelText(/Montant/)).toBeInTheDocument();
expect(screen.getByLabelText('Fréquence')).toBeInTheDocument();
- expect(screen.getByLabelText('Mois de référence')).toBeInTheDocument();
+ // THI-301: the anchor-month select is intentionally HIDDEN for monthly
+ // (the default) — the CadenceField summary line proves the cluster is
+ // mounted instead.
+ expect(screen.getByTestId('create-charge-summary')).toBeInTheDocument();
expect(screen.getByLabelText(/jour du mois/i)).toBeInTheDocument();
expect(screen.getByRole('button', { name: /^ajouter$/i })).toBeInTheDocument();
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen the create-flow test by asserting the CadenceField summary content, not just its presence.
This change only verifies that `create-charge-summary` renders, not that the cadence semantics are correct. To keep this test covering the default create scenario, please also assert that the summary text reflects the expected default copy (e.g. monthly at day N) and that the anchor-month select is not present in the DOM. That way the test still proves the form is wired to the correct default cadence, not just that the component mounts.
```suggestion
expect(screen.getByLabelText('Libellé')).toBeInTheDocument();
expect(screen.getByLabelText(/Montant/)).toBeInTheDocument();
expect(screen.getByLabelText('Fréquence')).toBeInTheDocument();
// THI-301: the anchor-month select is intentionally HIDDEN for monthly
// (the default) — the CadenceField summary line proves the cluster is
// mounted instead.
const createChargeSummary = screen.getByTestId('create-charge-summary');
expect(createChargeSummary).toBeInTheDocument();
// Verify that the default cadence semantics are correctly wired:
// monthly collection on a day-of-month (default create scenario).
expect(createChargeSummary).toHaveTextContent(/mensuel(le)?/i);
expect(createChargeSummary).toHaveTextContent(/mois/i);
// The anchor-month select should not be rendered for the default monthly cadence.
expect(screen.queryByLabelText('Mois de référence')).not.toBeInTheDocument();
expect(screen.getByLabelText(/jour du mois/i)).toBeInTheDocument();
expect(screen.getByRole('button', { name: /^ajouter$/i })).toBeInTheDocument();
});
```
</issue_to_address>
### Comment 4
<location path="src/app/[locale]/app/charges/__tests__/ChargesClient.test.tsx" line_range="280-281" />
<code_context>
expect(screen.getByTestId('charge-edit-label')).toHaveValue('Loyer appartement');
expect(screen.getByTestId('charge-edit-amount')).toHaveValue(1200);
- expect(screen.getByTestId('charge-edit-payment-day')).toHaveValue(5);
+ // THI-301: native <select> in CadenceField → string value.
+ expect(screen.getByTestId('edit-charge-day')).toHaveValue('5');
});
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding an edit-flow test that covers a non-monthly cadence to ensure day and anchor month are correctly pre-filled.
This still only covers the monthly case and now asserts the string value from the `<select>`. With the new `CadenceField` depending on `frequency`, `dueMonth`, and `paymentDay` from the existing charge, please add an integration test for a non-monthly charge (e.g. quarterly/semiannual/annual) that opens the edit drawer and asserts that `edit-charge-day` and `edit-charge-month` (or equivalent) are pre-populated with the expected values. That will verify the legacy charge data is correctly mapped into the new picker for all cadences.
Suggested implementation:
```typescript
await screen.findByTestId('charge-edit-drawer');
expect(screen.getByTestId('charge-edit-label')).toHaveValue('Loyer appartement');
expect(screen.getByTestId('charge-edit-amount')).toHaveValue(1200);
// THI-301: native <select> in CadenceField → string value.
expect(screen.getByTestId('edit-charge-day')).toHaveValue('5');
});
it('pre-fills cadence fields when editing a non-monthly charge', async () => {
/**
* THI-301:
* Ensure legacy non-monthly cadence values (frequency, dueMonth, paymentDay)
* are correctly mapped into CadenceField when opening the edit drawer.
*
* This test uses an annual charge as a representative non-monthly cadence.
* Adjust the seeded charge below if your fixtures use different labels
* or cadence shapes.
*/
const charges = [
{
id: 'annual-charge-id',
label: 'Assurance habitation',
amount: 35000,
// Legacy cadence fields feeding CadenceField:
frequency: 'ANNUAL',
paymentDay: 15,
dueMonth: 3, // e.g. March
},
];
renderChargesClientWithCharges(charges);
// Open the edit drawer for the non-monthly charge.
// Reuse the same interaction pattern as the monthly edit-flow test.
await userEvent.click(
screen.getByRole('button', { name: /modifier\s+assurance habitation/i }),
);
await screen.findByTestId('charge-edit-drawer');
// Label / amount sanity check.
expect(screen.getByTestId('charge-edit-label')).toHaveValue('Assurance habitation');
expect(screen.getByTestId('charge-edit-amount')).toHaveValue(350);
// CadenceField should be pre-populated from paymentDay / dueMonth.
expect(screen.getByTestId('edit-charge-day')).toHaveValue('15');
expect(screen.getByTestId('edit-charge-month')).toHaveValue('3');
});
it('calls updateChargeAction with the modified amount on Save', async () => {
```
1. Ensure that `renderChargesClientWithCharges` exists and accepts a list of charges as used above. If your test suite uses a different helper or a provider wrapper, adapt the call accordingly (e.g. `renderChargesClient({ charges })` or similar).
2. Align the seeded charge shape with your real API/fixture shape:
- If your domain uses `due_month` / `payment_day` or `cadence` objects instead of top-level `frequency`, `paymentDay`, `dueMonth`, adjust the property names in the seeded `charges` array.
- If the UI displays amounts in euros (e.g. 350 instead of 35000 cents), adjust the `amount` assertion to match your existing conventions.
3. Replace the `getByRole('button', { name: /modifier\s+assurance habitation/i })` selector with the exact edit-button selector used in the existing monthly edit-flow test (for consistency with how you open the edit drawer).
4. Confirm that the test IDs `edit-charge-day` and `edit-charge-month` match the actual `data-testid` attributes in the CadenceField edit form; if they differ, update the `getByTestId` selectors accordingly.
</issue_to_address>
### Comment 5
<location path="docs/plans/THI-301-cadencefield-plan.md" line_range="543" />
<code_context>
+
+### Task 7 : QA agents + DoD final
+
+- [ ] **Step 1 : Agents QA ciblés (voie lourde, par ce que le diff touche)**
+
+- `ui-auditor` (nouveaux selects natifs, labels, contraste tokens, mobile-first)
</code_context>
<issue_to_address>
**issue (typo):** Corriger "par ce que" en "parce que".
Formulation correcte en français : « parce que » en un seul mot.
```suggestion
- [ ] **Step 1 : Agents QA ciblés (voie lourde, parce que le diff touche)**
```
</issue_to_address>
### Comment 6
<location path="docs/plans/THI-301-cadencefield-design.md" line_range="65-72" />
<code_context>
+
+```
+MENSUEL TRIMESTRIEL
+ Frequence ( Mensuel v ) Frequence ( Trimestriel v )
+ Preleve le [ 15 v ] de chaque mois Preleve le [15 v] a partir de [mars v]
+ (... 1..28, 'Dernier jour') (jour: 1..28, 'Dernier jour')
+ -> 'le 15 de chaque mois' -> 'le 15 : mars, juin, sept, dec'
</code_context>
<issue_to_address>
**nitpick (typo):** Ajouter les accents manquants dans l’exemple ASCII (Fréquence, Prélevé, à partir de, déc.).
Dans l’exemple, ces mots devraient être accentués pour rester cohérents avec le reste de la doc : « Frequence » → « Fréquence », « Preleve » → « Prélevé », « a partir de » → « à partir de », et « dec » → « déc. ».
```suggestion
```
MENSUEL TRIMESTRIEL
Fréquence ( Mensuel v ) Fréquence ( Trimestriel v )
Prélevé le [ 15 v ] de chaque mois Prélevé le [15 v] à partir de [mars v]
(... 1..28, 'Dernier jour') (jour: 1..28, 'Dernier jour')
-> 'le 15 de chaque mois' -> 'le 15 : mars, juin, sept, déc.'
(changer ancre/freq = tout bouge)
```
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…t cadence flows (Sourcery #228) Four testing suggestions applied: disabled prop covers all three selects; 'dernier jour' asserted in the recurring (non-monthly) summary; the create flow asserts the summary TEXT (default 'Prélevé le 1 de chaque mois'); a new edit-flow test opens a NON-monthly charge and checks the pre-filled cluster (frequency/anchor/day + summary). Plus two doc typos (parce que, accents). 58 charges-page tests green.
…ery #228) The supported-cadence list was hard-coded in 3 places (CadenceField + ChargesClient + ChargeEditDrawer). It now lives once in the domain (src/lib/domain/types.ts) with ChargeFrequency inferred from it — adding a frequency updates every call-site at once.
Owner
Author
|
Retours globaux Sourcery traités :
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR-D — CadenceField : saisie de cadence unifiée (THI-301)
Le dernier gros irritant UX des charges : la cadence se saisissait via 3 champs séparés (fréquence / « Mois de référence » toujours affiché même en mensuel / jour en input libre) — et tu avais dit le trouver déroutant. Remplacé par un seul cluster clair, dans les deux points de saisie (form création + drawer édition).
Ce que ça donne
[15 ▾]de chaque mois » — le mois-ancre disparaît (il n'avait aucun sens).[15 ▾]à partir de[mars ▾]» — tout reste éditable à tout moment.Pour corriger TES dates réelles
C'est l'outil qui manquait : ouvre une charge (crayon), ajuste fréquence/ancre/jour avec l'aperçu des mois sous les yeux, sauve —
payment_monthsest recalculé proprement. (Ex. S.W.D.E : trimestriel à partir de mai → « le 10 : févr., mai, août, nov. ».)Gouvernance
Design + plan plan-reviewer APPROVED (2026-06-02), exécutés avec re-vérification du code actuel (mandat Fable 5). Zéro modif schéma/domaine/Server Action — voie légère. La démo design-playground du plan est délibérément omise (nice-to-have QA locale, à ajouter si tu la veux).
Tests
9 CadenceField (résumés, Dernier jour, a11y labels, émissions onChange) + 2 tests existants adaptés. Suite complète 1484 verte, typecheck + lint + build clean.
Smoke @Thierry (desktop + mobile)
/app/charges→ « + Ajouter une charge » : mensuel = pas de mois visible ; passe en trimestriel → « À partir de » apparaît + le résumé liste les 4 mois ; choisis « Dernier jour du mois » ; édite une charge existante (crayon) → même cluster pré-rempli.🤖 Generated with Claude Code
Summary by Sourcery
Introduire un composant unifié CadenceField pour éditer la cadence des frais récurrents dans les flux de création et de modification, en remplaçant les champs séparés précédents (fréquence, mois d’ancrage et jour de paiement) tout en préservant les contrats de domaine existants et les actions serveur.
New Features:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Introduce a unified CadenceField component for editing recurring charge cadence in both creation and edit flows, replacing the previous separate frequency, anchor month, and payment day inputs while preserving existing domain contracts and server actions.
New Features:
Enhancements:
Tests: