Skip to content

feat: admin Chart of Accounts editor (L4)#89

Merged
chitcommit merged 1 commit intomainfrom
feat/coa-admin-editor
Apr 10, 2026
Merged

feat: admin Chart of Accounts editor (L4)#89
chitcommit merged 1 commit intomainfrom
feat/coa-admin-editor

Conversation

@chitcommit
Copy link
Copy Markdown
Contributor

@chitcommit chitcommit commented Apr 10, 2026

Summary

Adds /coa admin page for L4 (owner/admin) users to view and manage tenant-specific Chart of Accounts overrides alongside the 80 global defaults seeded in #86.

Features

  • Searchable, filterable table: filter by scope (all/global/tenant) and type (asset/liability/equity/income/expense); text search over code and name
  • Create form: code, name, type, subtype, parent code, Schedule E line, tax deductible, description
  • Deactivate/reactivate tenant-specific accounts (global defaults are read-only)
  • Stats header: total / global / tenant-specific counts

validateNewAccountCode rules

Enforces the ChittyFinance code-range convention client-side before submitting:

Type Valid ranges
Asset 1000-1099, 1100-1199, 1500-1599, 1600-1699
Liability 2000-2099, 2500-2599
Equity 3000-3099
Income 4000-4099, 4100-4199
Expense 5000-5499, 5100-5199, 5200-5299, 5300-5399, 5400-5499, 6000-6099, 7000-7099, 9000-9999

Plus: 4-digit code required, no duplicates (tenant or global), parent code must exist if provided.

Authorization

  • L4 gate is server-side (PATCH/POST /api/coa* call requireL4() which checks tenant_users.role for owner/admin)
  • Frontend surfaces a friendly error if a non-L4 user attempts to create/edit
  • Sidebar link visible to cfo/accountant operating-model roles, but the actual write permission depends on the tenant-scoped role

Test plan

  • TypeScript check passes (0 errors)
  • 221 tests pass
  • Manual: visit /coa → verify 80 global defaults render sorted by code
  • Manual: click "New Account" with invalid code (e.g. "3050" for type=expense) → verify validation error
  • Manual: create a tenant-specific override (e.g. "5075 Plumbing Repairs") → verify it appears with Tenant scope badge
  • Manual: deactivate a tenant account → verify row dims and isActive toggles
  • Manual: as a viewer-role user, attempt create → verify 403 error surfaces as "need owner/admin role"

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added Chart of Accounts administration page for CFO and Accountant roles
    • Create and manage chart of accounts with built-in code validation
    • Search and filter accounts by type and scope
    • Activate or deactivate tenant-owned accounts from the management interface

Adds /coa page for L4 (owner/admin) users to view and manage tenant-specific
Chart of Accounts overrides alongside the 80 global defaults.

New: client/src/pages/CoaAdmin.tsx
- Searchable, filterable table (scope: all/global/tenant, type: 5 enum)
- Stats header: total / global / tenant-specific counts
- Create form with 8 fields (code, name, type, subtype, parent, schedule E,
  tax deductible, description)
- Toggle activate/deactivate for tenant-specific accounts
- Global defaults are displayed read-only (cannot be edited through this UI)
- L4 authorization enforced server-side; UI shows friendly "need owner/admin
  role" error if PATCH/POST returns 403

validateNewAccountCode() enforces the ChittyFinance code-range convention:
- 4-digit code required
- Code must fall inside one of the ranges defined for its type
  (e.g. expense codes must be in 5000-5499, 5100-5199, 6000-6099, 7000-7099,
  or 9000-9999)
- Code must not already exist for this tenant or as a global default
- Parent code must exist in the COA if provided

Hook additions (client/src/hooks/use-classification.ts):
- useCreateCoaAccount() — POST /api/coa
- useUpdateCoaAccount() — PATCH /api/coa/:id
- Both invalidate the tenant-scoped /api/coa cache on success

Wiring:
- /coa route added to App.tsx
- Sidebar link added (cfo, accountant roles; L4 check is server-side)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 10, 2026 22:50
@chitcommit chitcommit enabled auto-merge (squash) April 10, 2026 22:50
@github-actions
Copy link
Copy Markdown
Contributor

@coderabbitai review

Please evaluate:

  • Security implications
  • Credential exposure risk
  • Dependency supply chain concerns
  • Breaking API changes

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 10, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The PR introduces a new Chart of Accounts administration feature to the frontend, including a new routing entry, sidebar navigation link, two React Query mutation hooks for create and update operations, and a full-featured admin page component with form validation, filtering, and account management capabilities.

