fix(site-scope): preserve ?site= across sidebar navigation
Bug: picking a site in the topbar pill updated both URL and cookie,
but the sidebar nav Links were plain paths ('/dashboard/sites',
'/dashboard/financial/purchase-orders', ...). Clicking any of them
stripped the '?site=' query string. The page's getSiteScope() then
fell back to the still-set cookie and filtered the data correctly,
but the pill's useSearchParams() and the layout's frozen fallbackValue
both produced mode='all' — so the topbar pill showed 'All sites · N'
while every table was filtered to the picked site. The visual pill
and the data disagreed on every screen the user clicked.
Fix: extract scopeAwareHref() and route every sidebar Link through
it. The helper carries the current '?site=' (and nothing else) into
the destination URL, so the URL stays the single source of truth
for both the page filter and the topbar pill across navigation.
Layouts do not re-render on navigation, so preserving the URL is
the only viable fix — making the layout re-run getSiteScope() on
every navigation is what the plan explicitly avoided by design.
Tests: 5 new unit tests for scopeAwareHref covering the bare-path,
scope-only, drop-other-params, url-encoding, and 'all' sentinel
cases. Total: 651 tests passing, typecheck clean.
This commit is contained in:
parent
eeb4975df1
commit
4ac5f3a220
3 changed files with 83 additions and 5 deletions
49
src/components/layout/sidebar-scope-href.test.ts
Normal file
49
src/components/layout/sidebar-scope-href.test.ts
Normal file
|
|
@ -0,0 +1,49 @@
|
|||
// Unit test for the sidebar's scopeAwareHref helper. The behaviour
|
||||
// itself lives in sidebar.tsx (a "use client" component), but the
|
||||
// pure helper is exported here so it can be tested without React.
|
||||
//
|
||||
// The contract: a sidebar nav link to /foo must carry the current
|
||||
// `?site=` forward so the URL stays the single source of truth for
|
||||
// the topbar pill. Without this, picking a site in the switcher
|
||||
// updates URL+cookie, but clicking any sidebar link would strip
|
||||
// `?site=` and leave the pill showing "All sites · N" while the
|
||||
// page's data still filtered through the cookie.
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { scopeAwareHref } from "./sidebar-scope-href";
|
||||
|
||||
describe("scopeAwareHref", () => {
|
||||
it("returns the bare path when no scope is active", () => {
|
||||
expect(scopeAwareHref("/dashboard/sites", new URLSearchParams())).toBe(
|
||||
"/dashboard/sites",
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves ?site= when one is active", () => {
|
||||
const params = new URLSearchParams("site=abc123");
|
||||
expect(scopeAwareHref("/dashboard/sites", params)).toBe(
|
||||
"/dashboard/sites?site=abc123",
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves only ?site= and drops unrelated params", () => {
|
||||
const params = new URLSearchParams("site=abc123&foo=bar&baz=qux");
|
||||
expect(scopeAwareHref("/dashboard/financial/purchase-orders", params)).toBe(
|
||||
"/dashboard/financial/purchase-orders?site=abc123",
|
||||
);
|
||||
});
|
||||
|
||||
it("url-encodes the site id so paths with special chars still work", () => {
|
||||
const params = new URLSearchParams("site=has%20space");
|
||||
expect(scopeAwareHref("/dashboard/sites", params)).toBe(
|
||||
"/dashboard/sites?site=has%20space",
|
||||
);
|
||||
});
|
||||
|
||||
it("treats 'all' as no scope — bare path, no query string", () => {
|
||||
const params = new URLSearchParams("site=all");
|
||||
expect(scopeAwareHref("/dashboard/sites", params)).toBe(
|
||||
"/dashboard/sites",
|
||||
);
|
||||
});
|
||||
});
|
||||
27
src/components/layout/sidebar-scope-href.ts
Normal file
27
src/components/layout/sidebar-scope-href.ts
Normal file
|
|
@ -0,0 +1,27 @@
|
|||
/**
|
||||
* Build a sidebar nav href that preserves the current `?site=` scope.
|
||||
*
|
||||
* The pill reads its active value from `useSearchParams()` and a
|
||||
* server-rendered cookie fallback — and Next.js layouts do NOT
|
||||
* re-render on navigation, so the cookie fallback is frozen at the
|
||||
* value it had when the user first hit the dashboard. Without this
|
||||
* preservation, picking a site would update both the URL and the
|
||||
* cookie, then clicking any sidebar link would strip the URL to a
|
||||
* bare path: the page's `getSiteScope()` would fall back to the
|
||||
* (still-set) cookie and filter the data correctly, but the pill
|
||||
* would resolve to `mode: "all"` and display "All sites · N" —
|
||||
* pill and data disagreeing is the user-visible bug.
|
||||
*
|
||||
* Only the `?site=` param is preserved. Every other query string on
|
||||
* the current URL is dropped because the sidebar links are coarse
|
||||
* top-level routes that don't reference them. The literal `all`
|
||||
* sentinel is treated as "no scope" so a sidebar link from an
|
||||
* already-"All sites" URL stays clean.
|
||||
*/
|
||||
import { SITE_SCOPE_ALL, SITE_SCOPE_PARAM } from "@/lib/site-scope";
|
||||
|
||||
export function scopeAwareHref(path: string, currentParams: URLSearchParams): string {
|
||||
const site = currentParams.get(SITE_SCOPE_PARAM);
|
||||
if (!site || site === SITE_SCOPE_ALL) return path;
|
||||
return `${path}?${SITE_SCOPE_PARAM}=${encodeURIComponent(site)}`;
|
||||
}
|
||||
|
|
@ -2,7 +2,7 @@
|
|||
|
||||
import * as React from "react";
|
||||
import Link from "next/link";
|
||||
import { usePathname } from "next/navigation";
|
||||
import { usePathname, useSearchParams } from "next/navigation";
|
||||
import {
|
||||
Building2,
|
||||
Coins,
|
||||
|
|
@ -21,6 +21,7 @@ import {
|
|||
X,
|
||||
} from "lucide-react";
|
||||
import { SiteScopeSwitcher } from "@/components/layout/site-scope-switcher";
|
||||
import { scopeAwareHref } from "@/components/layout/sidebar-scope-href";
|
||||
import { Logo } from "@/components/logo";
|
||||
import { ThemeToggle } from "@/components/theme-toggle";
|
||||
import { Button } from "@/components/ui/button";
|
||||
|
|
@ -106,6 +107,7 @@ function groupIsActive(group: NavGroup, pathname: string): boolean {
|
|||
|
||||
export function Sidebar() {
|
||||
const pathname = usePathname();
|
||||
const params = useSearchParams();
|
||||
const { lock } = useAuth();
|
||||
const [mobileOpen, setMobileOpen] = React.useState(false);
|
||||
|
||||
|
|
@ -129,7 +131,7 @@ export function Sidebar() {
|
|||
const content = (
|
||||
<div className="flex h-full min-h-0 flex-col gap-2 overflow-y-auto scrollbar-thin">
|
||||
<div className="flex shrink-0 items-center justify-between px-4 py-4">
|
||||
<Link href="/dashboard" className="flex items-center gap-2">
|
||||
<Link href={scopeAwareHref("/dashboard", params)} className="flex items-center gap-2">
|
||||
<Logo />
|
||||
</Link>
|
||||
<Button
|
||||
|
|
@ -151,7 +153,7 @@ export function Sidebar() {
|
|||
return (
|
||||
<Link
|
||||
key={entry.href}
|
||||
href={entry.href}
|
||||
href={scopeAwareHref(entry.href, params)}
|
||||
className={cn(
|
||||
"group flex items-start gap-3 rounded-lg border border-transparent px-3 py-2.5 text-sm transition-all",
|
||||
active
|
||||
|
|
@ -181,7 +183,7 @@ export function Sidebar() {
|
|||
return (
|
||||
<div key={entry.href} className="space-y-1">
|
||||
<Link
|
||||
href={entry.href}
|
||||
href={scopeAwareHref(entry.href, params)}
|
||||
className={cn(
|
||||
"group flex items-start gap-3 rounded-lg border border-transparent px-3 py-2.5 text-sm transition-all",
|
||||
groupActive
|
||||
|
|
@ -208,7 +210,7 @@ export function Sidebar() {
|
|||
return (
|
||||
<li key={child.href}>
|
||||
<Link
|
||||
href={child.href}
|
||||
href={scopeAwareHref(child.href, params)}
|
||||
className={cn(
|
||||
"flex items-center gap-2 rounded-md px-2 py-1.5 text-xs transition-all",
|
||||
childActive
|
||||
|
|
|
|||
Loading…
Reference in a new issue