From 3e16b81be801513704f07592a67e959f30a8ee0a Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Tue, 1 Sep 2026 16:29:33 +0200 Subject: [PATCH] fix(settings): clear the accounting flag when its parent is turned off Turning Invoices off left the Accounting master flag -- and its sidebar entry -- silently on and freshly unlocked, because the bills=true => accounting=true force-enable had no reverse. A dependency model already exists and handles every true parent->child pair (quotes->bills, calendar->calendarBooking, accounting->{taxReport, incomingInvoices,expenses}), mirrored client-side in applyDependencyRules and server-side in adminFeatureFlags.js. The gap is only this asymmetric rule. Interpretation, two decisions: - Cascade on the client at toggle time, not on the server at persist time. applyDependencyRules is a pure invariant over a single state (the GET handler runs it too), so it structurally cannot distinguish "accounting is on because the admin wants it" from "...because bills forced it". The Features tab PUTs the full flag set, so on the wire an explicit true and a stale forced true are byte-identical -- a server-side transition rule would silently discard an admin who turns Invoices off and deliberately keeps Accounting on in the same save. The client is where the gesture is known. The persisted result is still server-enforced: the client sends accounting:false and the existing server invariant forces the sub-flags off. - Re-enabling the parent does NOT restore children. Flags are state, not history, and silently re-lighting a sub-feature with its routes and sidebar entries is the exact failure this bug is about. Refs testplan REPORT.md #8 (Part 8, S9). --- frontend/src/contexts/FeatureFlagsContext.tsx | 14 +++- .../featureFlagsAccountingCascade.test.tsx | 78 +++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 frontend/src/contexts/__tests__/featureFlagsAccountingCascade.test.tsx diff --git a/frontend/src/contexts/FeatureFlagsContext.tsx b/frontend/src/contexts/FeatureFlagsContext.tsx index 80a53af2..4283f970 100644 --- a/frontend/src/contexts/FeatureFlagsContext.tsx +++ b/frontend/src/contexts/FeatureFlagsContext.tsx @@ -195,7 +195,19 @@ export const FeatureFlagsProvider: React.FC = ({ children }) => { const setFlag = useCallback((key: FeatureKey, value: boolean) => { if (key === 'galleries') return; // locked - setStaged((prev) => applyDependencyRules({ ...prev, [key]: value })); + setStaged((prev) => { + const next = { ...prev, [key]: value }; + // Reverse the bills→accounting force-enable. applyDependencyRules is a + // pure invariant over one state — it can't tell "Accounting is on + // because the admin wants it" from "…because Invoices forced it on", so + // turning Invoices off used to leave the Accounting master (and its + // sidebar entry) silently on and freshly unlocked (QA S9). The + // reversal has to live here, at the toggle, where the transition is + // known; the admin sees the switch flip in the same staged state and + // can turn Accounting back on before saving if they want it standalone. + if (key === 'bills' && !value && prev.bills) next.accounting = false; + return applyDependencyRules(next); + }); }, []); const reset = useCallback(() => { diff --git a/frontend/src/contexts/__tests__/featureFlagsAccountingCascade.test.tsx b/frontend/src/contexts/__tests__/featureFlagsAccountingCascade.test.tsx new file mode 100644 index 00000000..641f01c8 --- /dev/null +++ b/frontend/src/contexts/__tests__/featureFlagsAccountingCascade.test.tsx @@ -0,0 +1,78 @@ +/** + * Turning Invoices (`bills`) on force-enables the Accounting master, but + * turning it back off used to leave Accounting silently on and freshly + * unlocked — an orphaned parent area after a trial toggle (QA S9). + */ +import React from 'react'; +import { describe, it, expect, vi } from 'vitest'; +import { renderHook, act, waitFor } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; + +const serverFlags: Record = {}; +vi.mock('../../services/featureFlags.service', () => ({ + featureFlagsService: { + get: vi.fn(async () => ({ ...serverFlags })), + update: vi.fn(async (f: Record) => f), + }, +})); + +import { FeatureFlagsProvider, useFeatureFlags, DEFAULT_FLAGS } from '../FeatureFlagsContext'; + +function renderFlags(overrides: Record) { + Object.assign(serverFlags, DEFAULT_FLAGS, overrides); + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + return renderHook(() => useFeatureFlags(), { wrapper }); +} + +describe('feature-flag parent/child cascade (QA S9)', () => { + it('turns Accounting (and its sub-features) off when Invoices is turned off', async () => { + const { result } = renderFlags({ + quotes: true, bills: true, accounting: true, expenses: true, taxReport: true, + }); + await waitFor(() => expect(result.current.staged.bills).toBe(true)); + + act(() => result.current.setFlag('bills', false)); + + expect(result.current.staged.bills).toBe(false); + expect(result.current.staged.accounting).toBe(false); + expect(result.current.staged.expenses).toBe(false); + expect(result.current.staged.taxReport).toBe(false); + }); + + it('leaves a standalone Accounting alone when an unrelated flag is toggled', async () => { + const { result } = renderFlags({ accounting: true, expenses: true }); + await waitFor(() => expect(result.current.staged.accounting).toBe(true)); + + act(() => result.current.setFlag('workflows', true)); + + expect(result.current.staged.accounting).toBe(true); + expect(result.current.staged.expenses).toBe(true); + }); + + it('lets the admin keep Accounting standalone by re-enabling it before saving', async () => { + const { result } = renderFlags({ quotes: true, bills: true, accounting: true }); + await waitFor(() => expect(result.current.staged.bills).toBe(true)); + + act(() => result.current.setFlag('bills', false)); + act(() => result.current.setFlag('accounting', true)); + + expect(result.current.staged.bills).toBe(false); + expect(result.current.staged.accounting).toBe(true); + }); + + it('does not resurrect sub-features when Invoices is turned back on', async () => { + const { result } = renderFlags({ quotes: true, bills: true, accounting: true, expenses: true }); + await waitFor(() => expect(result.current.staged.expenses).toBe(true)); + + act(() => result.current.setFlag('bills', false)); + act(() => result.current.setFlag('bills', true)); + + expect(result.current.staged.accounting).toBe(true); // forced back on by bills + expect(result.current.staged.expenses).toBe(false); // stays off until re-enabled + }); +});