Changes

Cohort / File(s) Summary
Routing & Navigation
client/src/App.tsx, client/src/components/layout/Sidebar.tsx
Added new /coa route in router and corresponding sidebar navigation item labeled "Chart of Accounts" with BookOpen icon, visible to CFO and accountant roles.
React Query Hooks
client/src/hooks/use-classification.ts
Introduced useCreateCoaAccount and useUpdateCoaAccount mutation hooks that POST/PATCH to tenant-scoped COA endpoints and invalidate query cache on success.
Chart of Accounts Admin Page
client/src/pages/CoaAdmin.tsx
New page component implementing full COA management UI with: create form featuring code validation (4-digit, type-specific ranges, uniqueness, parent code existence), filterable table with search/scope/type filters, activation/deactivation toggle for tenant accounts, and form-level error/success messaging.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • PR #86: Implements the backend POST /api/coa and PATCH /api/coa/:id endpoints that this PR's React Query hooks and CoaAdmin page directly consume.

Suggested reviewers

  • Copilot

Poem

🐰 A chart of accounts, oh what a sight!
With validation so keen and code ranges tight,
Creating and toggling with mutations so swift,
Our finances organized—what a gift! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: admin Chart of Accounts editor (L4)' directly and clearly summarizes the main change: adding a new admin Chart of Accounts editor page for L4 users.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/coa-admin-editor

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
chittyfinance 3566bd0 Apr 10 2026, 10:51 PM

@chitcommit chitcommit merged commit 2ca8e18 into main Apr 10, 2026
12 of 13 checks passed
@chitcommit chitcommit deleted the feat/coa-admin-editor branch April 10, 2026 22:51
@claude
Copy link
Copy Markdown

claude bot commented Apr 10, 2026

Code Review — feat: admin Chart of Accounts editor (L4)

Overall the feature is well-structured and the client-side validation logic is clear. A few issues worth addressing before merge:


Bugs / Correctness

1. Dead editing state — feature is half-implemented
client/src/pages/CoaAdmin.tsx declares editing and setEditing but they're never used:

const [editing, setEditing] = useState<ChartOfAccount | null>(null);

The table has no Edit button wired to it. Either add inline editing or remove this state. As-is it implies an incomplete feature and will produce a TypeScript unused-variable warning.

2. maxLength={10} on the Code input should be maxLength={4}
The validateNewAccountCode function rejects anything that isn't exactly 4 digits, but the input accepts up to 10 characters. Tighten it:

<input maxLength={4} ... />

3. Expense range 5000-5499 swallows all sub-ranges — labels are misleading
CODE_RANGES.expense has both { min: 5000, max: 5499, label: 'Property Operating' } and then { min: 5100, max: 5199, label: 'Utilities' }, 5200-5299, 5300-5399, 5400-5499. Code 5150 passes validation (falls in the parent range first), but the error message for an out-of-range code will list every range including the duplicates, making it confusing. The parent range is probably meant to be 5000-5099. Worth verifying against the seed data in #86.

4. updateAccount toggle button has no disabled/loading state
Rapid double-clicks on Deactivate/Activate will fire multiple PATCH requests. Add disabled={updateAccount.isPending}:

<button
  onClick={() => handleToggleActive(a)}
  disabled={updateAccount.isPending}
  ...
>

Design / Architecture

5. Server-side routes not in this PR
The PR description states POST/PATCH /api/coa* have requireL4() guards, but none of the 4 changed files are server-side. If those routes landed in a prior PR, please link it. If they don't exist yet, the frontend will 404 silently and the L4 authorization claim in the description is untested by this change.

6. useUpdateCoaAccount sends Partial<ChartOfAccount> — overly broad

mutationFn: ({ id, ...data }: { id: string } & Partial<ChartOfAccount>) =>
  apiRequest('PATCH', `/api/coa/${id}`, data)...

Partial<ChartOfAccount> includes read-only fields like tenantId, createdAt, classifiedAt. Define a narrow UpdateCoaPayload type (e.g., { isActive?: boolean; name?: string; ... }) to avoid sending unintended fields to the server and to make the mutation's contract explicit.

7. Not using shadcn/ui components (CLAUDE.md convention)
CoaAdmin.tsx uses raw <input>, <select>, <button>, and <table> elements. The CLAUDE.md says "Use shadcn/ui components for consistency (@/components/ui/*)". The Classification page (the most similar existing page) uses Input, Select, Button, and Table from shadcn. This page should do the same to stay consistent and pick up the design system's focus/error/disabled handling for free.


Minor

