From 848e9a7fa9f1e28fc1ea07613fbead4089859132 Mon Sep 17 00:00:00 2001 From: Alan Buscaglia Date: Thu, 25 Jun 2026 12:40:05 +0200 Subject: [PATCH] fix(ui): stabilize provider wizard modal state --- .../use-provider-wizard-controller.test.tsx | 56 +++++++- .../hooks/use-provider-wizard-controller.ts | 78 +++++++++-- .../wizard/provider-wizard-modal.test.tsx | 126 ++++++++++++++++++ .../wizard/provider-wizard-modal.tsx | 6 +- .../wizard/steps/connect-step.test.tsx | 106 +++++++++++++++ .../providers/wizard/steps/connect-step.tsx | 49 +++++-- ui/components/shadcn/modal/modal.tsx | 5 + 7 files changed, 398 insertions(+), 28 deletions(-) create mode 100644 ui/components/providers/wizard/provider-wizard-modal.test.tsx create mode 100644 ui/components/providers/wizard/steps/connect-step.test.tsx diff --git a/ui/components/providers/wizard/hooks/use-provider-wizard-controller.test.tsx b/ui/components/providers/wizard/hooks/use-provider-wizard-controller.test.tsx index 0d0ce0b0a2..c733025ac7 100644 --- a/ui/components/providers/wizard/hooks/use-provider-wizard-controller.test.tsx +++ b/ui/components/providers/wizard/hooks/use-provider-wizard-controller.test.tsx @@ -9,6 +9,7 @@ import { PROVIDER_WIZARD_STEP, } from "@/types/provider-wizard"; +import { WIZARD_FOOTER_ACTION_TYPE } from "../steps/footer-controls"; import type { ProviderWizardInitialData } from "../types"; import { useProviderWizardController } from "./use-provider-wizard-controller"; @@ -237,7 +238,7 @@ describe("useProviderWizardController", () => { expect(onOpenChange).not.toHaveBeenCalled(); }); - it("closes the wizard after a successful connection test in update mode", async () => { + it("moves to launch step after a successful connection test in update mode", async () => { // Given const onOpenChange = vi.fn(); const { result } = renderHook(() => @@ -265,10 +266,55 @@ describe("useProviderWizardController", () => { result.current.handleTestSuccess(); }); - // Credential rotation skips the launch/schedule step. - expect(onOpenChange).toHaveBeenCalledWith(false); - expect(refreshMock).toHaveBeenCalledTimes(1); - expect(result.current.currentStep).not.toBe(PROVIDER_WIZARD_STEP.LAUNCH); + expect(result.current.currentStep).toBe(PROVIDER_WIZARD_STEP.LAUNCH); + expect(onOpenChange).not.toHaveBeenCalled(); + expect(refreshMock).not.toHaveBeenCalled(); + }); + + it("does not rerender when setting a semantically unchanged footer config", () => { + // Given + const onOpenChange = vi.fn(); + let renderCount = 0; + const { result } = renderHook(() => { + renderCount += 1; + + return useProviderWizardController({ + open: true, + onOpenChange, + }); + }); + + const firstFooterConfig = { + showBack: true, + backLabel: "Back", + onBack: vi.fn(), + showAction: true, + actionLabel: "Next", + actionDisabled: false, + actionType: WIZARD_FOOTER_ACTION_TYPE.BUTTON, + onAction: vi.fn(), + }; + + act(() => { + result.current.setFooterConfig(firstFooterConfig); + }); + const renderCountAfterFirstUpdate = renderCount; + const footerConfigAfterFirstUpdate = result.current.resolvedFooterConfig; + + // When + act(() => { + result.current.setFooterConfig({ + ...firstFooterConfig, + onBack: vi.fn(), + onAction: vi.fn(), + }); + }); + + // Then + expect(renderCount).toBe(renderCountAfterFirstUpdate); + expect(result.current.resolvedFooterConfig).toBe( + footerConfigAfterFirstUpdate, + ); }); it("does not override launch footer config in the controller", () => { diff --git a/ui/components/providers/wizard/hooks/use-provider-wizard-controller.ts b/ui/components/providers/wizard/hooks/use-provider-wizard-controller.ts index 80c2182840..29381b146d 100644 --- a/ui/components/providers/wizard/hooks/use-provider-wizard-controller.ts +++ b/ui/components/providers/wizard/hooks/use-provider-wizard-controller.ts @@ -1,7 +1,13 @@ "use client"; import { useRouter } from "next/navigation"; -import { useEffect, useRef, useState } from "react"; +import { + type Dispatch, + type SetStateAction, + useEffect, + useRef, + useState, +} from "react"; import { DOCS_URLS, getProviderHelpText } from "@/lib/external-urls"; import { isCloud } from "@/lib/shared/env"; @@ -48,6 +54,32 @@ const EMPTY_FOOTER_CONFIG: WizardFooterConfig = { actionType: WIZARD_FOOTER_ACTION_TYPE.BUTTON, }; +function isSameFooterConfig( + current: WizardFooterConfig, + next: WizardFooterConfig, +) { + return ( + current.showBack === next.showBack && + current.backLabel === next.backLabel && + Boolean(current.backDisabled) === Boolean(next.backDisabled) && + Boolean(current.showSecondaryAction) === + Boolean(next.showSecondaryAction) && + (current.secondaryActionLabel ?? "") === + (next.secondaryActionLabel ?? "") && + Boolean(current.secondaryActionDisabled) === + Boolean(next.secondaryActionDisabled) && + current.secondaryActionVariant === next.secondaryActionVariant && + current.secondaryActionType === next.secondaryActionType && + (current.secondaryActionFormId ?? "") === + (next.secondaryActionFormId ?? "") && + current.showAction === next.showAction && + current.actionLabel === next.actionLabel && + Boolean(current.actionDisabled) === Boolean(next.actionDisabled) && + current.actionType === next.actionType && + (current.actionFormId ?? "") === (next.actionFormId ?? "") + ); +} + interface UseProviderWizardControllerProps { open: boolean; onOpenChange: (open: boolean) => void; @@ -82,8 +114,9 @@ export function useProviderWizardController({ const [orgCurrentStep, setOrgCurrentStep] = useState( ORG_WIZARD_STEP.SETUP, ); - const [footerConfig, setFooterConfig] = + const [footerConfig, setFooterConfigState] = useState(EMPTY_FOOTER_CONFIG); + const footerConfigRef = useRef(EMPTY_FOOTER_CONFIG); const [providerTypeHint, setProviderTypeHint] = useState( null, ); @@ -123,7 +156,8 @@ export function useProviderWizardController({ ); setOrgCurrentStep(orgInitialData.targetStep); setOrgSetupPhase(orgInitialData.targetPhase); - setFooterConfig(EMPTY_FOOTER_CONFIG); + footerConfigRef.current = EMPTY_FOOTER_CONFIG; + setFooterConfigState(EMPTY_FOOTER_CONFIG); setProviderTypeHint(null); return; } @@ -146,7 +180,8 @@ export function useProviderWizardController({ ); setCurrentStep(PROVIDER_WIZARD_STEP.CREDENTIALS); setOrgCurrentStep(ORG_WIZARD_STEP.SETUP); - setFooterConfig(EMPTY_FOOTER_CONFIG); + footerConfigRef.current = EMPTY_FOOTER_CONFIG; + setFooterConfigState(EMPTY_FOOTER_CONFIG); setProviderTypeHint(initialProviderType); setOrgSetupPhase(ORG_SETUP_PHASE.DETAILS); return; @@ -157,7 +192,8 @@ export function useProviderWizardController({ setWizardVariant(WIZARD_VARIANT.PROVIDER); setCurrentStep(PROVIDER_WIZARD_STEP.CONNECT); setOrgCurrentStep(ORG_WIZARD_STEP.SETUP); - setFooterConfig(EMPTY_FOOTER_CONFIG); + footerConfigRef.current = EMPTY_FOOTER_CONFIG; + setFooterConfigState(EMPTY_FOOTER_CONFIG); setProviderTypeHint(null); setOrgSetupPhase(ORG_SETUP_PHASE.DETAILS); }, [ @@ -194,7 +230,8 @@ export function useProviderWizardController({ setWizardVariant(WIZARD_VARIANT.PROVIDER); setCurrentStep(PROVIDER_WIZARD_STEP.CONNECT); setOrgCurrentStep(ORG_WIZARD_STEP.SETUP); - setFooterConfig(EMPTY_FOOTER_CONFIG); + footerConfigRef.current = EMPTY_FOOTER_CONFIG; + setFooterConfigState(EMPTY_FOOTER_CONFIG); setProviderTypeHint(null); setOrgSetupPhase(ORG_SETUP_PHASE.DETAILS); onOpenChange(false); @@ -220,13 +257,24 @@ export function useProviderWizardController({ }; const handleTestSuccess = () => { - if ( - useProviderWizardStore.getState().mode === PROVIDER_WIZARD_MODE.UPDATE - ) { - handleClose(); + setCurrentStep(PROVIDER_WIZARD_STEP.LAUNCH); + }; + + const updateFooterConfig: Dispatch> = ( + nextFooterConfig, + ) => { + const currentFooterConfig = footerConfigRef.current; + const resolvedNextFooterConfig = + typeof nextFooterConfig === "function" + ? nextFooterConfig(currentFooterConfig) + : nextFooterConfig; + + if (isSameFooterConfig(currentFooterConfig, resolvedNextFooterConfig)) { return; } - setCurrentStep(PROVIDER_WIZARD_STEP.LAUNCH); + + footerConfigRef.current = resolvedNextFooterConfig; + setFooterConfigState(resolvedNextFooterConfig); }; const openOrganizationsFlow = () => { @@ -236,7 +284,8 @@ export function useProviderWizardController({ resetOrgWizard(); setWizardVariant(WIZARD_VARIANT.ORGANIZATIONS); setOrgCurrentStep(ORG_WIZARD_STEP.SETUP); - setFooterConfig(EMPTY_FOOTER_CONFIG); + footerConfigRef.current = EMPTY_FOOTER_CONFIG; + setFooterConfigState(EMPTY_FOOTER_CONFIG); setProviderTypeHint(null); setOrgSetupPhase(ORG_SETUP_PHASE.DETAILS); }; @@ -245,7 +294,8 @@ export function useProviderWizardController({ resetOrgWizard(); setWizardVariant(WIZARD_VARIANT.PROVIDER); setCurrentStep(PROVIDER_WIZARD_STEP.CONNECT); - setFooterConfig(EMPTY_FOOTER_CONFIG); + footerConfigRef.current = EMPTY_FOOTER_CONFIG; + setFooterConfigState(EMPTY_FOOTER_CONFIG); setProviderTypeHint(null); setOrgSetupPhase(ORG_SETUP_PHASE.DETAILS); }; @@ -274,7 +324,7 @@ export function useProviderWizardController({ providerTypeHint, resolvedFooterConfig, setCurrentStep, - setFooterConfig, + setFooterConfig: updateFooterConfig, setOrgCurrentStep, setOrgSetupPhase, setProviderTypeHint, diff --git a/ui/components/providers/wizard/provider-wizard-modal.test.tsx b/ui/components/providers/wizard/provider-wizard-modal.test.tsx new file mode 100644 index 0000000000..6509de65a4 --- /dev/null +++ b/ui/components/providers/wizard/provider-wizard-modal.test.tsx @@ -0,0 +1,126 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { useOrgSetupStore } from "@/store/organizations/store"; +import { useProviderWizardStore } from "@/store/provider-wizard/store"; +import { PROVIDER_WIZARD_MODE } from "@/types/provider-wizard"; + +import { ProviderWizardModal } from "./provider-wizard-modal"; + +vi.mock("next/navigation", () => ({ + useRouter: () => ({ + refresh: vi.fn(), + }), +})); + +vi.mock("@/hooks/use-scroll-hint", () => ({ + useScrollHint: () => ({ + containerRef: vi.fn(), + sentinelRef: vi.fn(), + showScrollHint: false, + }), +})); + +vi.mock("@/components/providers/wizard/steps/connect-step", () => ({ + ConnectStep: () =>
Connect step
, +})); + +vi.mock("@/components/providers/wizard/steps/credentials-step", () => ({ + CredentialsStep: ({ onNext }: { onNext: () => void }) => ( +
+
Credentials step
+ +
+ ), +})); + +vi.mock("@/components/providers/wizard/steps/test-connection-step", () => ({ + TestConnectionStep: ({ onSuccess }: { onSuccess: () => void }) => ( +
+
Test connection step
+ +
+ ), +})); + +vi.mock("@/components/providers/wizard/steps/launch-step", () => ({ + LaunchStep: () =>
Launch step
, +})); + +vi.mock("@/components/providers/organizations/org-setup-form", () => ({ + OrgSetupForm: () =>
Organization setup
, +})); + +vi.mock("@/components/providers/organizations/org-account-selection", () => ({ + OrgAccountSelection: () =>
Organization account selection
, +})); + +vi.mock("@/components/providers/organizations/org-launch-scan", () => ({ + OrgLaunchScan: () =>
Organization launch scan
, +})); + +describe("ProviderWizardModal", () => { + beforeEach(() => { + sessionStorage.clear(); + localStorage.clear(); + useProviderWizardStore.getState().reset(); + useOrgSetupStore.getState().reset(); + }); + + it("provides an accessible dialog description without requiring visible helper text", () => { + // Given + const onOpenChange = vi.fn(); + + // When + render(); + + // Then + const dialog = screen.getByRole("dialog", { name: /adding a provider/i }); + const descriptionId = dialog.getAttribute("aria-describedby"); + + expect(descriptionId).toBeTruthy(); + expect(document.getElementById(descriptionId ?? "")).toHaveTextContent( + /connect or update a provider/i, + ); + }); + + it("shows the launch progress step when update mode reaches launch", async () => { + // Given + const user = userEvent.setup(); + const onOpenChange = vi.fn(); + + render( + , + ); + expect(await screen.findByText("Credentials step")).toBeVisible(); + + // When + await user.click( + screen.getByRole("button", { name: /continue to validate connection/i }), + ); + await user.click( + await screen.findByRole("button", { name: /check connection/i }), + ); + + // Then + expect(screen.getByText("Launch step")).toBeVisible(); + expect(screen.getByText("Launch Scan")).toBeVisible(); + expect(onOpenChange).not.toHaveBeenCalledWith(false); + }); +}); diff --git a/ui/components/providers/wizard/provider-wizard-modal.tsx b/ui/components/providers/wizard/provider-wizard-modal.tsx index ec6f60fe64..4c20b0f51c 100644 --- a/ui/components/providers/wizard/provider-wizard-modal.tsx +++ b/ui/components/providers/wizard/provider-wizard-modal.tsx @@ -33,9 +33,12 @@ import { PROVIDER_WIZARD_STEPS, WizardStepper } from "./wizard-stepper"; const UPDATE_MODE_WIZARD_STEPS = PROVIDER_WIZARD_STEPS.slice( 0, - PROVIDER_WIZARD_STEP.LAUNCH, + PROVIDER_WIZARD_STEP.LAUNCH + 1, ); +const PROVIDER_WIZARD_MODAL_DESCRIPTION = + "Connect or update a provider by adding account details, credentials, testing the connection, and launching a scan."; + interface ProviderWizardModalProps { open: boolean; onOpenChange: (open: boolean) => void; @@ -100,6 +103,7 @@ export function ProviderWizardModal({ diff --git a/ui/components/providers/wizard/steps/connect-step.test.tsx b/ui/components/providers/wizard/steps/connect-step.test.tsx new file mode 100644 index 0000000000..6401b9bd34 --- /dev/null +++ b/ui/components/providers/wizard/steps/connect-step.test.tsx @@ -0,0 +1,106 @@ +import { act, render, waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { useProviderWizardStore } from "@/store/provider-wizard/store"; + +import { ConnectStep } from "./connect-step"; + +type ConnectStepUiState = { + showBack: boolean; + showAction: boolean; + actionLabel: string; + actionDisabled: boolean; + isLoading: boolean; +}; + +type CapturedConnectAccountFormProps = { + onUiStateChange?: (state: ConnectStepUiState) => void; +}; + +const { capturedConnectAccountFormProps } = vi.hoisted(() => ({ + capturedConnectAccountFormProps: { + current: null as CapturedConnectAccountFormProps | null, + }, +})); + +vi.mock("@/components/providers/workflow/forms", () => ({ + ConnectAccountForm: (props: CapturedConnectAccountFormProps) => { + capturedConnectAccountFormProps.current = props; + + return
; + }, +})); + +describe("ConnectStep", () => { + beforeEach(() => { + sessionStorage.clear(); + localStorage.clear(); + capturedConnectAccountFormProps.current = null; + useProviderWizardStore.getState().reset(); + }); + + it("does not publish a new footer config when form UI state is unchanged", async () => { + // Given + const onFooterChange = vi.fn(); + + render( + , + ); + + await waitFor(() => expect(onFooterChange).toHaveBeenCalledTimes(1)); + + // When + act(() => { + capturedConnectAccountFormProps.current?.onUiStateChange?.({ + showBack: false, + showAction: false, + actionLabel: "Next", + actionDisabled: true, + isLoading: false, + }); + }); + + // Then + expect(onFooterChange).toHaveBeenCalledTimes(1); + }); + + it("publishes a new footer config when form UI state changes", async () => { + // Given + const onFooterChange = vi.fn(); + + render( + , + ); + + await waitFor(() => expect(onFooterChange).toHaveBeenCalledTimes(1)); + + // When + act(() => { + capturedConnectAccountFormProps.current?.onUiStateChange?.({ + showBack: true, + showAction: true, + actionLabel: "Next", + actionDisabled: false, + isLoading: false, + }); + }); + + // Then + await waitFor(() => expect(onFooterChange).toHaveBeenCalledTimes(2)); + expect(onFooterChange.mock.calls.at(-1)?.[0]).toMatchObject({ + showBack: true, + showAction: true, + actionDisabled: false, + }); + }); +}); diff --git a/ui/components/providers/wizard/steps/connect-step.tsx b/ui/components/providers/wizard/steps/connect-step.tsx index ab040c1e43..75231fd28d 100644 --- a/ui/components/providers/wizard/steps/connect-step.tsx +++ b/ui/components/providers/wizard/steps/connect-step.tsx @@ -15,6 +15,35 @@ import { WizardFooterConfig, } from "./footer-controls"; +type ConnectStepUiState = { + showBack: boolean; + showAction: boolean; + actionLabel: string; + actionDisabled: boolean; + isLoading: boolean; +}; + +const CONNECT_STEP_INITIAL_UI_STATE: ConnectStepUiState = { + showBack: false, + showAction: false, + actionLabel: "Next", + actionDisabled: true, + isLoading: false, +}; + +function isSameConnectStepUiState( + current: ConnectStepUiState, + next: ConnectStepUiState, +) { + return ( + current.showBack === next.showBack && + current.showAction === next.showAction && + current.actionLabel === next.actionLabel && + current.actionDisabled === next.actionDisabled && + current.isLoading === next.isLoading + ); +} + interface ConnectStepProps { onNext: () => void; onSelectOrganizations: () => void; @@ -31,13 +60,7 @@ export function ConnectStep({ const { setProvider, setVia, setSecretId, setMode } = useProviderWizardStore(); const backHandlerRef = useRef<(() => void) | null>(null); - const [uiState, setUiState] = useState({ - showBack: false, - showAction: false, - actionLabel: "Next", - actionDisabled: true, - isLoading: false, - }); + const [uiState, setUiState] = useState(CONNECT_STEP_INITIAL_UI_STATE); const formId = "provider-wizard-connect-form"; @@ -54,6 +77,16 @@ export function ConnectStep({ onNext(); }; + const handleUiStateChange = (nextUiState: ConnectStepUiState) => { + setUiState((currentUiState) => { + if (isSameConnectStepUiState(currentUiState, nextUiState)) { + return currentUiState; + } + + return nextUiState; + }); + }; + useEffect(() => { onFooterChange({ showBack: uiState.showBack, @@ -75,7 +108,7 @@ export function ConnectStep({ onSuccess={handleSuccess} onSelectOrganizations={onSelectOrganizations} onProviderTypeChange={onProviderTypeChange} - onUiStateChange={setUiState} + onUiStateChange={handleUiStateChange} onBackHandlerChange={(handler) => { backHandlerRef.current = handler; }} diff --git a/ui/components/shadcn/modal/modal.tsx b/ui/components/shadcn/modal/modal.tsx index 7249b99f9d..0d6eb3842f 100644 --- a/ui/components/shadcn/modal/modal.tsx +++ b/ui/components/shadcn/modal/modal.tsx @@ -65,6 +65,11 @@ export const Modal = ({ )} )} + {!title && description && ( + + {description} + + )} {children}