From 1a9a81f04f43736df9d4265468f4e14fb0dd775a Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Sun, 11 Oct 2026 01:40:09 -0400 Subject: [PATCH] fix(clusters): let Back leave the new-cluster page after a redirect A saved cluster draft makes the clusters list redirect to the new-cluster form, and that redirect pushed a history entry. Back from the form then landed on the list, a page the user never saw, and while the form was still loading (before it clears the draft) the list redirected straight back to the form, so Back did nothing at all. Redirect in place instead. Billing's Stripe return page had the same shape: it strips the setup-intent secret from its own URL and then pushed the form, so Back from the form landed on a confirm page with no secret, which spins forever. It now replaces itself too. Refs #1326 Co-Authored-By: Claude Opus 5.5 --- src/features/clusters/ClustersList.test.tsx | 6 +- src/features/clusters/ClustersList.tsx | 2 +- src/features/clusters/DESIGN.md | 5 ++ .../confirm/ProcessSetupIntent.test.tsx | 55 +++++++++++++++++++ .../billing/confirm/ProcessSetupIntent.tsx | 2 +- 5 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 src/features/organization/billing/confirm/ProcessSetupIntent.test.tsx diff --git a/src/features/clusters/ClustersList.test.tsx b/src/features/clusters/ClustersList.test.tsx index 6b20eaa08..f9fe02e3e 100644 --- a/src/features/clusters/ClustersList.test.tsx +++ b/src/features/clusters/ClustersList.test.tsx @@ -22,7 +22,7 @@ vi.mock( vi.mock('@tanstack/react-router', () => ({ useParams: () => ({ organizationId: 'org-a' }), Link: ({ children, to, ...props }: { children: ReactNode; to: string }) => {children}, - Navigate: ({ to }: { to: string }) =>

Navigate {to}

, + Navigate: ({ to, replace }: { to: string; replace?: boolean }) =>

{replace ? 'Replace' : 'Push'} {to}