8. err: any in mutation callbacks
Both onError handlers use err: any. Prefer:

onError: (err: unknown) => {
  const msg = err instanceof Error ? err.message : 'Create failed';
  ...
}

9. lastResult never clears
Once set, the status banner persists until another action fires. A simple setTimeout(() => setLastResult(null), 5000) in both onSuccess and onError handlers, or a dismiss button, would improve UX.

10. useChartOfAccounts query key mismatch
The existing useChartOfAccounts hook (not changed here) presumably registers a query key. The new useCreateCoaAccount and useUpdateCoaAccount invalidate ['/api/coa', tenantId]. Confirm that key matches what useChartOfAccounts uses — if that hook uses ['/api/coa'] without tenantId, the invalidation won't trigger a refetch.


Summary

Severity Count
Bug / correctness 4
Architecture 3
Minor / style 3

The validation logic itself (validateNewAccountCode) is solid and well-commented. Main asks before merge: confirm the server routes exist (or add them), remove/wire the dead editing state, fix the code input maxLength, and narrow the useUpdateCoaAccount payload type.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3566bd0050

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

return;
}
updateAccount.mutate(
{ id: account.id, isActive: !account.isActive },
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Patch only fields accepted by the COA update API

handleToggleActive sends { id, isActive }, but the PATCH /api/coa/:id request body is validated with coaUpdateSchema = coaCreateSchema.partial() in server/routes/classification.ts, which does not include isActive. In Zod, unknown keys are stripped by default, so the server receives no activation change and the toggle silently does nothing for every tenant account even though the UI reports success.

Useful? React with 👍 / 👎.

Comment on lines +75 to +79
const duplicate = existing.find((a) => a.code === code);
if (duplicate) {
const scope = duplicate.tenantId ? 'this tenant' : 'global defaults';
return { valid: false, reason: `Code ${code} already exists in ${scope} (${duplicate.name})` };
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Allow tenant overrides of global account codes

This duplicate check rejects any code already present in existing, including global defaults, so the create form blocks tenant-specific overrides before the request is sent. That conflicts with the backend behavior (POST /api/coa only rejects duplicates when the same tenant already owns the code) and with override resolution logic that prefers tenant accounts over globals, so users cannot create intended tenant overrides for existing global codes from this page.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new client-side admin experience for managing tenant-specific Chart of Accounts (COA) entries, integrating with the existing /api/coa endpoints and exposing the feature through the main app navigation.

Changes:

  • Introduces a new /coa page with COA listing, filtering/search, and a create + activate/deactivate workflow for tenant-owned accounts.
  • Adds React Query mutation hooks for creating and updating COA accounts.
  • Wires the page into routing and adds a Sidebar navigation entry.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
client/src/pages/CoaAdmin.tsx New COA admin page UI with validation, create form, and tenant-only activation toggles.
client/src/hooks/use-classification.ts Adds useCreateCoaAccount and useUpdateCoaAccount hooks and query invalidation for COA refresh.
client/src/components/layout/Sidebar.tsx Adds “Chart of Accounts” nav item pointing to /coa.
client/src/App.tsx Registers the /coa route to render the new COA admin page.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +75 to +79
const duplicate = existing.find((a) => a.code === code);
if (duplicate) {
const scope = duplicate.tenantId ? 'this tenant' : 'global defaults';
return { valid: false, reason: `Code ${code} already exists in ${scope} (${duplicate.name})` };
}
Comment on lines +53 to +54
* (the DB permits tenant overrides of globals, but we warn to prevent
* accidental shadowing; the caller can ignore the warning)
Comment on lines +34 to +43
expense: [
{ min: 5000, max: 5499, label: 'Property Operating' },
{ min: 5100, max: 5199, label: 'Utilities' },
{ min: 5200, max: 5299, label: 'HOA & Association' },
{ min: 5300, max: 5399, label: 'Financial' },
{ min: 5400, max: 5499, label: 'Depreciation' },
{ min: 6000, max: 6099, label: 'Administrative' },
{ min: 7000, max: 7099, label: 'Capital Improvements' },
{ min: 9000, max: 9999, label: 'Suspense / Non-Deductible' },
],

function handleToggleActive(account: ChartOfAccount) {
// Only tenant-owned accounts can be deactivated; global defaults are read-only
if (!account.tenantId) {
const [scopeFilter, setScopeFilter] = useState<ScopeFilter>('all');
const [typeFilter, setTypeFilter] = useState<TypeFilter>('all');
const [showCreate, setShowCreate] = useState(false);
const [editing, setEditing] = useState<ChartOfAccount | null>(null);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants