status fix
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
import { useEffect, useState } from 'react';
|
||||
import { useEffect, useRef, useState } from 'react';
|
||||
import { useMutation, useQueryClient } from '@tanstack/react-query';
|
||||
import { Switch } from '@astryxdesign/core/Switch';
|
||||
import { Button } from '@astryxdesign/core/Button';
|
||||
@@ -56,17 +56,35 @@ export function StoreOpenControl({
|
||||
const [until, setUntil] = useState('');
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
|
||||
// Re-seed if the page refetches with a different answer, but never while the
|
||||
// confirmation is up — that would move the switch under the shopkeeper's hand.
|
||||
/*
|
||||
Re-seed only when the PROP ITSELF changes.
|
||||
|
||||
This used to run whenever `confirming` flipped, and that undid its own
|
||||
save: closing the branch set open to false and confirming to false, the
|
||||
change of `confirming` re-ran the effect, and it wrote the prop — still
|
||||
true, because the page behind had not refetched yet — straight back over
|
||||
the answer. The switch snapped open the instant the shop was closed.
|
||||
|
||||
Comparing against the last value seen means a genuinely new answer from the
|
||||
server still lands, and nothing else can reach in and overwrite what the
|
||||
shopkeeper just did.
|
||||
*/
|
||||
const lastSeen = useRef(isOpen);
|
||||
useEffect(() => {
|
||||
if (!confirming) setOpen(isOpen);
|
||||
}, [isOpen, confirming]);
|
||||
if (lastSeen.current !== isOpen) {
|
||||
lastSeen.current = isOpen;
|
||||
setOpen(isOpen);
|
||||
}
|
||||
}, [isOpen]);
|
||||
|
||||
const save = useMutation({
|
||||
mutationFn: (next: { isopen: boolean; closeduntil?: string }) =>
|
||||
tenantsApi.setStoreOpen({ tenantid, locationid, ...next }),
|
||||
onMutate: () => setError(null),
|
||||
onSuccess: (_data, next) => {
|
||||
// Move the reference too. Otherwise the next render carrying the old
|
||||
// prop reads as a change and reverts what was just saved.
|
||||
lastSeen.current = next.isopen;
|
||||
setOpen(next.isopen);
|
||||
setConfirming(false);
|
||||
setUntil('');
|
||||
@@ -75,6 +93,7 @@ export function StoreOpenControl({
|
||||
// A close that fails silently is the worst outcome here: the shopkeeper
|
||||
// walks away believing the shop is shut while it keeps taking orders.
|
||||
onError: (cause) => {
|
||||
lastSeen.current = isOpen;
|
||||
setOpen(isOpen);
|
||||
setError(errorMessage(cause));
|
||||
},
|
||||
|
||||
82
src/features/store-admin/storeOpenControl.test.ts
Normal file
82
src/features/store-admin/storeOpenControl.test.ts
Normal file
@@ -0,0 +1,82 @@
|
||||
/**
|
||||
* The switch must not undo its own save.
|
||||
*
|
||||
* It shipped doing exactly that. Closing a branch set the local flag false and
|
||||
* dismissed the confirmation, the change of `confirming` re-ran the seeding
|
||||
* effect, and the effect wrote the prop — still `true`, because the page behind
|
||||
* had not refetched — back over the answer. The shop closed and the switch
|
||||
* snapped open, with nothing on screen to say the save had worked.
|
||||
*
|
||||
* A source-text guard rather than a render test: the defect was entirely in
|
||||
* which values the effect watched, which is readable without a DOM, and this
|
||||
* fails the moment somebody puts `confirming` back in the list.
|
||||
*/
|
||||
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { describe, it } from 'node:test';
|
||||
|
||||
const SOURCE = readFileSync(
|
||||
join(process.cwd(), 'src/features/store-admin/StoreOpenControl.tsx'),
|
||||
'utf8',
|
||||
);
|
||||
|
||||
/** The dependency list of each useEffect in the file. */
|
||||
function effectDeps(source: string): string[] {
|
||||
return [...source.matchAll(/useEffect\([\s\S]*?\}, \[([^\]]*)\]\)/g)].map((m) =>
|
||||
(m[1] ?? '').trim(),
|
||||
);
|
||||
}
|
||||
|
||||
describe('StoreOpenControl seeding effect', () => {
|
||||
it('does not re-run when the confirmation dialog opens or closes', () => {
|
||||
for (const deps of effectDeps(SOURCE)) {
|
||||
assert.ok(
|
||||
!/\bconfirming\b/.test(deps),
|
||||
`an effect watches "confirming" (deps: [${deps}]). Dismissing the ` +
|
||||
'confirmation then re-seeds from the prop, which still holds the old ' +
|
||||
'value, and the save is undone on screen.',
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it('seeds from the prop only when the prop itself has changed', () => {
|
||||
// A bare `setOpen(isOpen)` in an effect overwrites local state on every
|
||||
// render that touches the dependencies. The guard is the comparison.
|
||||
assert.match(
|
||||
SOURCE,
|
||||
/lastSeen\.current !== isOpen/,
|
||||
'the seeding effect no longer compares against the last value seen, so ' +
|
||||
'it can overwrite a save that has not reached the prop yet',
|
||||
);
|
||||
});
|
||||
|
||||
it('moves the reference when a save succeeds', () => {
|
||||
// Without this the next render carrying the old prop reads as a change,
|
||||
// and reverts what was just written.
|
||||
assert.match(
|
||||
SOURCE,
|
||||
/onSuccess:[\s\S]{0,400}lastSeen\.current = next\.isopen/,
|
||||
'onSuccess does not update lastSeen, so a stale prop will look like a ' +
|
||||
'new answer and revert the switch',
|
||||
);
|
||||
});
|
||||
|
||||
it('surfaces a failed save', () => {
|
||||
// A close that fails silently is the worst outcome: the shopkeeper walks
|
||||
// away believing the shop is shut while it keeps taking orders.
|
||||
assert.match(SOURCE, /onError:/, 'the mutation has no onError');
|
||||
assert.match(SOURCE, /setError\(errorMessage\(cause\)\)/, 'the error is never shown');
|
||||
});
|
||||
|
||||
it('asks before closing, and does not ask before reopening', () => {
|
||||
// Closing stops the shop taking money; reopening is harmless.
|
||||
assert.match(
|
||||
SOURCE,
|
||||
/save\.mutate\(\{ isopen: true \}\)/,
|
||||
'reopening should happen on the tap, not behind a confirmation',
|
||||
);
|
||||
assert.match(SOURCE, /setConfirming\(true\)/, 'closing should ask first');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user