Skip to content

Commit 91e8ed1

Browse files
MartinCastroAlvarezmartin-castro-laminr-aiclaude
authored
fix(spa): smart-route action-handler redirect URLs (#620) + release 1.4.13 (#643)
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: Martin Castro Laminrs <mcastro@laminr.ai> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 568ee32 commit 91e8ed1

4 files changed

Lines changed: 162 additions & 2 deletions

File tree

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// Lock the action-redirect routing (#620). `navigate` for SPA-internal
2+
// paths, `window.location.assign` (injected as `assignLocation` for
3+
// testability) for everything else. The legacy behaviour piped every
4+
// redirect through `navigate` — which silently no-op'd for any URL
5+
// outside the SPA mount.
6+
import { describe, expect, it, vi } from 'vitest';
7+
8+
import { followActionRedirect } from './action-redirect';
9+
10+
const MOUNT = '/admin-react/';
11+
const ORIGIN = 'http://localhost:3000';
12+
13+
function makeArgs(redirect: string) {
14+
const navigate = vi.fn();
15+
const assignLocation = vi.fn();
16+
return {
17+
args: { redirect, mount: MOUNT, navigate, currentOrigin: ORIGIN, assignLocation },
18+
navigate,
19+
assignLocation,
20+
};
21+
}
22+
23+
describe('followActionRedirect', () => {
24+
it('uses navigate for a same-origin path inside the SPA mount', () => {
25+
const { args, navigate, assignLocation } = makeArgs('/admin-react/auth/user/42/');
26+
followActionRedirect(args);
27+
// The mount prefix is stripped so BrowserRouter's basename
28+
// doesn't double up. Trailing slash before the relative path is
29+
// preserved.
30+
expect(navigate).toHaveBeenCalledWith('/auth/user/42/');
31+
expect(assignLocation).not.toHaveBeenCalled();
32+
});
33+
34+
it('preserves search + hash when navigating client-side', () => {
35+
const { args, navigate } = makeArgs('/admin-react/auth/user/42/?tab=audit#log');
36+
followActionRedirect(args);
37+
expect(navigate).toHaveBeenCalledWith('/auth/user/42/?tab=audit#log');
38+
});
39+
40+
it('falls back to assignLocation for a same-origin path OUTSIDE the mount', () => {
41+
// Legacy admin path — must be a full browser navigation since
42+
// React Router only routes within the SPA mount.
43+
const { args, navigate, assignLocation } = makeArgs('/admin/auth/user/42/change/');
44+
followActionRedirect(args);
45+
expect(assignLocation).toHaveBeenCalledWith('/admin/auth/user/42/change/');
46+
expect(navigate).not.toHaveBeenCalled();
47+
});
48+
49+
it('falls back to assignLocation for cross-origin URLs', () => {
50+
// The signed-S3-download case — must be a real browser navigation
51+
// so the download starts on the operator's machine.
52+
const { args, navigate, assignLocation } = makeArgs('https://s3.example.com/signed/file.pdf');
53+
followActionRedirect(args);
54+
expect(assignLocation).toHaveBeenCalledWith('https://s3.example.com/signed/file.pdf');
55+
expect(navigate).not.toHaveBeenCalled();
56+
});
57+
58+
it('falls back to assignLocation for a hijack-style /hijack/... URL', () => {
59+
// Common third-party pattern (`django-hijack`) — action returns
60+
// `HttpResponseRedirect("/hijack/release-user/?next=...")`.
61+
const { args, navigate, assignLocation } = makeArgs('/hijack/release-user/?next=/admin/foo/');
62+
followActionRedirect(args);
63+
expect(assignLocation).toHaveBeenCalledWith('/hijack/release-user/?next=/admin/foo/');
64+
expect(navigate).not.toHaveBeenCalled();
65+
});
66+
67+
it('falls back to assignLocation on an unparseable URL', () => {
68+
// A malformed input shouldn't make the operator's click disappear
69+
// into the void — surface it as a real navigation and let the
70+
// browser report the failure to the user.
71+
//
72+
// Using "http://" alone — bare scheme with no authority — produces
73+
// a TypeError in the URL constructor on all majors.
74+
const { args, navigate, assignLocation } = makeArgs('http://');
75+
followActionRedirect(args);
76+
expect(assignLocation).toHaveBeenCalledWith('http://');
77+
expect(navigate).not.toHaveBeenCalled();
78+
});
79+
});
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
// Action-handler redirect routing (#620). Django ModelAdmin actions
2+
// can return any HttpResponse (HttpResponseRedirect, file streams,
3+
// HTML pages); the API extracts the response's Location header into
4+
// the JSON envelope's `redirect` field. Before this fix the SPA piped
5+
// `redirect` straight into React Router's `navigate` — which is
6+
// scoped to the SPA mount, so any redirect pointing OUTSIDE the SPA
7+
// silently no-op'd: legacy `/admin/.../` URLs, hijack/impersonate
8+
// flows, signed S3 download URLs all looked broken from the operator's
9+
// POV. This helper picks the right navigation primitive per URL.
10+
import type { NavigateFunction } from 'react-router-dom';
11+
12+
/**
13+
* Follow a redirect returned by a ModelAdmin action.
14+
*
15+
* - Same origin AND under the SPA mount → React Router `navigate`
16+
* (no full reload; the operator stays in the SPA).
17+
* - Anything else (off-origin, or same-origin outside the mount) →
18+
* `window.location.assign` (full browser navigation; the only way
19+
* to reach a non-SPA page).
20+
*
21+
* `mount` is the SPA's URL prefix (e.g. `/admin-react/`), already
22+
* read once at boot from the `<meta name="dar-mount">` tag in
23+
* `main.tsx` — pass it through; this helper doesn't query the DOM
24+
* so it stays unit-testable.
25+
*
26+
* If the redirect URL is unparseable (a defensive case — the API
27+
* should always emit a real URL) we fall back to `window.location
28+
* .assign` and let the browser report the failure to the user.
29+
*/
30+
export interface FollowRedirectArgs {
31+
redirect: string;
32+
mount: string;
33+
navigate: NavigateFunction;
34+
/** Current page origin (`http://localhost:3000` etc.). Defaults to
35+
* `window.location.origin` in production; the test injects a known
36+
* value because jsdom's `window.location.origin` is hard-coded. */
37+
currentOrigin?: string;
38+
/** Full-page navigation. Defaults to `window.location.assign` in
39+
* production; the test injects a spy because jsdom's
40+
* `window.location.assign` is non-configurable on most jsdom
41+
* versions and resists `vi.spyOn`. */
42+
assignLocation?: (url: string) => void;
43+
}
44+
45+
export function followActionRedirect(args: FollowRedirectArgs): void {
46+
const { redirect, mount, navigate } = args;
47+
const origin = args.currentOrigin ?? window.location.origin;
48+
const assignLocation = args.assignLocation ?? ((url: string) => window.location.assign(url));
49+
let parsed: URL;
50+
try {
51+
parsed = new URL(redirect, origin);
52+
} catch {
53+
assignLocation(redirect);
54+
return;
55+
}
56+
const sameOrigin = parsed.origin === origin;
57+
const inSpaMount = parsed.pathname.startsWith(mount);
58+
if (sameOrigin && inSpaMount) {
59+
// BrowserRouter is initialised with `basename={mount}` (main.tsx)
60+
// so React Router routes are relative to that. Strip the mount
61+
// prefix before passing — otherwise the basename auto-prepends
62+
// and we get `/admin-react/admin-react/...` which 404s.
63+
const relative = parsed.pathname.slice(mount.length - 1) + parsed.search + parsed.hash;
64+
navigate(relative);
65+
} else {
66+
assignLocation(redirect);
67+
}
68+
}

frontend/apps/web/src/pages/DetailPage.tsx

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import { HistoryModal } from '@dar/history';
5050
import { RecordSkeleton } from '../components/RecordSkeleton';
5151
import { useModelMeta } from '../useModelMeta';
5252
import { toastMessages, useToast } from '../toast';
53+
import { followActionRedirect } from '../action-redirect';
5354
import { carryPreservedFilters, listPathWithPreservedFilters } from '../changelistFilters';
5455
import { useUnsavedGuard } from '../useUnsavedGuard';
5556

@@ -313,7 +314,19 @@ export function DetailPage({
313314
}
314315
onSuccess={async ({ message, messages, redirect }) => {
315316
if (redirect) {
316-
navigate(redirect);
317+
// Smart-route the redirect (#620): an admin action
318+
// can hand back any URL — a same-SPA path, a legacy
319+
// `/admin/.../change/` page, a hijack URL, a signed
320+
// S3 download. React Router's `navigate` only
321+
// routes WITHIN the SPA mount, so non-SPA URLs
322+
// silently no-op'd before. `followActionRedirect`
323+
// picks `navigate` vs `window.location.assign`
324+
// based on origin + mount.
325+
const mountMeta = document
326+
.querySelector<HTMLMetaElement>('meta[name="dar-mount"]')
327+
?.content?.trim();
328+
const mount = (mountMeta ?? '/').replace(/\/?$/, '/');
329+
followActionRedirect({ redirect, mount, navigate });
317330
return;
318331
}
319332
await refresh();

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[tool.poetry]
22
name = "django-admin-react"
3-
version = "1.4.12"
3+
version = "1.4.13"
44
description = "A drop-in React single-page admin for Django, driven entirely by ModelAdmin."
55
authors = ["django-admin-react contributors"]
66
license = "MIT"

0 commit comments

Comments
 (0)