feat(charges): month-history navigation + clickable forgotten alert - #231
Merged
Conversation
…Thierry priority 2026-07-19) The paid/unpaid state of ANY past month is now browsable — the ledger had the data all along, the UI just never let you go back. - /app/charges?period=YYYY-MM selects the viewed month. The param is validated + clamped server-side (regex, floor 2026-01, never future); invalid values silently fall back to today. Past periods load their own RLS-scoped ledger read (current month stays free with the snapshot). - ◀ juillet 2026 ▶ navigator above the banner: chevron links (disabled at the floor / at today), viewed-month label, and a 'Revenir à {mois}' link when browsing history. - Ticks stay EDITABLE in past months (togglePaymentAction was already period-parameterized) — so a forgotten June bill can be settled from July, which clears the dashboard alert. - Period-scoped banner copy: 'Reste à payer en juin 2026' / 'Tout est payé pour juin 2026' when not on the current month. - The dashboard forgotten alert now links 'Voir juin →' straight to the concerned month's history view. i18n x5: app.charges.{periodNav.*,remainingLabelPeriod,allPaidPeriod} + dashboard.upcomingBills.forgottenSee. Link test stubs now serialize object hrefs. Full suite 1501 green, typecheck + build clean.
|
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
area:charges
Contributor
Guide du relecteurImplémente la navigation historique par mois pour les charges et fait en sorte que l’alerte de factures oubliées du tableau de bord pointe directement vers la vue des charges du mois concerné, incluant l’analyse côté serveur de la période, des lectures RLS pour les périodes passées, ainsi qu’une mise à jour des textes/tests i18n. Diagramme de séquence pour la navigation historique par mois et l’alerte cliquable de factures oubliéessequenceDiagram
actor User
participant DashboardPage
participant ProchainesFacturesCard
participant ChargesPage
participant Supabase
participant ChargesClient
User ->> DashboardPage: load /app
DashboardPage ->> ProchainesFacturesCard: ProchainesFacturesCard({ forgotten })
ProchainesFacturesCard ->> User: render forgotten alert with Link(/app/charges?period=periodParam)
User ->> ChargesPage: navigate /app/charges?period=YYYY-MM
ChargesPage ->> ChargesPage: parseViewedPeriod(params.period, snapshot.currentPeriod)
ChargesPage ->> ChargesPage: ordinal(viewed), shift(viewed, ±1), toParam(period)
alt viewed isCurrent
ChargesPage ->> ChargesPage: paidChargeIds = snapshot.currentMonthPayments.map
else viewed is past month
ChargesPage ->> Supabase: createClient()
ChargesPage ->> Supabase: charge_payments.select().eq(...)
Supabase -->> ChargesPage: data | error
ChargesPage ->> ChargesPage: log.error(...)
ChargesPage ->> ChargesPage: paidChargeIds = (data ?? []).map
end
ChargesPage ->> ChargesPage: monthLabel(viewed), monthLabel(current)
ChargesPage ->> ChargesClient: ChargesClient({ currentPeriod: viewed, periodNav, paidChargeIds, ... })
ChargesClient ->> User: render periodNav prev/next Links and back-to-current Link
ChargesClient ->> User: render remaining/allPaid labels using periodNav.isCurrent and periodNav.label
Modifications au niveau des fichiers
Conseils et commandesInteragir avec Sourcery
Personnaliser votre expérienceAccédez à votre tableau de bord pour :
Obtenir de l’aide
Original review guide in EnglishReviewer's GuideImplements month-history navigation for charges and makes the dashboard forgotten-bills alert link directly to the relevant month’s charges view, including server-side period parsing, RLS-scoped reads for past periods, and updated i18n copy/tests. Sequence diagram for month-history navigation and clickable forgotten alertsequenceDiagram
actor User
participant DashboardPage
participant ProchainesFacturesCard
participant ChargesPage
participant Supabase
participant ChargesClient
User ->> DashboardPage: load /app
DashboardPage ->> ProchainesFacturesCard: ProchainesFacturesCard({ forgotten })
ProchainesFacturesCard ->> User: render forgotten alert with Link(/app/charges?period=periodParam)
User ->> ChargesPage: navigate /app/charges?period=YYYY-MM
ChargesPage ->> ChargesPage: parseViewedPeriod(params.period, snapshot.currentPeriod)
ChargesPage ->> ChargesPage: ordinal(viewed), shift(viewed, ±1), toParam(period)
alt viewed isCurrent
ChargesPage ->> ChargesPage: paidChargeIds = snapshot.currentMonthPayments.map
else viewed is past month
ChargesPage ->> Supabase: createClient()
ChargesPage ->> Supabase: charge_payments.select().eq(...)
Supabase -->> ChargesPage: data | error
ChargesPage ->> ChargesPage: log.error(...)
ChargesPage ->> ChargesPage: paidChargeIds = (data ?? []).map
end
ChargesPage ->> ChargesPage: monthLabel(viewed), monthLabel(current)
ChargesPage ->> ChargesClient: ChargesClient({ currentPeriod: viewed, periodNav, paidChargeIds, ... })
ChargesClient ->> User: render periodNav prev/next Links and back-to-current Link
ChargesClient ->> User: render remaining/allPaid labels using periodNav.isCurrent and periodNav.label
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é 2 problèmes et laissé quelques remarques générales :
- La prop
currentPerioddansChargesClientreprésente désormais la période affichée plutôt que strictement le mois courant, ce qui peut prêter à confusion ; envisage de renommer cette prop (et les champs de snapshot associés si pertinent) pour mieux refléter sa nouvelle sémantique. - Les mocks de
LinkdansChargesClient.test.tsxetProchainesFacturesCard.test.tsximplémentent tous deux la même logique de sérialisation objet → href ; extraire cette logique dans un helper de test partagé réduirait la duplication et garantirait un comportement cohérent. - La signature de
ChargesPagetypée avecsearchParamscommePromise<{ period?: string }>le traite comme un objetsearchParamsde Next.js ; aligner ce type sur la forme réelle à l’exécution (non-Promise) éviterait la confusion et clarifierait le contrat.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `currentPeriod` prop in `ChargesClient` now represents the viewed period rather than strictly the current month, which can be confusing; consider renaming this prop (and related snapshot fields if appropriate) to better reflect its new semantics.
- The test `Link` mocks in `ChargesClient.test.tsx` and `ProchainesFacturesCard.test.tsx` both implement the same object-href serialization logic; factoring this into a shared test helper would reduce duplication and keep behavior consistent.
- The `ChargesPage` signature types `searchParams` as a `Promise<{ period?: string }>` but treats it like a Next.js `searchParams` object; aligning this type with the actual runtime shape (non-Promise) would avoid confusion and make the contract clearer.
## Individual Comments
### Comment 1
<location path="src/app/[locale]/app/charges/page.tsx" line_range="48-62" />
<code_context>
+
+const toParam = (p: Period): string => `${p.year}-${String(p.month).padStart(2, '0')}`;
+
+export default async function ChargesPage({
+ searchParams,
+}: {
+ searchParams: Promise<{ period?: string }>;
</code_context>
<issue_to_address>
**suggestion:** The `searchParams` prop is typed and used as a `Promise`, which diverges from Next App Router conventions and may confuse future readers.
In the App Router, `searchParams` should be a plain object, not a promise. While `Promise.all` with a non-promise happens to work, this typing is unconventional and makes the signature harder to understand. Prefer a signature like `({ searchParams }: { searchParams?: { period?: string } })` and access `searchParams.period` directly to match Next’s conventions and keep the code clearer.
```suggestion
const toParam = (p: Period): string => `${p.year}-${String(p.month).padStart(2, '0')}`;
export default async function ChargesPage({
searchParams,
}: {
searchParams?: { period?: string };
}) {
const [snapshot, locale] = await Promise.all([
getWorkspaceSnapshot(),
getLocale() as Promise<Locale>,
]);
const params = searchParams ?? {};
const current = snapshot.currentPeriod;
const viewed = parseViewedPeriod(params.period, current);
```
</issue_to_address>
### Comment 2
<location path="src/app/[locale]/app/charges/page.tsx" line_range="25" />
<code_context>
+/** No payment data can exist before the app's first release month. */
+const PERIOD_FLOOR: Period = { year: 2026, month: 1 };
+
+const ordinal = (p: Period): number => p.year * 12 + p.month;
+
+/**
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the period handling by using direct year/month comparisons and treating `searchParams` as a plain object to reduce indirection and helper overhead.
You can keep all the new functionality while reducing indirection by inlining the month arithmetic and treating `searchParams` as a plain object again.
### 1. Simplify period parsing and comparisons (remove `ordinal` / `shift` / `toParam`)
The main rules are “invalid/future/pre-floor → current” and “prev/next month navigation with a floor”. Those can be implemented with direct year/month comparisons:
```ts
type Period = { year: number; month: number };
const PERIOD_RE = /^\d{4}-(0[1-9]|1[0-2])$/;
const PERIOD_FLOOR: Period = { year: 2026, month: 1 };
function isBefore(a: Period, b: Period): boolean {
return a.year < b.year || (a.year === b.year && a.month < b.month);
}
function isAfter(a: Period, b: Period): boolean {
return a.year > b.year || (a.year === b.year && a.month > b.month);
}
function parseViewedPeriod(raw: string | undefined, current: Period): Period {
if (!raw || !PERIOD_RE.test(raw)) return current;
const [year, month] = raw.split('-').map(Number) as [number, number];
const candidate: Period = { year, month };
if (isAfter(candidate, current) || isBefore(candidate, PERIOD_FLOOR)) {
return current;
}
return candidate;
}
```
Then compute `prev` / `next` and params inline instead of via `shift` / `ordinal` / `toParam`:
```ts
const prev: Period =
viewed.month === 1
? { year: viewed.year - 1, month: 12 }
: { year: viewed.year, month: viewed.month - 1 };
const next: Period =
viewed.month === 12
? { year: viewed.year + 1, month: 1 }
: { year: viewed.year, month: viewed.month + 1 };
const prevParam =
isBefore(prev, PERIOD_FLOOR)
? null
: `${prev.year}-${String(prev.month).padStart(2, '0')}`;
const nextParam = isCurrent
? null
: `${next.year}-${String(next.month).padStart(2, '0')}`;
```
This keeps all behavior but removes the mental mapping to an ordinal index and the extra helpers.
### 2. Treat `searchParams` as a plain object (avoid promise-based indirection)
You can still use `Promise.all` for the actual async work without turning `searchParams` into a promise. This keeps the Next.js mental model intact and removes one layer of indirection:
```ts
export default async function ChargesPage({
searchParams,
}: {
searchParams?: { period?: string };
}) {
const [snapshot, locale] = await Promise.all([
getWorkspaceSnapshot(),
getLocale() as Promise<Locale>,
]);
const current = snapshot.currentPeriod;
const viewed = parseViewedPeriod(searchParams?.period, current);
const isCurrent = !isAfter(viewed, current) && !isBefore(viewed, current);
// ...rest of the logic (Supabase read when !isCurrent, periodNav, etc.)
}
```
This keeps the same behavior (concurrent snapshot + locale fetch, period-based Supabase query) but reduces control-flow complexity and matches typical Next.js usage.
</issue_to_address>Sourcery est gratuit pour l’open source — si nos reviews vous plaisent, pensez à les partager ✨
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The
currentPeriodprop inChargesClientnow represents the viewed period rather than strictly the current month, which can be confusing; consider renaming this prop (and related snapshot fields if appropriate) to better reflect its new semantics. - The test
Linkmocks inChargesClient.test.tsxandProchainesFacturesCard.test.tsxboth implement the same object-href serialization logic; factoring this into a shared test helper would reduce duplication and keep behavior consistent. - The
ChargesPagesignature typessearchParamsas aPromise<{ period?: string }>but treats it like a Next.jssearchParamsobject; aligning this type with the actual runtime shape (non-Promise) would avoid confusion and make the contract clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `currentPeriod` prop in `ChargesClient` now represents the viewed period rather than strictly the current month, which can be confusing; consider renaming this prop (and related snapshot fields if appropriate) to better reflect its new semantics.
- The test `Link` mocks in `ChargesClient.test.tsx` and `ProchainesFacturesCard.test.tsx` both implement the same object-href serialization logic; factoring this into a shared test helper would reduce duplication and keep behavior consistent.
- The `ChargesPage` signature types `searchParams` as a `Promise<{ period?: string }>` but treats it like a Next.js `searchParams` object; aligning this type with the actual runtime shape (non-Promise) would avoid confusion and make the contract clearer.
## Individual Comments
### Comment 1
<location path="src/app/[locale]/app/charges/page.tsx" line_range="48-62" />
<code_context>
+
+const toParam = (p: Period): string => `${p.year}-${String(p.month).padStart(2, '0')}`;
+
+export default async function ChargesPage({
+ searchParams,
+}: {
+ searchParams: Promise<{ period?: string }>;
</code_context>
<issue_to_address>
**suggestion:** The `searchParams` prop is typed and used as a `Promise`, which diverges from Next App Router conventions and may confuse future readers.
In the App Router, `searchParams` should be a plain object, not a promise. While `Promise.all` with a non-promise happens to work, this typing is unconventional and makes the signature harder to understand. Prefer a signature like `({ searchParams }: { searchParams?: { period?: string } })` and access `searchParams.period` directly to match Next’s conventions and keep the code clearer.
```suggestion
const toParam = (p: Period): string => `${p.year}-${String(p.month).padStart(2, '0')}`;
export default async function ChargesPage({
searchParams,
}: {
searchParams?: { period?: string };
}) {
const [snapshot, locale] = await Promise.all([
getWorkspaceSnapshot(),
getLocale() as Promise<Locale>,
]);
const params = searchParams ?? {};
const current = snapshot.currentPeriod;
const viewed = parseViewedPeriod(params.period, current);
```
</issue_to_address>
### Comment 2
<location path="src/app/[locale]/app/charges/page.tsx" line_range="25" />
<code_context>
+/** No payment data can exist before the app's first release month. */
+const PERIOD_FLOOR: Period = { year: 2026, month: 1 };
+
+const ordinal = (p: Period): number => p.year * 12 + p.month;
+
+/**
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the period handling by using direct year/month comparisons and treating `searchParams` as a plain object to reduce indirection and helper overhead.
You can keep all the new functionality while reducing indirection by inlining the month arithmetic and treating `searchParams` as a plain object again.
### 1. Simplify period parsing and comparisons (remove `ordinal` / `shift` / `toParam`)
The main rules are “invalid/future/pre-floor → current” and “prev/next month navigation with a floor”. Those can be implemented with direct year/month comparisons:
```ts
type Period = { year: number; month: number };
const PERIOD_RE = /^\d{4}-(0[1-9]|1[0-2])$/;
const PERIOD_FLOOR: Period = { year: 2026, month: 1 };
function isBefore(a: Period, b: Period): boolean {
return a.year < b.year || (a.year === b.year && a.month < b.month);
}
function isAfter(a: Period, b: Period): boolean {
return a.year > b.year || (a.year === b.year && a.month > b.month);
}
function parseViewedPeriod(raw: string | undefined, current: Period): Period {
if (!raw || !PERIOD_RE.test(raw)) return current;
const [year, month] = raw.split('-').map(Number) as [number, number];
const candidate: Period = { year, month };
if (isAfter(candidate, current) || isBefore(candidate, PERIOD_FLOOR)) {
return current;
}
return candidate;
}
```
Then compute `prev` / `next` and params inline instead of via `shift` / `ordinal` / `toParam`:
```ts
const prev: Period =
viewed.month === 1
? { year: viewed.year - 1, month: 12 }
: { year: viewed.year, month: viewed.month - 1 };
const next: Period =
viewed.month === 12
? { year: viewed.year + 1, month: 1 }
: { year: viewed.year, month: viewed.month + 1 };
const prevParam =
isBefore(prev, PERIOD_FLOOR)
? null
: `${prev.year}-${String(prev.month).padStart(2, '0')}`;
const nextParam = isCurrent
? null
: `${next.year}-${String(next.month).padStart(2, '0')}`;
```
This keeps all behavior but removes the mental mapping to an ordinal index and the extra helpers.
### 2. Treat `searchParams` as a plain object (avoid promise-based indirection)
You can still use `Promise.all` for the actual async work without turning `searchParams` into a promise. This keeps the Next.js mental model intact and removes one layer of indirection:
```ts
export default async function ChargesPage({
searchParams,
}: {
searchParams?: { period?: string };
}) {
const [snapshot, locale] = await Promise.all([
getWorkspaceSnapshot(),
getLocale() as Promise<Locale>,
]);
const current = snapshot.currentPeriod;
const viewed = parseViewedPeriod(searchParams?.period, current);
const isCurrent = !isAfter(viewed, current) && !isBefore(viewed, current);
// ...rest of the logic (Supabase read when !isCurrent, periodNav, etc.)
}
```
This keeps the same behavior (concurrent snapshot + locale fetch, period-based Supabase query) but reduces control-flow complexity and matches typical Next.js usage.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…e menu (@Thierry + Sourcery #231) - currentPeriod prop → viewedPeriod: the prop names the period IN VIEW (month-history), no longer strictly today's month (Sourcery overall-1). - AppBreadcrumbs removed from the app layout + component/test deleted: with the header nav already marking the active page, the breadcrumb bar read as a confusing double menu on EVERY app page (@Thierry 2026-07-19). The public glossary keeps its own breadcrumb (deep SEO pages). - searchParams stays Promise-typed: that IS the Next 15+/16 App Router contract (sync access removed in 16) and the repo-wide convention (login/page.tsx, glossaire, admin layout) — Sourcery's suggestion would break the build. Defended on the thread.
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.
Historique par mois — navigation temporelle des factures (priorité validée @Thierry)
Ta demande : « on n'a aucun moyen de revenir en arrière pour voir les factures payées ou non ». C'est réglé — les données étaient en base depuis le début, l'UI ne permettait juste pas d'y aller.
Ce que ça fait
/app/charges?period=2026-06: la page affiche l'état payé/non payé de n'importe quel mois (param validé + clampé serveur : jamais le futur, plancher janv. 2026, valeur invalide → mois courant).Sécurité / risque
Lecture supplémentaire RLS-scopée pour les périodes passées ; aucune migration, aucune nouvelle Server Action ; param URL jamais trusté (regex + clamp). Suite complète 1501 verte, typecheck + build clean.
Smoke @Thierry (desktop + mobile)
🤖 Generated with Claude Code
Summary by Sourcery
Ajouter une navigation par historique mensuel à la page des charges et faire en sorte que l’alerte de facture oubliée sur le tableau de bord pointe directement vers la vue du mois concerné.
Nouvelles fonctionnalités :
periodsur la page des charges, avec des libellés de mois localisés et un navigateur.Améliorations :
Tests :
Original summary in English
Summary by Sourcery
Add month-history navigation to the charges page and make the dashboard forgotten-bill alert link directly to the relevant month view.
New Features:
Enhancements:
Tests: