Explorar el Código

feat(nav): enhance branch selection logic for admin/dev users and improve route branch validation

Code_Uwe hace 1 semana
padre
commit
3bf0f6403b
Se han modificado 1 ficheros con 52 adiciones y 17 borrados
  1. 52 17
      components/app-shell/QuickNav.jsx

+ 52 - 17
components/app-shell/QuickNav.jsx

@@ -53,6 +53,8 @@ export default function QuickNav() {
 
 	const canRevalidate = typeof retry === "function";
 
+	// Persisted selection for admin/dev:
+	// - Used as the "safe fallback" when the route branch is invalid/non-existent.
 	const [selectedBranch, setSelectedBranch] = React.useState(null);
 
 	const [branchList, setBranchList] = React.useState({
@@ -67,27 +69,45 @@ export default function QuickNav() {
 	const isExplorerActive = activePrimaryNav?.active === PRIMARY_NAV.EXPLORER;
 	const isSearchActive = activePrimaryNav?.active === PRIMARY_NAV.SEARCH;
 
+	const routeBranch = React.useMemo(() => {
+		return readRouteBranchFromPathname(pathname);
+	}, [pathname]);
+
+	const knownBranches =
+		branchList.status === BRANCH_LIST_STATE.READY &&
+		Array.isArray(branchList.branches)
+			? branchList.branches
+			: null;
+
+	const isKnownRouteBranch = React.useMemo(() => {
+		if (!routeBranch) return false;
+		if (!knownBranches) return false; // do not "trust" route until we know the branch list
+		return knownBranches.includes(routeBranch);
+	}, [routeBranch, knownBranches]);
+
 	React.useEffect(() => {
 		if (!isAuthenticated) return;
 
+		// Branch users: fixed branch, no persisted selection needed.
 		if (isBranchUser) {
 			const own = user.branchId;
 			setSelectedBranch(own && isValidBranchParam(own) ? own : null);
 			return;
 		}
 
-		const fromRoute = readRouteBranchFromPathname(pathname);
+		// Admin/dev: initialize from localStorage only.
+		// IMPORTANT:
+		// We do NOT initialize from the route branch, because invalid-but-syntactically-valid
+		// branches (e.g. NL200) would pollute state and cause thrashing.
 		const fromStorage = safeReadLocalStorageBranch(STORAGE_KEY_LAST_BRANCH);
 
-		const initial = fromRoute || fromStorage || null;
-
-		if (initial && initial !== selectedBranch) {
-			setSelectedBranch(initial);
-			safeWriteLocalStorageBranch(STORAGE_KEY_LAST_BRANCH, initial);
+		if (fromStorage && fromStorage !== selectedBranch) {
+			setSelectedBranch(fromStorage);
 		}
-	}, [isAuthenticated, isBranchUser, user?.branchId, pathname, selectedBranch]);
+	}, [isAuthenticated, isBranchUser, user?.branchId, selectedBranch]);
 
 	React.useEffect(() => {
+		// Fetch the branch list once for admin/dev users (or when the user changes).
 		if (!isAdminDev) return;
 
 		let cancelled = false;
@@ -104,6 +124,7 @@ export default function QuickNav() {
 			} catch (err) {
 				if (cancelled) return;
 
+				// Fail open: do not block navigation if validation fails.
 				console.error("[QuickNav] getBranches failed:", err);
 				setBranchList({ status: BRANCH_LIST_STATE.ERROR, branches: null });
 			}
@@ -115,31 +136,46 @@ export default function QuickNav() {
 	}, [isAdminDev, user?.userId]);
 
 	React.useEffect(() => {
+		// Once we know the branch list, keep selectedBranch valid and stable.
 		if (!isAdminDev) return;
-		if (branchList.status !== BRANCH_LIST_STATE.READY) return;
-
-		const branches = Array.isArray(branchList.branches)
-			? branchList.branches
-			: [];
-		if (branches.length === 0) return;
+		if (!knownBranches || knownBranches.length === 0) return;
 
-		if (!selectedBranch || !branches.includes(selectedBranch)) {
-			const next = branches[0];
+		if (!selectedBranch || !knownBranches.includes(selectedBranch)) {
+			const next = knownBranches[0];
 			setSelectedBranch(next);
 			safeWriteLocalStorageBranch(STORAGE_KEY_LAST_BRANCH, next);
 		}
-	}, [isAdminDev, branchList.status, branchList.branches, selectedBranch]);
+	}, [isAdminDev, knownBranches, selectedBranch]);
+
+	React.useEffect(() => {
+		// Sync selectedBranch to the current route branch ONLY when that route branch is known to exist.
+		// This prevents the "NL200 thrash" while still keeping the dropdown in sync for valid routes.
+		if (!isAdminDev) return;
+		if (!isKnownRouteBranch) return;
+		if (!routeBranch) return;
+
+		if (routeBranch !== selectedBranch) {
+			setSelectedBranch(routeBranch);
+			safeWriteLocalStorageBranch(STORAGE_KEY_LAST_BRANCH, routeBranch);
+		}
+	}, [isAdminDev, isKnownRouteBranch, routeBranch, selectedBranch]);
 
 	React.useEffect(() => {
 		if (!isAdminDev) return;
 		if (!selectedBranch) return;
 
+		// Keep localStorage in sync (defense-in-depth).
 		safeWriteLocalStorageBranch(STORAGE_KEY_LAST_BRANCH, selectedBranch);
 	}, [isAdminDev, selectedBranch]);
 
 	if (!isAuthenticated) return null;
 
+	// Effective branch for navigation:
+	// - Branch users: always their own branch
+	// - Admin/dev: always the persisted/validated selection (NOT the route branch)
+	//   This guarantees that nav buttons still work even when the user is on /NL200.
 	const effectiveBranch = isBranchUser ? user.branchId : selectedBranch;
+
 	const canNavigate = Boolean(
 		effectiveBranch && isValidBranchParam(effectiveBranch),
 	);
@@ -163,7 +199,6 @@ export default function QuickNav() {
 
 		if (!nextUrl) return;
 
-		// Optional but desired (RHL-032):
 		// Trigger a session revalidation without causing content flicker.
 		if (canRevalidate) retry();