design

src/features/store-admin/BranchScope.tsx

@@ -1,4 +1,11 @@
-import { createContext, useContext, useMemo, type ReactNode } from 'react';
+import {
+  createContext,
+  useContext,
+  useEffect,
+  useMemo,
+  useState,
+  type ReactNode,
+} from 'react';
 import { useSearchParams } from 'react-router-dom';
 import { useAuth } from '@/auth/AuthContext';
 import { useTenantLocations } from '@/queries/hooks';
@@ -32,16 +39,18 @@ export interface BranchScopeValue {
 
 const BranchScopeContext = createContext<BranchScopeValue | null>(null);
 
-/** The URL param. In the URL so a link to a page carries its branch with it. */
+/** The URL param. Mirrors the selection so a link carries its branch with it. */
 const PARAM = 'branch';
 
 /**
  * Which branch the Store Admin is looking at.
  *
- * Held in the URL rather than in component state for two reasons. A link to
- * "Inventory, Peelamedu" has to survive being pasted into a chat, and a reload
- * during a shift must not silently drop the operator back to All branches while
- * they are reading a number that only makes sense for one shop.
+ * Held in component state, mirrored to the URL. The mirror is what lets a link
+ * to "Inventory, Peelamedu" survive being pasted into a chat; holding the state
+ * here rather than reading it back out of the address bar is what stops a nav
+ * click — which replaces the query string — from resetting the operator to All
+ * branches mid-shift while they read a number that only means anything for one
+ * shop. See the long note on `chosen` below.
  *
  * The tenant, by contrast, comes from the session and is deliberately NOT in
  * the URL. Fiesta has no web auth, so tenant scoping is enforced by this client
@@ -83,15 +92,68 @@ export function BranchScopeProvider({
   );
 
   const raw = params.get(PARAM);
-  const parsed = raw === null || raw === 'all' ? null : Number(raw);
-  // An id in the URL that this tenant does not own falls back to All rather
-  // than showing an empty page — the id is user-editable, so it is untrusted.
-  const selected =
-    pin !== undefined
-      ? pin
-      : parsed !== null && Number.isFinite(parsed) && branches.some((b) => b.locationid === parsed)
-        ? parsed
-        : null;
+
+  /*
+  The selection lives here, and the URL only mirrors it.
+
+  ── The bug this fixes ──────────────────────────────────────────────────────
+
+  It used to be derived straight from the search param, with an absent param
+  meaning All branches. That is wrong, because absent does not mean "show me
+  everything" — it mostly means "you just clicked a nav tab". Every link in
+  `AppShell` is a bare path (`to="/admin/sales"`), so React Router replaces the
+  whole location, query string included, and the param is simply gone. The
+  branch filter therefore reset to All on every navigation: pick a shop on
+  Console, click Sales, and you were back to all six with nothing saying so.
+  Reproduced on tenant 1087 (Ragul Stores, 6 branches) — the label went from
+  "Ragul stores Selvapuram" back to "All branches (6)".
+
+  Copying the whole search string onto the nav links would have fixed it and
+  broken something else: `InventoryPage` and the global catalogue keep their own
+  params, and those would then follow the operator from page to page.
+
+  ── The rule ────────────────────────────────────────────────────────────────
+
+  A param that is PRESENT is obeyed, so a link to "Inventory, Peelamedu" still
+  survives being pasted into a chat, and editing the id in the address bar still
+  works. A param that is ABSENT changes nothing, so navigation cannot silently
+  widen the operator's scope. The effect below then writes the param back, which
+  is what keeps the URL honest after a nav click.
+  */
+  const [chosen, setChosen] = useState<BranchSelection>(null);
+
+  useEffect(() => {
+    if (pin !== undefined || raw === null) return;
+    if (raw === 'all') {
+      setChosen(null);
+      return;
+    }
+    const parsed = Number(raw);
+    if (Number.isFinite(parsed) && branches.some((b) => b.locationid === parsed)) {
+      setChosen(parsed);
+    } else if (!isLoading) {
+      // An id this tenant does not own falls back to All rather than showing an
+      // empty page — the id is user-editable, so it is untrusted. Guarded on
+      // `isLoading` because `branches` is empty until the fetch lands, and
+      // resetting then would throw away a perfectly good deep link.
+      setChosen(null);
+    }
+  }, [raw, branches, isLoading, pin]);
+
+  const selected = pin !== undefined ? pin : chosen;
+
+  // The URL follows the selection, including putting the param back after a nav
+  // click has dropped it. Built from the current params so a page's own query
+  // state is carried through untouched.
+  useEffect(() => {
+    if (pin !== undefined) return;
+    const want = selected === null ? null : String(selected);
+    if ((params.get(PARAM) ?? null) === want) return;
+    const next = new URLSearchParams(params);
+    if (want === null) next.delete(PARAM);
+    else next.set(PARAM, want);
+    setParams(next, { replace: true });
+  }, [selected, params, setParams, pin]);
 
   const value = useMemo<BranchScopeValue>(() => {
     const current = selected === null ? undefined : branches.find((b) => b.locationid === selected);
@@ -103,15 +165,14 @@ export function BranchScopeProvider({
       current,
       isPinned: pin !== undefined,
       scoped: current ? [current] : branches,
+      // State only. The URL is updated by the mirroring effect above, so there
+      // is one place that writes the param rather than two that can disagree.
       select: (next) => {
         if (pin !== undefined) return;
-        const nextParams = new URLSearchParams(params);
-        if (next === null) nextParams.delete(PARAM);
-        else nextParams.set(PARAM, String(next));
-        setParams(nextParams, { replace: true });
+        setChosen(next);
       },
     };
-  }, [branches, isLoading, tenantid, selected, params, setParams, pin]);
+  }, [branches, isLoading, tenantid, selected, pin]);
 
   return <BranchScopeContext.Provider value={value}>{children}</BranchScopeContext.Provider>;
 }