maximus/Simpl-Resultat

Fork 0

fix: remove expense filter from Category Over Time report (#41) #42

Merged

maximus merged 3 commits from fix/simpl-resultat-41-category-time-report-filter into main

2026-03-30 01:14:12 +00:00

maximus commented

2026-03-18 04:05:49 +00:00

Owner

Summary

Removed the hard-coded t.amount < 0 filter from the Category Over Time report so all transaction types (expenses, income, transfers) are shown by default
Added a Type filter dropdown (expense/income/transfer) in the right-side filter panel for the Category Over Time tab
Updated i18n keys (FR/EN), changelogs, and user guide

Changes

File	Change
`src/services/reportService.ts`	Removed `t.amount < 0` WHERE clause, added optional `typeFilter` parameter
`src/hooks/useReports.ts`	Added `categoryType` state and `setCategoryType` action
`src/components/reports/ReportFilterPanel.tsx`	Added type filter dropdown UI
`src/pages/ReportsPage.tsx`	Wired type filter to panel and hook
`src/i18n/locales/{fr,en}.json`	Added `reports.filters.allTypes` key
`CHANGELOG.md` / `CHANGELOG.fr.md`	Added changelog entries
`docs/guide-utilisateur.md`	Updated report description

Test plan

Open Reports > Category Over Time tab
Verify the chart now shows all transaction types by default (not just expenses)
Use the Type filter dropdown in the right panel to filter by expense, income, or transfer
Verify 'All types' shows everything
Switch between FR/EN and verify translations
Run npm test — all 17 tests pass

Ref: simpl-resultat#41

## Summary - Removed the hard-coded `t.amount < 0` filter from the Category Over Time report so all transaction types (expenses, income, transfers) are shown by default - Added a **Type** filter dropdown (expense/income/transfer) in the right-side filter panel for the Category Over Time tab - Updated i18n keys (FR/EN), changelogs, and user guide ## Changes | File | Change | |------|--------| | `src/services/reportService.ts` | Removed `t.amount < 0` WHERE clause, added optional `typeFilter` parameter | | `src/hooks/useReports.ts` | Added `categoryType` state and `setCategoryType` action | | `src/components/reports/ReportFilterPanel.tsx` | Added type filter dropdown UI | | `src/pages/ReportsPage.tsx` | Wired type filter to panel and hook | | `src/i18n/locales/{fr,en}.json` | Added `reports.filters.allTypes` key | | `CHANGELOG.md` / `CHANGELOG.fr.md` | Added changelog entries | | `docs/guide-utilisateur.md` | Updated report description | ## Test plan - [ ] Open Reports > Category Over Time tab - [ ] Verify the chart now shows all transaction types by default (not just expenses) - [ ] Use the Type filter dropdown in the right panel to filter by expense, income, or transfer - [ ] Verify 'All types' shows everything - [ ] Switch between FR/EN and verify translations - [ ] Run `npm test` — all 17 tests pass Ref: simpl-resultat#41

maximus added 1 commit 2026-03-18 04:05:49 +00:00

Remove hard-coded expense filter from Category Over Time report 56b46f1dfa

The Category Over Time report previously only showed expenses (t.amount < 0).
This removes that filter so all transaction types are shown by default,
and adds a type filter (expense/income/transfer) in the right filter panel.

Ref: simpl-resultat#41

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

maximus commented

2026-03-18 05:00:43 +00:00

Author

Owner

Reviewer automatique — needs-fix

Le retrait du filtre t.amount < 0 résout l'issue mais introduit un problème de correctness dans le classement des top N catégories : les montants positifs (revenus) et négatifs (dépenses) sont maintenant mélangés, ce qui peut fausser le tri et masquer des catégories pertinentes. Aucun test n'est ajouté pour couvrir la nouvelle logique.

Suggestions de simplification

src/pages/ReportsPage.tsx : hasCategories inclut déjà state.tab === 'overTime' (quand filterCategories.length > 0). La condition showFilterPanel ajoute maintenant state.tab === 'overTime' séparément pour le cas où filterCategories est vide. C'est correct mais la logique devient redondante et difficile à suivre. Considérer simplifier en : const showFilterPanel = state.tab === 'overTime' || (state.tab === 'byCategory' && filterCategories.length > 0) || (state.tab === 'trends' && sources.length > 1);

Problemes detectes

src/services/reportService.ts:61 [high] La suppression de t.amount < 0 change silencieusement la requête 'top N categories by total spend' plus bas. Avant, elle triait par montant de dépense (négatif). Maintenant, revenus (positifs) et dépenses (négatifs) sont mélangés — le SUM() peut s'annuler ou les revenus dominer le classement. Il faut adapter la requête topCategories pour trier par ABS(SUM(t.amount)) ou par SUM(t.amount) selon le type, sinon le top N sera incohérent quand aucun typeFilter n'est appliqué.
src/services/reportService.ts:64 [medium] COALESCE(c.type, 'expense') suppose que les catégories sans type sont des dépenses. Si des catégories ont type = NULL mais ne sont pas des dépenses, elles seront mal classifiées. Vérifier que cette hypothèse est correcte dans le schéma, ou utiliser un type explicite ('unknown') pour ne pas les inclure par erreur dans le filtre 'expense'.
src/components/reports/ReportFilterPanel.tsx:75 [low] (e.target.value || null) as CategoryTypeFilter — le cast as contourne le type-checking. Si une valeur inattendue apparaît, aucune validation ne la bloque. Préférer une fonction de validation ou au minimum un type guard pour garantir que seules les valeurs valides passent.

## Reviewer automatique — needs-fix Le retrait du filtre `t.amount < 0` résout l'issue mais introduit un problème de correctness dans le classement des top N catégories : les montants positifs (revenus) et négatifs (dépenses) sont maintenant mélangés, ce qui peut fausser le tri et masquer des catégories pertinentes. Aucun test n'est ajouté pour couvrir la nouvelle logique. ### Suggestions de simplification - **src/pages/ReportsPage.tsx** : `hasCategories` inclut déjà `state.tab === 'overTime'` (quand filterCategories.length > 0). La condition `showFilterPanel` ajoute maintenant `state.tab === 'overTime'` séparément pour le cas où filterCategories est vide. C'est correct mais la logique devient redondante et difficile à suivre. Considérer simplifier en : `const showFilterPanel = state.tab === 'overTime' || (state.tab === 'byCategory' && filterCategories.length > 0) || (state.tab === 'trends' && sources.length > 1);` ### Problemes detectes - **src/services/reportService.ts:61** [high] La suppression de `t.amount < 0` change silencieusement la requête 'top N categories by total spend' plus bas. Avant, elle triait par montant de dépense (négatif). Maintenant, revenus (positifs) et dépenses (négatifs) sont mélangés — le SUM() peut s'annuler ou les revenus dominer le classement. Il faut adapter la requête topCategories pour trier par `ABS(SUM(t.amount))` ou par `SUM(t.amount)` selon le type, sinon le top N sera incohérent quand aucun typeFilter n'est appliqué. - **src/services/reportService.ts:64** [medium] `COALESCE(c.type, 'expense')` suppose que les catégories sans type sont des dépenses. Si des catégories ont `type = NULL` mais ne sont pas des dépenses, elles seront mal classifiées. Vérifier que cette hypothèse est correcte dans le schéma, ou utiliser un type explicite ('unknown') pour ne pas les inclure par erreur dans le filtre 'expense'. - **src/components/reports/ReportFilterPanel.tsx:75** [low] `(e.target.value || null) as CategoryTypeFilter` — le cast `as` contourne le type-checking. Si une valeur inattendue apparaît, aucune validation ne la bloque. Préférer une fonction de validation ou au minimum un type guard pour garantir que seules les valeurs valides passent.

maximus added 1 commit 2026-03-18 06:02:48 +00:00

fix: address reviewer feedback (#41 ) d099678ddf

- Default categoryType filter to "expense" so top-N ranking is not
  skewed by mixing income and expense amounts
- Remove redundant showFilterPanel condition (hasCategories already
  covers the overTime tab)
- Add unit tests for getCategoryOverTime query construction and output

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

maximus commented

2026-03-18 07:01:33 +00:00

Author

Owner

Reviewer automatique — needs-fix

Les commentaires du round 1 sont largement adressés (tests ajoutés, typeFilter avec défaut 'expense' pour éviter le mélange). Cependant, le CHANGELOG est inexact (dit 'all types by default' alors que le défaut est 'expense'), et le cast non validé dans le select onChange pourrait laisser passer une valeur inattendue.

Suggestions de simplification

src/services/reportService.ts : Le changement de sémantique (de t.amount < 0 vers COALESCE(c.type, 'expense')) est plus correct mais n'est pas équivalent : une transaction positive sur une catégorie 'expense' (ex: remboursement) sera maintenant incluse, et une dépense sans catégorie assignée ne sera plus filtrée par montant. C'est probablement le comportement voulu mais un commentaire inline clarifierait l'intention du COALESCE.

Problemes detectes

CHANGELOG.md:5 [medium] Le texte dit 'now shows all transaction types by default' mais initialState.categoryType est "expense" (useReports.ts:64). Le comportement par défaut reste filtré sur les dépenses, pas tous les types. Le CHANGELOG induit en erreur. Corriger : 'now filters by category type instead of amount sign, defaults to expense'.
CHANGELOG.fr.md:5 [medium] Même problème que CHANGELOG.md : 'affiche maintenant tous les types de transactions par défaut' est faux vu que le défaut est 'expense'.
src/components/reports/ReportFilterPanel.tsx:75 [low] (e.target.value || null) as CategoryTypeFilter est un cast non sécurisé. Si le DOM est manipulé ou si une valeur inattendue arrive, elle passerait directement dans la query SQL (paramétrisée, donc pas d'injection, mais résultat silencieusement vide). Préférer une validation runtime : const val = e.target.value; const valid: CategoryTypeFilter[] = ['expense','income','transfer']; onCategoryTypeChange(valid.includes(val as any) ? val as CategoryTypeFilter : null).

## Reviewer automatique — needs-fix Les commentaires du round 1 sont largement adressés (tests ajoutés, typeFilter avec défaut 'expense' pour éviter le mélange). Cependant, le CHANGELOG est inexact (dit 'all types by default' alors que le défaut est 'expense'), et le cast non validé dans le select onChange pourrait laisser passer une valeur inattendue. ### Suggestions de simplification - **src/services/reportService.ts** : Le changement de sémantique (de `t.amount < 0` vers `COALESCE(c.type, 'expense')`) est plus correct mais n'est pas équivalent : une transaction positive sur une catégorie 'expense' (ex: remboursement) sera maintenant incluse, et une dépense sans catégorie assignée ne sera plus filtrée par montant. C'est probablement le comportement voulu mais un commentaire inline clarifierait l'intention du COALESCE. ### Problemes detectes - **CHANGELOG.md:5** [medium] Le texte dit 'now shows all transaction types by default' mais `initialState.categoryType` est `"expense"` (useReports.ts:64). Le comportement par défaut reste filtré sur les dépenses, pas tous les types. Le CHANGELOG induit en erreur. Corriger : 'now filters by category type instead of amount sign, defaults to expense'. - **CHANGELOG.fr.md:5** [medium] Même problème que CHANGELOG.md : 'affiche maintenant tous les types de transactions par défaut' est faux vu que le défaut est 'expense'. - **src/components/reports/ReportFilterPanel.tsx:75** [low] `(e.target.value || null) as CategoryTypeFilter` est un cast non sécurisé. Si le DOM est manipulé ou si une valeur inattendue arrive, elle passerait directement dans la query SQL (paramétrisée, donc pas d'injection, mais résultat silencieusement vide). Préférer une validation runtime : `const val = e.target.value; const valid: CategoryTypeFilter[] = ['expense','income','transfer']; onCategoryTypeChange(valid.includes(val as any) ? val as CategoryTypeFilter : null)`.

maximus added 1 commit 2026-03-18 08:01:50 +00:00

fix: address reviewer feedback (#41 ) 97da6f9f71

- Correct CHANGELOG to reflect default type is expense, not all types
- Validate select onChange value against allowed CategoryTypeFilter values

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

maximus commented

2026-03-18 09:00:37 +00:00

Author

Owner

Reviewer automatique — approved

Les deux problèmes du round 2 sont corrigés : le CHANGELOG mentionne maintenant correctement 'dépense par défaut', et le select onChange valide la valeur contre une whitelist avant de la propager. Le code est correct, les requêtes SQL sont paramétrées, les tests couvrent les cas pertinents (filtres individuels, combinés, structure de retour, groupement Other).

Suggestions de simplification

CHANGELOG.md : Les deux lignes pour #41 pourraient être fusionnées en une seule entrée puisqu'elles décrivent le même changement (retrait du filtre + ajout du sélecteur). Idem pour CHANGELOG.fr.md.

## Reviewer automatique — approved Les deux problèmes du round 2 sont corrigés : le CHANGELOG mentionne maintenant correctement 'dépense par défaut', et le select onChange valide la valeur contre une whitelist avant de la propager. Le code est correct, les requêtes SQL sont paramétrées, les tests couvrent les cas pertinents (filtres individuels, combinés, structure de retour, groupement Other). ### Suggestions de simplification - **CHANGELOG.md** : Les deux lignes pour #41 pourraient être fusionnées en une seule entrée puisqu'elles décrivent le même changement (retrait du filtre + ajout du sélecteur). Idem pour CHANGELOG.fr.md.