From f98350d588e4930a37e36d88cfb283de8d1001f4 Mon Sep 17 00:00:00 2001
From: Martin Castro Laminrs <mcastro@laminr.ai>
Date: Sun, 31 May 2026 12:37:44 +0200
Subject: [PATCH] fix(spa): smart-route action-handler redirect URLs (#620) +
 release 1.4.13
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A ModelAdmin action that returns ``HttpResponseRedirect(some_url)``
was looking silently no-op'd to the operator: the click ran, the
toast didn't appear, and nothing visible happened. The diagnosis in
the issue blamed the API for swallowing the response, but the API
correctly extracts ``response["Location"]`` into the JSON envelope's
``redirect`` field (``api/views/actions.py:256``). The actual bug was
on the SPA: ``DetailPage`` piped the redirect URL straight into
React Router's ``navigate`` — which is scoped to the SPA's
``BrowserRouter`` ``basename``, so any URL outside the SPA mount
silently no-op'd:

  - legacy admin paths (``/admin/<app>/<model>/<pk>/change/``)
  - hijack / impersonate URLs (``/hijack/release-user/?next=…``)
  - cross-origin downloads (signed S3 URLs)

New ``followActionRedirect`` helper (`apps/web/src/action-redirect.ts`)
picks the right primitive per URL: ``navigate`` for same-origin paths
inside the SPA mount (no full reload), ``window.location.assign``
for everything else. Returns a stripped basename-relative path to
the navigate call so BrowserRouter doesn't double-prefix.

The helper is dependency-injected (``currentOrigin``,
``assignLocation``) so the test suite can lock the routing logic
without touching jsdom's non-configurable ``window.location``.

Locks: 6 new vitests in `action-redirect.test.ts` cover the SPA-
internal path, search + hash preservation, the legacy-admin path,
cross-origin URLs, the hijack pattern, and a malformed-URL fallback.

Release 1.4.13 bundles this with the unreleased changes since
1.4.12 (all already merged on main):

  - #631 / PR #641 — ``PRIMARY_COLOR`` reads ``site_primary_color``
    off the configured ``AdminSite`` before falling back to the
    setting + default.
  - #626 / PR #642 — ``raw_id_fields`` and ``radio_fields`` now
    render their intended widgets (plain-pk text input + lookup
    link, inline radio bank) instead of falling through to
    autocomplete / ``<select>``.
  - #623 / #624 / #633 / #634 / #635 — README "Stock-Django hooks
    that do NOT carry through" / "Writing safe ``list_display``
    callables" / "Hardening" / "Mounting the API on a different
    origin" sections (PR #640).
  - PR #638 — ``release.yml`` → ``publish.yml`` rename so PyPI's
    Trusted Publisher config matches the workflow filename.

Closes #620.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 frontend/apps/web/src/action-redirect.test.ts | 79 +++++++++++++++++++
 frontend/apps/web/src/action-redirect.ts      | 68 ++++++++++++++++
 frontend/apps/web/src/pages/DetailPage.tsx    | 15 +++-
 pyproject.toml                                |  2 +-
 4 files changed, 162 insertions(+), 2 deletions(-)
 create mode 100644 frontend/apps/web/src/action-redirect.test.ts
 create mode 100644 frontend/apps/web/src/action-redirect.ts

diff --git a/frontend/apps/web/src/action-redirect.test.ts b/frontend/apps/web/src/action-redirect.test.ts
new file mode 100644
index 0000000..fd73b65
--- /dev/null
+++ b/frontend/apps/web/src/action-redirect.test.ts
@@ -0,0 +1,79 @@
+// Lock the action-redirect routing (#620). `navigate` for SPA-internal
+// paths, `window.location.assign` (injected as `assignLocation` for
+// testability) for everything else. The legacy behaviour piped every
+// redirect through `navigate` — which silently no-op'd for any URL
+// outside the SPA mount.
+import { describe, expect, it, vi } from 'vitest';
+
+import { followActionRedirect } from './action-redirect';
+
+const MOUNT = '/admin-react/';
+const ORIGIN = 'http://localhost:3000';
+
+function makeArgs(redirect: string) {
+  const navigate = vi.fn();
+  const assignLocation = vi.fn();
+  return {
+    args: { redirect, mount: MOUNT, navigate, currentOrigin: ORIGIN, assignLocation },
+    navigate,
+    assignLocation,
+  };
+}
+
+describe('followActionRedirect', () => {
+  it('uses navigate for a same-origin path inside the SPA mount', () => {
+    const { args, navigate, assignLocation } = makeArgs('/admin-react/auth/user/42/');
+    followActionRedirect(args);
+    // The mount prefix is stripped so BrowserRouter's basename
+    // doesn't double up. Trailing slash before the relative path is
+    // preserved.
+    expect(navigate).toHaveBeenCalledWith('/auth/user/42/');
+    expect(assignLocation).not.toHaveBeenCalled();
+  });
+
+  it('preserves search + hash when navigating client-side', () => {
+    const { args, navigate } = makeArgs('/admin-react/auth/user/42/?tab=audit#log');
+    followActionRedirect(args);
+    expect(navigate).toHaveBeenCalledWith('/auth/user/42/?tab=audit#log');
+  });
+
+  it('falls back to assignLocation for a same-origin path OUTSIDE the mount', () => {
+    // Legacy admin path — must be a full browser navigation since
+    // React Router only routes within the SPA mount.
+    const { args, navigate, assignLocation } = makeArgs('/admin/auth/user/42/change/');
+    followActionRedirect(args);
+    expect(assignLocation).toHaveBeenCalledWith('/admin/auth/user/42/change/');
+    expect(navigate).not.toHaveBeenCalled();
+  });
+
+  it('falls back to assignLocation for cross-origin URLs', () => {
+    // The signed-S3-download case — must be a real browser navigation
+    // so the download starts on the operator's machine.
+    const { args, navigate, assignLocation } = makeArgs('https://s3.example.com/signed/file.pdf');
+    followActionRedirect(args);
+    expect(assignLocation).toHaveBeenCalledWith('https://s3.example.com/signed/file.pdf');
+    expect(navigate).not.toHaveBeenCalled();
+  });
+
+  it('falls back to assignLocation for a hijack-style /hijack/... URL', () => {
+    // Common third-party pattern (`django-hijack`) — action returns
+    // `HttpResponseRedirect("/hijack/release-user/?next=...")`.
+    const { args, navigate, assignLocation } = makeArgs('/hijack/release-user/?next=/admin/foo/');
+    followActionRedirect(args);
+    expect(assignLocation).toHaveBeenCalledWith('/hijack/release-user/?next=/admin/foo/');
+    expect(navigate).not.toHaveBeenCalled();
+  });
+
+  it('falls back to assignLocation on an unparseable URL', () => {
+    // A malformed input shouldn't make the operator's click disappear
+    // into the void — surface it as a real navigation and let the
+    // browser report the failure to the user.
+    //
+    // Using "http://" alone — bare scheme with no authority — produces
+    // a TypeError in the URL constructor on all majors.
+    const { args, navigate, assignLocation } = makeArgs('http://');
+    followActionRedirect(args);
+    expect(assignLocation).toHaveBeenCalledWith('http://');
+    expect(navigate).not.toHaveBeenCalled();
+  });
+});
diff --git a/frontend/apps/web/src/action-redirect.ts b/frontend/apps/web/src/action-redirect.ts
new file mode 100644
index 0000000..9ccc1ea
--- /dev/null
+++ b/frontend/apps/web/src/action-redirect.ts
@@ -0,0 +1,68 @@
+// Action-handler redirect routing (#620). Django ModelAdmin actions
+// can return any HttpResponse (HttpResponseRedirect, file streams,
+// HTML pages); the API extracts the response's Location header into
+// the JSON envelope's `redirect` field. Before this fix the SPA piped
+// `redirect` straight into React Router's `navigate` — which is
+// scoped to the SPA mount, so any redirect pointing OUTSIDE the SPA
+// silently no-op'd: legacy `/admin/.../` URLs, hijack/impersonate
+// flows, signed S3 download URLs all looked broken from the operator's
+// POV. This helper picks the right navigation primitive per URL.
+import type { NavigateFunction } from 'react-router-dom';
+
+/**
+ * Follow a redirect returned by a ModelAdmin action.
+ *
+ * - Same origin AND under the SPA mount → React Router `navigate`
+ *   (no full reload; the operator stays in the SPA).
+ * - Anything else (off-origin, or same-origin outside the mount) →
+ *   `window.location.assign` (full browser navigation; the only way
+ *   to reach a non-SPA page).
+ *
+ * `mount` is the SPA's URL prefix (e.g. `/admin-react/`), already
+ * read once at boot from the `<meta name="dar-mount">` tag in
+ * `main.tsx` — pass it through; this helper doesn't query the DOM
+ * so it stays unit-testable.
+ *
+ * If the redirect URL is unparseable (a defensive case — the API
+ * should always emit a real URL) we fall back to `window.location
+ * .assign` and let the browser report the failure to the user.
+ */
+export interface FollowRedirectArgs {
+  redirect: string;
+  mount: string;
+  navigate: NavigateFunction;
+  /** Current page origin (`http://localhost:3000` etc.). Defaults to
+   *  `window.location.origin` in production; the test injects a known
+   *  value because jsdom's `window.location.origin` is hard-coded. */
+  currentOrigin?: string;
+  /** Full-page navigation. Defaults to `window.location.assign` in
+   *  production; the test injects a spy because jsdom's
+   *  `window.location.assign` is non-configurable on most jsdom
+   *  versions and resists `vi.spyOn`. */
+  assignLocation?: (url: string) => void;
+}
+
+export function followActionRedirect(args: FollowRedirectArgs): void {
+  const { redirect, mount, navigate } = args;
+  const origin = args.currentOrigin ?? window.location.origin;
+  const assignLocation = args.assignLocation ?? ((url: string) => window.location.assign(url));
+  let parsed: URL;
+  try {
+    parsed = new URL(redirect, origin);
+  } catch {
+    assignLocation(redirect);
+    return;
+  }
+  const sameOrigin = parsed.origin === origin;
+  const inSpaMount = parsed.pathname.startsWith(mount);
+  if (sameOrigin && inSpaMount) {
+    // BrowserRouter is initialised with `basename={mount}` (main.tsx)
+    // so React Router routes are relative to that. Strip the mount
+    // prefix before passing — otherwise the basename auto-prepends
+    // and we get `/admin-react/admin-react/...` which 404s.
+    const relative = parsed.pathname.slice(mount.length - 1) + parsed.search + parsed.hash;
+    navigate(relative);
+  } else {
+    assignLocation(redirect);
+  }
+}
diff --git a/frontend/apps/web/src/pages/DetailPage.tsx b/frontend/apps/web/src/pages/DetailPage.tsx
index b378c09..c9680d0 100644
--- a/frontend/apps/web/src/pages/DetailPage.tsx
+++ b/frontend/apps/web/src/pages/DetailPage.tsx
@@ -50,6 +50,7 @@ import { HistoryModal } from '@dar/history';
 import { RecordSkeleton } from '../components/RecordSkeleton';
 import { useModelMeta } from '../useModelMeta';
 import { toastMessages, useToast } from '../toast';
+import { followActionRedirect } from '../action-redirect';
 import { carryPreservedFilters, listPathWithPreservedFilters } from '../changelistFilters';
 import { useUnsavedGuard } from '../useUnsavedGuard';
 
@@ -313,7 +314,19 @@ export function DetailPage({
                 }
                 onSuccess={async ({ message, messages, redirect }) => {
                   if (redirect) {
-                    navigate(redirect);
+                    // Smart-route the redirect (#620): an admin action
+                    // can hand back any URL — a same-SPA path, a legacy
+                    // `/admin/.../change/` page, a hijack URL, a signed
+                    // S3 download. React Router's `navigate` only
+                    // routes WITHIN the SPA mount, so non-SPA URLs
+                    // silently no-op'd before. `followActionRedirect`
+                    // picks `navigate` vs `window.location.assign`
+                    // based on origin + mount.
+                    const mountMeta = document
+                      .querySelector<HTMLMetaElement>('meta[name="dar-mount"]')
+                      ?.content?.trim();
+                    const mount = (mountMeta ?? '/').replace(/\/?$/, '/');
+                    followActionRedirect({ redirect, mount, navigate });
                     return;
                   }
                   await refresh();
diff --git a/pyproject.toml b/pyproject.toml
index 6287331..e5d8b73 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "django-admin-react"
-version = "1.4.12"
+version = "1.4.13"
 description = "A drop-in React single-page admin for Django, driven entirely by ModelAdmin."
 authors = ["django-admin-react contributors"]
 license = "MIT"