, })); vi.mock('@/components/SubNavMenu', () => ({ SubNavMenu: () => null })); vi.mock( @@ -109,10 +109,10 @@ describe('ClustersList', () => { expect(screen.getByRole('link', { name: 'New Cluster' }).getAttribute('href')).toBe('/org-a/new-cluster'); }); - it('preserves a saved cluster creation redirect', () => { + it('redirects a saved cluster draft in place, so Back skips the page that redirected (#1326)', () => { state.clusters = [cluster('Production', 'RUNNING', 'US East')]; state.saved = { name: 'Draft' }; render(); - expect(screen.getByText('Navigate /org-a/new-cluster')).toBeTruthy(); + expect(screen.getByText('Replace /org-a/new-cluster')).toBeTruthy(); }); }); diff --git a/src/features/clusters/ClustersList.tsx b/src/features/clusters/ClustersList.tsx index d54f9d117..6f55a7e03 100644 --- a/src/features/clusters/ClustersList.tsx +++ b/src/features/clusters/ClustersList.tsx @@ -58,7 +58,7 @@ export function ClustersList() { return ; } if (savedClusterState) { - return ; + return ; } return ( diff --git a/src/features/clusters/DESIGN.md b/src/features/clusters/DESIGN.md index ecea1d316..16bb0b7b1 100644 --- a/src/features/clusters/DESIGN.md +++ b/src/features/clusters/DESIGN.md @@ -2,6 +2,11 @@ `lib/clusterListModel.ts` derives lifecycle categories, counts and filters from the same organization snapshot; its tests pin missing-data and self-hosted semantics. A running lifecycle is not a monitoring-health assertion. Regions combine resolved plan labels with available non-retired instance labels, deduplicating matches, because organization responses need not expand instances; the list resolves opaque ids through the existing organization-scoped region catalog once, without polling. Missing labels remain unreported without discarding other reported instance regions. Running classification uses the same activeClusterStatuses rule as card navigation; UPDATED remains attention until RUNNING is reported. The model still derives instance count and version when instance data is supplied, but the card no longer renders them: the organization response does not currently expand instances, so those two cells only ever showed a dash, and the Regions cell (real plan labels) went with the row. The list adds no per-card detail polling. Existing `ClusterProgress` retains its lifecycle polling behavior. +A saved cluster draft makes the list redirect to the form, and that redirect replaces its history entry (#1326). The +form clears the draft only once it mounts, so a pushed redirect left Back on the list, which pushed the form again for +as long as the form was still loading. Billing's Stripe return page replaces itself on the way back to the form for the +same reason: it strips the setup-intent secret from its own URL first, so going back to it would show a spinner forever. + SystemStatus notices have no cluster association and remain in the cloud-wide notification banner/center. Per-cluster monitoring incidents and scheduled maintenance need a scoped backend contract before the list can attribute them. The UI preserves ClusterCard's permission checks and lifecycle actions. # Cluster configuration diff --git a/src/features/organization/billing/confirm/ProcessSetupIntent.test.tsx b/src/features/organization/billing/confirm/ProcessSetupIntent.test.tsx new file mode 100644 index 000000000..bcc16d7a0 --- /dev/null +++ b/src/features/organization/billing/confirm/ProcessSetupIntent.test.tsx @@ -0,0 +1,55 @@ +/** @vitest-environment jsdom */ +import { cleanup, render, waitFor } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { ProcessSetupIntent } from './ProcessSetupIntent'; + +const state = vi.hoisted(() => ({ + navigate: vi.fn(), + saved: null as unknown, + setupIntent: {} as { status: string; payment_method: string | null }, + stripe: { retrieveSetupIntent: vi.fn() }, +})); +vi.mock('@stripe/react-stripe-js', () => ({ useStripe: () => state.stripe })); +vi.mock('@tanstack/react-router', () => ({ + useNavigate: () => state.navigate, + useParams: () => ({ organizationId: 'org-a' }), + useSearch: () => ({}), +})); +vi.mock('@/hooks/useLocalStorage', () => ({ useLocalStorage: () => [state.saved] })); +vi.mock('@/integrations/stripe/useProcessStripePaymentMethod', () => { + const process = (_paymentMethod: string, onProcessed: () => void) => onProcessed(); + return { useProcessStripePaymentMethod: () => process }; +}); + +afterEach(() => { + cleanup(); + state.navigate.mockReset(); + state.stripe.retrieveSetupIntent.mockReset(); + state.saved = null; +}); + +function returnFromStripe(setupIntent: { status: string; payment_method: string | null }) { + window.history.pushState(null, '', '/?setup_intent_client_secret=seti_secret#/org-a/billing/confirm'); + state.stripe.retrieveSetupIntent.mockResolvedValue({ setupIntent }); + render(); +} + +describe('ProcessSetupIntent', () => { + it('replaces itself with the cluster draft, so Back from the form skips this page (#1326)', async () => { + state.saved = { clusterName: 'Draft' }; + returnFromStripe({ status: 'succeeded', payment_method: 'pm_card' }); + + await waitFor(() => expect(state.navigate).toHaveBeenCalled()); + expect(state.navigate).toHaveBeenCalledWith({ search: undefined, to: '../../new-cluster', replace: true }); + expect(window.location.search).toBe(''); + expect(window.location.hash).toBe('#/org-a/billing/confirm'); + }); + + it('replaces itself on the way back to a cluster edit too', async () => { + state.saved = { clusterId: 'clu-a' }; + returnFromStripe({ status: 'processing', payment_method: null }); + + await waitFor(() => expect(state.navigate).toHaveBeenCalled()); + expect(state.navigate).toHaveBeenCalledWith({ search: undefined, to: '../../clu-a/edit', replace: true }); + }); +}); diff --git a/src/features/organization/billing/confirm/ProcessSetupIntent.tsx b/src/features/organization/billing/confirm/ProcessSetupIntent.tsx index c6e804062..3cd766cc4 100644 --- a/src/features/organization/billing/confirm/ProcessSetupIntent.tsx +++ b/src/features/organization/billing/confirm/ProcessSetupIntent.tsx @@ -32,7 +32,7 @@ export function ProcessSetupIntent() { : '../../new-cluster' : '../'; window.history.replaceState(null, '', currentUrlIncludingHash()); - void navigate({ search: undefined, to }); + void navigate({ search: undefined, to, replace: true }); }, [navigate, savedClusterState]); useEffect(() => {