fix(clusters): close out the remaining engine v2 review threads

libs/domains/clusters/feature/src/lib/cluster-add/cluster-add.spec.tsx

@@ -3,11 +3,13 @@ import { renderWithProviders, screen, waitFor } from '@qovery/shared/util-tests'
 import { type SelfManagedClusterCreationFlowProps } from '../self-managed-cluster-creation/self-managed-cluster-creation-flow'
 import { ClusterAdd } from './cluster-add'
 
+const mockUseBlocker = jest.fn()
 jest.mock('@tanstack/react-router', () => {
   const React = jest.requireActual('react')
   return {
     ...jest.requireActual('@tanstack/react-router'),
     useParams: () => ({ organizationId: 'org-123' }),
+    useBlocker: (options: unknown) => mockUseBlocker(options),
     Link: React.forwardRef(
       ({ children, ...props }: { children?: React.ReactNode }, ref: React.Ref<HTMLAnchorElement>) =>
         React.createElement('a', { ref, ...props }, children)
@@ -42,7 +44,12 @@ jest.mock('../self-managed-cluster-creation/self-managed-cluster-creation-flow',
   ),
 }))
 
+type BlockerOptions = { disabled: boolean; shouldBlockFn: () => boolean; enableBeforeUnload: boolean }
+const getBlocker = () => mockUseBlocker.mock.calls.at(-1)?.[0] as BlockerOptions
+
 describe('ClusterAdd', () => {
+  afterEach(() => jest.restoreAllMocks())
+
   async function openCreationFlow(userEvent: ReturnType<typeof renderWithProviders>['userEvent']) {
     await userEvent.click(screen.getByRole('button', { name: 'AWS' }))
     expect(await screen.findByText('Creation flow')).toBeInTheDocument()
@@ -76,6 +83,7 @@ describe('ClusterAdd', () => {
       selectedCloudProvider: 'AWS',
       selectedInstallationType: 'self-managed',
     })
+    expect(screen.getByRole('button', { name: 'Close' })).toBeInTheDocument()
     await userEvent.keyboard('{Escape}')
 
     await waitFor(() => expect(screen.queryByText('Creation flow')).not.toBeInTheDocument())
@@ -89,10 +97,49 @@ describe('ClusterAdd', () => {
     await userEvent.keyboard('{Escape}')
 
     expect(screen.getByText('Creation flow')).toBeInTheDocument()
-    expect(document.querySelector('.fa-xmark')).not.toBeInTheDocument()
+    expect(screen.queryByRole('button', { name: 'Close' })).not.toBeInTheDocument()
 
     await userEvent.click(screen.getByRole('button', { name: 'Operator connected' }))
 
     await waitFor(() => expect(screen.queryByText('Creation flow')).not.toBeInTheDocument())
   })
+
+  describe('leaving the install step', () => {
+    it('does not guard the page before the cluster is created', async () => {
+      const { userEvent } = renderWithProviders(<ClusterAdd />)
+
+      expect(getBlocker().disabled).toBe(true)
+
+      await openCreationFlow(userEvent)
+
+      expect(getBlocker().disabled).toBe(true)
+    })
+
+    it('asks for confirmation before leaving, and enables the before-unload warning', async () => {
+      const confirm = jest.spyOn(window, 'confirm')
+      const { userEvent } = renderWithProviders(<ClusterAdd />)
+
+      await openCreationFlow(userEvent)
+      await userEvent.click(screen.getByRole('button', { name: 'Reach install step' }))
+
+      expect(getBlocker()).toMatchObject({ disabled: false, enableBeforeUnload: true })
+      confirm.mockReturnValueOnce(false)
+      expect(getBlocker().shouldBlockFn()).toBe(true)
+      confirm.mockReturnValueOnce(true)
+      expect(getBlocker().shouldBlockFn()).toBe(false)
+    })
+
+    it('lets the flow navigate away once the Operator connects', async () => {
+      const confirm = jest.spyOn(window, 'confirm')
+      const { userEvent } = renderWithProviders(<ClusterAdd />)
+
+      await openCreationFlow(userEvent)
+      await userEvent.click(screen.getByRole('button', { name: 'Reach install step' }))
+      await userEvent.click(screen.getByRole('button', { name: 'Operator connected' }))
+
+      expect(getBlocker().shouldBlockFn()).toBe(false)
+      expect(getBlocker().disabled).toBe(true)
+      expect(confirm).not.toHaveBeenCalled()
+    })
+  })
 })

libs/domains/clusters/feature/src/lib/cluster-add/cluster-add.tsx

@@ -1,11 +1,11 @@
-import { Link, useParams } from '@tanstack/react-router'
+import { Link, useBlocker, useParams } from '@tanstack/react-router'
 import Azure from 'devicon/icons/azure/azure-original.svg'
 import DigitalOcean from 'devicon/icons/digitalocean/digitalocean-original.svg'
 import GCP from 'devicon/icons/googlecloud/googlecloud-original.svg'
 import Kubernetes from 'devicon/icons/kubernetes/kubernetes-original.svg'
 import posthog from 'posthog-js'
 import { useFeatureFlagEnabled } from 'posthog-js/react'
-import { type ReactElement, type ReactNode, cloneElement, useState } from 'react'
+import { type ReactElement, type ReactNode, cloneElement, useRef, useState } from 'react'
 import { Callout, Heading, Icon, Modal, Section, useModal } from '@qovery/shared/ui'
 import { useSupportChat } from '@qovery/shared/util-hooks'
 import { twMerge } from '@qovery/shared/util-js'
@@ -17,6 +17,9 @@ import {
   type SelfManagedClusterCreationStep,
 } from '../self-managed-cluster-creation/self-managed-cluster-creation-flow'
 
+const LEAVE_INSTALL_STEP_MESSAGE =
+  'The Operator installation instructions will be lost if you leave this page. Leave anyway?'
+
 type ProviderCardProps = {
   title: string
   icon: string | ReactElement
@@ -38,7 +41,9 @@ function ProviderCard({ title, icon, disabled = false, actionLabel, analytics, .
   const { organizationId = '' } = useParams({ strict: false })
   const className = twMerge(
     'flex h-[52px] items-center gap-2 rounded-md border border-neutral bg-surface-neutral px-4 text-left text-sm font-medium text-neutral transition-colors',
-    disabled ? 'cursor-not-allowed bg-surface-neutral-component text-neutral-subtle' : 'hover:border-brand-strong'
+    disabled ? 'cursor-not-allowed bg-surface-neutral-component text-neutral-subtle' : 'hover:border-brand-strong',
+    // Not available to the organization yet: it opens an access request, not the creation flow.
+    actionLabel && 'border-dashed text-neutral-subtle'
   )
   const content = (
     <>
@@ -111,13 +116,25 @@ export function ClusterAdd() {
   const [isCreationFlowOpen, setIsCreationFlowOpen] = useState(false)
   const [creationStep, setCreationStep] = useState<SelfManagedClusterCreationStep>('general')
 
+  // The flow closes itself before navigating once the Operator connects: that navigation must not be blocked.
+  const isFlowClosingRef = useRef(false)
+  // Once the cluster exists, leaving the page would lose the Operator installation instructions.
+  const isInstalling = isCreationFlowOpen && creationStep === 'install'
+
+  useBlocker({
+    disabled: !isInstalling,
+    shouldBlockFn: () => !isFlowClosingRef.current && !window.confirm(LEAVE_INSTALL_STEP_MESSAGE),
+    enableBeforeUnload: isInstalling,
+  })
+
   const openInstallationGuideModal = ({ isDemo = false }: { isDemo?: boolean } = {}) =>
     openModal({
       options: { width: 500 },
       content: <ClusterInstallationGuideModal mode="CREATE" isDemo={isDemo} type="ON_PREMISE" onClose={closeModal} />,
     })
 
   const openCreationFlow = () => {
+    isFlowClosingRef.current = false
     setCreationStep('general')
     setIsCreationFlowOpen(true)
   }
@@ -248,7 +265,10 @@ export function ClusterAdd() {
         <SelfManagedClusterCreationFlow
           organizationId={organizationId}
           onStepChange={setCreationStep}
-          onClose={() => setIsCreationFlowOpen(false)}
+          onClose={() => {
+            isFlowClosingRef.current = true
+            setIsCreationFlowOpen(false)
+          }}
         />
       </Modal>
     </div>

libs/domains/clusters/feature/src/lib/cluster-profile/cluster-profile-context.tsx

@@ -26,7 +26,7 @@ export type ProfileValues = Record<string, Record<string, unknown>>
 export type ClusterInputValues = Record<string, Record<string, string>>
 
 // Scalars are compared as the inputs display them: an unset toggle shows as off, an unset text as empty.
-function getDisplayedScalarValue(field: PlatformScalarField, value: unknown) {
+function getDisplayedScalarValue(field: Pick<PlatformFieldDescriptor, 'type' | 'defaultValue'>, value: unknown) {
   return getCatalogVariableValue(field, value) ?? (field.type === 'bool' ? false : '')
 }
 
@@ -106,6 +106,7 @@ function getConfigurationRequest(
   }
   const nextClusterInputs = { ...configuration?.clusterInputs }
   for (const [componentKey, values] of Object.entries(clusterInputs)) {
+    if (!Object.keys(values).length) continue
     nextClusterInputs[componentKey] = { ...configuration?.clusterInputs[componentKey], ...values }
   }
 
@@ -163,7 +164,14 @@ export function ClusterProfileProvider({ children }: PropsWithChildren) {
   }
 
   const updateClusterInput = (componentKey: string, field: PlatformFieldDescriptor, value: CatalogVariableValue) => {
-    setClusterInputs((currentValues) => updateComponentValue(currentValues, componentKey, field.key, String(value)))
+    // The input shows its default while unsaved: setting it back to what is displayed is no edit.
+    const nextValue = String(value)
+    const isDisplayedValue =
+      getDisplayedScalarValue(field, nextValue) ===
+      getDisplayedScalarValue(field, configuration?.clusterInputs[componentKey]?.[field.key])
+    setClusterInputs((currentValues) =>
+      updateComponentValue(currentValues, componentKey, field.key, isDisplayedValue ? undefined : nextValue)
+    )
   }
 
   const resetChanges = () => {

libs/domains/clusters/feature/src/lib/cluster-profile/cluster-profile.spec.tsx

@@ -869,6 +869,78 @@ describe('ClusterProfileFeature', () => {
       expect(mockDeployCluster).not.toHaveBeenCalled()
     })
 
+    it('does not count a cluster input set back to its default as a change', async () => {
+      const [loki, alloy] = createComponentQueries(['loki', 'alloy'])
+      mockUsePlatformComponentConfigurations.mockReturnValue([
+        {
+          ...loki,
+          data: {
+            ...createResolution('loki'),
+            requirements: [
+              {
+                key: 'infra.region',
+                label: 'Region',
+                type: 'string',
+                required: false,
+                sensitive: false,
+                constraints: {},
+                defaultValue: 'eu-west-1',
+                status: 'READY',
+              },
+            ],
+          },
+        },
+        alloy,
+      ] as ReturnType<typeof usePlatformComponentConfigurations>)
+      const { userEvent } = renderWithProviders(<ClusterProfileFeature />)
+      const region = screen.getByRole('textbox', { name: 'Region' })
+      expect(region).toHaveValue('eu-west-1')
+
+      await userEvent.type(region, '2')
+      expect(screen.getByRole('region', { name: 'Unsaved profile changes' })).toHaveTextContent('1 change ongoing')
+
+      await userEvent.type(region, '{Backspace}')
+
+      await waitFor(() =>
+        expect(screen.queryByRole('region', { name: 'Unsaved profile changes' })).not.toBeInTheDocument()
+      )
+    })
+
+    it('does not count a toggle cluster input switched back off as a change', async () => {
+      const [loki, alloy] = createComponentQueries(['loki', 'alloy'])
+      mockUsePlatformComponentConfigurations.mockReturnValue([
+        {
+          ...loki,
+          data: {
+            ...createResolution('loki'),
+            requirements: [
+              {
+                key: 'infra.useSpot',
+                label: 'Use spot instances',
+                type: 'bool',
+                required: false,
+                sensitive: false,
+                constraints: {},
+                status: 'READY',
+              },
+            ],
+          },
+        },
+        alloy,
+      ] as ReturnType<typeof usePlatformComponentConfigurations>)
+      const { userEvent } = renderWithProviders(<ClusterProfileFeature />)
+      const useSpot = screen.getByRole('switch', { name: 'Use spot instances' })
+
+      await userEvent.click(useSpot)
+      expect(screen.getByRole('region', { name: 'Unsaved profile changes' })).toHaveTextContent('1 change ongoing')
+
+      await userEvent.click(useSpot)
+
+      await waitFor(() =>
+        expect(screen.queryByRole('region', { name: 'Unsaved profile changes' })).not.toBeInTheDocument()
+      )
+    })
+
     it('does not save edits that were reverted by hand', async () => {
       const { userEvent } = renderWithProviders(<ClusterProfileFeature />)
 

libs/shared/console-shared/src/lib/catalog-variable-input/catalog-variable-input.spec.tsx

@@ -1,3 +1,4 @@
+import { useState } from 'react'
 import { renderWithProviders, screen } from '@qovery/shared/util-tests'
 import { CatalogVariableInput, type CatalogVariableInputProps } from './catalog-variable-input'
 
@@ -102,6 +103,82 @@ describe('CatalogVariableInput', () => {
     expect(screen.getByTestId('input-toggle').parentElement).toHaveClass('items-end')
   })
 
+  it.each([
+    ['a toggle in the row layout', { type: 'bool' }, 'row', 'switch', undefined],
+    ['a select in the row layout', { type: 'string', allowedValues: ['small', 'large'] }, 'row', 'combobox', undefined],
+    ['a toggle in the card layout', { type: 'bool' }, 'card', 'switch', undefined],
+    ['a checkbox in the card layout', { type: 'bool' }, 'card', 'checkbox', 'checkbox'],
+  ] as const)('ties the error to %s', (_, fieldType, layout, role, booleanControl) => {
+    renderWithProviders(
+      <CatalogVariableInput
+        {...defaultProps}
+        error="This value is not allowed."
+        field={{ key: 'profile', label: 'Profile', ...fieldType }}
+        layout={layout}
+        booleanControl={booleanControl}
+        value={fieldType.type === 'bool' ? false : 'small'}
+      />
+    )
+
+    const control = screen.getByRole(role, { name: 'Profile' })
+    expect(control).toHaveAttribute('aria-invalid', 'true')
+    expect(control).toHaveAccessibleDescription('This value is not allowed.')
+  })
+
+  it('does not keep a stale option once the value is cleared', async () => {
+    const field = { key: 'profile', label: 'Profile', type: 'string', allowedValues: ['small', 'large'] } as const
+    function ClearableSelect() {
+      const [value, setValue] = useState('large')
+      return (
+        <>
+          <CatalogVariableInput {...defaultProps} field={field} layout="row" value={value} />
+          <button type="button" onClick={() => setValue('')}>
+            Clear
+          </button>
+        </>
+      )
+    }
+    const { userEvent } = renderWithProviders(<ClearableSelect />)
+    expect(screen.getByRole('combobox', { name: 'Profile' })).toHaveValue('large')
+
+    await userEvent.click(screen.getByRole('button', { name: 'Clear' }))
+
+    expect(screen.getByRole('combobox', { name: 'Profile' })).not.toHaveValue('large')
+  })
+
+  it('keeps error ids unique when two inputs share a field key', () => {
+    const field = { key: 'enabled', label: 'Enabled', type: 'bool' } as const
+    renderWithProviders(
+      <>
+        <CatalogVariableInput {...defaultProps} field={{ ...field, label: 'First' }} layout="row" error="First error" />
+        <CatalogVariableInput
+          {...defaultProps}
+          field={{ ...field, label: 'Second' }}
+          layout="row"
+          error="Second error"
+        />
+      </>
+    )
+
+    expect(screen.getByRole('switch', { name: 'First' })).toHaveAccessibleDescription('First error')
+    expect(screen.getByRole('switch', { name: 'Second' })).toHaveAccessibleDescription('Second error')
+  })
+
+  it('does not flag a control without error', () => {
+    renderWithProviders(
+      <CatalogVariableInput
+        {...defaultProps}
+        field={{ key: 'high-availability', label: 'High availability', type: 'bool' }}
+        layout="row"
+        value={false}
+      />
+    )
+
+    const control = screen.getByRole('switch', { name: 'High availability' })
+    expect(control).not.toHaveAttribute('aria-invalid')
+    expect(control).not.toHaveAttribute('aria-describedby')
+  })
+
   it('renders sensitive values as passwords', () => {
     renderWithProviders(
       <CatalogVariableInput