Navigation "return to caller" fix — `returnTo` query param

Status: Done · Author: AI Review · Date: 2026-07-12 · Implemented: 2026-07-13

Permanent docs: the convention now lives in architecture.md → Navigation, hooks.md → useReturnTo, and the ButtonCancel entry in buttons.md.

Problem

This app implements every "modal" (PartnerDetails, ContractDetails, PartnerDnDPicker, DefaultActivityAttributes, TemplatesCaption, TemplateEditor, OrgLanguages, …) as a plain page route rendering a MUI <Dialog open> — there are no Next.js intercepting/parallel routes anywhere in src/app. Because these "modal shell" components are ordinary pages, they get reached from multiple different parent contexts (the flat list, an activity's controller/processor chip, a contract's partner chip, another partner's contract card, a partner-picker shortcut, etc.), but their default Save/Delete/Close handlers hardcode a single navigation target — effectively "one level up in the route tree" — instead of returning to wherever the user actually came from.

Reported example: ActivityDetails → click a controller/processor chip → opens PartnerDetails at /e/org/partners/[id] → click Save → lands on the flat /e/org/partners list, not back on the activity. Same thing happens via the partner-picker's "Edit Partners" shortcut (PartnerDnDPicker), which also unconditionally discards the in-progress activity edit context by hard-navigating away.

Chosen solution

A returnTo query param, read by one shared hook, with the existing hardcoded path kept as the fallback-of-fallback. This fits the app's existing conventions (useUrlParams + toQueryString/buildUrl/ mergeParams in src/lib/url/toQueryString.ts, nuqs for query-param state) and is backward-compatible/incrementally adoptable — routes reached with no returnTo behave exactly as they do today.

returnTo is kept in clear text, not encrypted/signed: it only ever holds an internal relative path (nothing confidential — already visible in the address bar/history like any other link), and the real risk (open redirect via a crafted absolute-URL value) is closed off by an allow-list validator rather than obfuscation. nuqs has no crypto features and isn't a fit for that regardless.

0. New shared infrastructure (do first)

0.1 src/lib/url/returnTo.ts — new pure validator, exported from the src/lib/url barrel:

export const isSafeReturnPath = (value: string | null | undefined): value is string => {
  if (!value || typeof value !== "string") return false;
  if (!value.startsWith("/")) return false;
  if (value.startsWith("//") || value.startsWith("/\\")) return false;
  if (/^[a-zA-Z][a-zA-Z\d+\-.]*:/.test(value)) return false;
  if (/\s/.test(value)) return false;
  return true;
};

Rejects absolute URLs, protocol-relative //, scheme: values, and embedded whitespace — same-origin relative paths only.

0.2 src/lib/hooks/useReturnTo.ts — new hook, exported from src/lib/hooks/index.ts:

const useReturnTo = () => {
  const router = useRouter();             // @/lib/i18n
  const pathname = usePathname();          // @/lib/i18n
  const searchParams = useSearchParams();  // next/navigation
  const [rawReturnTo] = useQueryState("returnTo", parseAsString); // nuqs

  const returnTo = useMemo(
    () => (isSafeReturnPath(rawReturnTo) ? rawReturnTo : null),
    [rawReturnTo],
  );

  // fallback is optional: omit it (e.g. ButtonCancel) to fall through to
  // router.back() when there's no returnTo; pass it (the shell components
  // in section 1) to fall through to today's hardcoded path instead.
  // Zero-regression either way: identical to today's behavior when
  // returnTo is absent/unsafe.
  const navigateBack = useCallback(
    (fallback?: string) => {
      if (returnTo) {
        router.push(returnTo);
        return;
      }
      if (fallback) {
        router.push(fallback);
        return;
      }
      router.back();
    },
    [router, returnTo],
  );

  // Build a link into a shell that carries the current location forward.
  const withReturnTo = useCallback(
    (path: string, extraParams: ParamRecord = {}) => {
      const search = searchParams.toString();
      const current = pathname + (search ? `?${search}` : "");
      return buildUrl(path, mergeParams(extraParams, { returnTo: current }));
    },
    [pathname, searchParams],
  );

  return { returnTo, navigateBack, withReturnTo };
};

withReturnTo composes with the existing buildUrl/mergeParams helpers rather than reinventing query-string handling. Because returnTo values are always URL-encoded through URLSearchParams, nested chains (Partner ← Contract ← Activity) round-trip safely with no special-case logic.

Full priority chain at each call site (the hook only implements the middle two tiers — the first is already how every one of these components works today, and stays untouched):

Explicit override prop (onSave/onClose/onDelete/onClick, already optional on every shell in section 1 and on ButtonCancel) → use it. Unchanged by this work.
No prop, returnTo present & safe → navigateBack(...) pushes to it.
No prop, no returnTo → navigateBack(fallback) pushes to the existing hardcoded fallback path (section 1's shell components), or navigateBack() with no argument falls through to router.back() (ButtonCancel, item 1.12).

1. Shell components — swap hardcoded `router.push` for `navigateBack(sameFallback)`

Save and Delete already get this same treatment here — there's no separate ButtonSave/ButtonDelete item like ButtonCancel (1.12) because there's nothing to unify at the button-component level. ButtonCancel could be centralized because "cancel" has one universal default action (router.back()) regardless of which page you're on. Save and Delete don't: what to save/delete is always shell-specific business logic (contractCreate, activityDelete, …), so there's no generic default behavior a shared button component could provide.

src/lib/ui/buttons/Button.tsx (renders Save actions) has no navigation default at all when onClick is omitted — it throws a dev warning (noOnClickWarning), not a fallback push. Save always requires an explicit handler.
src/lib/ui/buttons/ButtonHold.tsx (renders Delete actions, e.g. in EditActivityButtons) is the same — onClick is always required, no built-in "after delete" navigation to intercept.

So the fix for Save/Delete lives where the "what happens after the action succeeds" logic actually lives: each shell's own onSave ?? (() => navigateBack(fallback)) / onDelete ?? (() => navigateBack(fallback)) default. That's exactly what 1.1, 1.3, 1.5, and 1.11 below already do — every Save/Delete handler found in the audit is covered by this list, with no separate item needed.

1.1 src/lib/ui/modals/PartnerDetails.tsx:31-32 — handleSave, handleDelete (fallback /e/org/partners)
1.2 src/lib/ui/modals/ContractDetails.tsx:49 — handleClose (fallback /e/org/contracts)
1.3 src/lib/ui/modals/DefaultActivityAttributes.tsx:90,105 — save & cancel (fallback /e/org)
1.4 src/lib/ui/modals/TemplatesCaption.tsx:78-80 — close (fallback /e/org)
1.5 src/lib/ui/modals/TemplateEditor.tsx:464,516,663 — save, delete, close (fallback /e/tmp)
1.6 src/lib/ui/modals/OrgLanguages.tsx:256 — cancel's else-branch (fallback /e/org). Leave :239 untouched — that push deliberately needs the freshly-set locale from the redirect, not a caller-supplied path.
1.7 src/lib/ui/buttongroups/PartnersGridButtons.tsx:29 — close (fallback /e/org + current urlParams)
1.8 src/lib/ui/buttongroups/ContractsGridButtons.tsx:30 — close (fallback /e/org + urlParams)
1.9 src/lib/ui/buttongroups/OuFormButtons.tsx:77 — close (fallback /e + urlParams)
1.10 src/lib/ui/grids/OrganizationSettingsGrid.tsx:103 — (fallback /)
1.11 src/lib/ui/buttongroups/EditActivityButtons.tsx:46-48,66,80 — confirmed same anti-pattern, not just theoretical (see correction below). Replace:
```
const basePath = pathname.split("/").slice(0, -1).join("/") || "/";
...
router.push(basePath + toQueryString(urlParams)); // handleCancel, handleDelete
```
with:
```
const { navigateBack } = useReturnTo();
...
navigateBack(basePath + toQueryString(urlParams)); // same fallback, zero regression
```
in both handleCancel and handleDelete. basePath itself stays — it's still needed as the fallback-of-fallback for standalone entry (typing /e/[id] directly, or opening from the flat activity list).
1.12 src/lib/ui/buttons/ButtonCancel.tsx:87 — replace:
```
const defaultOnClick = onClick ?? (() => router.back());
```
with:
```
const { navigateBack } = useReturnTo();
const defaultOnClick = onClick ?? (() => navigateBack());
```
No fallback path passed — navigateBack() already falls through to router.back() per the priority chain in section 0, so this is a pure upgrade: identical behavior when no returnTo is present, and correct when one is. ButtonCancel's own onClick prop (already the top-priority override — e.g. EditActivityButtons's custom handleCancel, unaffected by this change) keeps working exactly as it does today. Every caller currently relying on ButtonCancel's default (e.g. OuFormButtons.tsx) gets fixed for free, without a per-caller edit.

Leave src/app/[locale]/(app)/e/orgchange/page.tsx:16 as-is — it only fires on an error/no-op toast path, not a real multi-context navigation.

Correction to the original plan: section 3 below previously listed EditActivityButtons.tsx as "out of scope — brittle but not currently buggy," reasoning that ActivityDetails only has one known parent context (the flat activity list at /e). That was wrong — grep for navLink={`/e/${activity.activityId}`} and router.push(`/e/${...}`) shows /e/[id] is already linked into from multiple distinct parent contexts today:

src/lib/forms/organizationForm/OrganizationDetailsHeader.tsx:143,152,161 — a partner's own details page, listing that partner's activities (as controller / joint controller / processor)
src/lib/ui/cards/PartnerCaption.tsx:183 — the flat partners grid's per-partner activity chips
src/lib/ui/cards/ContractCaption.tsx:144 — a contract's associated activities

So today, opening an activity from a partner's or contract's activity chip and then clicking Cancel or Delete already sends the user to the flat /e activity list instead of back to the partner/contract they came from — the exact same bug class as the PartnerDetails case, just previously missed. This item is in scope, not deferred.

2. Link-building call sites — attach `returnTo` via `withReturnTo`

ElementChip.tsx needs no change — it already just does router.push(navLink); the returnTo param rides along inside navLink once the callers above attach it.

3. Out of scope (flagged, not fixed in this pass)

EditActivityButtons.tsx was previously listed here as out of scope; it has been moved to 1.11 / 2.8-2.10 above since it turned out to be the same live bug, not a hypothetical one — see the correction note under section 1.

3.1 PartnerDnDPicker's missing confirm-on-navigate-away — "Edit Partners" abandons in-progress activity edits with no warning. This fix makes it land in the correct place but does not add an unsaved-changes guard — separate UX issue, separate ticket.

Root cause: src/lib/ui/pickers/PartnerDnDPicker.tsx:84-86 (handleEditOrganizations) unconditionally calls router.push(...). DragAndDrop.tsx's own "Edit Partners" button (leftButtonText/ onLeftButtonClick, lines 864-873) already disables itself via disabled={hasChanges} when there are uncommitted drag-and-drop edits — but that hasChanges is local to the DnD widget only. It has no visibility into the parent ActivityDetails form's own pending edits (name, description, other attributes not yet saved), which live in ActivityDetails.tsx's hasChanges (line 221, diff of activities vs savedActivities). Clicking "Edit Partners" today silently discards those.

Planned fix — thread a hasPendingChanges flag down and gate the button behind a confirm dialog (no new i18n keys needed — reuse existing common.unsavedChangesConfirm / common.changeLost, already present in all 10 locale files):
1. src/lib/ui/pickers/DragAndDrop.tsx — add optional prop leftButtonConfirmDialog?: boolean; when true, pass confirmDialog, confirmQuestion={tCommon("unsavedChangesConfirm")}, confirmText={tCommon("changeLost")} to the left button's <Button> (same confirmDialog mechanism already used by ButtonCancel.tsx and EditActivityButtons.tsx's Save/Delete). Needs a useTranslations("common") call alongside the existing tEditOrg.
2. src/lib/ui/pickers/PartnerDnDPicker.tsx — add optional prop hasPendingChanges?: boolean; pass leftButtonConfirmDialog={hasPendingChanges} to DragAndDrop.
3. src/lib/edit/EditActivityOrganizations.tsx — add optional prop hasPendingChanges?: boolean; forward to <PartnerDnDPicker>.
4. src/lib/ui/AttributeRow/AttributeRowOrganizations.tsx — add optional prop hasPendingChanges?: boolean; forward to <EditActivityOrganizations>.
5. src/lib/ui/AttributeRow/renderAttributeRow.tsx — add hasChanges: boolean to RenderAttributeRowProps; pass as hasPendingChanges={hasChanges} to AttributeRowOrganizations in the isControllers(attribute) branch only.
6. src/lib/ui/modals/ActivityDetails.tsx — pass the component's existing hasChanges (line 221) into the renderAttributeRow({...}) call (line 645) as hasChanges.
Verification: open an activity, edit a text attribute (don't save), then click "Edit Partners" → confirm dialog should appear ("You have unsaved changes. Are you sure you want to close?"); Cancel stays on the activity with the edit intact, Confirm navigates away and the edit is lost (expected, now with informed consent). With no pending activity edits, "Edit Partners" should navigate immediately, same as today.

4. Verification checklist

Click through manually (dev server). Confirm both "no returnTo → old behavior unchanged" and "returnTo present → returns to true origin":

5. Deploy testing

Section 4 is local/dev-server only. Before calling this done, repeat the critical path on a real deployed preview, since router.push/query-param behavior, locale-prefixed routing (useRouter/usePathname from @/lib/i18n), and the NuqsAdapter wiring can behave differently once built for production and served through Vercel/whatever host is used.

5.1 Push the branch and open its preview deployment (or deploy to staging, per this repo's normal flow — see ../infrastructure for the deploy pipeline).
5.2 Re-run at minimum 4.3 (activity → controller chip → edit partner → Save → back to activity), 4.7 (the headline partner-picker bug), and 4.11 (partner → activity chip → Cancel/Delete → back to partner) against the preview URL, in a non-default locale (e.g. /es/... or /fr/...) to also confirm the returnTo path survives locale prefixing.
5.3 Re-run 4.9 (the isSafeReturnPath open-redirect check) against the deployed URL specifically — confirm no CDN/edge rewrite or middleware strips/rewrites the returnTo param in a way that changes the safety check's outcome.
5.4 Watch for hydration or build-time console errors from the new useReturnTo hook (in particular useSearchParams() requiring a Suspense boundary) that a local dev server might not surface as loudly as a production build.
5.5 Confirm no regression in a cold/direct load of each modified route (/e/org/partners/[id], /e/org/contracts, /e/[id], etc.) with no returnTo param at all — the zero-regression guarantee this whole design depends on.

6. Update `/docs/dev` documentation

Once the fix is implemented and deploy-verified (sections 1-5 done), fold the new convention into the permanent docs so it doesn't only live in this TODO file:

6.1 docs/dev/components/hooks/hooks.md — add a ### useReturnTo entry under "Standalone hooks" (alongside the existing useDate/useEditMode/useUrlParams entries), documenting returnTo, navigateBack(fallback?) (fallback optional — defaults to router.back() when omitted), and withReturnTo(path, extraParams), plus the safety rule (same-origin relative paths only, via isSafeReturnPath).
6.2 docs/dev/architecture.md — add a short "Navigation" note (near "Request Lifecycle" or as its own subsection) explaining that modals are plain page routes (no intercepting/parallel routes), and that any shell reachable from more than one parent context must use useReturnTo for its Save/Close/Delete/Cancel handlers instead of hardcoding a parent path — so the next person adding a modal follows the convention instead of reintroducing this bug class.
6.3 docs/dev/components/ui/buttons.md — update the ButtonCancel entry to reflect 1.12: its default onClick is now navigateBack() (via useReturnTo), not a bare router.back(). Document the resulting priority chain — explicit onClick prop overrides everything (unchanged), then returnTo if present, then router.back() — so it's clear ButtonCancel and the shell components' navigateBack(fallback) calls (1.1-1.11) are the same mechanism now, just with or without a fallback path, not two competing patterns.
6.4 Mark this file's status header as Status: Done (from Draft) once all of the above is implemented and documented, or leave a short "superseded by docs/dev/..." pointer if the content is fully absorbed elsewhere and this file is archived.

Problem​

Chosen solution​

0. New shared infrastructure (do first)​

1. Shell components — swap hardcoded router.push for navigateBack(sameFallback)​

2. Link-building call sites — attach returnTo via withReturnTo​

3. Out of scope (flagged, not fixed in this pass)​

4. Verification checklist​

5. Deploy testing​

6. Update /docs/dev documentation​