Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions apps/app-frontend/src/shell/StorageRoutes.test.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,22 @@
import { Route, Routes } from 'react-router-dom';
import { screen, waitFor } from '@testing-library/react';
import { describe, expect, it } from 'vitest';
import { describe, expect, it, vi } from 'vitest';

import { renderWithProviders } from '@osac/ui-components/test-utils/TestProviders';

import { StorageRoutes } from './StorageRoutes';

vi.mock('react-router-dom', async (importOriginal) => {
const actual = await importOriginal<typeof import('react-router-dom')>();
return {
...actual,
// useBlocker requires a data router; this test harness renders under a
// plain MemoryRouter, so StorageBackendCreatePage's LeaveFormConfirmation
// is stubbed out here rather than exercised (see its own test file).
useBlocker: () => ({ state: 'unblocked' as const }),
};
});

const renderAt = (path: string) =>
renderWithProviders(
<Routes>
Expand All @@ -21,10 +32,11 @@ describe('StorageRoutes', () => {
expect(screen.getByRole('button', { name: 'Create backend' })).toBeInTheDocument();
});

it('renders a placeholder for backends/create', () => {
it('renders the real create form for backends/create', () => {
renderAt('/admin/infrastructure/storage/backends/create');

expect(screen.getByText('Create storage backend')).toBeInTheDocument();
expect(screen.getByRole('heading', { name: 'Create storage backend' })).toBeInTheDocument();
expect(screen.getByRole('textbox', { name: 'Name' })).toBeInTheDocument();
});

it('renders a placeholder for backends/:id/edit', () => {
Expand Down
6 changes: 2 additions & 4 deletions apps/app-frontend/src/shell/StorageRoutes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Navigate, Route, Routes } from 'react-router-dom';

import StorageTierCreatePage from '@osac/ui-components/components/Storage/StorageTierCreatePage';
import { useTranslation } from '@osac/ui-components/hooks/useTranslation';
import { StorageBackendCreatePage } from '@osac/ui-components/pages/admin/StorageBackendCreatePage';
import { StorageManagementPage } from '@osac/ui-components/pages/admin/StorageManagementPage';
import { StoragePlaceholder } from '@osac/ui-components/pages/admin/StoragePlaceholder';

Expand All @@ -12,10 +13,7 @@ export const StorageRoutes = () => {
<Routes>
<Route index element={<Navigate to="backends" replace />} />
<Route path="backends" element={<StorageManagementPage activeTab="backends" />} />
<Route
path="backends/create"
element={<StoragePlaceholder title={t('Create storage backend')} />}
/>
<Route path="backends/create" element={<StorageBackendCreatePage />} />
<Route
path="backends/:id/edit"
element={<StoragePlaceholder title={t('Edit storage backend')} />}
Expand Down
13 changes: 11 additions & 2 deletions libs/i18n/locales/en/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
"All": "All",
"An external IP pool is required": "An external IP pool is required",
"API URL": "API URL",
"Are you sure you want to cancel? Your selections and entered data will be lost.": "Are you sure you want to cancel? Your selections and entered data will be lost.",
"Are you sure you want to disable Identity provider {{idpName}}": "Are you sure you want to disable Identity provider {{idpName}}",
"Are you sure you want to enable Identity provider {{idpName}}": "Are you sure you want to enable Identity provider {{idpName}}",
"Are you sure you want to leave? Your entered data will be lost.": "Are you sure you want to leave? Your entered data will be lost.",
"At least one CIDR (IPv4 or IPv6) is required": "At least one CIDR (IPv4 or IPv6) is required",
"At least one node set is required": "At least one node set is required",
"At least one user is required": "At least one user is required",
Expand Down Expand Up @@ -94,6 +94,7 @@
"catalogProvision.vm.wizardDescription": "Select a catalog item, configure, and provision.",
"catalogProvision.vm.wizardTitle": "Create virtual machine",
"catalogProvision.wizard.navAria": "{{title}} steps",
"Ceph": "Ceph",
"CIDR": "CIDR",
"CIDR must be within parent virtual network range": "CIDR must be within parent virtual network range",
"CIDR overlaps with existing subnet \"{{name}}\" ({{cidr}})": "CIDR overlaps with existing subnet \"{{name}}\" ({{cidr}})",
Expand Down Expand Up @@ -167,7 +168,7 @@
"Disable {{idpName}}?": "Disable {{idpName}}?",
"Disabled": "Disabled",
"Discard and close": "Discard and close",
"Discard wizard progress?": "Discard wizard progress?",
"Discard unsaved changes?": "Discard unsaved changes?",
"Domain {{number}}": "Domain {{number}}",
"Domain is required": "Domain is required",
"Domains": "Domains",
Expand All @@ -185,6 +186,7 @@
"Enable {{idpName}}?": "Enable {{idpName}}?",
"Encryption enabled": "Encryption enabled",
"Endpoint": "Endpoint",
"Endpoint is required": "Endpoint is required",
"Error": "Error",
"Error loading external IP pools": "Error loading external IP pools",
"Error loading virtual networks": "Error loading virtual networks",
Expand All @@ -204,6 +206,7 @@
"Failed to connect to the console.": "Failed to connect to the console.",
"Failed to create resource": "Failed to create resource",
"Failed to create role binding": "Failed to create role binding",
"Failed to create storage backend": "Failed to create storage backend",
"Failed to create storage tier": "Failed to create storage tier",
"Failed to create subnet": "Failed to create subnet",
"Failed to create tenant": "Failed to create tenant",
Expand Down Expand Up @@ -348,6 +351,7 @@
"Overview": "Overview",
"Parent virtual network": "Parent virtual network",
"Password": "Password",
"Password is required": "Password is required",
"Paste a public SSH key for remote access. Supported types: ssh-rsa, ssh-ed25519, and ecdsa-sha2-nistp256/384/521.": "Paste a public SSH key for remote access. Supported types: ssh-rsa, ssh-ed25519, and ecdsa-sha2-nistp256/384/521.",
"Paste from clipboard": "Paste from clipboard",
"Paused": "Paused",
Expand All @@ -368,6 +372,7 @@
"Protocol is required": "Protocol is required",
"Protocol(s)": "Protocol(s)",
"Provider": "Provider",
"Provider is required": "Provider is required",
"Provision a bare metal instance from a catalog item.": "Provision a bare metal instance from a catalog item.",
"Provision bare metal": "Provision bare metal",
"Provisioning": "Provisioning",
Expand All @@ -376,6 +381,7 @@
"Pull secret": "Pull secret",
"Pull secret is required": "Pull secret is required",
"Pull secrets download OpenShift components and connect clusters to your Red Hat account. Copy the full JSON from OpenShift Cluster Manager (console.redhat.com/openshift/install/pull-secret).": "Pull secrets download OpenShift components and connect clusters to your Red Hat account. Copy the full JSON from OpenShift Cluster Manager (console.redhat.com/openshift/install/pull-secret).",
"Pure": "Pure",
"Quota (GiB)": "Quota (GiB)",
"Reactivate": "Reactivate",
"Ready": "Ready",
Expand Down Expand Up @@ -467,6 +473,7 @@
"Unspecified": "Unspecified",
"Update": "Update",
"Update role binding": "Update role binding",
"Use credentials": "Use credentials",
"Use IPv4 CIDR notation (for example 10.128.0.0/14).": "Use IPv4 CIDR notation (for example 10.128.0.0/14).",
"Use IPv4 CIDR notation (for example 172.30.0.0/16).": "Use IPv4 CIDR notation (for example 172.30.0.0/16).",
"User data": "User data",
Expand All @@ -475,7 +482,9 @@
"User data must not exceed 64 KB.": "User data must not exceed 64 KB.",
"User info URL": "User info URL",
"Username": "Username",
"Username is required": "Username is required",
"Users": "Users",
"VAST": "VAST",
"View and manage your bare metal instances.": "View and manage your bare metal instances.",
"View password": "View password",
"Virtual machine conditions": "Virtual machine conditions",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ const LeaveFormConfirmation = () => {
return (
<Modal isOpen variant="small" aria-labelledby="leave-form-title">
<ModalHeader
title={t('Discard wizard progress?')}
title={t('Discard unsaved changes?')}
titleIconVariant="warning"
labelId="leave-form-title"
/>
<ModalBody>
<Content component="p">
{t('Are you sure you want to cancel? Your selections and entered data will be lost.')}
{t('Are you sure you want to leave? Your entered data will be lost.')}
</Content>
</ModalBody>
<ModalFooter>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import { create } from '@bufbuild/protobuf';
import { Code, ConnectError } from '@connectrpc/connect';
import { screen, waitFor } from '@testing-library/react';
import { beforeEach, describe, expect, it, vi } from 'vitest';

import type { StorageBackend } from '@osac/types/private';
import { StorageBackendState, StorageProtocol } from '@osac/types/private';
import type { StorageBackend, StorageTiersCreateResponse } from '@osac/types/private';
import {
StorageBackendState,
StorageProtocol,
StorageTiersCreateResponseSchema,
} from '@osac/types/private';

import StorageTierCreatePage from './StorageTierCreatePage';
import { renderWithProviders } from '../../test-utils/TestProviders';
Expand All @@ -14,6 +19,11 @@ vi.mock('react-router-dom', async (importOriginal) => {
return {
...actual,
useNavigate: () => mockNavigate,
// useBlocker requires a data router; this test harness renders under a plain
// MemoryRouter, so LeaveFormConfirmation's blocking behavior is stubbed out
// rather than exercised here (see StorageBackendCreatePage.test.tsx for the
// same pattern).
useBlocker: () => ({ state: 'unblocked' as const }),
};
});

Expand Down Expand Up @@ -245,6 +255,37 @@ describe('StorageTierCreatePage', () => {
});
});

it('disables Create while the submission is pending, to prevent duplicate submissions', async () => {
let resolveCreate: (() => void) | undefined;
const onStorageTierCreate = () =>
new Promise<StorageTiersCreateResponse>((resolve) => {
resolveCreate = () =>
resolve(create(StorageTiersCreateResponseSchema, { object: { id: 'new-tier-1' } }));
});

const { user } = renderWithProviders(<StorageTierCreatePage />, {
apiFixtures: { storageBackends: [readyBackend] },
transportOverrides: { onStorageTierCreate },
});

await fillValidForm(user);
await user.click(screen.getByRole('button', { name: 'Create' }));

// Once isLoading is true, PatternFly's Spinner contributes its own "Contents"
// accessible name to the button, so an exact "Create" match no longer
// resolves — match by substring instead.
await waitFor(() => {
expect(screen.getByRole('button', { name: /Create/ })).toBeDisabled();
});
expect(screen.getByRole('button', { name: 'Cancel' })).toBeDisabled();

resolveCreate?.();

await waitFor(() => {
expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/tiers');
});
}, 15000);

it('shows the ALREADY_EXISTS error as a form-level error without navigating away', async () => {
const { user } = renderWithProviders(<StorageTierCreatePage />, {
apiFixtures: { storageBackends: [readyBackend] },
Expand Down
39 changes: 22 additions & 17 deletions libs/ui-components/src/components/Storage/StorageTierCreatePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@ import { useTranslation } from '../../hooks/useTranslation';
import { getErrorMessage } from '../../utils/error';
import { positiveIntegerSchema } from '../../validation/positive-integer';
import { resourceNameSchema } from '../../validation/resource-name';
import NameField from '../catalogProvision/wizard/fields/NameField';
import { CheckboxField } from '../Form/CheckboxField';
import { InputField } from '../Form/InputField';
import LeaveFormConfirmation from '../Form/LeaveFormConfirmation';
import OsacForm from '../Form/OsacForm';
import { RadioButtonField } from '../Form/RadioButtonField';
import { SelectField } from '../Form/SelectField';
Expand All @@ -50,13 +52,13 @@ interface BackendAssociationValues {
}

interface StorageTierCreateFormValues {
name: string;
metadata: { name: string };
description: string;
backends: [BackendAssociationValues];
}

const initialValues: StorageTierCreateFormValues = {
name: '',
metadata: { name: '' },
description: '',
backends: [
{
Expand All @@ -73,7 +75,7 @@ const initialValues: StorageTierCreateFormValues = {
const StorageTierCreatePage = () => {
const { t } = useTranslation();
const navigate = useNavigate();
const { mutate, error, isPending } = useCreateStorageTier();
const { mutateAsync, error } = useCreateStorageTier();
const { data: backends = [], isLoading: backendsLoading } = usePrivateStorageBackends({
filter: STORAGE_BACKEND_READY_LIST_FILTER,
});
Expand All @@ -84,7 +86,7 @@ const StorageTierCreatePage = () => {
}));

const schema = Yup.object({
name: resourceNameSchema(t),
metadata: Yup.object({ name: resourceNameSchema(t) }),
description: Yup.string(),
backends: Yup.array()
.of(
Expand Down Expand Up @@ -124,11 +126,11 @@ const StorageTierCreatePage = () => {
<Formik
initialValues={initialValues}
validationSchema={schema}
onSubmit={(values) => {
const backend = values.backends[0];
mutate(
{
metadata: { name: values.name },
onSubmit={async (values) => {
try {
const backend = values.backends[0];
await mutateAsync({
metadata: values.metadata,
spec: {
description: values.description,
backends: [
Expand All @@ -142,16 +144,19 @@ const StorageTierCreatePage = () => {
},
],
},
},
{ onSuccess: () => navigate(TIERS_LIST_PATH) },
);
});
navigate(TIERS_LIST_PATH);
} catch {
// Surfaced via the mutation's own `error` state below; nothing further to do here.
}
}}
>
{({ submitForm }) => (
{({ submitForm, isSubmitting }) => (
<Stack hasGutter>
<LeaveFormConfirmation />
<StackItem>
<OsacForm>
<InputField name="name" label={t('Name')} fieldId="tier-name" isRequired />
<NameField />
<InputField
name="description"
label={t('Description')}
Expand Down Expand Up @@ -220,8 +225,8 @@ const StorageTierCreatePage = () => {
<Button
variant="primary"
onClick={submitForm}
isDisabled={isPending}
isLoading={isPending}
isDisabled={isSubmitting}
isLoading={isSubmitting}
>
{t('Create')}
</Button>
Expand All @@ -230,7 +235,7 @@ const StorageTierCreatePage = () => {
<Button
variant="link"
onClick={() => navigate(TIERS_LIST_PATH)}
isDisabled={isPending}
isDisabled={isSubmitting}
>
{t('Cancel')}
</Button>
Expand Down
Loading
Loading