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).
This commit is contained in:
@@ -195,7 +195,19 @@ export const FeatureFlagsProvider: React.FC<ProviderProps> = ({ 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(() => {
|
||||
|
||||
@@ -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<string, boolean> = {};
|
||||
vi.mock('../../services/featureFlags.service', () => ({
|
||||
featureFlagsService: {
|
||||
get: vi.fn(async () => ({ ...serverFlags })),
|
||||
update: vi.fn(async (f: Record<string, boolean>) => f),
|
||||
},
|
||||
}));
|
||||
|
||||
import { FeatureFlagsProvider, useFeatureFlags, DEFAULT_FLAGS } from '../FeatureFlagsContext';
|
||||
|
||||
function renderFlags(overrides: Record<string, boolean>) {
|
||||
Object.assign(serverFlags, DEFAULT_FLAGS, overrides);
|
||||
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||
const wrapper = ({ children }: { children: React.ReactNode }) => (
|
||||
<QueryClientProvider client={qc}>
|
||||
<FeatureFlagsProvider>{children}</FeatureFlagsProvider>
|
||||
</QueryClientProvider>
|
||||
);
|
||||
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
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user