From 4a8ed918144cbfc46b9a393c56e6d82821b4e6de Mon Sep 17 00:00:00 2001 From: Elay Aharoni Date: Sun, 9 Aug 2026 14:15:20 +0300 Subject: [PATCH 01/10] OSAC-3604: add StorageTierStatusLabel Assisted-by: Claude Code Signed-off-by: Elay Aharoni --- .../Storage/StorageTierStatusLabel.test.tsx | 23 +++++++++++++++++ .../Storage/StorageTierStatusLabel.tsx | 25 +++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 libs/ui-components/src/components/Storage/StorageTierStatusLabel.test.tsx create mode 100644 libs/ui-components/src/components/Storage/StorageTierStatusLabel.tsx diff --git a/libs/ui-components/src/components/Storage/StorageTierStatusLabel.test.tsx b/libs/ui-components/src/components/Storage/StorageTierStatusLabel.test.tsx new file mode 100644 index 00000000..f568c6a0 --- /dev/null +++ b/libs/ui-components/src/components/Storage/StorageTierStatusLabel.test.tsx @@ -0,0 +1,23 @@ +import { render, screen } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; + +import { StorageTierState } from '@osac/types/private'; + +import { StorageTierStatusLabel } from './StorageTierStatusLabel'; + +describe('StorageTierStatusLabel', () => { + it('maps ACTIVE to ready/Active', () => { + render(); + expect(screen.getByText('Active')).toBeInTheDocument(); + }); + + it('maps undefined to unspecified/Unspecified', () => { + render(); + expect(screen.getByText('Unspecified')).toBeInTheDocument(); + }); + + it('maps UNSPECIFIED to unspecified/Unspecified', () => { + render(); + expect(screen.getByText('Unspecified')).toBeInTheDocument(); + }); +}); diff --git a/libs/ui-components/src/components/Storage/StorageTierStatusLabel.tsx b/libs/ui-components/src/components/Storage/StorageTierStatusLabel.tsx new file mode 100644 index 00000000..dc12faa5 --- /dev/null +++ b/libs/ui-components/src/components/Storage/StorageTierStatusLabel.tsx @@ -0,0 +1,25 @@ +import { StorageTierState } from '@osac/types/private'; + +import { ResourceStatusLabel, type StatusKind } from '../Resource/ResourceStatusLabel'; + +interface StorageTierStatusLabelProps { + state?: StorageTierState; +} + +const STORAGE_TIER_STATUS_MAP: Record = { + [StorageTierState.UNSPECIFIED]: { status: 'unspecified', text: 'Unspecified' }, + [StorageTierState.ACTIVE]: { status: 'ready', text: 'Active' }, +}; + +const resolveStorageTierStatus = ( + state?: StorageTierState, +): { status: StatusKind; text: string } => + state !== undefined && state in STORAGE_TIER_STATUS_MAP + ? STORAGE_TIER_STATUS_MAP[state] + : STORAGE_TIER_STATUS_MAP[StorageTierState.UNSPECIFIED]; + +export const StorageTierStatusLabel = ({ state }: StorageTierStatusLabelProps) => { + const { status, text } = resolveStorageTierStatus(state); + + return ; +}; From 1767ccb313b730f25076d2ec3d3dd1c66a1e1298 Mon Sep 17 00:00:00 2001 From: Elay Aharoni Date: Sun, 9 Aug 2026 14:17:54 +0300 Subject: [PATCH 02/10] OSAC-3604: add StorageBackendList/StorageTierDelete mock transport overrides Assisted-by: Claude Code Signed-off-by: Elay Aharoni --- .../src/test-utils/createMockConnectTransport.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/libs/ui-components/src/test-utils/createMockConnectTransport.ts b/libs/ui-components/src/test-utils/createMockConnectTransport.ts index c88f00f0..c9c9c413 100644 --- a/libs/ui-components/src/test-utils/createMockConnectTransport.ts +++ b/libs/ui-components/src/test-utils/createMockConnectTransport.ts @@ -36,11 +36,15 @@ import type { StorageBackend, StorageBackendsCreateRequest, StorageBackendsCreateResponse, + StorageBackendsListRequest, + StorageBackendsListResponse, StorageBackendsUpdateRequest, StorageBackendsUpdateResponse, StorageTier, StorageTiersCreateRequest, StorageTiersCreateResponse, + StorageTiersDeleteRequest, + StorageTiersDeleteResponse, StorageTiersListRequest, StorageTiersListResponse, StorageTiersUpdateRequest, @@ -146,11 +150,13 @@ export type MockTransportOverrides = { req: IdentityProvidersUpdateRequest, ) => IdentityProvidersUpdateResponse; onTenantCreate?: (req: TenantsCreateRequest) => TenantsCreateResponse; + onStorageBackendList?: (req: StorageBackendsListRequest) => StorageBackendsListResponse; onStorageBackendCreate?: (req: StorageBackendsCreateRequest) => StorageBackendsCreateResponse; onStorageBackendUpdate?: (req: StorageBackendsUpdateRequest) => StorageBackendsUpdateResponse; onStorageTierList?: (req: StorageTiersListRequest) => StorageTiersListResponse; onStorageTierCreate?: (req: StorageTiersCreateRequest) => StorageTiersCreateResponse; onStorageTierUpdate?: (req: StorageTiersUpdateRequest) => StorageTiersUpdateResponse; + onStorageTierDelete?: (req: StorageTiersDeleteRequest) => StorageTiersDeleteResponse; }; export const createMockConnectTransport = ( @@ -292,6 +298,9 @@ export const createMockConnectTransport = ( router.service(StorageBackends, { list: (req) => { + if (overrides.onStorageBackendList) { + return overrides.onStorageBackendList(req); + } const items = storageBackends.filter((item) => matchesStorageBackendReadyFilter(req.filter, item.status?.state), ); @@ -355,7 +364,12 @@ export const createMockConnectTransport = ( } return { object: req.object }; }, - delete: () => ({}), + delete: (req) => { + if (overrides.onStorageTierDelete) { + return overrides.onStorageTierDelete(req); + } + return {}; + }, }); router.service(PrivateTenants, { From c357bfab361458661e175ddbab6e301e60c13014 Mon Sep 17 00:00:00 2001 From: Elay Aharoni Date: Sun, 9 Aug 2026 14:20:43 +0300 Subject: [PATCH 03/10] OSAC-3604: add StorageTierActionsMenu and delete confirmation Assisted-by: Claude Code Signed-off-by: Elay Aharoni --- .../Storage/StorageTierActionsMenu.tsx | 63 +++++++++++ .../StorageTierDeleteConfirmModal.test.tsx | 106 ++++++++++++++++++ .../Storage/StorageTierDeleteConfirmModal.tsx | 77 +++++++++++++ 3 files changed, 246 insertions(+) create mode 100644 libs/ui-components/src/components/Storage/StorageTierActionsMenu.tsx create mode 100644 libs/ui-components/src/components/Storage/StorageTierDeleteConfirmModal.test.tsx create mode 100644 libs/ui-components/src/components/Storage/StorageTierDeleteConfirmModal.tsx diff --git a/libs/ui-components/src/components/Storage/StorageTierActionsMenu.tsx b/libs/ui-components/src/components/Storage/StorageTierActionsMenu.tsx new file mode 100644 index 00000000..de6dfee2 --- /dev/null +++ b/libs/ui-components/src/components/Storage/StorageTierActionsMenu.tsx @@ -0,0 +1,63 @@ +import { useState } from 'react'; +import { useNavigate } from 'react-router-dom'; +import { Dropdown, DropdownItem, DropdownList, MenuToggle } from '@patternfly/react-core'; +import { EllipsisVIcon } from '@patternfly/react-icons/dist/esm/icons/ellipsis-v-icon'; + +import type { StorageTier } from '@osac/types/private'; + +import StorageTierDeleteConfirmModal from './StorageTierDeleteConfirmModal'; +import { useTranslation } from '../../hooks/useTranslation'; + +interface StorageTierActionsMenuProps { + tier: StorageTier; +} + +const StorageTierActionsMenu = ({ tier }: StorageTierActionsMenuProps) => { + const { t } = useTranslation(); + const navigate = useNavigate(); + const [open, setOpen] = useState(false); + const [deleteOpen, setDeleteOpen] = useState(false); + + return ( + <> + {deleteOpen && ( + setDeleteOpen(false)} + onSuccess={() => setDeleteOpen(false)} + /> + )} + ( + setOpen((o) => !o)} + aria-label={t('Actions for {{name}}', { name: tier.metadata?.name ?? tier.id })} + > + + + )} + popperProps={{ position: 'right' }} + > + + navigate(`/admin/storage/tiers/${tier.id}/edit`)}> + {t('Edit')} + + { + setDeleteOpen(true); + setOpen(false); + }} + > + {t('Delete')} + + + + + ); +}; + +export default StorageTierActionsMenu; diff --git a/libs/ui-components/src/components/Storage/StorageTierDeleteConfirmModal.test.tsx b/libs/ui-components/src/components/Storage/StorageTierDeleteConfirmModal.test.tsx new file mode 100644 index 00000000..a9a717b1 --- /dev/null +++ b/libs/ui-components/src/components/Storage/StorageTierDeleteConfirmModal.test.tsx @@ -0,0 +1,106 @@ +import { Code, ConnectError } from '@connectrpc/connect'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import StorageTierDeleteConfirmModal from './StorageTierDeleteConfirmModal'; +import * as storageTiersApi from '../../api/v1/private/storage-tiers'; + +vi.mock('../../api/v1/private/storage-tiers', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useDeleteStorageTier: vi.fn(), + }; +}); + +const mockTier = { + id: 'tier-1', + metadata: { name: 'fast' }, + spec: { description: '', backends: [] }, +}; + +describe('StorageTierDeleteConfirmModal', () => { + const mutate = vi.fn(); + const reset = vi.fn(); + + beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(storageTiersApi.useDeleteStorageTier).mockReturnValue({ + mutate, + reset, + isPending: false, + error: null, + } as unknown as ReturnType); + }); + + it('deletes the tier and calls onSuccess', async () => { + const user = userEvent.setup(); + mutate.mockImplementation((_id: string, options?: { onSuccess?: () => void }) => { + options?.onSuccess?.(); + return Promise.resolve(undefined); + }); + const onSuccess = vi.fn(); + + render( + , + ); + + expect(screen.getByRole('dialog')).toBeInTheDocument(); + + await user.click(screen.getByRole('button', { name: /^Delete$/i })); + + await waitFor(() => { + expect(mutate).toHaveBeenCalledWith('tier-1', { + onSuccess: expect.any(Function) as unknown, + }); + expect(onSuccess).toHaveBeenCalled(); + }); + }); + + it('shows the FAILED_PRECONDITION error verbatim and does not call onSuccess when the tier is referenced by a Tenant', async () => { + const user = userEvent.setup(); + vi.mocked(storageTiersApi.useDeleteStorageTier).mockReturnValue({ + mutate, + reset, + isPending: false, + error: new ConnectError('Storage tier is referenced by a Tenant', Code.FailedPrecondition), + } as unknown as ReturnType); + const onSuccess = vi.fn(); + + render( + , + ); + + await user.click(screen.getByRole('button', { name: /^Delete$/i })); + + await waitFor(() => { + expect(screen.getByText('Storage tier is referenced by a Tenant')).toBeInTheDocument(); + }); + expect(onSuccess).not.toHaveBeenCalled(); + }); + + it('calls onClose when Cancel is clicked', async () => { + const user = userEvent.setup(); + const onClose = vi.fn(); + + render( + , + ); + + await user.click(screen.getByRole('button', { name: /Cancel/i })); + expect(onClose).toHaveBeenCalled(); + }); +}); diff --git a/libs/ui-components/src/components/Storage/StorageTierDeleteConfirmModal.tsx b/libs/ui-components/src/components/Storage/StorageTierDeleteConfirmModal.tsx new file mode 100644 index 00000000..1c11ce33 --- /dev/null +++ b/libs/ui-components/src/components/Storage/StorageTierDeleteConfirmModal.tsx @@ -0,0 +1,77 @@ +import { + Alert, + Button, + Modal, + ModalBody, + ModalFooter, + ModalHeader, + Stack, + StackItem, +} from '@patternfly/react-core'; + +import type { StorageTier } from '@osac/types/private'; + +import { useDeleteStorageTier } from '../../api/v1/private/storage-tiers'; +import { useTranslation } from '../../hooks/useTranslation'; +import { getErrorMessage } from '../../utils/error'; + +interface StorageTierDeleteConfirmModalProps { + tier: StorageTier; + onClose: () => void; + onSuccess: () => void; +} + +const StorageTierDeleteConfirmModal = ({ + tier, + onClose, + onSuccess, +}: StorageTierDeleteConfirmModalProps) => { + const { t } = useTranslation(); + const { mutate, isPending, error } = useDeleteStorageTier(); + + const tierName = tier.metadata?.name ?? tier.id; + + return ( + + + + + + {t('This permanently deletes the storage tier. This action cannot be undone.')} + + {error && ( + + + {getErrorMessage(error)} + + + )} + + + + + + + + ); +}; + +export default StorageTierDeleteConfirmModal; From 3fe675a04d4db61788bb36ea8abb0293d6f78e27 Mon Sep 17 00:00:00 2001 From: Elay Aharoni Date: Sun, 9 Aug 2026 14:29:51 +0300 Subject: [PATCH 04/10] OSAC-3604: type mock transport list/delete overrides as MessageInitShape MockTransportOverrides declared these fields with the strict generated Message type, which requires $typeName on every literal returned from a test override. No existing test exercised onStorageTierList, onStorageBackendList, or onStorageTierDelete with a literal response until this story's StorageTiersListPage tests, which is why the gap was latent. MessageInitShape matches how the rest of the codebase accepts plain-object message inputs elsewhere (e.g. useCreateStorageTier). Assisted-by: Claude Code Signed-off-by: Elay Aharoni --- .../test-utils/createMockConnectTransport.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/libs/ui-components/src/test-utils/createMockConnectTransport.ts b/libs/ui-components/src/test-utils/createMockConnectTransport.ts index c9c9c413..d94bf306 100644 --- a/libs/ui-components/src/test-utils/createMockConnectTransport.ts +++ b/libs/ui-components/src/test-utils/createMockConnectTransport.ts @@ -1,3 +1,4 @@ +import type { MessageInitShape } from '@bufbuild/protobuf'; import { Code, ConnectError, type Transport, createRouterTransport } from '@connectrpc/connect'; import type { @@ -37,16 +38,13 @@ import type { StorageBackendsCreateRequest, StorageBackendsCreateResponse, StorageBackendsListRequest, - StorageBackendsListResponse, StorageBackendsUpdateRequest, StorageBackendsUpdateResponse, StorageTier, StorageTiersCreateRequest, StorageTiersCreateResponse, StorageTiersDeleteRequest, - StorageTiersDeleteResponse, StorageTiersListRequest, - StorageTiersListResponse, StorageTiersUpdateRequest, StorageTiersUpdateResponse, TenantsCreateRequest, @@ -56,8 +54,11 @@ import { Tenants as PrivateTenants, StorageBackendState, StorageBackends, + StorageBackendsListResponseSchema, StorageTierState, StorageTiers, + StorageTiersDeleteResponseSchema, + StorageTiersListResponseSchema, } from '@osac/types/private'; import { UnauthorizedError } from '../utils/unauthorizedError'; @@ -150,13 +151,19 @@ export type MockTransportOverrides = { req: IdentityProvidersUpdateRequest, ) => IdentityProvidersUpdateResponse; onTenantCreate?: (req: TenantsCreateRequest) => TenantsCreateResponse; - onStorageBackendList?: (req: StorageBackendsListRequest) => StorageBackendsListResponse; + onStorageBackendList?: ( + req: StorageBackendsListRequest, + ) => MessageInitShape; onStorageBackendCreate?: (req: StorageBackendsCreateRequest) => StorageBackendsCreateResponse; onStorageBackendUpdate?: (req: StorageBackendsUpdateRequest) => StorageBackendsUpdateResponse; - onStorageTierList?: (req: StorageTiersListRequest) => StorageTiersListResponse; + onStorageTierList?: ( + req: StorageTiersListRequest, + ) => MessageInitShape; onStorageTierCreate?: (req: StorageTiersCreateRequest) => StorageTiersCreateResponse; onStorageTierUpdate?: (req: StorageTiersUpdateRequest) => StorageTiersUpdateResponse; - onStorageTierDelete?: (req: StorageTiersDeleteRequest) => StorageTiersDeleteResponse; + onStorageTierDelete?: ( + req: StorageTiersDeleteRequest, + ) => MessageInitShape; }; export const createMockConnectTransport = ( From d3947ea9684e91b9694b138fe4d02c5ee2107e83 Mon Sep 17 00:00:00 2001 From: Elay Aharoni Date: Sun, 9 Aug 2026 14:36:40 +0300 Subject: [PATCH 05/10] OSAC-3604: add StorageTiersListPage Assisted-by: Claude Code Signed-off-by: Elay Aharoni --- libs/i18n/locales/en/translation.json | 8 + .../pages/admin/StorageTiersListPage.test.tsx | 203 ++++++++++++++++++ .../src/pages/admin/StorageTiersListPage.tsx | 123 +++++++++++ 3 files changed, 334 insertions(+) create mode 100644 libs/ui-components/src/pages/admin/StorageTiersListPage.test.tsx create mode 100644 libs/ui-components/src/pages/admin/StorageTiersListPage.tsx diff --git a/libs/i18n/locales/en/translation.json b/libs/i18n/locales/en/translation.json index 4e223a3d..8569e528 100644 --- a/libs/i18n/locales/en/translation.json +++ b/libs/i18n/locales/en/translation.json @@ -26,6 +26,7 @@ "Bare metal instances": "Bare metal instances", "Bare Metal Machines": "Bare Metal Machines", "Bare metal provisioning wizard": "Bare metal provisioning wizard", + "Block": "Block", "Boot disk": "Boot disk", "Break-glass credentials": "Break-glass credentials", "Browse catalog items and launch virtual machines, clusters, or bare metal machines from published offerings.": "Browse catalog items and launch virtual machines, clusters, or bare metal machines from published offerings.", @@ -128,6 +129,7 @@ "Create storage backend": "Create storage backend", "Create subnet": "Create subnet", "Create tenant": "Create tenant", + "Create tier": "Create tier", "Create virtual machine": "Create virtual machine", "Create virtual network": "Create virtual network", "Created": "Created", @@ -189,6 +191,7 @@ "Failed to delete cluster": "Failed to delete cluster", "Failed to delete compute instance": "Failed to delete compute instance", "Failed to delete Identity provider": "Failed to delete Identity provider", + "Failed to delete storage tier": "Failed to delete storage tier", "Failed to delete tenant": "Failed to delete tenant", "Failed to disable Identity provider": "Failed to disable Identity provider", "Failed to download kubeconfig": "Failed to download kubeconfig", @@ -263,6 +266,7 @@ "Network class is required": "Network class is required", "Networking": "Networking", "Next": "Next", + "NFS": "NFS", "No bare metal instances match your search.": "No bare metal instances match your search.", "No bare metal instances yet.": "No bare metal instances yet.", "No catalog items found": "No catalog items found", @@ -277,6 +281,7 @@ "No published catalog items are available yet.": "No published catalog items are available yet.", "No security groups match your search.": "No security groups match your search.", "No security groups yet. Create one to get started.": "No security groups yet. Create one to get started.", + "No storage tiers yet. Create one to get started.": "No storage tiers yet. Create one to get started.", "No subnets yet. Create one to get started.": "No subnets yet. Create one to get started.", "No tenants match your search.": "No tenants match your search.", "No tenants yet. Register one to get started.": "No tenants yet. Register one to get started.", @@ -314,6 +319,7 @@ "Primary domain": "Primary domain", "Protocol": "Protocol", "Protocol is required": "Protocol is required", + "Protocol(s)": "Protocol(s)", "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", @@ -359,6 +365,7 @@ "SSH public key must be in the form \"[TYPE] key [[EMAIL]]\". Supported types are ssh-rsa, ssh-ed25519, and ecdsa-sha2-nistp256/384/521.": "SSH public key must be in the form \"[TYPE] key [[EMAIL]]\". Supported types are ssh-rsa, ssh-ed25519, and ecdsa-sha2-nistp256/384/521.", "Start": "Start", "Starting": "Starting", + "State": "State", "Status": "Status", "Stop": "Stop", "Stopped": "Stopped", @@ -386,6 +393,7 @@ "This permanently deletes the cluster and all its resources. This action cannot be undone.": "This permanently deletes the cluster and all its resources. This action cannot be undone.", "This permanently deletes the compute instance. This action cannot be undone.": "This permanently deletes the compute instance. This action cannot be undone.", "This permanently deletes the Identity provider and all its resources. This action cannot be undone.": "This permanently deletes the Identity provider and all its resources. This action cannot be undone.", + "This permanently deletes the storage tier. This action cannot be undone.": "This permanently deletes the storage tier. This action cannot be undone.", "This permanently deletes the tenant and all its resources. This action cannot be undone.": "This permanently deletes the tenant and all its resources. This action cannot be undone.", "This will permanently delete the rule. This action cannot be undone. Traffic matching this rule will be blocked.": "This will permanently delete the rule. This action cannot be undone. Traffic matching this rule will be blocked.", "This will permanently delete the security group and all its rules. This action cannot be undone.": "This will permanently delete the security group and all its rules. This action cannot be undone.", diff --git a/libs/ui-components/src/pages/admin/StorageTiersListPage.test.tsx b/libs/ui-components/src/pages/admin/StorageTiersListPage.test.tsx new file mode 100644 index 00000000..48b60889 --- /dev/null +++ b/libs/ui-components/src/pages/admin/StorageTiersListPage.test.tsx @@ -0,0 +1,203 @@ +import { Route, Routes } from 'react-router-dom'; +import { screen, waitFor } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; + +import type { StorageBackend, StorageTier } from '@osac/types/private'; +import { StorageBackendState, StorageProtocol, StorageTierState } from '@osac/types/private'; + +import { StorageTiersListPage } from './StorageTiersListPage'; +import { storageBackendIdsFilter } from '../../api/v1/private/storage-backends'; +import { renderWithProviders } from '../../test-utils/TestProviders'; + +const makeBackend = (id: string, name: string): StorageBackend => + ({ + id, + metadata: { name }, + spec: { provider: 'vast', endpoint: `${id}.example.com`, credentials: {} }, + status: { state: StorageBackendState.READY }, + }) as StorageBackend; + +const makeTier = ( + id: string, + name: string, + backends: { backendId: string; protocol: StorageProtocol }[], +): StorageTier => + ({ + id, + metadata: { name }, + spec: { + description: '', + backends: backends.map((b) => ({ + backendId: b.backendId, + protocol: b.protocol, + maxReadBandwidthMbs: 0, + maxWriteBandwidthMbs: 0, + quotaGib: BigInt(0), + encryptionEnabled: false, + })), + }, + status: { state: StorageTierState.ACTIVE }, + }) as StorageTier; + +const backendA = makeBackend('backend-a', 'Fast NVMe'); +const backendB = makeBackend('backend-b', 'Bulk HDD'); +const backendUnused = makeBackend('backend-unused', 'Unused Backend'); + +const singleBackendTier = makeTier('tier-1', 'fast', [ + { backendId: 'backend-a', protocol: StorageProtocol.NFS }, +]); +const mixedTier = makeTier('tier-2', 'mixed', [ + { backendId: 'backend-b', protocol: StorageProtocol.BLOCK }, + { backendId: 'missing-backend', protocol: StorageProtocol.NFS }, +]); + +const defaultTiers = [singleBackendTier, mixedTier]; +const defaultBackends = [backendA, backendB, backendUnused]; + +const renderPage = ( + tiers: StorageTier[] = defaultTiers, + backends: StorageBackend[] = defaultBackends, +) => + renderWithProviders(, { + apiFixtures: { storageTiers: tiers, storageBackends: backends }, + }); + +describe('StorageTiersListPage', () => { + it('renders tier rows with name, resolved backend names, and protocols', async () => { + renderPage(); + + await waitFor(() => { + expect(screen.getByText('fast')).toBeInTheDocument(); + }); + expect(screen.getByText('Fast NVMe')).toBeInTheDocument(); + expect(screen.getByText('NFS')).toBeInTheDocument(); + }); + + it('renders comma-separated backend names and protocols for a tier with multiple backend associations, falling back to the raw id when a backend cannot be resolved', async () => { + renderPage(); + + await waitFor(() => { + expect(screen.getByText('mixed')).toBeInTheDocument(); + }); + expect(screen.getByText('Bulk HDD, missing-backend')).toBeInTheDocument(); + expect(screen.getByText('Block, NFS')).toBeInTheDocument(); + }); + + it('renders the STATE column via StorageTierStatusLabel', async () => { + renderPage(); + + await waitFor(() => { + expect(screen.getAllByText('Active')).toHaveLength(2); + }); + }); + + it('requests exactly the set of backend ids referenced by the rendered tiers, not every registered backend', async () => { + let capturedFilter: string | undefined; + + renderWithProviders(, { + apiFixtures: { storageTiers: defaultTiers, storageBackends: defaultBackends }, + transportOverrides: { + onStorageBackendList: (req) => { + capturedFilter = req.filter; + return { + items: defaultBackends, + size: defaultBackends.length, + total: defaultBackends.length, + }; + }, + }, + }); + + await waitFor(() => { + expect(capturedFilter).toBeDefined(); + }); + + const expectedIds = ['backend-a', 'backend-b', 'missing-backend'].sort(); + expect(capturedFilter).toBe(storageBackendIdsFilter(expectedIds)); + }); + + it('shows the empty state and no table when there are no tiers', async () => { + renderPage([], []); + + await waitFor(() => { + expect( + screen.getByText('No storage tiers yet. Create one to get started.'), + ).toBeInTheDocument(); + }); + expect(screen.queryByRole('table')).not.toBeInTheDocument(); + }); + + it('navigates to the create route when Create tier is clicked', async () => { + const { user } = renderWithProviders( + + } /> + navigated-to-create} /> + , + { + apiFixtures: { storageTiers: defaultTiers, storageBackends: defaultBackends }, + routerEntries: ['/admin/storage/tiers'], + }, + ); + + await waitFor(() => { + expect(screen.getByText('fast')).toBeInTheDocument(); + }); + + await user.click(screen.getByRole('button', { name: 'Create tier' })); + + await waitFor(() => { + expect(screen.getByText('navigated-to-create')).toBeInTheDocument(); + }); + }); + + it('navigates to the edit route when a row Edit action is clicked', async () => { + const { user } = renderWithProviders( + + } /> + navigated-to-edit} /> + , + { + apiFixtures: { storageTiers: defaultTiers, storageBackends: defaultBackends }, + routerEntries: ['/admin/storage/tiers'], + }, + ); + + await waitFor(() => { + expect(screen.getByText('fast')).toBeInTheDocument(); + }); + + await user.click(screen.getByRole('button', { name: 'Actions for fast' })); + await user.click(screen.getByRole('menuitem', { name: 'Edit' })); + + await waitFor(() => { + expect(screen.getByText('navigated-to-edit')).toBeInTheDocument(); + }); + }); + + it('removes the row from the table when delete succeeds', async () => { + let tiers = [...defaultTiers]; + const { user } = renderWithProviders(, { + apiFixtures: { storageBackends: defaultBackends }, + transportOverrides: { + onStorageTierList: () => ({ items: tiers, size: tiers.length, total: tiers.length }), + onStorageTierDelete: (req) => { + tiers = tiers.filter((tier) => tier.id !== req.id); + return {}; + }, + }, + }); + + await waitFor(() => { + expect(screen.getByText('fast')).toBeInTheDocument(); + }); + + await user.click(screen.getByRole('button', { name: 'Actions for fast' })); + await user.click(screen.getByRole('menuitem', { name: 'Delete' })); + await user.click(screen.getByRole('button', { name: /^Delete$/i })); + + await waitFor(() => { + expect(screen.queryByText('fast')).not.toBeInTheDocument(); + }); + expect(screen.getByText('mixed')).toBeInTheDocument(); + }); +}); diff --git a/libs/ui-components/src/pages/admin/StorageTiersListPage.tsx b/libs/ui-components/src/pages/admin/StorageTiersListPage.tsx new file mode 100644 index 00000000..4412c738 --- /dev/null +++ b/libs/ui-components/src/pages/admin/StorageTiersListPage.tsx @@ -0,0 +1,123 @@ +import { useMemo } from 'react'; +import { useNavigate } from 'react-router-dom'; +import { Button, Flex, FlexItem, Stack, StackItem } from '@patternfly/react-core'; +import { Table, Tbody, Td, Th, Thead, Tr } from '@patternfly/react-table'; +import type { TFunction } from 'i18next'; + +import type { StorageTier } from '@osac/types/private'; +import { StorageProtocol } from '@osac/types/private'; + +import { + storageBackendIdsFilter, + usePrivateStorageBackends, +} from '../../api/v1/private/storage-backends'; +import { usePrivateStorageTiers } from '../../api/v1/private/storage-tiers'; +import ListPageBody from '../../components/Page/ListPageBody'; +import StorageTierActionsMenu from '../../components/Storage/StorageTierActionsMenu'; +import { StorageTierStatusLabel } from '../../components/Storage/StorageTierStatusLabel'; +import { SubtleContent } from '../../components/SubtleContent/SubtleContent'; +import { useTranslation } from '../../hooks/useTranslation'; + +const protocolLabel = (t: TFunction, protocol: StorageProtocol): string => { + switch (protocol) { + case StorageProtocol.NFS: + return t('NFS'); + case StorageProtocol.BLOCK: + return t('Block'); + default: + return '—'; + } +}; + +const uniqueBackendIds = (tiers: StorageTier[]): string[] => { + const ids = new Set(); + tiers.forEach((tier) => { + tier.spec?.backends.forEach((backend) => ids.add(backend.backendId)); + }); + return Array.from(ids).sort(); +}; + +export const StorageTiersListPage = () => { + const { t } = useTranslation(); + const navigate = useNavigate(); + + const { data: tiers = [], isLoading, error } = usePrivateStorageTiers(); + + const backendIds = useMemo(() => uniqueBackendIds(tiers), [tiers]); + + const { data: backends = [] } = usePrivateStorageBackends({ + filter: storageBackendIdsFilter(backendIds), + }); + + const backendsById = useMemo( + () => new Map(backends.map((backend) => [backend.id, backend])), + [backends], + ); + + return ( + + {!error && ( + + + + + + + + )} + + + {tiers.length === 0 ? ( + + {t('No storage tiers yet. Create one to get started.')} + + ) : ( + + + + + + + + + + + {tiers.map((tier) => { + const backendAssociations = tier.spec?.backends ?? []; + return ( + + + + + + + + ); + })} + +
{t('Name')}{t('Backends')}{t('Protocol(s)')}{t('State')} +
{tier.metadata?.name || tier.id} + {backendAssociations + .map( + (association) => + backendsById.get(association.backendId)?.metadata?.name ?? + association.backendId, + ) + .join(', ')} + + {backendAssociations + .map((association) => protocolLabel(t, association.protocol)) + .join(', ')} + + + + +
+ )} + + + + ); +}; From ed7ab19b00e3222a645e6a50c7b3c5fda33688f8 Mon Sep 17 00:00:00 2001 From: Elay Aharoni Date: Sun, 9 Aug 2026 14:38:55 +0300 Subject: [PATCH 06/10] OSAC-3604: wire StorageTiersListPage into the Tiers tab Assisted-by: Claude Code Signed-off-by: Elay Aharoni --- .../src/pages/admin/StorageManagementPage.test.tsx | 8 +++++--- .../src/pages/admin/StorageManagementPage.tsx | 3 ++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/libs/ui-components/src/pages/admin/StorageManagementPage.test.tsx b/libs/ui-components/src/pages/admin/StorageManagementPage.test.tsx index d97263c6..37271b63 100644 --- a/libs/ui-components/src/pages/admin/StorageManagementPage.test.tsx +++ b/libs/ui-components/src/pages/admin/StorageManagementPage.test.tsx @@ -31,10 +31,12 @@ describe('StorageManagementPage', () => { expect(screen.getByRole('tabpanel')).toHaveTextContent('Storage backends'); }); - it('shows the Tiers placeholder when activeTab is tiers', () => { + it('renders the Storage Tiers list page when activeTab is tiers', async () => { renderPage('tiers'); - expect(screen.getByRole('tabpanel')).toHaveTextContent('Storage tiers'); + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Create tier' })).toBeInTheDocument(); + }); }); it('navigates to /admin/storage/tiers when the Tiers tab is clicked', async () => { @@ -43,7 +45,7 @@ describe('StorageManagementPage', () => { await user.click(screen.getByRole('tab', { name: 'Tiers' })); await waitFor(() => { - expect(screen.getByRole('tabpanel')).toHaveTextContent('Storage tiers'); + expect(screen.getByRole('button', { name: 'Create tier' })).toBeInTheDocument(); }); }); }); diff --git a/libs/ui-components/src/pages/admin/StorageManagementPage.tsx b/libs/ui-components/src/pages/admin/StorageManagementPage.tsx index f2f079a5..e7006b5b 100644 --- a/libs/ui-components/src/pages/admin/StorageManagementPage.tsx +++ b/libs/ui-components/src/pages/admin/StorageManagementPage.tsx @@ -6,6 +6,7 @@ import ListPageBody from '@osac/ui-components/components/Page/ListPageBody'; import { useTranslation } from '@osac/ui-components/hooks/useTranslation'; import { StoragePlaceholder } from './StoragePlaceholder'; +import { StorageTiersListPage } from './StorageTiersListPage'; type StorageTab = 'backends' | 'tiers'; @@ -35,7 +36,7 @@ export const StorageManagementPage = ({ activeTab }: { activeTab: StorageTab }) {t('Tiers')}}> - + From 8c12f8e609562cd6942c25c1c4715bc1662dee32 Mon Sep 17 00:00:00 2001 From: Elay Aharoni Date: Sun, 9 Aug 2026 14:43:20 +0300 Subject: [PATCH 07/10] OSAC-3604: add tiers/create and tiers/:id/edit routes Assisted-by: Claude Code Signed-off-by: Elay Aharoni --- .../src/shell/StorageRoutes.test.tsx | 20 ++++++++++++++++--- apps/app-frontend/src/shell/StorageRoutes.tsx | 8 ++++++++ libs/i18n/locales/en/translation.json | 2 ++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/apps/app-frontend/src/shell/StorageRoutes.test.tsx b/apps/app-frontend/src/shell/StorageRoutes.test.tsx index d1911a6b..9648527b 100644 --- a/apps/app-frontend/src/shell/StorageRoutes.test.tsx +++ b/apps/app-frontend/src/shell/StorageRoutes.test.tsx @@ -1,5 +1,5 @@ import { Route, Routes } from 'react-router-dom'; -import { screen } from '@testing-library/react'; +import { screen, waitFor } from '@testing-library/react'; import { describe, expect, it } from 'vitest'; import { renderWithProviders } from '@osac/ui-components/test-utils/TestProviders'; @@ -33,9 +33,23 @@ describe('StorageRoutes', () => { expect(screen.getByText('Edit storage backend')).toBeInTheDocument(); }); - it('renders the Tiers tab at /admin/storage/tiers', () => { + it('renders the Tiers tab at /admin/storage/tiers', async () => { renderAt('/admin/storage/tiers'); - expect(screen.getByRole('tabpanel')).toHaveTextContent('Storage tiers'); + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Create tier' })).toBeInTheDocument(); + }); + }); + + it('renders a placeholder for tiers/create', () => { + renderAt('/admin/storage/tiers/create'); + + expect(screen.getByText('Create storage tier')).toBeInTheDocument(); + }); + + it('renders a placeholder for tiers/:id/edit', () => { + renderAt('/admin/storage/tiers/tier-123/edit'); + + expect(screen.getByText('Edit storage tier')).toBeInTheDocument(); }); }); diff --git a/apps/app-frontend/src/shell/StorageRoutes.tsx b/apps/app-frontend/src/shell/StorageRoutes.tsx index 108a974b..7abe10fd 100644 --- a/apps/app-frontend/src/shell/StorageRoutes.tsx +++ b/apps/app-frontend/src/shell/StorageRoutes.tsx @@ -20,6 +20,14 @@ export const StorageRoutes = () => { element={} /> } /> + } + /> + } + /> ); }; diff --git a/libs/i18n/locales/en/translation.json b/libs/i18n/locales/en/translation.json index 8569e528..7b6d8a01 100644 --- a/libs/i18n/locales/en/translation.json +++ b/libs/i18n/locales/en/translation.json @@ -127,6 +127,7 @@ "Create Identity provider steps": "Create Identity provider steps", "Create security group": "Create security group", "Create storage backend": "Create storage backend", + "Create storage tier": "Create storage tier", "Create subnet": "Create subnet", "Create tenant": "Create tenant", "Create tier": "Create tier", @@ -162,6 +163,7 @@ "Edit Identity provider": "Edit Identity provider", "Edit rule": "Edit rule", "Edit storage backend": "Edit storage backend", + "Edit storage tier": "Edit storage tier", "Editable": "Editable", "Editable fields can be changed when creating from this catalog item. Fixed fields use the default value shown.": "Editable fields can be changed when creating from this catalog item. Fixed fields use the default value shown.", "Enable": "Enable", From 0fa4deb82d894b9e3bb2086831f29294f8c212be Mon Sep 17 00:00:00 2001 From: Elay Aharoni Date: Sun, 9 Aug 2026 15:08:28 +0300 Subject: [PATCH 08/10] OSAC-3604: address validation review findings StorageManagementPage's Tabs mounted both tab bodies simultaneously (PatternFly's default, hiding the inactive one via CSS), so every visit to the Backends tab also mounted StorageTiersListPage and fired its usePrivateStorageTiers/usePrivateStorageBackends fetches for content the user never sees. Harmless while both tabs held inert placeholders, but StorageTiersListPage is now a real, self-fetching page. Add mountOnEnter/unmountOnExit so only the active tab's content mounts. Assisted-by: Claude Code Signed-off-by: Elay Aharoni --- .../admin/StorageManagementPage.test.tsx | 24 ++++++++++++++++++- .../src/pages/admin/StorageManagementPage.tsx | 2 ++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/libs/ui-components/src/pages/admin/StorageManagementPage.test.tsx b/libs/ui-components/src/pages/admin/StorageManagementPage.test.tsx index 37271b63..b8cc3efa 100644 --- a/libs/ui-components/src/pages/admin/StorageManagementPage.test.tsx +++ b/libs/ui-components/src/pages/admin/StorageManagementPage.test.tsx @@ -1,6 +1,6 @@ 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 { StorageManagementPage } from './StorageManagementPage'; import { renderWithProviders } from '../../test-utils/TestProviders'; @@ -48,4 +48,26 @@ describe('StorageManagementPage', () => { expect(screen.getByRole('button', { name: 'Create tier' })).toBeInTheDocument(); }); }); + + it('does not mount the Tiers list page (and its data fetches) while the Backends tab is active', async () => { + const onStorageTierList = vi.fn(() => ({ items: [], size: 0, total: 0 })); + + renderWithProviders( + + } + /> + , + { + routerEntries: ['/admin/storage/backends'], + transportOverrides: { onStorageTierList }, + }, + ); + + await waitFor(() => { + expect(screen.getByRole('tabpanel')).toHaveTextContent('Storage backends'); + }); + expect(onStorageTierList).not.toHaveBeenCalled(); + }); }); diff --git a/libs/ui-components/src/pages/admin/StorageManagementPage.tsx b/libs/ui-components/src/pages/admin/StorageManagementPage.tsx index e7006b5b..b5408437 100644 --- a/libs/ui-components/src/pages/admin/StorageManagementPage.tsx +++ b/libs/ui-components/src/pages/admin/StorageManagementPage.tsx @@ -31,6 +31,8 @@ export const StorageManagementPage = ({ activeTab }: { activeTab: StorageTab }) } }} aria-label={t('Storage tabs')} + mountOnEnter + unmountOnExit > {t('Backends')}}> From 0920ea63d3709862aea38b0ff547fa2097a4e095 Mon Sep 17 00:00:00 2001 From: Elay Aharoni Date: Sun, 9 Aug 2026 17:21:40 +0300 Subject: [PATCH 09/10] OSAC-3604: address code review feedback Gate the Tiers list page's backend-name lookup with an enabled option on usePrivateStorageBackends (mirroring the existing pattern in networking.ts), avoiding a wasted this.id in [] request on every page load. Surface backend-lookup failures as a distinct inline warning instead of letting them look identical to the per-row deleted-backend fallback. Assisted-by: Claude Code Signed-off-by: Elay Aharoni --- libs/i18n/locales/en/translation.json | 2 + .../src/api/v1/private/storage-backends.ts | 10 +- .../pages/admin/StorageTiersListPage.test.tsx | 35 +++++- .../src/pages/admin/StorageTiersListPage.tsx | 114 ++++++++++-------- 4 files changed, 109 insertions(+), 52 deletions(-) diff --git a/libs/i18n/locales/en/translation.json b/libs/i18n/locales/en/translation.json index 7b6d8a01..65c70498 100644 --- a/libs/i18n/locales/en/translation.json +++ b/libs/i18n/locales/en/translation.json @@ -19,6 +19,7 @@ "Authorization URL": "Authorization URL", "Authorization URL is required": "Authorization URL is required", "Back": "Back", + "Backend IDs are shown in place of names until this recovers. This is separate from the normal fallback shown when a tier references a backend that no longer exists.": "Backend IDs are shown in place of names until this recovers. This is separate from the normal fallback shown when a tier references a backend that no longer exists.", "Backends": "Backends", "Bare Metal": "Bare Metal", "bare metal instance": "bare metal instance", @@ -406,6 +407,7 @@ "Token URL is required": "Token URL is required", "Type": "Type", "UDP": "UDP", + "Unable to resolve backend names": "Unable to resolve backend names", "Unauthorized": "Unauthorized", "Unknown": "Unknown", "Unspecified": "Unspecified", diff --git a/libs/ui-components/src/api/v1/private/storage-backends.ts b/libs/ui-components/src/api/v1/private/storage-backends.ts index 87ec175a..0b95eb0d 100644 --- a/libs/ui-components/src/api/v1/private/storage-backends.ts +++ b/libs/ui-components/src/api/v1/private/storage-backends.ts @@ -20,12 +20,20 @@ export const STORAGE_BACKEND_READY_LIST_FILTER = `this.status.state == ${Storage export const storageBackendIdsFilter = (ids: string[]): string => `this.id in [${ids.map((id) => `"${escapeCelStringLiteral(id)}"`).join(', ')}]`; -export const usePrivateStorageBackends = (params: ListParams = {}) => { +type StorageBackendsListOptions = { + enabled?: boolean; +}; + +export const usePrivateStorageBackends = ( + params: ListParams = {}, + options: StorageBackendsListOptions = {}, +) => { const client = useApiFetch(StorageBackends); return useApiQuery({ queryKey: apiQueryKey('v1/private/storage_backends', undefined, params), queryFn: () => client.list(params), select: (data) => data.items, + enabled: options.enabled ?? true, }); }; diff --git a/libs/ui-components/src/pages/admin/StorageTiersListPage.test.tsx b/libs/ui-components/src/pages/admin/StorageTiersListPage.test.tsx index 48b60889..943135d3 100644 --- a/libs/ui-components/src/pages/admin/StorageTiersListPage.test.tsx +++ b/libs/ui-components/src/pages/admin/StorageTiersListPage.test.tsx @@ -1,6 +1,7 @@ import { Route, Routes } from 'react-router-dom'; +import { Code, ConnectError } from '@connectrpc/connect'; import { screen, waitFor } from '@testing-library/react'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import type { StorageBackend, StorageTier } from '@osac/types/private'; import { StorageBackendState, StorageProtocol, StorageTierState } from '@osac/types/private'; @@ -116,6 +117,38 @@ describe('StorageTiersListPage', () => { expect(capturedFilter).toBe(storageBackendIdsFilter(expectedIds)); }); + it('does not request backend names when there are no tiers', async () => { + const onStorageBackendList = vi.fn(() => ({ items: [], size: 0, total: 0 })); + + renderWithProviders(, { + apiFixtures: { storageTiers: [], storageBackends: defaultBackends }, + transportOverrides: { onStorageBackendList }, + }); + + await waitFor(() => { + expect( + screen.getByText('No storage tiers yet. Create one to get started.'), + ).toBeInTheDocument(); + }); + expect(onStorageBackendList).not.toHaveBeenCalled(); + }); + + it('shows a warning banner when the backend-name lookup fails, without breaking the per-row id fallback', async () => { + renderWithProviders(, { + apiFixtures: { storageTiers: defaultTiers, storageBackends: defaultBackends }, + transportOverrides: { + onStorageBackendList: () => { + throw new ConnectError('backend service unavailable', Code.Unavailable); + }, + }, + }); + + await waitFor(() => { + expect(screen.getByText('Unable to resolve backend names')).toBeInTheDocument(); + }); + expect(screen.getByText('backend-a')).toBeInTheDocument(); + }); + it('shows the empty state and no table when there are no tiers', async () => { renderPage([], []); diff --git a/libs/ui-components/src/pages/admin/StorageTiersListPage.tsx b/libs/ui-components/src/pages/admin/StorageTiersListPage.tsx index 4412c738..585505e6 100644 --- a/libs/ui-components/src/pages/admin/StorageTiersListPage.tsx +++ b/libs/ui-components/src/pages/admin/StorageTiersListPage.tsx @@ -1,6 +1,6 @@ import { useMemo } from 'react'; import { useNavigate } from 'react-router-dom'; -import { Button, Flex, FlexItem, Stack, StackItem } from '@patternfly/react-core'; +import { Alert, Button, Flex, FlexItem, Stack, StackItem } from '@patternfly/react-core'; import { Table, Tbody, Td, Th, Thead, Tr } from '@patternfly/react-table'; import type { TFunction } from 'i18next'; @@ -45,9 +45,10 @@ export const StorageTiersListPage = () => { const backendIds = useMemo(() => uniqueBackendIds(tiers), [tiers]); - const { data: backends = [] } = usePrivateStorageBackends({ - filter: storageBackendIdsFilter(backendIds), - }); + const { data: backends = [], error: backendsError } = usePrivateStorageBackends( + { filter: storageBackendIdsFilter(backendIds) }, + { enabled: backendIds.length > 0 }, + ); const backendsById = useMemo( () => new Map(backends.map((backend) => [backend.id, backend])), @@ -69,53 +70,66 @@ export const StorageTiersListPage = () => { )} - {tiers.length === 0 ? ( - - {t('No storage tiers yet. Create one to get started.')} - - ) : ( - - - - - - - - - - - {tiers.map((tier) => { - const backendAssociations = tier.spec?.backends ?? []; - return ( - - - - - - + + {Boolean(backendsError) && ( + + + {t( + 'Backend IDs are shown in place of names until this recovers. This is separate from the normal fallback shown when a tier references a backend that no longer exists.', + )} + + + )} + + {tiers.length === 0 ? ( + + {t('No storage tiers yet. Create one to get started.')} + + ) : ( +
{t('Name')}{t('Backends')}{t('Protocol(s)')}{t('State')} -
{tier.metadata?.name || tier.id} - {backendAssociations - .map( - (association) => - backendsById.get(association.backendId)?.metadata?.name ?? - association.backendId, - ) - .join(', ')} - - {backendAssociations - .map((association) => protocolLabel(t, association.protocol)) - .join(', ')} - - - - -
+ + + + + + + - ); - })} - -
{t('Name')}{t('Backends')}{t('Protocol(s)')}{t('State')}
- )} + + + {tiers.map((tier) => { + const backendAssociations = tier.spec?.backends ?? []; + return ( + + {tier.metadata?.name || tier.id} + + {backendAssociations + .map( + (association) => + backendsById.get(association.backendId)?.metadata?.name ?? + association.backendId, + ) + .join(', ')} + + + {backendAssociations + .map((association) => protocolLabel(t, association.protocol)) + .join(', ')} + + + + + + + + + ); + })} + + + )} + + From 21b95ed62fed64df4324bfc43fb1206c2f49897a Mon Sep 17 00:00:00 2001 From: Elay Aharoni Date: Mon, 10 Aug 2026 10:06:04 +0300 Subject: [PATCH 10/10] OSAC-3604: address CodeRabbit and batzionb PR feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Keep the Create tier action available when the tier list fails to load — a failed List doesn't mean creation is broken (CodeRabbit). - Add a test covering unmountOnExit for the Tiers tab, not just mountOnEnter (CodeRabbit). - Rename the STATE column to Status and move it right after Name, to match every other resource table in this codebase (batzionb). Assisted-by: Claude Code Signed-off-by: Elay Aharoni --- libs/i18n/locales/en/translation.json | 1 - .../admin/StorageManagementPage.test.tsx | 31 +++++++++++++++++++ .../pages/admin/StorageTiersListPage.test.tsx | 17 +++++++++- .../src/pages/admin/StorageTiersListPage.tsx | 28 ++++++++--------- 4 files changed, 60 insertions(+), 17 deletions(-) diff --git a/libs/i18n/locales/en/translation.json b/libs/i18n/locales/en/translation.json index 65c70498..e6fdfdf9 100644 --- a/libs/i18n/locales/en/translation.json +++ b/libs/i18n/locales/en/translation.json @@ -368,7 +368,6 @@ "SSH public key must be in the form \"[TYPE] key [[EMAIL]]\". Supported types are ssh-rsa, ssh-ed25519, and ecdsa-sha2-nistp256/384/521.": "SSH public key must be in the form \"[TYPE] key [[EMAIL]]\". Supported types are ssh-rsa, ssh-ed25519, and ecdsa-sha2-nistp256/384/521.", "Start": "Start", "Starting": "Starting", - "State": "State", "Status": "Status", "Stop": "Stop", "Stopped": "Stopped", diff --git a/libs/ui-components/src/pages/admin/StorageManagementPage.test.tsx b/libs/ui-components/src/pages/admin/StorageManagementPage.test.tsx index b8cc3efa..eabb229f 100644 --- a/libs/ui-components/src/pages/admin/StorageManagementPage.test.tsx +++ b/libs/ui-components/src/pages/admin/StorageManagementPage.test.tsx @@ -70,4 +70,35 @@ describe('StorageManagementPage', () => { }); expect(onStorageTierList).not.toHaveBeenCalled(); }); + + it('unmounts the Tiers list page (and stops its data fetches) after switching away to the Backends tab', async () => { + const onStorageTierList = vi.fn(() => ({ items: [], size: 0, total: 0 })); + + const { user } = renderWithProviders( + + } + /> + } /> + , + { + routerEntries: ['/admin/storage/tiers'], + transportOverrides: { onStorageTierList }, + }, + ); + + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Create tier' })).toBeInTheDocument(); + }); + expect(onStorageTierList).toHaveBeenCalledTimes(1); + + await user.click(screen.getByRole('tab', { name: 'Backends' })); + + await waitFor(() => { + expect(screen.getByRole('tabpanel')).toHaveTextContent('Storage backends'); + }); + expect(screen.queryByRole('button', { name: 'Create tier' })).not.toBeInTheDocument(); + expect(onStorageTierList).toHaveBeenCalledTimes(1); + }); }); diff --git a/libs/ui-components/src/pages/admin/StorageTiersListPage.test.tsx b/libs/ui-components/src/pages/admin/StorageTiersListPage.test.tsx index 943135d3..0a4da055 100644 --- a/libs/ui-components/src/pages/admin/StorageTiersListPage.test.tsx +++ b/libs/ui-components/src/pages/admin/StorageTiersListPage.test.tsx @@ -84,7 +84,7 @@ describe('StorageTiersListPage', () => { expect(screen.getByText('Block, NFS')).toBeInTheDocument(); }); - it('renders the STATE column via StorageTierStatusLabel', async () => { + it('renders the STATUS column via StorageTierStatusLabel', async () => { renderPage(); await waitFor(() => { @@ -183,6 +183,21 @@ describe('StorageTiersListPage', () => { }); }); + it('keeps the Create tier action available when the tier list fails to load', async () => { + renderWithProviders(, { + apiFixtures: { storageBackends: defaultBackends }, + transportOverrides: { + onStorageTierList: () => { + throw new ConnectError('tier service unavailable', Code.Unavailable); + }, + }, + }); + + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Create tier' })).toBeInTheDocument(); + }); + }); + it('navigates to the edit route when a row Edit action is clicked', async () => { const { user } = renderWithProviders( diff --git a/libs/ui-components/src/pages/admin/StorageTiersListPage.tsx b/libs/ui-components/src/pages/admin/StorageTiersListPage.tsx index 585505e6..dd319de8 100644 --- a/libs/ui-components/src/pages/admin/StorageTiersListPage.tsx +++ b/libs/ui-components/src/pages/admin/StorageTiersListPage.tsx @@ -57,17 +57,15 @@ export const StorageTiersListPage = () => { return ( - {!error && ( - - - - - - - - )} + + + + + + + @@ -90,9 +88,9 @@ export const StorageTiersListPage = () => { {t('Name')} + {t('Status')} {t('Backends')} {t('Protocol(s)')} - {t('State')} @@ -102,6 +100,9 @@ export const StorageTiersListPage = () => { return ( {tier.metadata?.name || tier.id} + + + {backendAssociations .map( @@ -116,9 +117,6 @@ export const StorageTiersListPage = () => { .map((association) => protocolLabel(t, association.protocol)) .join(', ')} - - -