fix(spa): list-action redirect refresh + popup-blocked fallback (#644) + 1.5.1

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

@@ -41,6 +41,7 @@ import { FilterBar } from '@dar/search';
 
 import { useToast } from '../toast';
 import { CHANGELIST_FILTERS_PARAM, withPreservedFilters } from '../changelistFilters';
+import { handleActionResult } from './action-result';
 
 // Lazy-loaded so the @dnd-kit suite (the heaviest dep in this modal)
 // only lands in the bundle of users who open the Customize modal
@@ -132,6 +133,14 @@ export function ListPage() {
     lockedColsKey(appLabel, modelName),
   );
   const [colsOpen, setColsOpen] = useState(false);
+  // Popup-blocked fallback (#644). When an action returns a redirect
+  // and `window.open` is suppressed by the browser's popup blocker
+  // (the call isn't a direct user gesture by then — we're inside an
+  // async handler after the POST resolves), surface a banner with a
+  // clickable link so the redirect isn't silently swallowed.
+  const [pendingRedirect, setPendingRedirect] = useState<{ url: string; label: string } | null>(
+    null,
+  );
 
   // Drag-to-resize column widths, persisted per app/model (a UI
   // preference, like hidden columns) via @dar/customization. The Table
@@ -334,31 +343,18 @@ export function ListPage() {
       const result = await client.runAction(appLabel, modelName, action.name, pks);
       setSelected(new Set());
       setSelectAcross(false);
-      // Intermediate / form-returning action (#250). When the admin action
-      // returns an HttpResponse (e.g. Django's delete-selected, or a
-      // custom action that needs a confirmation/parameter page), the
-      // backend forwards its Location as `redirect`. Open it in a new tab
-      // so the operator can complete the flow there — the SPA stays
-      // mounted, no silent no-op. (Parameterised in-SPA forms are a
-      // follow-up; this closes the "silent no-op" minimum.)
-      if (result.redirect) {
-        window.open(result.redirect, '_blank', 'noopener,noreferrer');
-        toast.info(`${action.label} opened in a new tab.`);
-        return;
-      }
-      await refresh();
-      // Prefer the action's own message_user output (#442); fall back to a
-      // generic confirmation when the action queued nothing.
-      const msgs = result.messages ?? [];
-      if (msgs.length > 0) {
-        for (const m of msgs) {
-          if (m.level === 'error' || m.level === 'warning') toast.error(m.message);
-          else if (m.level === 'info' || m.level === 'debug') toast.info(m.message);
-          else toast.success(m.message);
-        }
-      } else {
-        toast.success(`${action.label} — ${count} item${count === 1 ? '' : 's'}.`);
-      }
+      // Per-result dispatch lives in `handleActionResult` so the
+      // popup-blocked + refresh + level-toast flow is unit-testable
+      // without rendering the whole page (#250, #442, #632, #644).
+      await handleActionResult({
+        result,
+        action,
+        count,
+        toast,
+        refresh,
+        openLink: (url) => window.open(url, '_blank', 'noopener,noreferrer'),
+        onPopupBlocked: (info) => setPendingRedirect(info),
+      });
     } catch (e) {
       toast.error(e instanceof Error ? e.message : 'Action failed.');
     } finally {
@@ -661,6 +657,34 @@ export function ListPage() {
         </div>
       )}
 
+      {/* Popup-blocked fallback for an action redirect (#644). If
+          window.open returns null (the browser suppressed the
+          new-tab because we're past the direct-user-gesture moment),
+          render a clickable link banner so the redirect URL isn't
+          silently swallowed. The user opens it manually with a real
+          click; dismiss closes the banner. Amber = "this needs your
+          attention but it isn't an error." */}
+      {pendingRedirect && (
+        <div className="flex items-center justify-between rounded-md border border-amber-300 bg-amber-50 px-3 py-2 text-sm text-amber-800">
+          <span>
+            {pendingRedirect.label} wants to open a page —{' '}
+            <a
+              href={pendingRedirect.url}
+              target="_blank"
+              rel="noopener noreferrer"
+              className="font-medium underline decoration-amber-500 underline-offset-2 hover:decoration-amber-700"
+              onClick={() => setPendingRedirect(null)}
+            >
+              click to open in a new tab
+            </a>
+            .
+          </span>
+          <Button variant="secondary" onClick={() => setPendingRedirect(null)}>
+            Dismiss
+          </Button>
+        </div>
+      )}
+
       {/* Select-all-across-pages (#386). Once the whole page is ticked and
           more rows match, offer to extend the selection to the entire
           filtered set (Django changelist parity); while active, say so and

frontend/apps/web/src/pages/action-result.test.ts

@@ -0,0 +1,130 @@
+// Lock the bulk-action result handling (#250 / #442 / #632 / #644):
+//
+//   1. redirect → `openLink` opens new tab → info toast
+//   2. redirect → `openLink` returns null (popup blocked) →
+//      pendingRedirect banner, NOT silently swallowed
+//   3. refresh always fires (BOTH redirect and no-redirect paths)
+//   4. messages[] toasts by level (error/warning → red,
+//      info/debug → blue, success → green)
+//   5. No messages + no redirect → generic "<action> — N item(s)" toast
+//   6. No messages + WITH redirect → no generic toast (the redirect's
+//      destination surfaces the next-step UX)
+
+import { describe, expect, it, vi } from 'vitest';
+
+import type { ActionDescriptor, ActionRunResponse } from '@dar/data';
+
+import { handleActionResult } from './action-result';
+
+const action: ActionDescriptor = {
+  name: 'export',
+  label: 'Export selected',
+  description: '',
+  requires_confirmation: false,
+};
+
+function makeArgs(
+  result: ActionRunResponse,
+  overrides: { openReturns?: Window | null } = {},
+) {
+  const toast = {
+    success: vi.fn<(m: string) => void>(),
+    error: vi.fn<(m: string) => void>(),
+    info: vi.fn<(m: string) => void>(),
+  };
+  const refresh = vi.fn<() => Promise<void>>(async () => {});
+  // `openReturns` may be `null` (popup blocked); `??` would
+  // coalesce a real null away, so check for `undefined` (omitted).
+  const openLink = vi.fn<(url: string) => Window | null>(
+    () => ('openReturns' in overrides ? overrides.openReturns! : ({} as Window)),
+  );
+  const onPopupBlocked = vi.fn<(info: { url: string; label: string }) => void>();
+  return {
+    args: { result, action, count: 3, toast, refresh, openLink, onPopupBlocked },
+    toast,
+    refresh,
+    openLink,
+    onPopupBlocked,
+  };
+}
+
+describe('handleActionResult', () => {
+  it('opens redirect in a new tab and shows an info toast', async () => {
+    const { args, toast, openLink, onPopupBlocked } = makeArgs({
+      executed: true,
+      action: 'export',
+      redirect: '/exports/build/?ids=1,2,3',
+    });
+    await handleActionResult(args);
+    expect(openLink).toHaveBeenCalledWith('/exports/build/?ids=1,2,3');
+    expect(toast.info).toHaveBeenCalledWith('Export selected opened in a new tab.');
+    expect(onPopupBlocked).not.toHaveBeenCalled();
+  });
+
+  it('surfaces a pendingRedirect when window.open is suppressed (popup blocker)', async () => {
+    const { args, toast, openLink, onPopupBlocked } = makeArgs(
+      { executed: true, action: 'export', redirect: 'https://exports.example/123' },
+      { openReturns: null },
+    );
+    await handleActionResult(args);
+    expect(openLink).toHaveBeenCalledOnce();
+    expect(onPopupBlocked).toHaveBeenCalledWith({
+      url: 'https://exports.example/123',
+      label: 'Export selected',
+    });
+    // Crucially: no info toast in this path — the URL would be lost
+    // without the banner the caller renders from the pendingRedirect.
+    expect(toast.info).not.toHaveBeenCalled();
+  });
+
+  it('refreshes the changelist on BOTH the redirect and no-redirect paths', async () => {
+    const withRedirect = makeArgs({ executed: true, action: 'export', redirect: '/x/' });
+    await handleActionResult(withRedirect.args);
+    expect(withRedirect.refresh).toHaveBeenCalledOnce();
+
+    const noRedirect = makeArgs({ executed: true, action: 'export' });
+    await handleActionResult(noRedirect.args);
+    expect(noRedirect.refresh).toHaveBeenCalledOnce();
+  });
+
+  it('toasts each messages[] entry by level (error → red, info → blue, success → green)', async () => {
+    const { args, toast } = makeArgs({
+      executed: true,
+      action: 'export',
+      messages: [
+        { level: 'error', message: 'one failed' },
+        { level: 'info', message: 'two queued' },
+        { level: 'success', message: 'three done' },
+      ],
+    });
+    await handleActionResult(args);
+    expect(toast.error).toHaveBeenCalledWith('one failed');
+    expect(toast.info).toHaveBeenCalledWith('two queued');
+    expect(toast.success).toHaveBeenCalledWith('three done');
+  });
+
+  it('falls back to a generic success toast when no messages and no redirect', async () => {
+    const { args, toast } = makeArgs({ executed: true, action: 'export' });
+    await handleActionResult(args);
+    expect(toast.success).toHaveBeenCalledWith('Export selected — 3 items.');
+  });
+
+  it('uses singular "item" when count is 1', async () => {
+    const { args, toast } = makeArgs({ executed: true, action: 'export' });
+    args.count = 1;
+    await handleActionResult(args);
+    expect(toast.success).toHaveBeenCalledWith('Export selected — 1 item.');
+  });
+
+  it('does NOT emit the generic toast when the action redirected (the destination shows the next-step UX)', async () => {
+    const { args, toast } = makeArgs({
+      executed: true,
+      action: 'export',
+      redirect: '/exports/build/',
+    });
+    await handleActionResult(args);
+    // info toast for the new-tab open is fine; the generic
+    // "— N item(s)" success toast must NOT fire.
+    expect(toast.success).not.toHaveBeenCalled();
+  });
+});

frontend/apps/web/src/pages/action-result.ts

@@ -0,0 +1,61 @@
+// handleActionResult — what to do with the JSON envelope a
+// changelist bulk-action returns (#250, #442, #632, #644). Pure
+// function so the redirect / popup-blocked / messages flow is
+// unit-testable without rendering the whole ListPage.
+//
+// Mirrors Django admin's `ModelAdmin.response_action` contract:
+//
+//   - An action that returns `HttpResponse` (typically a
+//     `HttpResponseRedirect` to an intermediate / confirmation
+//     page, an export wizard, a hijack flow) → the API surfaces
+//     the `Location` as `result.redirect`. Open it in a NEW TAB
+//     (`noopener,noreferrer`) so the changelist stays in view.
+//   - Popup blocker eats `window.open` → return a
+//     `{url, label}` pendingRedirect for the caller to render as a
+//     clickable banner; the redirect must never be silently lost.
+//   - Refresh the changelist on BOTH branches so any side-effects
+//     the action had on the rows are visible.
+//   - Messages with a `level` tag drive the toast colour (#632).
+//   - When there are no messages AND no redirect, fall back to a
+//     generic "<Action> — N item(s)." success toast — a redirect's
+//     own destination usually surfaces next-step UX, no extra
+//     "Done" toast needed for that path.
+
+import type { ActionDescriptor, ActionRunResponse } from '@dar/data';
+
+import type { ToastApi } from '../toast';
+import { toastMessages } from '../toast';
+
+export interface HandleActionResultArgs {
+  result: ActionRunResponse;
+  action: ActionDescriptor;
+  count: number;
+  toast: ToastApi;
+  refresh: () => Promise<void>;
+  /** Injected for testability. Production: `(url) =>
+   *  window.open(url, '_blank', 'noopener,noreferrer')`. Returns
+   *  `null` when the popup blocker suppresses the call. */
+  openLink: (url: string) => Window | null;
+  /** Called when `openLink` returns `null`. Caller renders a
+   *  clickable fallback banner so the redirect URL isn't lost. */
+  onPopupBlocked: (info: { url: string; label: string }) => void;
+}
+
+export async function handleActionResult(args: HandleActionResultArgs): Promise<void> {
+  const { result, action, count, toast, refresh, openLink, onPopupBlocked } = args;
+  if (result.redirect) {
+    const opened = openLink(result.redirect);
+    if (opened) {
+      toast.info(`${action.label} opened in a new tab.`);
+    } else {
+      onPopupBlocked({ url: result.redirect, label: action.label });
+    }
+  }
+  await refresh();
+  const msgs = result.messages ?? [];
+  if (msgs.length > 0) {
+    toastMessages(toast, msgs);
+  } else if (!result.redirect) {
+    toast.success(`${action.label} — ${count} item${count === 1 ? '' : 's'}.`);
+  }
+}

frontend/apps/web/src/toast.tsx

@@ -18,7 +18,7 @@ interface Toast {
   message: string;
 }
 
-interface ToastApi {
+export interface ToastApi {
   success: (message: string) => void;
   error: (message: string) => void;
   info: (message: string) => void;

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "django-admin-react"
-version = "1.5.0"
+version = "1.5.1"
 description = "A drop-in React single-page admin for Django, driven entirely by ModelAdmin."
 authors = ["django-admin-react contributors"]
 license = "MIT"