Skip to content

feat(ledger): native chart of accounts — template picker and sever disposition - #363

Merged
jfrench9 merged 2 commits into
mainfrom
feature/native-chart-ui
Sep 12, 2026
Merged

jfrench9 merged 2 commits into
mainfrom
feature/native-chart-ui

Conversation

@jfrench9

Copy link
Copy Markdown
Member

Summary

The app side of the native accounting cutover (RoboFinSystems/robosystems#1378, #1379; client 1.16.0): the chart-of-accounts page's empty state becomes a template picker that creates the chart in one step, and deleting a QuickBooks connection asks whether to keep it revivable or sever it and go native.

Changes

Chart of accounts (ledger/chart-of-accounts)

  • New components/ChartTemplatePicker.tsx: lists the shipped templates via clients.ledger.listChartTemplates, takes an optional legal form for the equity mapping (corporation / LLC / partnership, defaulting to the entity's own), runs clients.ledger.initializeChartOfAccounts, and hands the result back. A refusal (409 once a chart exists) or a load failure surfaces as an alert. The copy tells a QuickBooks user to skip it, since their chart arrives with the first sync.
  • content.tsx: the "No Accounts Found / import accounting data" empty state is replaced by the picker when a graph is selected; a reloadKey re-runs the loader for the same graph once the chart exists, so the new accounts replace the empty state without a route bounce.

Connections (connections)

  • New components/DeleteConnectionModal.tsx wrapping ConfirmModal: for QuickBooks, two radio dispositions — Disconnect (default; today's behaviour, revivable on re-OAuth) and Sever and go native (keep the chart QuickBooks created as the graph's own; QuickBooks can never resume; bank feeds connect after). Every other provider keeps the single confirmation. The dialog names the provider from PROVIDER_LABELS (now exported from ConnectionCard) instead of the raw slug.
  • content.tsx: handleDeleteConnection(disposition) passes query: { disposition } to SDK.deleteConnection; the success toast says which happened.

Tests

  • ChartTemplatePicker.test.tsx (5): listing and default selection, initialize with template + legal form, null legal form when the entity's own is kept, the 409 refusal, a load failure.
  • DeleteConnectionModal.test.tsx (4): QuickBooks defaults to disconnect, sever once chosen, single disposition for other providers, hidden renders nothing.

Dependency: @robosystems/client ^1.15.0 → ^1.16.0 (package.json, lockfile).

Deploy Notes

  • Needs a RoboSystems API release carrying #1378 and #1379 — both are on main but the latest release is v1.11.18 (2026-09-09), which predates them. Deploy the API first: until then the picker's chartTemplates query errors and initialize-chart-of-accounts returns 404, and disposition=sever is ignored by deleteConnection.
  • @robosystems/client 1.16.0: additive (two new facade methods, one new optional query parameter); no existing call site changed shape.
  • No new env vars, no CloudFormation change.

Testing

  • npm run test:all (vitest, format, lint:fix, typecheck, cf-lint): 409 tests passed across 55 files; ESLint, tsc and cfn-lint clean. The pre-commit gate ran the same.
  • Not exercised against a live graph in this session; both new components are covered by unit tests with the client mocked.

🤖 Generated with Claude Code

https://claude.ai/code/session_0188CbjDiNBtxdEYjSJ5Y7mX

…sposition

Consumes @robosystems/client 1.16.0 (the native chart surface from
robosystems #1378 and #1379).

The chart-of-accounts page's empty state becomes a template picker: it
lists the shipped templates (chartTemplates), takes an optional legal form
for the equity mapping, runs initialize-chart-of-accounts and reloads the
page with the new chart. One-time by design; a 409 from a graph that
already has a chart surfaces as the alert.

Deleting a QuickBooks connection now asks which way the books go: keep the
connection revivable (disconnect, the default and today's behaviour) or
sever it and go native with the chart QuickBooks created. The modal is its
own component; other providers keep the single confirmation. The provider
label map is exported from ConnectionCard so the dialog names the provider
instead of interpolating the slug.

Claude-Session: https://claude.ai/code/session_0188CbjDiNBtxdEYjSJ5Y7mX
@github-actions github-actions Bot added the change:normal Normal change: automated review documented on the pull request before merge label Sep 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Change classification: normal — platform or application code changed.

A normal change carries a documented automated review on this pull request before merge, in place of a second human approver. The change-classification job requests that review and passes once it has been posted. Recorded by the change-classification job.

@jfrench9

Copy link
Copy Markdown
Member Author

@claude please review this PR

@claude

claude Bot commented Sep 12, 2026

Copy link
Copy Markdown

Claude finished @jfrench9's task in 1m 52s —— View job


Review

  • Read trigger comment and gather PR context
  • Review ChartTemplatePicker.tsx + its test
  • Review DeleteConnectionModal.tsx + its test
  • Review connections/content.tsx and chart-of-accounts/content.tsx diffs
  • Review ConnectionCard.tsx export change and dependency bump
  • Post final review feedback

Nice, focused PR — the template picker and the sever/disconnect fork are both clean, well-tested implementations of the two flows described. Two things worth a look before/around merge:

1. ChartTemplatePicker bypasses the app's error-mapping convention (src/app/(app)/ledger/chart-of-accounts/components/ChartTemplatePicker.tsx:63-65,85-89)

Both loadError and initError are set directly from err.message:

setLoadError(err instanceof Error ? err.message : 'Failed to load chart templates.')
...
setInitError(err instanceof Error ? err.message : 'Failed to initialize the chart of accounts.')

Per CLAUDE.md's "Error mapping" convention, the SDK facade throws Error("<label> failed: " + JSON.stringify(error)), so err.message on a real failure will be something like Initialize chart of accounts failed: {"detail":"This graph already has a chart of accounts"} — the raw envelope, not the clean string the unit tests use (ChartTemplatePicker.test.tsx:112-127 mocks a plain Error('This graph already has a chart of accounts'), which doesn't exercise the real shape). src/lib/ledger/errors.ts exists specifically so new write surfaces don't re-derive this — friendlyError(err.message).message here would both unwrap the JSON and let the 409-already-exists case get a clearer, on-brand message instead of the raw FastAPI detail.

Fix this →

2. Template picker can render alongside a genuine load failure (src/app/(app)/ledger/chart-of-accounts/content.tsx:847-874)

The empty-state branch is keyed only on accounts.length === 0, which is also true when getAccountTree throws (the catch at line 433-436 never touches accounts, and error gets set to 'Failed to load chart of accounts. Please try again.'). In that case the page now shows the red error Card and, right below it, "Start from a template" inviting the user to create a new chart — even though one may already exist and just failed to load. Previously this only produced the (less actionable) "No Accounts Found" empty state. Worth gating the picker on !error as well, e.g. accounts.length === 0 && !error ? <ChartTemplatePicker .../> : ..., so a transient load failure doesn't nudge someone into initializeChartOfAccounts on a graph that may already have a chart (the 409 refusal covers correctness, but the UX is confusing either way).

Minor / non-blocking:

  • ChartTemplatePicker's template cards use role="radio" on <button> elements (ChartTemplatePicker.tsx:133-146) without arrow-key navigation, which a native role="radiogroup" implies. DeleteConnectionModal in the same PR uses real <input type="radio"> for its disposition choice — consider the same here for consistency/accessibility, though this is low severity given it's still clickable and labeled.
  • Everything else checked out: the PROVIDER_LABELS export, the disposition query param wiring in connections/content.tsx, the reload-key pattern for re-running the account loader, and the @robosystems/client bump are all straightforward and match the described deploy-ordering caveat (API must ship #1378/#1379 first).

Test coverage for both new components (ChartTemplatePicker.test.tsx, DeleteConnectionModal.test.tsx) is thorough for the described scenarios.

Review fixes on #363. The picker now runs SDK errors through friendlyError
instead of showing the raw "<label> failed: {json}" envelope, and the
one-time refusal (a chart already exists) gets its own mapping. The chart
page no longer shows the picker under a load-failure card, since an
empty account list also follows a failed load. The template cards are
real radio inputs inside a fieldset, matching the disposition dialog.

Claude-Session: https://claude.ai/code/session_0188CbjDiNBtxdEYjSJ5Y7mX
@jfrench9
jfrench9 merged commit fb43b32 into main Sep 12, 2026
7 checks passed
@jfrench9
jfrench9 deleted the feature/native-chart-ui branch September 12, 2026 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:normal Normal change: automated review documented on the pull request before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant