From 2c351bf0c936a66b5d13ff1609426fee61f3ea74 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Thu, 11 Jun 2026 00:17:44 +0200 Subject: [PATCH] refactor(accounting): make Accounting a master flag with sub-toggles Replaces the earlier peer-`accounting` flag (which only *conditionally* relocated Tax) with a cleaner top-level master + sub-toggle model, per design discussion: - `accounting` = explicit top-level MASTER (Settings -> Features). Off hides the whole Accounting section. - Sub-toggles, gated under the master: - `taxReport` ("Tax export") moves PERMANENTLY out of CRM. Removed from the Clients sub-nav and from the derived `clients` flag. Now INDEPENDENT of Bills (per decision). Old /admin/clients/tax-report -> redirect to /admin/accounting/tax-report. - `incomingInvoices` (new) gates the supplier-invoice capture / expenses / re-bill feature; the /api/admin/expenses router now checks it. - Dependency rules (backend + frontend): accounting off forces taxReport + incomingInvoices off; taxReport dropped from the clients derivation; the bills->taxReport rule removed. - Preserve visuals: migration 122 rewritten to auto-enable `accounting` on installs that already had Tax on (so the tab doesn't vanish), and to seed `incomingInvoices` off. Verified with a SQLite harness (taxReport on -> accounting on; off -> off). - Settings -> Features: new "Accounting" section with the master card + Tax export + Incoming invoices sub-cards (disabled until the master is on). - i18n: navigation.accounting, accounting.*, settings.features.{accounting, incomingInvoices,taxReport.requiresAccounting}, sections.accounting (EN + DE, DE authored natively); Tax report relabelled "Tax export"/"Steuerexport". Verified: node -c, migration-122 harness, en/de JSON valid, npm run build green. --- .../core/122_seed_accounting_feature_flag.js | 39 +++++--- backend/src/routes/adminExpenses.js | 11 ++- backend/src/routes/adminFeatureFlags.js | 19 +++- frontend/src/App.tsx | 8 +- .../src/components/admin/ClientsLayout.tsx | 19 +--- frontend/src/contexts/FeatureFlagsContext.tsx | 18 +++- .../features/settings/tabs/FeaturesTab.tsx | 91 ++++++++++++------- frontend/src/i18n/locales/de.json | 14 ++- frontend/src/i18n/locales/en.json | 14 ++- frontend/src/services/featureFlags.service.ts | 12 ++- 10 files changed, 157 insertions(+), 88 deletions(-) diff --git a/backend/migrations/core/122_seed_accounting_feature_flag.js b/backend/migrations/core/122_seed_accounting_feature_flag.js index ce416999..94ed6de6 100644 --- a/backend/migrations/core/122_seed_accounting_feature_flag.js +++ b/backend/migrations/core/122_seed_accounting_feature_flag.js @@ -1,22 +1,39 @@ /** - * Migration 122: seed the `accounting` feature flag (default OFF). + * Migration 122: seed the Accounting feature flags. * - * Gates the new top-level Accounting area (inbound supplier invoices, - * billable / re-billable expenses, Erfolgsrechnung). Admins opt in under - * Settings → Features. Separate from the CRM `bills` flag. + * - `accounting` : top-level MASTER for the Accounting section + * (separate from CRM). Default OFF — EXCEPT on + * installs that already had the Tax report + * (`taxReport`) enabled: the Tax export relocated + * permanently into Accounting, so we auto-enable the + * master there to preserve the existing menu (per the + * "migrations preserve visual state" rule). Otherwise + * admins opt in under Settings → Features. + * - `incomingInvoices` : Accounting sub-feature (supplier-invoice capture + + * expenses + re-bill). Always default OFF (new). * - * Idempotent: inserts only when the row is missing (mirrors the - * 095_add_customer_portal_flag pattern). 107_crm_consolidated already - * shipped its flag set on fresh installs and won't re-run. + * Idempotent: each row is inserted only when missing. 107_crm_consolidated + * already shipped its flag set and won't re-run. */ exports.up = async function (knex) { if (!(await knex.schema.hasTable('feature_flags'))) return; - const existing = await knex('feature_flags').where({ key: 'accounting' }).first(); - if (existing) return; - await knex('feature_flags').insert({ key: 'accounting', value: false }); + + const existingAccounting = await knex('feature_flags').where({ key: 'accounting' }).first(); + if (!existingAccounting) { + // Preserve visuals: if Tax was already on, light up the Accounting + // master so the relocated Tax export doesn't vanish on upgrade. + const taxRow = await knex('feature_flags').where({ key: 'taxReport' }).first(); + const taxOn = !!(taxRow && (taxRow.value === true || taxRow.value === 1 || taxRow.value === '1')); + await knex('feature_flags').insert({ key: 'accounting', value: taxOn }); + } + + const existingIncoming = await knex('feature_flags').where({ key: 'incomingInvoices' }).first(); + if (!existingIncoming) { + await knex('feature_flags').insert({ key: 'incomingInvoices', value: false }); + } }; exports.down = async function (knex) { if (!(await knex.schema.hasTable('feature_flags'))) return; - await knex('feature_flags').where({ key: 'accounting' }).del(); + await knex('feature_flags').whereIn('key', ['accounting', 'incomingInvoices']).del(); }; diff --git a/backend/src/routes/adminExpenses.js b/backend/src/routes/adminExpenses.js index 3f718e11..04f806af 100644 --- a/backend/src/routes/adminExpenses.js +++ b/backend/src/routes/adminExpenses.js @@ -43,17 +43,20 @@ const inboundUpload = multer({ }, }); -async function requireAccountingFlag(req, res, next) { +// Gated on the `incomingInvoices` sub-feature, which the feature-flag +// dependency rules force OFF whenever the `accounting` master is off — so +// this single check covers both. +async function requireIncomingInvoicesFlag(req, res, next) { try { - const row = await db('feature_flags').where({ key: 'accounting' }).first(); + const row = await db('feature_flags').where({ key: 'incomingInvoices' }).first(); const enabled = row && (row.value === true || row.value === 1 || row.value === '1'); - if (!enabled) return res.status(403).json({ error: 'Accounting feature is disabled', code: 'ACCOUNTING_DISABLED' }); + if (!enabled) return res.status(403).json({ error: 'Incoming invoices feature is disabled', code: 'INCOMING_INVOICES_DISABLED' }); return next(); } catch (err) { return next(err); } } router.use(adminAuth); -router.use(requireAccountingFlag); +router.use(requireIncomingInvoicesFlag); // ── Expense categories (literal path — register BEFORE '/:id') ────────────── router.get('/categories', requirePermission('accounting.view'), handleAsync(async (_req, res) => { diff --git a/backend/src/routes/adminFeatureFlags.js b/backend/src/routes/adminFeatureFlags.js index 81063cce..a17c4183 100644 --- a/backend/src/routes/adminFeatureFlags.js +++ b/backend/src/routes/adminFeatureFlags.js @@ -66,6 +66,10 @@ const KNOWN_FLAGS = [ // supplier invoices, expenses + re-bill, and the tax report (which // relocates here from CRM when this is on). Strictly opt-in. 'accounting', + // Incoming invoices (migration 124) — supplier-invoice capture + + // expenses + re-bill. Accounting sub-feature; forced off when the + // `accounting` master is off. + 'incomingInvoices', ]; // Spec defaults for any flag missing from the DB (e.g. a row added by a @@ -89,6 +93,7 @@ const DEFAULT_FLAGS = { hoursLogging: false, contracts: false, accounting: false, + incomingInvoices: false, }; async function readAllFlags() { @@ -109,10 +114,13 @@ function applyDependencyRules(flags) { // Sub-features can't outlive their parents. if (out.quotes === false) out.bills = false; if (out.calendar === false) out.calendarBooking = false; - // Tax report only makes sense when bills are on — turning bills off - // implicitly turns the tax report off too. Admins enabling tax - // report must first enable bills. - if (out.bills === false) out.taxReport = false; + // Accounting is a top-level MASTER; its sub-features can't outlive it. + // Tax export is now independent of Bills — it relocated permanently + // into the Accounting section (its own master gate). + if (out.accounting === false) { + out.taxReport = false; + out.incomingInvoices = false; + } // Clients parent flag is DERIVED from its children. Admins don't // toggle it directly in the Features tab — they enable a specific // sub-feature (Accounts today; Calendar/Quotes/Bills/Messaging @@ -125,9 +133,10 @@ function applyDependencyRules(flags) { || out.crmDevelopment || out.quotes || out.bills - || out.taxReport || out.hoursLogging || out.contracts + // NOTE: taxReport intentionally removed — Tax export moved to the + // Accounting section (its own master), no longer a CRM sub-feature. // Migration 137 — admin calendar lights up the Clients section. // (calendarBooking is gated behind `calendar` so adding the parent // is sufficient.) diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index e78f7e4c..a15515c2 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -244,10 +244,10 @@ function App() { /> - {/* Tax / Steuer report — gated by `taxReport`. */} - }> - } /> - + {/* Tax export moved permanently to the Accounting + section. Keep this path as a redirect so old + bookmarks / links don't 404. */} + } /> {/* Developer tools — gated by `crmDevelopment`. */} }> } /> diff --git a/frontend/src/components/admin/ClientsLayout.tsx b/frontend/src/components/admin/ClientsLayout.tsx index be53b2e4..2aca0ec5 100644 --- a/frontend/src/components/admin/ClientsLayout.tsx +++ b/frontend/src/components/admin/ClientsLayout.tsx @@ -14,7 +14,7 @@ import React from 'react'; import { NavLink, Outlet, useLocation, useNavigate } from 'react-router-dom'; import { useTranslation } from 'react-i18next'; -import { Briefcase, UserCog, FileText, Receipt, Wrench, Calculator, Clock, ScrollText, Calendar } from 'lucide-react'; +import { Briefcase, UserCog, FileText, Receipt, Wrench, Clock, ScrollText, Calendar } from 'lucide-react'; import type { LucideIcon } from 'lucide-react'; import { useFeatureFlags, type FeatureKey } from '../../contexts/FeatureFlagsContext'; @@ -81,13 +81,8 @@ export const ClientsLayout: React.FC = () => { icon: Receipt, featureFlag: 'bills', }, - { - key: 'tax-report', - to: '/admin/clients/tax-report', - label: t('clients.subnav.taxReport', 'Tax'), - icon: Calculator, - featureFlag: 'taxReport', - }, + // Tax export moved permanently to the Accounting section (it is no + // longer a CRM sub-feature). See AccountingLayout. // Future sub-features: // { key: 'messaging', ... featureFlag: 'messaging' } { @@ -99,13 +94,7 @@ export const ClientsLayout: React.FC = () => { }, ]; - const enabledItems = navItems.filter((item) => { - if (!flags[item.featureFlag]) return false; - // When the Accounting area is enabled, the tax report relocates out - // of CRM and under Accounting — hide it here so it isn't in both. - if (item.key === 'tax-report' && flags.accounting) return false; - return true; - }); + const enabledItems = navItems.filter((item) => flags[item.featureFlag]); // When the parent `clients` flag is on but no sub-feature is enabled, // there's nothing to render. Settings → Features is one click away diff --git a/frontend/src/contexts/FeatureFlagsContext.tsx b/frontend/src/contexts/FeatureFlagsContext.tsx index 4b80eb53..2899b543 100644 --- a/frontend/src/contexts/FeatureFlagsContext.tsx +++ b/frontend/src/contexts/FeatureFlagsContext.tsx @@ -45,10 +45,12 @@ export const DEFAULT_FLAGS: FeatureFlags = { // Settings → Features once they've reviewed the seeded block // library with their lawyer. contracts: false, - // Accounting (migration 122). Top-level Accounting area (inbound - // supplier invoices, expenses + re-bill). When ON, the tax report - // moves out of the CRM sub-nav and under Accounting. Strictly opt-in. + // Accounting (migration 122). Top-level MASTER for the Accounting + // section (separate from CRM). Sub-features below require it. accounting: false, + // Incoming invoices (migration 124) — supplier-invoice capture + + // expenses + re-bill. Accounting sub-feature; requires `accounting`. + incomingInvoices: false, }; export const FEATURE_FLAGS_QUERY_KEY = ['feature-flags'] as const; @@ -80,7 +82,12 @@ function applyDependencyRules(flags: FeatureFlags): FeatureFlags { out.galleries = true; // foundation — always on if (out.quotes === false) out.bills = false; // bills depend on quotes if (out.calendar === false) out.calendarBooking = false; // booking depends on calendar - if (out.bills === false) out.taxReport = false; // tax report depends on bills + // Accounting sub-features require the Accounting master. Tax export is + // independent of Bills now — it relocated permanently into Accounting. + if (out.accounting === false) { + out.taxReport = false; + out.incomingInvoices = false; + } // Clients parent flag is DERIVED from its children. Admins don't // toggle it directly — enabling any CRM-area sub-feature // (Accounts today; future Calendar / Quotes / Bills / Messaging) @@ -91,11 +98,12 @@ function applyDependencyRules(flags: FeatureFlags): FeatureFlags { || out.crmDevelopment || out.quotes || out.bills - || out.taxReport || out.hoursLogging || out.contracts // Migration 137 — admin calendar lights up the Clients section. || out.calendar + // NOTE: taxReport is intentionally NOT here anymore — the Tax export + // moved permanently into the Accounting section (its own master). // future siblings: || out.messaging ); return out; diff --git a/frontend/src/features/settings/tabs/FeaturesTab.tsx b/frontend/src/features/settings/tabs/FeaturesTab.tsx index 6563af5c..d7ef030b 100644 --- a/frontend/src/features/settings/tabs/FeaturesTab.tsx +++ b/frontend/src/features/settings/tabs/FeaturesTab.tsx @@ -17,6 +17,7 @@ import { Wrench, Calculator, Landmark, + ScanLine, } from 'lucide-react'; import { useTranslation } from 'react-i18next'; import { Button, Card } from '../../../components/common'; @@ -260,39 +261,6 @@ export const FeaturesTab: React.FC = () => { onToggle={(next) => setFlag('bills', next)} /> - setFlag('taxReport', next)} - disabled={!staged.bills} - lockedReason={!staged.bills ? t( - 'settings.features.taxReport.requiresBills', - 'Enable Bills first — the tax report reads from your invoices.', - ) : undefined} - /> - - setFlag('accounting', next)} - /> - { /> + {/* Accounting — top-level master + sub-toggles. The Tax export + relocated here permanently out of CRM. Sub-toggles are disabled + until the Accounting master is on. */} +
+ setFlag('accounting', next)} + /> + + setFlag('taxReport', next)} + disabled={!staged.accounting} + lockedReason={!staged.accounting ? t( + 'settings.features.taxReport.requiresAccounting', + 'Enable Accounting first — Tax export lives in the Accounting section.', + ) : undefined} + /> + + setFlag('incomingInvoices', next)} + disabled={!staged.accounting} + lockedReason={!staged.accounting ? t( + 'settings.features.incomingInvoices.requiresAccounting', + 'Enable Accounting first — Incoming invoices live in the Accounting section.', + ) : undefined} + /> +
+ {/* Insights & Access */}
;