maximus/Simpl-Resultat
1
0
Fork 0
Code Issues 19 Pull requests 8 Projects Releases 34 Packages Wiki Activity Actions

feat(onboarding): transaction count, onboarding counters and checklist reducer (#351) #377

Merged
maximus merged 2 commits from issue-351-onboarding-core into main 2026-10-04 12:51:11 +00:00
Conversation 2 Commits 2 Files changed 9 +785
maximus commented 2026-10-03 23:22:41 +00:00
Owner
Copy link

Fixes #351

What

Data foundation of the getting-started checklist (wave planned-2026-10-03-onboarding-phase1, link 1 of 7). No UI, no DB migration, no CHANGELOG entry (centralised in #357).

  • transactionService.getTransactionCount() — unfiltered SELECT COUNT(*) FROM transactions.
  • src/services/onboardingService.ts — getOnboardingCounters() reads the aggregated five-count query, getImportFolder() and the onboarding_checklist_hidden preference in one Promise.all; hideChecklist() / showChecklist() set / delete the preference (value "1") and logInfo.
  • src/utils/onboardingChecklist.ts — pure computeChecklist(counters, { importFolderSet, budgetAllowed }) → { items, done, total, complete }; six rows in order (profile always done, importFolder, source, import, manualCategory, budget); the budget row is locked when Budget is not in the edition, excluded from total (5 instead of 6) and from complete, and links to /settings/users.
  • src/hooks/useOnboardingChecklist.ts — onboardingChecklistReducer + initialOnboardingChecklistState exported; state { status, counters, importFolderSet, hidden, error }; actions refresh(), hide(), show(); composed with useEntitlement("budget") → returns { status, checklist, hidden, budgetReady, refresh, hide, show }. A load error sets status: "error" and never touches hidden.
  • Types OnboardingCounters, ChecklistItem, ChecklistItemKey in src/shared/types/index.ts.

Why

#352 (dashboard empty state + checklist) and #353 (Transactions/Budget empty states) consume these pieces; keeping them pure and exported lets the state machine be tested without a renderHook harness.

Decisions

  • OnboardingCounters fields are camelCase (as the issue names them in computeChecklist); the SQL aliases stay snake_case and are mapped in the service.
  • The hook state carries importFolderSet beside the four listed fields, since computeChecklist takes it separately from counters.
  • hide() / show() persist first, then dispatch; a failed write is logged (logError) and leaves the state unchanged.
  • The issue lists five to values for six rows; #352 renders a link only on unchecked rows and profile is always checked, so: importFolder/source/import → /import, manualCategory → /transactions, budget → /budget (/settings/users when locked); profile.to = /settings/users, never rendered.

How tested

  • src/utils/onboardingChecklist.test.ts (14): each row in isolation, zero counters → 1/6, folder without source → 2/6, budgetAllowed=false → total=5 + budget.locked, complete at 5/5 locked and 6/6 open, not at 5/6, routes.
  • src/hooks/useOnboardingChecklist.test.ts (10): every action; error keeps hidden and the last counters; refresh after error → loading → ready.
  • src/services/onboardingService.test.ts (10): aggregated SQL shape, preference keys, mapping, hidden only for "1", empty folder treated as unset, hide/show writes + log.
  • src/services/transactionService.test.ts (3): 0, N, no row.
  • npm test: 72 files, 1385 tests green. npm run build (tsc + vite) green.

Generated autonomously by /autopilot run of 2026-10-03

🤖 Generated with Claude Code

Fixes #351 ## What Data foundation of the getting-started checklist (wave `planned-2026-10-03-onboarding-phase1`, link 1 of 7). No UI, no DB migration, no CHANGELOG entry (centralised in #357). - `transactionService.getTransactionCount()` — unfiltered `SELECT COUNT(*) FROM transactions`. - `src/services/onboardingService.ts` — `getOnboardingCounters()` reads the aggregated five-count query, `getImportFolder()` and the `onboarding_checklist_hidden` preference in one `Promise.all`; `hideChecklist()` / `showChecklist()` set / delete the preference (value `"1"`) and `logInfo`. - `src/utils/onboardingChecklist.ts` — pure `computeChecklist(counters, { importFolderSet, budgetAllowed })` → `{ items, done, total, complete }`; six rows in order (`profile` always done, `importFolder`, `source`, `import`, `manualCategory`, `budget`); the budget row is `locked` when Budget is not in the edition, excluded from `total` (5 instead of 6) and from `complete`, and links to `/settings/users`. - `src/hooks/useOnboardingChecklist.ts` — `onboardingChecklistReducer` + `initialOnboardingChecklistState` exported; state `{ status, counters, importFolderSet, hidden, error }`; actions `refresh()`, `hide()`, `show()`; composed with `useEntitlement("budget")` → returns `{ status, checklist, hidden, budgetReady, refresh, hide, show }`. A load error sets `status: "error"` and never touches `hidden`. - Types `OnboardingCounters`, `ChecklistItem`, `ChecklistItemKey` in `src/shared/types/index.ts`. ## Why #352 (dashboard empty state + checklist) and #353 (Transactions/Budget empty states) consume these pieces; keeping them pure and exported lets the state machine be tested without a renderHook harness. ## Decisions - `OnboardingCounters` fields are camelCase (as the issue names them in `computeChecklist`); the SQL aliases stay snake_case and are mapped in the service. - The hook state carries `importFolderSet` beside the four listed fields, since `computeChecklist` takes it separately from `counters`. - `hide()` / `show()` persist first, then dispatch; a failed write is logged (`logError`) and leaves the state unchanged. - The issue lists five `to` values for six rows; #352 renders a link only on unchecked rows and `profile` is always checked, so: importFolder/source/import → `/import`, manualCategory → `/transactions`, budget → `/budget` (`/settings/users` when locked); `profile.to` = `/settings/users`, never rendered. ## How tested - `src/utils/onboardingChecklist.test.ts` (14): each row in isolation, zero counters → 1/6, folder without source → 2/6, `budgetAllowed=false` → `total=5` + `budget.locked`, complete at 5/5 locked and 6/6 open, not at 5/6, routes. - `src/hooks/useOnboardingChecklist.test.ts` (10): every action; error keeps `hidden` and the last counters; refresh after error → loading → ready. - `src/services/onboardingService.test.ts` (10): aggregated SQL shape, preference keys, mapping, hidden only for `"1"`, empty folder treated as unset, hide/show writes + log. - `src/services/transactionService.test.ts` (3): 0, N, no row. - `npm test`: 72 files, 1385 tests green. `npm run build` (tsc + vite) green. Generated autonomously by /autopilot run of 2026-10-03 🤖 Generated with [Claude Code](https://claude.com/claude-code)
maximus added 1 commit 2026-10-03 23:22:41 +00:00
feat(onboarding): add transaction count, onboarding counters and checklist reducer (#351)
All checks were successful
PR Check — Frontend / frontend (pull_request) Successful in 1m46s
Details
f77ce94505 
Lay the data foundation of the getting-started checklist, without UI:

- transactionService.getTransactionCount(): unfiltered COUNT(*) of transactions
- onboardingService: getOnboardingCounters() reads the five aggregated counts,
  the import folder and the onboarding_checklist_hidden preference in one
  Promise.all; hideChecklist()/showChecklist() persist the preference and log
- utils/onboardingChecklist.computeChecklist(): pure computation of the six
  rows (profile, importFolder, source, import, manualCategory, budget), with
  the budget row locked and excluded from the total when the edition has no
  Budget
- hooks/useOnboardingChecklist: exported reducer + initial state, actions
  refresh/hide/show, composed with useEntitlement("budget"); a load error
  sets status "error" and keeps the hidden flag
- types OnboardingCounters, ChecklistItem, ChecklistItemKey

Tests cover every row in isolation, the budget gating (total 5 vs 6, complete
at 5/5 locked and 6/6 open), the aggregated query and preference reads, and
every reducer action. No DB migration, no CHANGELOG entry (centralised in #357).

Resolves #351

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
maximus added the
autopilot:pending-human
 label 2026-10-03 23:22:47 +00:00
maximus added 1 commit 2026-10-03 23:23:47 +00:00
refactor(onboarding): compute the load error message once (#351)
All checks were successful
PR Check — Frontend / frontend (pull_request) Successful in 1m43s
Details
76fd8d376a 
Self-review tidy: `errorMessage(e)` was evaluated twice in `refresh`.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
maximus commented 2026-10-03 23:24:10 +00:00
Author
Owner
Copy link

Self-review — Verdict: APPROVE

Autonomous review of the full diff (GET /pulls/377.diff, 9 files, 2 commits f77ce94 + 76fd8d3) against the acceptance criteria of #351.

Summary

Pure data layer: one unfiltered count, one aggregated five-count query read with the two preferences in a single Promise.all, a pure computeChecklist, and an exported reducer composed with useEntitlement("budget"). No UI, no migration, no i18n key, no CHANGELOG edit. The tidy commit 76fd8d3 only removes a duplicated errorMessage(e) call found during this review.

Security

  • No secrets, no personal data (public repo). SQL is constant strings; preference reads/writes go through userPreferenceService (parametrised $1/$2). No string interpolation into SQL.

Acceptance criteria

Criterion Status Evidence
computeChecklist: each row tested in isolation OK onboardingChecklist.test.ts — source, import, manualCategory (and not transactions), budget, importFolder
budgetAllowed=false → total=5, budget.locked=true OK "budgetAllowed=false → total=5 and the budget row is locked" (+ to="/settings/users")
complete true at 5/5 locked and 6/6 open OK two dedicated tests, plus a 5/6 negative case
folder chosen without source → done=2 OK "import folder chosen without any source → done=2"
zero counters → done=1 OK "zero counters → only profile is done (1/6)"
Reducer: every action tested OK LOAD_START, LOAD_DONE, LOAD_ERROR, HIDE, SHOW in useOnboardingChecklist.test.ts
load error keeps hidden, sets status="error" OK tested from a hidden-ready state and from the fresh state
refresh after error → loading then ready OK "refresh after an error goes back to loading, then ready"
getTransactionCount with mocked db (0 and N) OK transactionService.test.ts (0, 1234, no row)
npm test + npm run build green, no UI touched OK 72 files / 1385 tests; tsc + vite green; no .tsx in the diff
Six rows in order, to per row, /settings/users for locked budget OK "lists the six rows in order", "links each row to its page"
Query shape as given in the issue OK test asserts status = 'completed', is_manually_categorized = 1, budget_entries, unfiltered transactions
hideChecklist → "1", showChecklist → delete, logInfo both OK service tests check params and one logInfo per call, none on failure
Types in src/shared/types/index.ts OK OnboardingCounters, ChecklistItem, ChecklistItemKey

Quality

  • No dead code, no debug output. Comments in English. Reducer/initial-state naming follows licenseReducer / feedbackReducer / migrationReducer.
  • getOnboardingCounters tolerates an empty result set (falls back to zeros) like the other COUNT(*) readers.

Concerns (non-blocking)

  1. Race hide() vs in-flight refresh(): LOAD_DONE applies the hidden read at the start of the load. A dismissal clicked while a refresh is running could be undone by that load's result. The window is a few ms of local SQLite reads and the Hide button only exists once the list is shown; if #352 triggers refresh() after imports, it may want to call it before re-rendering the Hide control, or a sequence guard can be added then.
  2. Five to values for six rows in the issue: resolved by mapping the five uncheckable rows (import → /import, manualCategory → /transactions); profile.to is never rendered (#352 links unchecked rows only). Flagged in the PR body for Max.

Generated autonomously by /autopilot run of 2026-10-03

## Self-review — Verdict: **APPROVE** Autonomous review of the full diff (`GET /pulls/377.diff`, 9 files, 2 commits `f77ce94` + `76fd8d3`) against the acceptance criteria of #351. ### Summary Pure data layer: one unfiltered count, one aggregated five-count query read with the two preferences in a single `Promise.all`, a pure `computeChecklist`, and an exported reducer composed with `useEntitlement("budget")`. No UI, no migration, no i18n key, no CHANGELOG edit. The tidy commit `76fd8d3` only removes a duplicated `errorMessage(e)` call found during this review. ### Security - No secrets, no personal data (public repo). SQL is constant strings; preference reads/writes go through `userPreferenceService` (parametrised `$1/$2`). No string interpolation into SQL. ### Acceptance criteria | Criterion | Status | Evidence | |---|---|---| | `computeChecklist`: each row tested in isolation | OK | `onboardingChecklist.test.ts` — source, import, manualCategory (and not `transactions`), budget, importFolder | | `budgetAllowed=false` → `total=5`, `budget.locked=true` | OK | "budgetAllowed=false → total=5 and the budget row is locked" (+ `to="/settings/users"`) | | `complete` true at 5/5 locked and 6/6 open | OK | two dedicated tests, plus a 5/6 negative case | | folder chosen without source → `done=2` | OK | "import folder chosen without any source → done=2" | | zero counters → `done=1` | OK | "zero counters → only profile is done (1/6)" | | Reducer: every action tested | OK | LOAD_START, LOAD_DONE, LOAD_ERROR, HIDE, SHOW in `useOnboardingChecklist.test.ts` | | load error keeps `hidden`, sets `status="error"` | OK | tested from a hidden-ready state and from the fresh state | | `refresh` after `error` → `loading` then `ready` | OK | "refresh after an error goes back to loading, then ready" | | `getTransactionCount` with mocked `db` (0 and N) | OK | `transactionService.test.ts` (0, 1234, no row) | | `npm test` + `npm run build` green, no UI touched | OK | 72 files / 1385 tests; tsc + vite green; no `.tsx` in the diff | | Six rows in order, `to` per row, `/settings/users` for locked budget | OK | "lists the six rows in order", "links each row to its page" | | Query shape as given in the issue | OK | test asserts `status = 'completed'`, `is_manually_categorized = 1`, `budget_entries`, unfiltered `transactions` | | `hideChecklist` → `"1"`, `showChecklist` → delete, `logInfo` both | OK | service tests check params and one `logInfo` per call, none on failure | | Types in `src/shared/types/index.ts` | OK | `OnboardingCounters`, `ChecklistItem`, `ChecklistItemKey` | ### Quality - No dead code, no debug output. Comments in English. Reducer/initial-state naming follows `licenseReducer` / `feedbackReducer` / `migrationReducer`. - `getOnboardingCounters` tolerates an empty result set (falls back to zeros) like the other `COUNT(*)` readers. ### Concerns (non-blocking) 1. **Race `hide()` vs in-flight `refresh()`**: `LOAD_DONE` applies the `hidden` read at the start of the load. A dismissal clicked while a refresh is running could be undone by that load's result. The window is a few ms of local SQLite reads and the Hide button only exists once the list is shown; if #352 triggers `refresh()` after imports, it may want to call it before re-rendering the Hide control, or a sequence guard can be added then. 2. Five `to` values for six rows in the issue: resolved by mapping the five uncheckable rows (import → `/import`, manualCategory → `/transactions`); `profile.to` is never rendered (#352 links unchecked rows only). Flagged in the PR body for Max. Generated autonomously by /autopilot run of 2026-10-03
maximus commented 2026-10-04 01:26:15 +00:00
Author
Owner
Copy link

Revue adversariale (passe indépendante) — Verdict : APPROVE

Revue de la tête 76fd8d3 contre main, indépendante du self-review du worker (commentaire 19000). Rien d'exécuté dans le dépôt : copie hors dépôt (git archive FETCH_HEAD), supprimée en fin de revue.

Résumé

Socle de données conforme au contrat de #351 : requête agrégée identique à celle de l'issue (tables et colonnes vérifiées dans consolidated_schema.sql ; status = 'completed' correspond aux deux seules valeurs écrites par le wizard, useImportWizard.ts:1009 et :1060), computeChecklist pure, reducer exporté qui préserve hidden sur erreur, useEntitlement("budget") composé comme demandé. Aucune migration, aucune chaîne UI, CHANGELOG laissé à #357 comme la vague l'impose.

Ce qui a tourné

  • Suite complète sur la copie : 72 fichiers, 1385 tests verts ; tsc --noEmit propre.
  • 7 mutations du relecteur, chacune détectée (copie restaurée au byte près après chaque) :
    • unlocked = items (ligne verrouillée comptée dans total) → 3/14 rouges
    • hidden: hiddenPref !== null → 1/10
    • importFolderSet: importFolder !== null (dossier vide = choisi) → 1/10
    • LOAD_ERROR remet hidden: false → 1/10
    • return rows[0].cnt sans repli → 1/3
    • route budget verrouillé → /budget → 1/14
    • LOAD_DONE ignore snapshot.hidden → 2/10
  • CI frontend verte sur la tête (1m43s) ; check-rust correctement sauté (aucun fichier Rust).
  • Timeline de #351 : pull_ref closes depuis #377 → fermeture au merge. Commits en format conventionnel, les deux référencent #351.
  • Aucun skip/only dans les 4 fichiers de test.

Suggestions (non bloquantes)

  1. src/utils/onboardingChecklist.ts:50 — la ligne budget peut être { done: true, locked: true } (entrées de budget créées avant le gating, puis édition Gratuite). #378 s'en sort en résolvant locked avant done ; une phrase dans le doc-comment de ChecklistItem (« locked prime sur done ») protégerait un troisième consommateur.
  2. src/hooks/useOnboardingChecklist.ts:92-113 — hide() persiste puis dispatche ; un refresh() en vol qui a lu hidden=false avant l'écriture et se résout après remet hidden=false jusqu'au prochain chargement. Sans effet tant que le dashboard ne rafraîchit qu'au montage ; à revoir si #352/#353 ajoutent un refresh sur focus (compteur de requête pour ignorer un LOAD_DONE périmé).
  3. docs/architecture.md non mis à jour pour le nouveau service et le nouveau hook — confirmé centralisé dans #357, rien à faire ici.

Actions faites

  • Verdict posté ; #351 porte déjà status:approved (self-review), label inchangé.
  • Copie hors dépôt et pr378.diff supprimés du scratchpad ; dépôt de travail intact (git status propre).
## Revue adversariale (passe indépendante) — Verdict : **APPROVE** Revue de la tête `76fd8d3` contre `main`, indépendante du self-review du worker (commentaire 19000). Rien d'exécuté dans le dépôt : copie hors dépôt (`git archive FETCH_HEAD`), supprimée en fin de revue. ### Résumé Socle de données conforme au contrat de #351 : requête agrégée identique à celle de l'issue (tables et colonnes vérifiées dans `consolidated_schema.sql` ; `status = 'completed'` correspond aux deux seules valeurs écrites par le wizard, `useImportWizard.ts:1009` et `:1060`), `computeChecklist` pure, reducer exporté qui préserve `hidden` sur erreur, `useEntitlement("budget")` composé comme demandé. Aucune migration, aucune chaîne UI, CHANGELOG laissé à #357 comme la vague l'impose. ### Ce qui a tourné - Suite complète sur la copie : **72 fichiers, 1385 tests verts** ; `tsc --noEmit` propre. - **7 mutations du relecteur**, chacune détectée (copie restaurée au byte près après chaque) : - `unlocked = items` (ligne verrouillée comptée dans `total`) → 3/14 rouges - `hidden: hiddenPref !== null` → 1/10 - `importFolderSet: importFolder !== null` (dossier vide = choisi) → 1/10 - `LOAD_ERROR` remet `hidden: false` → 1/10 - `return rows[0].cnt` sans repli → 1/3 - route budget verrouillé → `/budget` → 1/14 - `LOAD_DONE` ignore `snapshot.hidden` → 2/10 - CI frontend **verte** sur la tête (1m43s) ; `check-rust` correctement sauté (aucun fichier Rust). - Timeline de #351 : `pull_ref` `closes` depuis #377 → fermeture au merge. Commits en format conventionnel, les deux référencent #351. - Aucun `skip`/`only` dans les 4 fichiers de test. ### Suggestions (non bloquantes) 1. `src/utils/onboardingChecklist.ts:50` — la ligne `budget` peut être `{ done: true, locked: true }` (entrées de budget créées avant le gating, puis édition Gratuite). #378 s'en sort en résolvant `locked` avant `done` ; une phrase dans le doc-comment de `ChecklistItem` (« `locked` prime sur `done` ») protégerait un troisième consommateur. 2. `src/hooks/useOnboardingChecklist.ts:92-113` — `hide()` persiste puis dispatche ; un `refresh()` en vol qui a lu `hidden=false` avant l'écriture et se résout après remet `hidden=false` jusqu'au prochain chargement. Sans effet tant que le dashboard ne rafraîchit qu'au montage ; à revoir si #352/#353 ajoutent un refresh sur focus (compteur de requête pour ignorer un `LOAD_DONE` périmé). 3. `docs/architecture.md` non mis à jour pour le nouveau service et le nouveau hook — confirmé centralisé dans #357, rien à faire ici. ### Actions faites - Verdict posté ; #351 porte déjà `status:approved` (self-review), label inchangé. - Copie hors dépôt et `pr378.diff` supprimés du scratchpad ; dépôt de travail intact (`git status` propre).
maximus merged commit 76fd8d376a into main 2026-10-04 12:51:11 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
autopilot:pending-human

PR produced by /autopilot — awaits human review/merge

  
source:analyste

Créée par le Super-Analyste

  
source:defenseur

Créée par l Escouade Défenseur

  
source:human

Créée par un humain

  
source:medic

Créée par le Medic (auto-détection)

  
status:approved

PR approuvée, prête à merger

  
status:blocked

Bloquée (question, dépendance, clarification)

  
status:in-progress

Un agent (Medic/Dev) travaille dessus

  
status:needs-clarification

Issue blocked by ambiguity — Max needs to clarify before re-running autopilot

  
status:needs-fix

Reviewer demande des corrections

  
status:ready

Analysée et documentée, prête à prendre

  
status:review

PR créée, en attente du Reviewer

  
status:triage

Nouvelle issue, pas encore analysée

  
type:bug

Correction de bug → Medic

  
type:feature

Nouvelle fonctionnalité → Dev

  
type:infra

Config, CI/CD, déploiement → Dev

  
type:refactor

Refactoring / amélioration technique → Dev

  
type:schema

Modèle de données / migrations → Dev

  
type:security

Issue de sécurité → Medic ou Dev

No labels
autopilot:pending-human
source:analyste
source:defenseur
source:human
source:medic
status:approved
status:blocked
status:in-progress
status:needs-clarification
status:needs-fix
status:ready
status:review
status:triage
type:bug
type:feature
type:infra
type:refactor
type:schema
type:security
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: maximus/Simpl-Resultat#377
No description provided.
Delete branch "issue-351-onboarding-core"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?