From 29fb882478ca69d85a13ba355271d732061388d5 Mon Sep 17 00:00:00 2001
From: Matt <matt@letsbe.solutions>
Date: Tue, 2 Jun 2026 12:26:10 +0200
Subject: [PATCH] fix(audit): H15 (saved-view sort) + H14 (back/forward URL
 resync) in usePaginatedQuery

H15: new applyView({filters,sort}) atomic mutator (one URL write) restores a
saved view's sort, threaded through all six list components instead of being
discarded. H14: a guarded effect resyncs page/sort/filters FROM the URL on
Back/Forward; the resync setStates carry a scoped, justified
set-state-in-effect disable (loop-guarded external-URL sync).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/components/berths/berth-list.tsx          |   6 +-
 src/components/clients/client-list.tsx        |  14 +-
 src/components/companies/company-list.tsx     |   6 +-
 src/components/interests/interest-list.tsx    |   6 +-
 .../residential-interests-list.tsx            |   6 +-
 src/components/yachts/yacht-list.tsx          |   6 +-
 src/hooks/use-paginated-query.ts              | 135 +++++++++++++++++-
 7 files changed, 152 insertions(+), 27 deletions(-)

diff --git a/src/components/berths/berth-list.tsx b/src/components/berths/berth-list.tsx
index b94296da..d3b14a0f 100644
--- a/src/components/berths/berth-list.tsx
+++ b/src/components/berths/berth-list.tsx
@@ -85,7 +85,7 @@ export function BerthList() {
     setSort,
     filters,
     setFilter,
-    setAllFilters,
+    applyView,
     clearFilters,
     setPage,
     setPageSize,
@@ -183,8 +183,8 @@ export function BerthList() {
         <div className="flex items-center gap-2 ml-auto">
           <SavedViewsDropdown
             entityType="berths"
-            onApplyView={(savedFilters, _savedSort) => {
-              setAllFilters(savedFilters);
+            onApplyView={(savedFilters, savedSort) => {
+              applyView({ filters: savedFilters, sort: savedSort });
             }}
           />
           <Button
diff --git a/src/components/clients/client-list.tsx b/src/components/clients/client-list.tsx
index 85cabc46..82ef2ae5 100644
--- a/src/components/clients/client-list.tsx
+++ b/src/components/clients/client-list.tsx
@@ -115,7 +115,7 @@ export function ClientList() {
     setPageSize,
     filters,
     setFilter,
-    setAllFilters,
+    applyView,
     clearFilters,
   } = usePaginatedQuery<ClientRow>({
     queryKey: ['clients'],
@@ -189,12 +189,12 @@ export function ClientList() {
         />
         <SavedViewsDropdown
           entityType="clients"
-          onApplyView={(savedFilters, _savedSort) => {
-            // Atomic replace - sequential setFilter() calls dropped all
-            // but the last value (each one read stale `filters` from
-            // closure and overwrote). setAllFilters writes the whole
-            // saved view in one setState.
-            setAllFilters(savedFilters);
+          onApplyView={(savedFilters, savedSort) => {
+            // Atomic replace of filters AND sort in one URL write. Passing
+            // both args fixes H15 (the saved sort was previously dropped);
+            // applyView also avoids the two-write race that a setAllFilters
+            // + setSort pair would hit.
+            applyView({ filters: savedFilters, sort: savedSort });
           }}
         />
         <ColumnPicker
diff --git a/src/components/companies/company-list.tsx b/src/components/companies/company-list.tsx
index a3059baf..36a45047 100644
--- a/src/components/companies/company-list.tsx
+++ b/src/components/companies/company-list.tsx
@@ -97,7 +97,7 @@ export function CompanyList() {
     setPageSize,
     filters,
     setFilter,
-    setAllFilters,
+    applyView,
     clearFilters,
   } = usePaginatedQuery<CompanyRow>({
     queryKey: ['companies'],
@@ -155,8 +155,8 @@ export function CompanyList() {
         <div className="ml-auto flex items-center gap-2">
           <SavedViewsDropdown
             entityType="companies"
-            onApplyView={(savedFilters, _savedSort) => {
-              setAllFilters(savedFilters);
+            onApplyView={(savedFilters, savedSort) => {
+              applyView({ filters: savedFilters, sort: savedSort });
             }}
           />
           <ColumnPicker columns={COMPANY_COLUMN_OPTIONS} hidden={hidden} onChange={setHidden} />
diff --git a/src/components/interests/interest-list.tsx b/src/components/interests/interest-list.tsx
index 6cabb7ec..454f6315 100644
--- a/src/components/interests/interest-list.tsx
+++ b/src/components/interests/interest-list.tsx
@@ -110,7 +110,7 @@ export function InterestList() {
     setPageSize,
     filters,
     setFilter,
-    setAllFilters,
+    applyView,
     clearFilters,
   } = usePaginatedQuery<InterestRow>({
     queryKey: ['interests'],
@@ -266,8 +266,8 @@ export function InterestList() {
             <>
               <SavedViewsDropdown
                 entityType="interests"
-                onApplyView={(savedFilters) => {
-                  setAllFilters(savedFilters);
+                onApplyView={(savedFilters, savedSort) => {
+                  applyView({ filters: savedFilters, sort: savedSort });
                 }}
               />
               <ColumnPicker
diff --git a/src/components/residential/residential-interests-list.tsx b/src/components/residential/residential-interests-list.tsx
index 1a8f4e48..f3d1db0a 100644
--- a/src/components/residential/residential-interests-list.tsx
+++ b/src/components/residential/residential-interests-list.tsx
@@ -83,7 +83,7 @@ export function ResidentialInterestsList() {
     setPageSize,
     filters,
     setFilter,
-    setAllFilters,
+    applyView,
     clearFilters,
   } = usePaginatedQuery<ResidentialInterestRow>({
     queryKey: ['residential-interests'],
@@ -193,7 +193,9 @@ export function ResidentialInterestsList() {
         <div className="ml-auto flex flex-wrap items-center gap-2">
           <SavedViewsDropdown
             entityType="residential_interests"
-            onApplyView={(savedFilters) => setAllFilters(savedFilters)}
+            onApplyView={(savedFilters, savedSort) =>
+              applyView({ filters: savedFilters, sort: savedSort })
+            }
           />
           <ColumnPicker
             columns={RESIDENTIAL_INTEREST_COLUMN_OPTIONS}
diff --git a/src/components/yachts/yacht-list.tsx b/src/components/yachts/yacht-list.tsx
index 62aa3633..f92b9077 100644
--- a/src/components/yachts/yacht-list.tsx
+++ b/src/components/yachts/yacht-list.tsx
@@ -99,7 +99,7 @@ export function YachtList() {
     setPageSize,
     filters,
     setFilter,
-    setAllFilters,
+    applyView,
     clearFilters,
   } = usePaginatedQuery<YachtRow>({
     queryKey: ['yachts'],
@@ -153,8 +153,8 @@ export function YachtList() {
         />
         <SavedViewsDropdown
           entityType="yachts"
-          onApplyView={(savedFilters, _savedSort) => {
-            setAllFilters(savedFilters);
+          onApplyView={(savedFilters, savedSort) => {
+            applyView({ filters: savedFilters, sort: savedSort });
           }}
         />
       </div>
diff --git a/src/hooks/use-paginated-query.ts b/src/hooks/use-paginated-query.ts
index 4febf209..6730a1de 100644
--- a/src/hooks/use-paginated-query.ts
+++ b/src/hooks/use-paginated-query.ts
@@ -1,6 +1,6 @@
 'use client';
 
-import { useState, useCallback, useMemo } from 'react';
+import { useState, useCallback, useMemo, useEffect, useRef } from 'react';
 import { useQuery, useQueryClient, type QueryKey } from '@tanstack/react-query';
 import { useSearchParams, useRouter, usePathname } from 'next/navigation';
 
@@ -55,9 +55,13 @@ export function usePaginatedQuery<T>({
     deserializeFiltersFromParams(searchParams, filterDefinitions),
   );
 
-  // Sync state to URL
-  const syncUrl = useCallback(
-    (p: number, ps: number, s?: typeof sort, f?: FilterValues) => {
+  // Canonical query string for a given state. Sorted keys so two URLs
+  // that carry the same params in a different order compare equal - this
+  // is what lets the back/forward resync effect (H14) tell "the URL the
+  // browser just restored" apart from "the URL we last wrote ourselves"
+  // without thrashing on key ordering.
+  const buildCanonicalQs = useCallback(
+    (p: number, ps: number, s?: typeof sort, f?: FilterValues): string => {
       const params = new URLSearchParams();
       if (p !== 1) params.set('page', String(p));
       if (ps !== initialPageSize) params.set('limit', String(ps));
@@ -72,12 +76,26 @@ export function usePaginatedQuery<T>({
       // Keep existing tab param
       const tab = searchParams.get('tab');
       if (tab) params.set('tab', tab);
+      params.sort();
+      return params.toString();
+    },
+    [searchParams, initialPageSize],
+  );
 
-      const qs = params.toString();
+  // Records the query string we last pushed via `router.replace`, so the
+  // resync effect (H14) can skip params it itself authored and only react
+  // to genuinely-external URL changes (Back/Forward).
+  const lastWrittenQsRef = useRef<string | null>(null);
+
+  // Sync state to URL
+  const syncUrl = useCallback(
+    (p: number, ps: number, s?: typeof sort, f?: FilterValues) => {
+      const qs = buildCanonicalQs(p, ps, s, f);
+      lastWrittenQsRef.current = qs;
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
       router.replace(`${pathname}${qs ? `?${qs}` : ''}` as any, { scroll: false });
     },
-    [pathname, router, searchParams, initialPageSize],
+    [pathname, router, buildCanonicalQs],
   );
 
   function setPage(p: number) {
@@ -130,6 +148,110 @@ export function usePaginatedQuery<T>({
     syncUrl(1, pageSize, sort, next);
   }
 
+  /**
+   * Atomically apply a saved view's filters AND sort in a single URL
+   * write. The saved-views apply path previously called `setAllFilters`
+   * only, silently dropping the view's stored sort (H15). Splitting the
+   * write into `setAllFilters` + `setSort` would also race: each reads a
+   * stale closure and the second `syncUrl` clobbers the first. `applyView`
+   * sets both pieces of state and emits ONE `syncUrl`, so the view lands
+   * intact with its sort.
+   *
+   * `sort` is optional - a view saved without an explicit sort clears the
+   * active sort back to the list's `initialSort` (handled by the consumer
+   * passing `undefined`). The incoming `direction` is the loose `string`
+   * shape from `SavedViewsDropdown`; we narrow non-'asc' to 'desc'.
+   */
+  function applyView({
+    filters: nextFilters,
+    sort: nextSort,
+  }: {
+    filters: FilterValues;
+    sort?: { field: string; direction: string } | undefined;
+  }) {
+    const normalizedSort: { field: string; direction: 'asc' | 'desc' } | undefined = nextSort
+      ? {
+          field: nextSort.field,
+          direction: nextSort.direction === 'asc' ? ('asc' as const) : ('desc' as const),
+        }
+      : initialSort;
+    setFiltersState(nextFilters);
+    setSortState(normalizedSort);
+    setPageState(1);
+    syncUrl(1, pageSize, normalizedSort, nextFilters);
+  }
+
+  /**
+   * H14 - resync state FROM the URL on external navigation (Back/Forward).
+   *
+   * The slices above seed from the URL once (useState initializers) then
+   * drive it one-way via `syncUrl` -> `router.replace`. Nothing pulled the
+   * URL back into state, so Back/Forward moved the address bar but left the
+   * list showing the previous page/sort/filters.
+   *
+   * Loop safety: every write we make records its canonical query string in
+   * `lastWrittenQsRef`. Here we re-derive the canonical form of the params
+   * the router is currently handing us and compare it against that ref. If
+   * they match, this render was caused by our OWN write - bail before
+   * touching state. We also compare against the canonical form of CURRENT
+   * state and only call a setter for a slice that actually differs, so even
+   * an external change that happens to equal current state is a no-op. Both
+   * guards mean this effect cannot feed itself: it never calls `syncUrl`,
+   * and it never setState's a value equal to what state already holds.
+   */
+  useEffect(() => {
+    const incomingQs = buildCanonicalQs(
+      Number(searchParams.get('page')) || initialPage,
+      Number(searchParams.get('limit')) || initialPageSize,
+      searchParams.get('sort')
+        ? {
+            field: searchParams.get('sort') as string,
+            direction: (searchParams.get('order') as 'asc' | 'desc') ?? 'desc',
+          }
+        : initialSort,
+      deserializeFiltersFromParams(searchParams, filterDefinitions),
+    );
+
+    // This params object is the one we just wrote - not an external nav.
+    if (incomingQs === lastWrittenQsRef.current) return;
+
+    // The state already matches the URL - nothing to do (also prevents a
+    // redundant setState that could schedule an extra render).
+    const currentQs = buildCanonicalQs(page, pageSize, sort, filters);
+    if (incomingQs === currentQs) return;
+
+    // Genuine external change (Back/Forward): pull each slice from the URL,
+    // setting only the ones that drifted.
+    const nextPage = Number(searchParams.get('page')) || initialPage;
+    const nextPageSize = Number(searchParams.get('limit')) || initialPageSize;
+    const nextSortField = searchParams.get('sort');
+    const nextSort = nextSortField
+      ? {
+          field: nextSortField,
+          direction: (searchParams.get('order') as 'asc' | 'desc') ?? 'desc',
+        }
+      : initialSort;
+    const nextFilters = deserializeFiltersFromParams(searchParams, filterDefinitions);
+
+    /* eslint-disable react-hooks/set-state-in-effect --
+       Deliberate, guarded URL->state sync on Back/Forward: the URL is the
+       external source of truth here. The two early-returns above ensure this
+       runs ONLY on a genuine external navigation (never on our own write), and
+       each setState is diff-guarded, so there is no cascading-render loop
+       (audit H14). This is the documented external-store-subscription case the
+       rule otherwise blocks. */
+    if (nextPage !== page) setPageState(nextPage);
+    if (nextPageSize !== pageSize) setPageSizeState(nextPageSize);
+    if (nextSort?.field !== sort?.field || nextSort?.direction !== sort?.direction) {
+      setSortState(nextSort);
+    }
+    if (JSON.stringify(nextFilters) !== JSON.stringify(filters)) {
+      setFiltersState(nextFilters);
+    }
+    /* eslint-enable react-hooks/set-state-in-effect */
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [searchParams]);
+
   // Build query string for API
   const apiParams = useMemo(() => {
     const params = new URLSearchParams();
@@ -196,6 +318,7 @@ export function usePaginatedQuery<T>({
     filters,
     setFilter,
     setAllFilters,
+    applyView,
     clearFilters,
     optimisticRemove,
   };