From 20f2c2bb08006fac4c5e36b205595da4bf0face3 Mon Sep 17 00:00:00 2001 From: batzionb Date: Sun, 9 Aug 2026 14:59:37 +0300 Subject: [PATCH 1/8] OSAC-3780: add admin instance type admin routes and list page Wire the provider-admin instance type pages into the admin shell, add the private instance type hook and lifecycle labels, and rebalance the list table for the reviewed browser layout. Assisted-by: Cursor Signed-off-by: batzionb --- .gitignore | 1 + apps/app-frontend/src/shell/AppShell.test.tsx | 45 ++++++ apps/app-frontend/src/shell/AppShell.tsx | 11 +- .../src/shell/InstanceTypeRoutes.test.tsx | 35 +++++ .../src/shell/InstanceTypeRoutes.tsx | 11 ++ .../src/shell/StorageRoutes.test.tsx | 14 +- apps/app-frontend/src/shell/shellNav.test.ts | 26 +++- apps/app-frontend/src/shell/shellNav.ts | 18 ++- libs/i18n/locales/en/translation.json | 12 ++ libs/ui-components/src/api/types.ts | 1 + .../src/api/v1/private/instance-type.test.ts | 71 ++++++++++ .../src/api/v1/private/instance-type.ts | 14 ++ .../AdminInstanceTypeCreatePage.tsx | 40 ++++++ .../AdminInstanceTypeListPage.test.tsx | 128 ++++++++++++++++++ .../AdminInstanceTypeListPage.tsx | 91 +++++++++++++ .../InstanceTypeLifecycleLabel.test.tsx | 45 ++++++ .../InstanceTypeLifecycleLabel.tsx | 36 +++++ .../admin/StorageManagementPage.test.tsx | 11 +- .../src/pages/admin/StorageManagementPage.tsx | 2 +- .../test-utils/createMockConnectTransport.ts | 15 ++ 20 files changed, 610 insertions(+), 17 deletions(-) create mode 100644 apps/app-frontend/src/shell/AppShell.test.tsx create mode 100644 apps/app-frontend/src/shell/InstanceTypeRoutes.test.tsx create mode 100644 apps/app-frontend/src/shell/InstanceTypeRoutes.tsx create mode 100644 libs/ui-components/src/api/v1/private/instance-type.test.ts create mode 100644 libs/ui-components/src/api/v1/private/instance-type.ts create mode 100644 libs/ui-components/src/components/InstanceType/AdminInstanceTypeCreatePage.tsx create mode 100644 libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.test.tsx create mode 100644 libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.tsx create mode 100644 libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx create mode 100644 libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx diff --git a/.gitignore b/.gitignore index 8ab78d26..7149abe6 100644 --- a/.gitignore +++ b/.gitignore @@ -55,6 +55,7 @@ proxy/osac-proxy # an e2e test suite. Live session state and run output, never committed. apps/playwright/.auth/ apps/playwright/test-results/ +/test-results/ apps/playwright/playwright-report/ apps/playwright/blob-report/ diff --git a/apps/app-frontend/src/shell/AppShell.test.tsx b/apps/app-frontend/src/shell/AppShell.test.tsx new file mode 100644 index 00000000..3ae5f9c6 --- /dev/null +++ b/apps/app-frontend/src/shell/AppShell.test.tsx @@ -0,0 +1,45 @@ +import { screen, waitFor } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; + +import { SessionProvider } from '@osac/ui-components/hooks/use-session'; +import { renderWithProviders } from '@osac/ui-components/test-utils/TestProviders'; + +vi.mock('./StorageRoutes', () => ({ + StorageRoutes: () =>

Storage routes

, +})); + +import { AppShell } from './AppShell'; + +const renderAppShell = (entry: string) => + renderWithProviders( + + + , + { + apiFixtures: { privateInstanceTypes: [] }, + routerEntries: [entry], + }, + ); + +describe('AppShell', () => { + it('renders the storage route through the admin shell', () => { + renderAppShell('/admin/infrastructure/storage/backends'); + + expect(screen.getByRole('heading', { name: 'Storage routes' })).toBeInTheDocument(); + }); + + it('renders the instance type list route through the admin shell', async () => { + renderAppShell('/admin/infrastructure/instance-types'); + + expect(screen.getByRole('heading', { name: 'Instance types' })).toBeInTheDocument(); + await waitFor(() => { + expect(screen.getByText('No instance types yet.')).toBeInTheDocument(); + }); + }); + + it('renders the instance type create shell through the admin shell', () => { + renderAppShell('/admin/infrastructure/instance-types/create'); + + expect(screen.getByRole('heading', { name: 'Create instance type' })).toBeInTheDocument(); + }); +}); diff --git a/apps/app-frontend/src/shell/AppShell.tsx b/apps/app-frontend/src/shell/AppShell.tsx index bad4b903..119954b2 100644 --- a/apps/app-frontend/src/shell/AppShell.tsx +++ b/apps/app-frontend/src/shell/AppShell.tsx @@ -16,6 +16,7 @@ import { ClusterRoutes } from '@osac/ui-components/pages/tenant/ClusterRoutes'; import { VmCreatePage } from '@osac/ui-components/pages/tenant/VmCreatePage'; import { VmListPage } from '@osac/ui-components/pages/tenant/VmListPage'; +import { InstanceTypeRoutes } from './InstanceTypeRoutes'; import { ShellMasthead } from './ShellMasthead'; import { defaultRouteForRole } from './shellRoutes'; import { ShellSidebar } from './ShellSidebar'; @@ -49,13 +50,21 @@ export const AppShell = ({ logout }: { logout: () => Promise }) => { } /> } /> + + + + } + /> ({ + default: () =>

Instance types

, +})); + +vi.mock('@osac/ui-components/components/InstanceType/AdminInstanceTypeCreatePage', () => ({ + default: () =>

Create instance type

, +})); + +import { InstanceTypeRoutes } from './InstanceTypeRoutes'; + +const renderRoutes = (initialEntry: string) => ( + + + } /> + + +); + +describe('InstanceTypeRoutes', () => { + it('renders the list page on the index route', () => { + render(renderRoutes('/admin/infrastructure/instance-types')); + + expect(screen.getByRole('heading', { name: 'Instance types' })).toBeInTheDocument(); + }); + + it('renders the create page shell on the create route', () => { + render(renderRoutes('/admin/infrastructure/instance-types/create')); + + expect(screen.getByRole('heading', { name: 'Create instance type' })).toBeInTheDocument(); + }); +}); diff --git a/apps/app-frontend/src/shell/InstanceTypeRoutes.tsx b/apps/app-frontend/src/shell/InstanceTypeRoutes.tsx new file mode 100644 index 00000000..a4116453 --- /dev/null +++ b/apps/app-frontend/src/shell/InstanceTypeRoutes.tsx @@ -0,0 +1,11 @@ +import { Route, Routes } from 'react-router-dom'; + +import AdminInstanceTypeCreatePage from '@osac/ui-components/components/InstanceType/AdminInstanceTypeCreatePage'; +import AdminInstanceTypeListPage from '@osac/ui-components/components/InstanceType/AdminInstanceTypeListPage'; + +export const InstanceTypeRoutes = () => ( + + } /> + } /> + +); diff --git a/apps/app-frontend/src/shell/StorageRoutes.test.tsx b/apps/app-frontend/src/shell/StorageRoutes.test.tsx index d1911a6b..0775d7ee 100644 --- a/apps/app-frontend/src/shell/StorageRoutes.test.tsx +++ b/apps/app-frontend/src/shell/StorageRoutes.test.tsx @@ -9,32 +9,32 @@ import { StorageRoutes } from './StorageRoutes'; const renderAt = (path: string) => renderWithProviders( - } /> + } /> , { routerEntries: [path] }, ); describe('StorageRoutes', () => { - it('redirects the bare /admin/storage path to the Backends tab', () => { - renderAt('/admin/storage'); + it('redirects the bare /admin/infrastructure/storage path to the Backends tab', () => { + renderAt('/admin/infrastructure/storage'); expect(screen.getByRole('tabpanel')).toHaveTextContent('Storage backends'); }); it('renders a placeholder for backends/create', () => { - renderAt('/admin/storage/backends/create'); + renderAt('/admin/infrastructure/storage/backends/create'); expect(screen.getByText('Create storage backend')).toBeInTheDocument(); }); it('renders a placeholder for backends/:id/edit', () => { - renderAt('/admin/storage/backends/abc-123/edit'); + renderAt('/admin/infrastructure/storage/backends/abc-123/edit'); expect(screen.getByText('Edit storage backend')).toBeInTheDocument(); }); - it('renders the Tiers tab at /admin/storage/tiers', () => { - renderAt('/admin/storage/tiers'); + it('renders the Tiers tab at /admin/infrastructure/storage/tiers', () => { + renderAt('/admin/infrastructure/storage/tiers'); expect(screen.getByRole('tabpanel')).toHaveTextContent('Storage tiers'); }); diff --git a/apps/app-frontend/src/shell/shellNav.test.ts b/apps/app-frontend/src/shell/shellNav.test.ts index 9cf96eee..76fbb94e 100644 --- a/apps/app-frontend/src/shell/shellNav.test.ts +++ b/apps/app-frontend/src/shell/shellNav.test.ts @@ -43,13 +43,35 @@ describe('navRowsForRole', () => { } }); - it('includes Tenants and Storage under Administration for admin role', () => { + it('includes only Tenants under Administration for admin role', () => { expect(findSection('admin', 'nav-administration')?.children).toEqual([ { id: 'tenant', label: 'Tenants', path: '/admin/tenants' }, - { id: 'storage', label: 'Storage', path: '/admin/storage' }, ]); }); + it('Infrastructure section shows up only for admin role and contains storage and instance types', () => { + expect(findSection('admin', 'nav-infrastructure')).toEqual({ + kind: 'section', + sectionId: 'nav-infrastructure', + label: 'Infrastructure', + children: [ + { + id: 'storage', + label: 'Storage', + path: '/admin/infrastructure/storage', + }, + { + id: 'instance-types', + label: 'Instance Types', + path: '/admin/infrastructure/instance-types', + }, + ], + }); + for (const role of ['tenant-user', 'tenant-admin', 'tenant-idp-manager'] as UserRole[]) { + expect(findSection(role, 'nav-infrastructure')).toBeUndefined(); + } + }); + it('IDP administration shows up only for idp manager', () => { expect(findSection('tenant-idp-manager', 'nav-tenant-administration')).toBeDefined(); for (const role of ['tenant-user', 'tenant-admin', 'admin'] as UserRole[]) { diff --git a/apps/app-frontend/src/shell/shellNav.ts b/apps/app-frontend/src/shell/shellNav.ts index 90e87d35..470f7269 100644 --- a/apps/app-frontend/src/shell/shellNav.ts +++ b/apps/app-frontend/src/shell/shellNav.ts @@ -26,9 +26,23 @@ const getAdminNav = (t: TFunction): NavSection[] => [ kind: 'section', sectionId: 'nav-administration', label: t('Administration'), + children: [{ id: 'tenant', label: t('Tenants'), path: '/admin/tenants' }], + }, + { + kind: 'section', + sectionId: 'nav-infrastructure', + label: t('Infrastructure'), children: [ - { id: 'tenant', label: t('Tenants'), path: '/admin/tenants' }, - { id: 'storage', label: t('Storage'), path: '/admin/storage' }, + { + id: 'storage', + label: t('Storage'), + path: '/admin/infrastructure/storage', + }, + { + id: 'instance-types', + label: t('Instance Types'), + path: '/admin/infrastructure/instance-types', + }, ], }, ...getBaseNav(t), diff --git a/libs/i18n/locales/en/translation.json b/libs/i18n/locales/en/translation.json index 4e223a3d..9907a1a9 100644 --- a/libs/i18n/locales/en/translation.json +++ b/libs/i18n/locales/en/translation.json @@ -1,6 +1,7 @@ { "Actions": "Actions", "Actions for {{name}}": "Actions for {{name}}", + "Active": "Active", "Add": "Add", "Add domain": "Add domain", "Add node set": "Add node set", @@ -118,12 +119,14 @@ "Copy": "Copy", "Could not load host types": "Could not load host types", "Could not load instance types": "Could not load instance types", + "CPU cores": "CPU cores", "Create": "Create", "Create cluster": "Create cluster", "Create cluster wizard": "Create cluster wizard", "Create identity provider": "Create identity provider", "Create Identity provider": "Create Identity provider", "Create Identity provider steps": "Create Identity provider steps", + "Create instance type": "Create instance type", "Create security group": "Create security group", "Create storage backend": "Create storage backend", "Create subnet": "Create subnet", @@ -142,6 +145,7 @@ "Delete security group?": "Delete security group?", "Deleting": "Deleting", "deprecated": "deprecated", + "Deprecated": "Deprecated", "Description": "Description", "Destination CIDR": "Destination CIDR", "Details": "Details", @@ -224,7 +228,10 @@ "Identity providers": "Identity providers", "IdP manager": "IdP manager", "Inbound Rules": "Inbound Rules", + "Infrastructure": "Infrastructure", "Instance type": "Instance type", + "Instance types": "Instance types", + "Instance Types": "Instance Types", "Internal IP": "Internal IP", "Invalid IPv4 CIDR format (e.g., 192.168.1.0/24)": "Invalid IPv4 CIDR format (e.g., 192.168.1.0/24)", "Invalid IPv4 CIDR notation": "Invalid IPv4 CIDR notation", @@ -240,15 +247,18 @@ "Issuer is required": "Issuer is required", "JWKS URL": "JWKS URL", "Keep editing": "Keep editing", + "Lifecycle State": "Lifecycle State", "Loading cluster password": "Loading cluster password", "Loading security groups...": "Loading security groups...", "Loading subnets...": "Loading subnets...", "Logout URL": "Logout URL", "Manage firewall rules for your virtual networks.": "Manage firewall rules for your virtual networks.", "Manage identity providers for your tenant.": "Manage identity providers for your tenant.", + "Manage provider-defined instance types for this cloud platform.": "Manage provider-defined instance types for this cloud platform.", "Manage storage backends and tiers for this cloud platform.": "Manage storage backends and tiers for this cloud platform.", "Manage tenants for this cloud platform.": "Manage tenants for this cloud platform.", "Manage virtual networks for your compute instances.": "Manage virtual networks for your compute instances.", + "Memory (GiB)": "Memory (GiB)", "Message": "Message", "Must be a valid domain (e.g. example.com)": "Must be a valid domain (e.g. example.com)", "Must be a valid URL (e.g. https://example.com)": "Must be a valid URL (e.g. https://example.com)", @@ -271,6 +281,7 @@ "No identity providers match your search.": "No identity providers match your search.", "No identity providers yet. Create one to get started.": "No identity providers yet. Create one to get started.", "No inbound rules yet. Add one to allow incoming traffic.": "No inbound rules yet. Add one to allow incoming traffic.", + "No instance types yet.": "No instance types yet.", "No node sets added yet.": "No node sets added yet.", "No node sets configured.": "No node sets configured.", "No outbound rules yet. Add one to allow outgoing traffic.": "No outbound rules yet. Add one to allow outgoing traffic.", @@ -288,6 +299,7 @@ "Node Sets": "Node Sets", "Nodes": "Nodes", "Not defined": "Not defined", + "Obsolete": "Obsolete", "OIDC": "OIDC", "OIDC configuration": "OIDC configuration", "Open catalog item details for {{title}}": "Open catalog item details for {{title}}", diff --git a/libs/ui-components/src/api/types.ts b/libs/ui-components/src/api/types.ts index d7311c9a..2bf0463c 100644 --- a/libs/ui-components/src/api/types.ts +++ b/libs/ui-components/src/api/types.ts @@ -27,6 +27,7 @@ export type ApiRoute = | 'v1/external_ip_attachments' | 'v1/external_ip_pools' | 'v1/console_sessions' + | 'v1/private/instance_types' | 'v1/private/tenants' | 'v1/private/storage_backends' | 'v1/private/storage_tiers' diff --git a/libs/ui-components/src/api/v1/private/instance-type.test.ts b/libs/ui-components/src/api/v1/private/instance-type.test.ts new file mode 100644 index 00000000..28bb06a7 --- /dev/null +++ b/libs/ui-components/src/api/v1/private/instance-type.test.ts @@ -0,0 +1,71 @@ +import React, { type ReactNode, createElement } from 'react'; +import { create } from '@bufbuild/protobuf'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { renderHook, waitFor } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; + +import { + InstanceTypeSchema, + InstanceTypeState, + type InstanceType as PrivateInstanceType, +} from '@osac/types/private'; + +import { useAdminInstanceTypes } from './instance-type'; +import { createMockConnectTransport } from '../../../test-utils/createMockConnectTransport'; +import { ApiProvider } from '../../api-context'; + +const makeInstanceType = ( + id: string, + state: InstanceTypeState = InstanceTypeState.ACTIVE, +): PrivateInstanceType => + create(InstanceTypeSchema, { + id, + metadata: { name: `instance-type-${id}` }, + spec: { + description: `${id} description`, + cores: 4, + memoryGib: 16, + state, + }, + }); + +const makeWrapper = (transport: ReturnType) => { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + const wrapper = ({ children }: { children: ReactNode }) => + createElement( + ApiProvider, + { transport } as React.ComponentProps, + createElement(QueryClientProvider, { client: queryClient }, children), + ); + return { wrapper, queryClient }; +}; + +describe('useAdminInstanceTypes', () => { + it('returns all private instance type items from the list response', async () => { + const transport = createMockConnectTransport({ + privateInstanceTypes: [ + makeInstanceType('active-1', InstanceTypeState.ACTIVE), + makeInstanceType('deprecated-1', InstanceTypeState.DEPRECATED), + ], + }); + const { wrapper } = makeWrapper(transport); + const { result } = renderHook(() => useAdminInstanceTypes(), { wrapper }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(result.current.data?.map((item) => item.id)).toEqual(['active-1', 'deprecated-1']); + }); + + it('stores query results under the private instance type cache key', async () => { + const transport = createMockConnectTransport({ + privateInstanceTypes: [makeInstanceType('active-1')], + }); + const { wrapper, queryClient } = makeWrapper(transport); + const { result } = renderHook(() => useAdminInstanceTypes(), { wrapper }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(queryClient.getQueryData(['v1/private/instance_types'])).toBeDefined(); + expect(queryClient.getQueryData(['v1/instance_types'])).toBeUndefined(); + }); +}); diff --git a/libs/ui-components/src/api/v1/private/instance-type.ts b/libs/ui-components/src/api/v1/private/instance-type.ts new file mode 100644 index 00000000..9d371922 --- /dev/null +++ b/libs/ui-components/src/api/v1/private/instance-type.ts @@ -0,0 +1,14 @@ +import { InstanceTypes } from '@osac/types/private'; + +import { useApiFetch } from '../../api-context'; +import { type ListParams, apiQueryKey } from '../../types'; +import { useApiQuery } from '../../use-api-query'; + +export const useAdminInstanceTypes = (params: ListParams = {}) => { + const client = useApiFetch(InstanceTypes); + return useApiQuery({ + queryKey: apiQueryKey('v1/private/instance_types', undefined, params), + queryFn: () => client.list(params), + select: (data) => data.items, + }); +}; diff --git a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeCreatePage.tsx b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeCreatePage.tsx new file mode 100644 index 00000000..40386379 --- /dev/null +++ b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeCreatePage.tsx @@ -0,0 +1,40 @@ +import { useNavigate } from 'react-router-dom'; +import { + Breadcrumb, + BreadcrumbItem, + Button, + PageSection, + Stack, + Title, +} from '@patternfly/react-core'; + +import { useTranslation } from '../../hooks/useTranslation'; + +const AdminInstanceTypeCreatePage = () => { + const { t } = useTranslation(); + const navigate = useNavigate(); + + return ( + + + + + + + {t('Create')} + + + {t('Create instance type')} + + + + ); +}; + +export default AdminInstanceTypeCreatePage; diff --git a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.test.tsx b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.test.tsx new file mode 100644 index 00000000..6b07a522 --- /dev/null +++ b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.test.tsx @@ -0,0 +1,128 @@ +import { create } from '@bufbuild/protobuf'; +import { screen } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; + +import { + InstanceTypeSchema, + InstanceTypeState, + type InstanceType as PrivateInstanceType, +} from '@osac/types/private'; +import { mockQueryResult } from '@osac/ui-components/test-utils/query'; + +import AdminInstanceTypeListPage from './AdminInstanceTypeListPage'; +import { renderWithProviders } from '../../test-utils/TestProviders'; + +vi.mock('@osac/ui-components/api/v1/private/instance-type', () => ({ + useAdminInstanceTypes: vi.fn(), +})); + +const { useAdminInstanceTypes } = await import('@osac/ui-components/api/v1/private/instance-type'); + +const longDescription = + 'A provider-curated general-purpose instance type for sustained workloads that need predictable CPU and memory capacity, room for sidecar processes, and enough headroom for bursty background tasks without immediately resizing the virtual machine.'; + +const makeInstanceType = ( + id: string, + state: InstanceTypeState, + description = `${id} description`, +): PrivateInstanceType => + create(InstanceTypeSchema, { + id, + metadata: { + name: `instance-type-${id}`, + creationTimestamp: { seconds: BigInt(1717000000), nanos: 0 }, + }, + spec: { + cores: 4, + memoryGib: 16, + description, + state, + }, + }); + +const renderPage = () => renderWithProviders(); + +describe('AdminInstanceTypeListPage', () => { + it('renders the required columns and lifecycle labels for populated data', () => { + vi.mocked(useAdminInstanceTypes).mockReturnValue( + mockQueryResult({ + data: [ + makeInstanceType('active-1', InstanceTypeState.ACTIVE), + makeInstanceType('deprecated-1', InstanceTypeState.DEPRECATED), + makeInstanceType('obsolete-1', InstanceTypeState.OBSOLETE), + makeInstanceType('long-description-1', InstanceTypeState.ACTIVE, longDescription), + ], + }), + ); + + renderPage(); + + expect(screen.getByRole('heading', { name: 'Instance types' })).toBeInTheDocument(); + expect(screen.getAllByRole('columnheader').map((header) => header.textContent)).toEqual([ + 'Name', + 'Lifecycle State', + 'CPU cores', + 'Memory (GiB)', + 'Description', + 'Created', + ]); + expect(screen.getByText('instance-type-active-1')).toBeInTheDocument(); + expect(screen.getByText('active-1 description')).toBeInTheDocument(); + const truncatedDescription = Array.from( + document.querySelectorAll('.pf-v6-c-truncate__text'), + ).find((element) => element.textContent?.startsWith('A provider-curated general-purpose')); + expect(truncatedDescription).toBeDefined(); + expect(truncatedDescription).not.toBeNull(); + expect(truncatedDescription?.textContent).toContain('A provider-curated'); + expect(truncatedDescription?.closest('.pf-v6-c-truncate')).toHaveClass('pf-m-fixed'); + expect( + Array.from(document.querySelectorAll('.pf-v6-c-truncate__omission')).some( + (element) => element.textContent === '...', + ), + ).toBe(true); + expect(screen.getAllByText('Active')).toHaveLength(2); + expect(screen.getByText('Deprecated')).toBeInTheDocument(); + expect(screen.getByText('Obsolete')).toBeInTheDocument(); + }); + + it('shows a loading spinner while the query is in flight', () => { + vi.mocked(useAdminInstanceTypes).mockReturnValue( + mockQueryResult({ + data: undefined, + isLoading: true, + }), + ); + + renderPage(); + + expect(screen.getByRole('progressbar')).toBeInTheDocument(); + }); + + it('shows the empty state when no instance types are returned', () => { + vi.mocked(useAdminInstanceTypes).mockReturnValue( + mockQueryResult({ + data: [], + }), + ); + + renderPage(); + + expect(screen.getByText('No instance types yet.')).toBeInTheDocument(); + expect(screen.queryByRole('table')).not.toBeInTheDocument(); + }); + + it('uses the page-level error state when the query fails', () => { + vi.mocked(useAdminInstanceTypes).mockReturnValue( + mockQueryResult({ + data: [], + error: new Error('Private instance types unavailable'), + }), + ); + + renderPage(); + + expect(screen.getByText('An error occurred')).toBeInTheDocument(); + expect(screen.getByText('Private instance types unavailable')).toBeInTheDocument(); + expect(screen.queryByRole('table')).not.toBeInTheDocument(); + }); +}); diff --git a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.tsx b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.tsx new file mode 100644 index 00000000..e9f76ecb --- /dev/null +++ b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.tsx @@ -0,0 +1,91 @@ +import { Truncate } from '@patternfly/react-core'; +import { Table, Tbody, Td, Th, Thead, Tr } from '@patternfly/react-table'; + +import { InstanceTypeLifecycleLabel } from './InstanceTypeLifecycleLabel'; +import { useAdminInstanceTypes } from '../../api/v1/private/instance-type'; +import { useTranslation } from '../../hooks/useTranslation'; +import ListPage from '../Page/ListPage'; +import ListPageBody from '../Page/ListPageBody'; +import { Timestamp } from '../Primitives/Timestamp'; +import { SubtleContent } from '../SubtleContent/SubtleContent'; + +const INSTANCE_TYPE_DESCRIPTION_PREVIEW_LENGTH = 120; +const INSTANCE_TYPE_NAME_PREVIEW_LENGTH = 32; +const NAME_COLUMN_WIDTH = 15; +const LIFECYCLE_STATE_COLUMN_WIDTH = 10; +const CPU_CORES_COLUMN_WIDTH = 10; +const MEMORY_COLUMN_WIDTH = 10; +const DESCRIPTION_COLUMN_WIDTH = 40; +const CREATED_COLUMN_WIDTH = 15; + +const AdminInstanceTypeListPage = () => { + const { t } = useTranslation(); + const { data: instanceTypes = [], isLoading, error } = useAdminInstanceTypes(); + + return ( + + + {instanceTypes.length === 0 ? ( + {t('No instance types yet.')} + ) : ( + + + + + + + + + + + + + {instanceTypes.map((instanceType) => ( + + + + + + + + + ))} + +
{t('Name')}{t('Lifecycle State')}{t('CPU cores')}{t('Memory (GiB)')}{t('Description')}{t('Created')}
+ + + + + {instanceType.spec?.cores ?? '—'} + + {instanceType.spec?.memoryGib ?? '—'} + + + + +
+ )} + + + ); +}; + +export default AdminInstanceTypeListPage; diff --git a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx new file mode 100644 index 00000000..3568ce68 --- /dev/null +++ b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx @@ -0,0 +1,45 @@ +import { render, screen } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; + +import { InstanceTypeState } from '@osac/types/private'; + +import { InstanceTypeLifecycleLabel } from './InstanceTypeLifecycleLabel'; + +const expectLabelColor = (text: string, colorClass?: string) => { + const label = screen.getByText(text).closest('.pf-v6-c-label'); + + expect(label).not.toBeNull(); + if (colorClass) { + expect(label).toHaveClass(colorClass); + return; + } + + expect(label).not.toHaveClass('pf-m-green'); + expect(label).not.toHaveClass('pf-m-orange'); +}; + +describe('InstanceTypeLifecycleLabel', () => { + it('renders active instance types in green', () => { + render(); + + expectLabelColor('Active', 'pf-m-green'); + }); + + it('renders deprecated instance types in orange', () => { + render(); + + expectLabelColor('Deprecated', 'pf-m-orange'); + }); + + it('renders obsolete instance types in grey', () => { + render(); + + expectLabelColor('Obsolete'); + }); + + it('falls back to unspecified when the state is missing', () => { + render(); + + expectLabelColor('Unspecified'); + }); +}); diff --git a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx new file mode 100644 index 00000000..cb785fea --- /dev/null +++ b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx @@ -0,0 +1,36 @@ +import type { ComponentProps } from 'react'; +import { Label } from '@patternfly/react-core'; + +import { InstanceTypeState } from '@osac/types/private'; + +import { useTranslation } from '../../hooks/useTranslation'; + +export interface InstanceTypeLifecycleLabelProps { + state?: InstanceTypeState; +} + +type LifecycleLabelConfig = { + color: ComponentProps['color']; + text: string; +}; + +export const InstanceTypeLifecycleLabel = ({ state }: InstanceTypeLifecycleLabelProps) => { + const { t } = useTranslation(); + + const label = (): LifecycleLabelConfig => { + switch (state) { + case InstanceTypeState.ACTIVE: + return { color: 'green', text: t('Active') }; + case InstanceTypeState.DEPRECATED: + return { color: 'orange', text: t('Deprecated') }; + case InstanceTypeState.OBSOLETE: + return { color: 'grey', text: t('Obsolete') }; + default: + return { color: 'grey', text: t('Unspecified') }; + } + }; + + const { color, text } = label(); + + return ; +}; diff --git a/libs/ui-components/src/pages/admin/StorageManagementPage.test.tsx b/libs/ui-components/src/pages/admin/StorageManagementPage.test.tsx index d97263c6..7f7271c7 100644 --- a/libs/ui-components/src/pages/admin/StorageManagementPage.test.tsx +++ b/libs/ui-components/src/pages/admin/StorageManagementPage.test.tsx @@ -9,12 +9,15 @@ const renderPage = (activeTab: 'backends' | 'tiers') => renderWithProviders( } /> - } /> + } + /> , - { routerEntries: [`/admin/storage/${activeTab}`] }, + { routerEntries: [`/admin/infrastructure/storage/${activeTab}`] }, ); describe('StorageManagementPage', () => { @@ -37,7 +40,7 @@ describe('StorageManagementPage', () => { expect(screen.getByRole('tabpanel')).toHaveTextContent('Storage tiers'); }); - it('navigates to /admin/storage/tiers when the Tiers tab is clicked', async () => { + it('navigates to /admin/infrastructure/storage/tiers when the Tiers tab is clicked', async () => { const { user } = renderPage('backends'); await user.click(screen.getByRole('tab', { name: 'Tiers' })); diff --git a/libs/ui-components/src/pages/admin/StorageManagementPage.tsx b/libs/ui-components/src/pages/admin/StorageManagementPage.tsx index f2f079a5..e0cd8563 100644 --- a/libs/ui-components/src/pages/admin/StorageManagementPage.tsx +++ b/libs/ui-components/src/pages/admin/StorageManagementPage.tsx @@ -26,7 +26,7 @@ export const StorageManagementPage = ({ activeTab }: { activeTab: StorageTab }) activeKey={activeTab} onSelect={(_event, tabKey) => { if (isStorageTab(tabKey)) { - navigate(`/admin/storage/${tabKey}`, { replace: true }); + navigate(`/admin/infrastructure/storage/${tabKey}`, { replace: true }); } }} aria-label={t('Storage tabs')} diff --git a/libs/ui-components/src/test-utils/createMockConnectTransport.ts b/libs/ui-components/src/test-utils/createMockConnectTransport.ts index c88f00f0..916059c5 100644 --- a/libs/ui-components/src/test-utils/createMockConnectTransport.ts +++ b/libs/ui-components/src/test-utils/createMockConnectTransport.ts @@ -32,6 +32,7 @@ import { VirtualNetworks, } from '@osac/types'; import type { + InstanceType as PrivateInstanceType, Tenant as PrivateTenant, StorageBackend, StorageBackendsCreateRequest, @@ -49,6 +50,7 @@ import type { TenantsCreateResponse, } from '@osac/types/private'; import { + InstanceTypes as PrivateInstanceTypes, Tenants as PrivateTenants, StorageBackendState, StorageBackends, @@ -69,6 +71,7 @@ export type MockApiFixtures = { securityGroups?: SecurityGroup[]; identityProviders?: IdentityProvider[]; instanceTypes?: InstanceType[]; + privateInstanceTypes?: PrivateInstanceType[]; storageBackends?: StorageBackend[]; storageTiers?: StorageTier[]; }; @@ -167,6 +170,7 @@ export const createMockConnectTransport = ( const subnets = fixtures.subnets ?? []; const securityGroups = fixtures.securityGroups ?? []; const instanceTypes = fixtures.instanceTypes ?? []; + const privateInstanceTypes = fixtures.privateInstanceTypes ?? []; const storageBackends = fixtures.storageBackends ?? []; const storageTiers = fixtures.storageTiers ?? []; @@ -358,6 +362,17 @@ export const createMockConnectTransport = ( delete: () => ({}), }); + router.service(PrivateInstanceTypes, { + list: () => ({ + items: privateInstanceTypes, + size: privateInstanceTypes.length, + total: privateInstanceTypes.length, + }), + get: (req) => ({ + object: privateInstanceTypes.find((item) => item.id === req.id), + }), + }); + router.service(PrivateTenants, { list: () => ({ items: tenants, From 330f263cd06c49e36a0e48dcfb0379f20da15688 Mon Sep 17 00:00:00 2001 From: batzionb Date: Sun, 9 Aug 2026 15:26:14 +0300 Subject: [PATCH 2/8] OSAC-3780: address cross-cutting review findings Align the Infrastructure nav "Instance types" label casing with the list page and create-page shell so they share one i18n key instead of two near-duplicate entries. Assisted-by: Claude Code Signed-off-by: batzionb --- apps/app-frontend/src/shell/shellNav.test.ts | 2 +- apps/app-frontend/src/shell/shellNav.ts | 2 +- libs/i18n/locales/en/translation.json | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/apps/app-frontend/src/shell/shellNav.test.ts b/apps/app-frontend/src/shell/shellNav.test.ts index 76fbb94e..0b52c53e 100644 --- a/apps/app-frontend/src/shell/shellNav.test.ts +++ b/apps/app-frontend/src/shell/shellNav.test.ts @@ -62,7 +62,7 @@ describe('navRowsForRole', () => { }, { id: 'instance-types', - label: 'Instance Types', + label: 'Instance types', path: '/admin/infrastructure/instance-types', }, ], diff --git a/apps/app-frontend/src/shell/shellNav.ts b/apps/app-frontend/src/shell/shellNav.ts index 470f7269..6ab8e212 100644 --- a/apps/app-frontend/src/shell/shellNav.ts +++ b/apps/app-frontend/src/shell/shellNav.ts @@ -40,7 +40,7 @@ const getAdminNav = (t: TFunction): NavSection[] => [ }, { id: 'instance-types', - label: t('Instance Types'), + label: t('Instance types'), path: '/admin/infrastructure/instance-types', }, ], diff --git a/libs/i18n/locales/en/translation.json b/libs/i18n/locales/en/translation.json index 9907a1a9..a5c4a20a 100644 --- a/libs/i18n/locales/en/translation.json +++ b/libs/i18n/locales/en/translation.json @@ -231,7 +231,6 @@ "Infrastructure": "Infrastructure", "Instance type": "Instance type", "Instance types": "Instance types", - "Instance Types": "Instance Types", "Internal IP": "Internal IP", "Invalid IPv4 CIDR format (e.g., 192.168.1.0/24)": "Invalid IPv4 CIDR format (e.g., 192.168.1.0/24)", "Invalid IPv4 CIDR notation": "Invalid IPv4 CIDR notation", From 1c71ff8c03321905a917e907751fb88ba247eea8 Mon Sep 17 00:00:00 2001 From: batzionb Date: Sun, 9 Aug 2026 15:37:31 +0300 Subject: [PATCH 3/8] OSAC-3780: add create button to instance type list page Wire a "Create instance type" action into the list page so the create-page shell (AC-4) is reachable from the UI, matching the pattern used by TenantListPage. Assisted-by: Claude Code Signed-off-by: batzionb --- .../AdminInstanceTypeListPage.test.tsx | 27 +++++++++++++++++++ .../AdminInstanceTypeListPage.tsx | 12 ++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.test.tsx b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.test.tsx index 6b07a522..e6528fa9 100644 --- a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.test.tsx +++ b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.test.tsx @@ -1,3 +1,4 @@ +import { Route, Routes } from 'react-router-dom'; import { create } from '@bufbuild/protobuf'; import { screen } from '@testing-library/react'; import { describe, expect, it, vi } from 'vitest'; @@ -42,6 +43,18 @@ const makeInstanceType = ( const renderPage = () => renderWithProviders(); +const renderPageWithCreateRoute = () => + renderWithProviders( + + } /> + Create instance type page} + /> + , + { routerEntries: ['/admin/infrastructure/instance-types'] }, + ); + describe('AdminInstanceTypeListPage', () => { it('renders the required columns and lifecycle labels for populated data', () => { vi.mocked(useAdminInstanceTypes).mockReturnValue( @@ -125,4 +138,18 @@ describe('AdminInstanceTypeListPage', () => { expect(screen.getByText('Private instance types unavailable')).toBeInTheDocument(); expect(screen.queryByRole('table')).not.toBeInTheDocument(); }); + + it('navigates to the create route when the create button is clicked', async () => { + vi.mocked(useAdminInstanceTypes).mockReturnValue( + mockQueryResult({ + data: [], + }), + ); + + const { user } = renderPageWithCreateRoute(); + + await user.click(screen.getByRole('button', { name: 'Create instance type' })); + + expect(screen.getByRole('heading', { name: 'Create instance type page' })).toBeInTheDocument(); + }); }); diff --git a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.tsx b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.tsx index e9f76ecb..9c876e48 100644 --- a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.tsx +++ b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.tsx @@ -1,4 +1,5 @@ -import { Truncate } from '@patternfly/react-core'; +import { useNavigate } from 'react-router-dom'; +import { Button, Truncate } from '@patternfly/react-core'; import { Table, Tbody, Td, Th, Thead, Tr } from '@patternfly/react-table'; import { InstanceTypeLifecycleLabel } from './InstanceTypeLifecycleLabel'; @@ -20,6 +21,7 @@ const CREATED_COLUMN_WIDTH = 15; const AdminInstanceTypeListPage = () => { const { t } = useTranslation(); + const navigate = useNavigate(); const { data: instanceTypes = [], isLoading, error } = useAdminInstanceTypes(); return ( @@ -27,6 +29,14 @@ const AdminInstanceTypeListPage = () => { title={t('Instance types')} description={t('Manage provider-defined instance types for this cloud platform.')} error={error} + actions={ + + } > {instanceTypes.length === 0 ? ( From 211abd9621b6ac27acde99914a804cb0c795eddd Mon Sep 17 00:00:00 2001 From: batzionb Date: Sun, 9 Aug 2026 16:40:36 +0300 Subject: [PATCH 4/8] OSAC-3780: extract instance type table and improve empty state Extract the instance type table markup into AdminInstanceTypeTable so the list page stays focused on data wiring, add a richer empty state with an icon and guidance text, and fall back to the raw lifecycle state (or an em dash) instead of a generic 'Unspecified' label. Assisted-by: Claude Code Signed-off-by: batzionb --- libs/i18n/locales/en/translation.json | 1 + .../AdminInstanceTypeListPage.test.tsx | 5 +- .../AdminInstanceTypeListPage.tsx | 71 +----------- .../InstanceType/AdminInstanceTypeTable.tsx | 103 ++++++++++++++++++ .../InstanceTypeLifecycleLabel.test.tsx | 4 +- .../InstanceTypeLifecycleLabel.tsx | 2 +- 6 files changed, 114 insertions(+), 72 deletions(-) create mode 100644 libs/ui-components/src/components/InstanceType/AdminInstanceTypeTable.tsx diff --git a/libs/i18n/locales/en/translation.json b/libs/i18n/locales/en/translation.json index a5c4a20a..46364ff8 100644 --- a/libs/i18n/locales/en/translation.json +++ b/libs/i18n/locales/en/translation.json @@ -121,6 +121,7 @@ "Could not load instance types": "Could not load instance types", "CPU cores": "CPU cores", "Create": "Create", + "Create an instance type to start defining provider-managed sizes.": "Create an instance type to start defining provider-managed sizes.", "Create cluster": "Create cluster", "Create cluster wizard": "Create cluster wizard", "Create identity provider": "Create identity provider", diff --git a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.test.tsx b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.test.tsx index e6528fa9..461ff563 100644 --- a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.test.tsx +++ b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.test.tsx @@ -121,7 +121,10 @@ describe('AdminInstanceTypeListPage', () => { renderPage(); expect(screen.getByText('No instance types yet.')).toBeInTheDocument(); - expect(screen.queryByRole('table')).not.toBeInTheDocument(); + expect( + screen.getByText('Create an instance type to start defining provider-managed sizes.'), + ).toBeInTheDocument(); + expect(screen.getByRole('grid', { name: 'Instance types' })).toBeInTheDocument(); }); it('uses the page-level error state when the query fails', () => { diff --git a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.tsx b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.tsx index 9c876e48..be36dc73 100644 --- a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.tsx +++ b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.tsx @@ -1,23 +1,11 @@ import { useNavigate } from 'react-router-dom'; -import { Button, Truncate } from '@patternfly/react-core'; -import { Table, Tbody, Td, Th, Thead, Tr } from '@patternfly/react-table'; +import { Button } from '@patternfly/react-core'; -import { InstanceTypeLifecycleLabel } from './InstanceTypeLifecycleLabel'; +import AdminInstanceTypeTable from './AdminInstanceTypeTable'; import { useAdminInstanceTypes } from '../../api/v1/private/instance-type'; import { useTranslation } from '../../hooks/useTranslation'; import ListPage from '../Page/ListPage'; import ListPageBody from '../Page/ListPageBody'; -import { Timestamp } from '../Primitives/Timestamp'; -import { SubtleContent } from '../SubtleContent/SubtleContent'; - -const INSTANCE_TYPE_DESCRIPTION_PREVIEW_LENGTH = 120; -const INSTANCE_TYPE_NAME_PREVIEW_LENGTH = 32; -const NAME_COLUMN_WIDTH = 15; -const LIFECYCLE_STATE_COLUMN_WIDTH = 10; -const CPU_CORES_COLUMN_WIDTH = 10; -const MEMORY_COLUMN_WIDTH = 10; -const DESCRIPTION_COLUMN_WIDTH = 40; -const CREATED_COLUMN_WIDTH = 15; const AdminInstanceTypeListPage = () => { const { t } = useTranslation(); @@ -39,60 +27,7 @@ const AdminInstanceTypeListPage = () => { } > - {instanceTypes.length === 0 ? ( - {t('No instance types yet.')} - ) : ( - - - - - - - - - - - - - {instanceTypes.map((instanceType) => ( - - - - - - - - - ))} - -
{t('Name')}{t('Lifecycle State')}{t('CPU cores')}{t('Memory (GiB)')}{t('Description')}{t('Created')}
- - - - - {instanceType.spec?.cores ?? '—'} - - {instanceType.spec?.memoryGib ?? '—'} - - - - -
- )} + ); diff --git a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeTable.tsx b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeTable.tsx new file mode 100644 index 00000000..af4041d5 --- /dev/null +++ b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeTable.tsx @@ -0,0 +1,103 @@ +import { + Bullseye, + EmptyState, + EmptyStateBody, + EmptyStateVariant, + Truncate, +} from '@patternfly/react-core'; +import SearchIcon from '@patternfly/react-icons/dist/esm/icons/search-icon'; +import { Table, Tbody, Td, Th, Thead, Tr } from '@patternfly/react-table'; + +import type { InstanceType as PrivateInstanceType } from '@osac/types/private'; + +import { InstanceTypeLifecycleLabel } from './InstanceTypeLifecycleLabel'; +import { useTranslation } from '../../hooks/useTranslation'; +import { Timestamp } from '../Primitives/Timestamp'; + +const INSTANCE_TYPE_DESCRIPTION_PREVIEW_LENGTH = 120; +const INSTANCE_TYPE_NAME_PREVIEW_LENGTH = 32; +const NAME_COLUMN_WIDTH = 15; +const LIFECYCLE_STATE_COLUMN_WIDTH = 10; +const CPU_CORES_COLUMN_WIDTH = 10; +const MEMORY_COLUMN_WIDTH = 10; +const DESCRIPTION_COLUMN_WIDTH = 40; +const CREATED_COLUMN_WIDTH = 15; +const EMPTY_STATE_COLUMN_SPAN = 6; + +interface AdminInstanceTypeTableProps { + instanceTypes: PrivateInstanceType[]; +} + +const AdminInstanceTypeTable = ({ instanceTypes }: AdminInstanceTypeTableProps) => { + const { t } = useTranslation(); + + return ( + + + + + + + + + + + + + {instanceTypes.length === 0 ? ( + + + + ) : ( + instanceTypes.map((instanceType) => ( + + + + + + + + + )) + )} + +
{t('Name')}{t('Lifecycle State')}{t('CPU cores')}{t('Memory (GiB)')}{t('Description')}{t('Created')}
+ + + + {t('Create an instance type to start defining provider-managed sizes.')} + + + +
+ + + + + {instanceType.spec?.cores ?? '—'} + + {instanceType.spec?.memoryGib ?? '—'} + + + + +
+ ); +}; + +export default AdminInstanceTypeTable; diff --git a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx index 3568ce68..b8354867 100644 --- a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx +++ b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx @@ -37,9 +37,9 @@ describe('InstanceTypeLifecycleLabel', () => { expectLabelColor('Obsolete'); }); - it('falls back to unspecified when the state is missing', () => { + it('falls back to an em dash when the state is missing', () => { render(); - expectLabelColor('Unspecified'); + expectLabelColor('—'); }); }); diff --git a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx index cb785fea..f7bda163 100644 --- a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx +++ b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx @@ -26,7 +26,7 @@ export const InstanceTypeLifecycleLabel = ({ state }: InstanceTypeLifecycleLabel case InstanceTypeState.OBSOLETE: return { color: 'grey', text: t('Obsolete') }; default: - return { color: 'grey', text: t('Unspecified') }; + return { color: 'grey', text: state || '—' }; } }; From 6ae157adc828c219c46f89253f57c6d78b859824 Mon Sep 17 00:00:00 2001 From: batzionb Date: Sun, 9 Aug 2026 16:58:42 +0300 Subject: [PATCH 5/8] OSAC-3780: address cross-cutting review findings Add a dedicated ResourceLifecycleLabel primitive instead of duplicating PatternFly Label rendering in InstanceTypeLifecycleLabel, default-export the component to match its siblings, show the raw state text for any lifecycle value outside the three designed states, add coming-soon messaging to the create-page shell, align the Lifecycle state column header casing, and drop an unrelated .gitignore change. Assisted-by: Claude Code Signed-off-by: batzionb --- .gitignore | 1 - libs/i18n/locales/en/translation.json | 2 +- .../AdminInstanceTypeCreatePage.tsx | 9 ++--- .../AdminInstanceTypeListPage.test.tsx | 2 +- .../InstanceType/AdminInstanceTypeTable.tsx | 6 ++-- .../InstanceTypeLifecycleLabel.test.tsx | 8 ++++- .../InstanceTypeLifecycleLabel.tsx | 33 +++++++++---------- .../Resource/ResourceLifecycleLabel.test.tsx | 26 +++++++++++++++ .../Resource/ResourceLifecycleLabel.tsx | 21 ++++++++++++ 9 files changed, 80 insertions(+), 28 deletions(-) create mode 100644 libs/ui-components/src/components/Resource/ResourceLifecycleLabel.test.tsx create mode 100644 libs/ui-components/src/components/Resource/ResourceLifecycleLabel.tsx diff --git a/.gitignore b/.gitignore index 7149abe6..8ab78d26 100644 --- a/.gitignore +++ b/.gitignore @@ -55,7 +55,6 @@ proxy/osac-proxy # an e2e test suite. Live session state and run output, never committed. apps/playwright/.auth/ apps/playwright/test-results/ -/test-results/ apps/playwright/playwright-report/ apps/playwright/blob-report/ diff --git a/libs/i18n/locales/en/translation.json b/libs/i18n/locales/en/translation.json index 46364ff8..60b16771 100644 --- a/libs/i18n/locales/en/translation.json +++ b/libs/i18n/locales/en/translation.json @@ -247,7 +247,7 @@ "Issuer is required": "Issuer is required", "JWKS URL": "JWKS URL", "Keep editing": "Keep editing", - "Lifecycle State": "Lifecycle State", + "Lifecycle state": "Lifecycle state", "Loading cluster password": "Loading cluster password", "Loading security groups...": "Loading security groups...", "Loading subnets...": "Loading subnets...", diff --git a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeCreatePage.tsx b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeCreatePage.tsx index 40386379..4c983917 100644 --- a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeCreatePage.tsx +++ b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeCreatePage.tsx @@ -3,9 +3,10 @@ import { Breadcrumb, BreadcrumbItem, Button, + EmptyState, + EmptyStateBody, PageSection, Stack, - Title, } from '@patternfly/react-core'; import { useTranslation } from '../../hooks/useTranslation'; @@ -29,9 +30,9 @@ const AdminInstanceTypeCreatePage = () => { {t('Create')} - - {t('Create instance type')} - + + {t('This feature is coming soon.')} + ); diff --git a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.test.tsx b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.test.tsx index 461ff563..85b0e420 100644 --- a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.test.tsx +++ b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeListPage.test.tsx @@ -73,7 +73,7 @@ describe('AdminInstanceTypeListPage', () => { expect(screen.getByRole('heading', { name: 'Instance types' })).toBeInTheDocument(); expect(screen.getAllByRole('columnheader').map((header) => header.textContent)).toEqual([ 'Name', - 'Lifecycle State', + 'Lifecycle state', 'CPU cores', 'Memory (GiB)', 'Description', diff --git a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeTable.tsx b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeTable.tsx index af4041d5..c5e38373 100644 --- a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeTable.tsx +++ b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeTable.tsx @@ -10,7 +10,7 @@ import { Table, Tbody, Td, Th, Thead, Tr } from '@patternfly/react-table'; import type { InstanceType as PrivateInstanceType } from '@osac/types/private'; -import { InstanceTypeLifecycleLabel } from './InstanceTypeLifecycleLabel'; +import InstanceTypeLifecycleLabel from './InstanceTypeLifecycleLabel'; import { useTranslation } from '../../hooks/useTranslation'; import { Timestamp } from '../Primitives/Timestamp'; @@ -36,7 +36,7 @@ const AdminInstanceTypeTable = ({ instanceTypes }: AdminInstanceTypeTableProps) {t('Name')} - {t('Lifecycle State')} + {t('Lifecycle state')} {t('CPU cores')} {t('Memory (GiB)')} {t('Description')} @@ -73,7 +73,7 @@ const AdminInstanceTypeTable = ({ instanceTypes }: AdminInstanceTypeTableProps) omissionContent="..." /> - + diff --git a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx index b8354867..fb7c5384 100644 --- a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx +++ b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx @@ -3,7 +3,7 @@ import { describe, expect, it } from 'vitest'; import { InstanceTypeState } from '@osac/types/private'; -import { InstanceTypeLifecycleLabel } from './InstanceTypeLifecycleLabel'; +import InstanceTypeLifecycleLabel from './InstanceTypeLifecycleLabel'; const expectLabelColor = (text: string, colorClass?: string) => { const label = screen.getByText(text).closest('.pf-v6-c-label'); @@ -42,4 +42,10 @@ describe('InstanceTypeLifecycleLabel', () => { expectLabelColor('—'); }); + + it('shows the raw state when it is not one of the known lifecycle states', () => { + render(); + + expectLabelColor('99'); + }); }); diff --git a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx index f7bda163..23cec253 100644 --- a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx +++ b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx @@ -1,36 +1,35 @@ -import type { ComponentProps } from 'react'; -import { Label } from '@patternfly/react-core'; - import { InstanceTypeState } from '@osac/types/private'; import { useTranslation } from '../../hooks/useTranslation'; +import { + ResourceLifecycleLabel, + ResourceLifecycleLabelProps, +} from '../Resource/ResourceLifecycleLabel'; export interface InstanceTypeLifecycleLabelProps { state?: InstanceTypeState; } -type LifecycleLabelConfig = { - color: ComponentProps['color']; - text: string; -}; - -export const InstanceTypeLifecycleLabel = ({ state }: InstanceTypeLifecycleLabelProps) => { +const InstanceTypeLifecycleLabel = ({ state }: InstanceTypeLifecycleLabelProps) => { const { t } = useTranslation(); - const label = (): LifecycleLabelConfig => { + const props = (): ResourceLifecycleLabelProps => { switch (state) { case InstanceTypeState.ACTIVE: - return { color: 'green', text: t('Active') }; + return { lifecycle: 'active', text: t('Active') }; case InstanceTypeState.DEPRECATED: - return { color: 'orange', text: t('Deprecated') }; + return { lifecycle: 'deprecated', text: t('Deprecated') }; case InstanceTypeState.OBSOLETE: - return { color: 'grey', text: t('Obsolete') }; + return { lifecycle: 'obsolete', text: t('Obsolete') }; + case InstanceTypeState.UNSPECIFIED: + case undefined: + return { lifecycle: 'unspecified', text: '—' }; default: - return { color: 'grey', text: state || '—' }; + return { lifecycle: 'unspecified', text: String(state) }; } }; - const { color, text } = label(); - - return ; + return ; }; + +export default InstanceTypeLifecycleLabel; diff --git a/libs/ui-components/src/components/Resource/ResourceLifecycleLabel.test.tsx b/libs/ui-components/src/components/Resource/ResourceLifecycleLabel.test.tsx new file mode 100644 index 00000000..388eacab --- /dev/null +++ b/libs/ui-components/src/components/Resource/ResourceLifecycleLabel.test.tsx @@ -0,0 +1,26 @@ +import { render, screen } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; + +import { ResourceLifecycleLabel } from './ResourceLifecycleLabel'; + +describe('ResourceLifecycleLabel', () => { + it('renders active in green', () => { + render(); + + expect(screen.getByText('Active').closest('.pf-v6-c-label')).toHaveClass('pf-m-green'); + }); + + it('renders deprecated in orange', () => { + render(); + + expect(screen.getByText('Deprecated').closest('.pf-v6-c-label')).toHaveClass('pf-m-orange'); + }); + + it('renders obsolete in grey', () => { + render(); + + const label = screen.getByText('Obsolete').closest('.pf-v6-c-label'); + expect(label).not.toHaveClass('pf-m-green'); + expect(label).not.toHaveClass('pf-m-orange'); + }); +}); diff --git a/libs/ui-components/src/components/Resource/ResourceLifecycleLabel.tsx b/libs/ui-components/src/components/Resource/ResourceLifecycleLabel.tsx new file mode 100644 index 00000000..3c1715ac --- /dev/null +++ b/libs/ui-components/src/components/Resource/ResourceLifecycleLabel.tsx @@ -0,0 +1,21 @@ +import { Label } from '@patternfly/react-core'; + +export type LifecycleKind = 'active' | 'deprecated' | 'obsolete' | 'unspecified'; + +type LifecycleLabelColor = 'green' | 'orange' | 'grey'; + +const LIFECYCLE_COLOR: Record = { + active: 'green', + deprecated: 'orange', + obsolete: 'grey', + unspecified: 'grey', +}; + +export interface ResourceLifecycleLabelProps { + lifecycle: LifecycleKind; + text: string; +} + +export const ResourceLifecycleLabel = ({ lifecycle, text }: ResourceLifecycleLabelProps) => { + return ; +}; From 2bfb4cd43aeb6e0cd137527371e4f8af34b4e3e4 Mon Sep 17 00:00:00 2001 From: batzionb Date: Mon, 10 Aug 2026 13:59:13 +0300 Subject: [PATCH 6/8] =?UTF-8?q?OSAC-3780:=20Address=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20use=20Record=20for=20instance=20type=20lifecycle=20?= =?UTF-8?q?mapping?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switch InstanceTypeLifecycleLabel to a Record map, following the IdentityProviderStatusLabel pattern, so an unrecognized backend state falls back to the unspecified treatment instead of surfacing the raw backend value in the UI. Assisted-by: Claude Code Signed-off-by: batzionb --- .../InstanceTypeLifecycleLabel.test.tsx | 4 +-- .../InstanceTypeLifecycleLabel.tsx | 36 ++++++++++--------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx index fb7c5384..04940e57 100644 --- a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx +++ b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx @@ -43,9 +43,9 @@ describe('InstanceTypeLifecycleLabel', () => { expectLabelColor('—'); }); - it('shows the raw state when it is not one of the known lifecycle states', () => { + it('falls back to an em dash when the state is not one of the known lifecycle states', () => { render(); - expectLabelColor('99'); + expectLabelColor('—'); }); }); diff --git a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx index 23cec253..f1765ce8 100644 --- a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx +++ b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx @@ -1,3 +1,5 @@ +import type { TFunction } from 'i18next'; + import { InstanceTypeState } from '@osac/types/private'; import { useTranslation } from '../../hooks/useTranslation'; @@ -10,26 +12,26 @@ export interface InstanceTypeLifecycleLabelProps { state?: InstanceTypeState; } +const instanceTypeLifecycleMap = ( + t: TFunction, +): Record => ({ + [InstanceTypeState.ACTIVE]: { lifecycle: 'active', text: t('Active') }, + [InstanceTypeState.DEPRECATED]: { lifecycle: 'deprecated', text: t('Deprecated') }, + [InstanceTypeState.OBSOLETE]: { lifecycle: 'obsolete', text: t('Obsolete') }, + [InstanceTypeState.UNSPECIFIED]: { lifecycle: 'unspecified', text: '—' }, +}); + const InstanceTypeLifecycleLabel = ({ state }: InstanceTypeLifecycleLabelProps) => { const { t } = useTranslation(); - const props = (): ResourceLifecycleLabelProps => { - switch (state) { - case InstanceTypeState.ACTIVE: - return { lifecycle: 'active', text: t('Active') }; - case InstanceTypeState.DEPRECATED: - return { lifecycle: 'deprecated', text: t('Deprecated') }; - case InstanceTypeState.OBSOLETE: - return { lifecycle: 'obsolete', text: t('Obsolete') }; - case InstanceTypeState.UNSPECIFIED: - case undefined: - return { lifecycle: 'unspecified', text: '—' }; - default: - return { lifecycle: 'unspecified', text: String(state) }; - } - }; - - return ; + const lifecycleMap = instanceTypeLifecycleMap(t); + + const props = + state !== undefined + ? (lifecycleMap[state] ?? lifecycleMap[InstanceTypeState.UNSPECIFIED]) + : lifecycleMap[InstanceTypeState.UNSPECIFIED]; + + return ; }; export default InstanceTypeLifecycleLabel; From 93528c84692a454877d1a946113610e218a9009a Mon Sep 17 00:00:00 2001 From: batzionb Date: Mon, 10 Aug 2026 14:13:18 +0300 Subject: [PATCH 7/8] =?UTF-8?q?OSAC-3780:=20Address=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20use=20translated=20Unspecified=20label=20text?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the shared 'Unspecified' translation for the lifecycle label's fallback text instead of an em dash, matching IdentityProviderStatusLabel. Assisted-by: Claude Code Signed-off-by: batzionb --- .../InstanceType/InstanceTypeLifecycleLabel.test.tsx | 8 ++++---- .../InstanceType/InstanceTypeLifecycleLabel.tsx | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx index 04940e57..452cb2e8 100644 --- a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx +++ b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.test.tsx @@ -37,15 +37,15 @@ describe('InstanceTypeLifecycleLabel', () => { expectLabelColor('Obsolete'); }); - it('falls back to an em dash when the state is missing', () => { + it('falls back to unspecified when the state is missing', () => { render(); - expectLabelColor('—'); + expectLabelColor('Unspecified'); }); - it('falls back to an em dash when the state is not one of the known lifecycle states', () => { + it('falls back to unspecified when the state is not one of the known lifecycle states', () => { render(); - expectLabelColor('—'); + expectLabelColor('Unspecified'); }); }); diff --git a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx index f1765ce8..5157ac62 100644 --- a/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx +++ b/libs/ui-components/src/components/InstanceType/InstanceTypeLifecycleLabel.tsx @@ -18,7 +18,7 @@ const instanceTypeLifecycleMap = ( [InstanceTypeState.ACTIVE]: { lifecycle: 'active', text: t('Active') }, [InstanceTypeState.DEPRECATED]: { lifecycle: 'deprecated', text: t('Deprecated') }, [InstanceTypeState.OBSOLETE]: { lifecycle: 'obsolete', text: t('Obsolete') }, - [InstanceTypeState.UNSPECIFIED]: { lifecycle: 'unspecified', text: '—' }, + [InstanceTypeState.UNSPECIFIED]: { lifecycle: 'unspecified', text: t('Unspecified') }, }); const InstanceTypeLifecycleLabel = ({ state }: InstanceTypeLifecycleLabelProps) => { From c1a49dc2b94ffb3685e0f318b0e459a58a3d2cc9 Mon Sep 17 00:00:00 2001 From: batzionb Date: Mon, 10 Aug 2026 14:16:24 +0300 Subject: [PATCH 8/8] =?UTF-8?q?OSAC-3780:=20Address=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20extract=20shared=20TruncatedText=20primitive?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract the whitespace-normalize-then-truncate pattern used by the Name and Description columns in AdminInstanceTypeTable into a shared TruncatedText primitive, so other tables can reuse it. Assisted-by: Claude Code Signed-off-by: batzionb --- .../InstanceType/AdminInstanceTypeTable.tsx | 21 +++------- .../Primitives/TruncatedText.test.tsx | 39 +++++++++++++++++++ .../components/Primitives/TruncatedText.tsx | 31 +++++++++++++++ 3 files changed, 76 insertions(+), 15 deletions(-) create mode 100644 libs/ui-components/src/components/Primitives/TruncatedText.test.tsx create mode 100644 libs/ui-components/src/components/Primitives/TruncatedText.tsx diff --git a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeTable.tsx b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeTable.tsx index c5e38373..47831a1b 100644 --- a/libs/ui-components/src/components/InstanceType/AdminInstanceTypeTable.tsx +++ b/libs/ui-components/src/components/InstanceType/AdminInstanceTypeTable.tsx @@ -1,10 +1,4 @@ -import { - Bullseye, - EmptyState, - EmptyStateBody, - EmptyStateVariant, - Truncate, -} from '@patternfly/react-core'; +import { Bullseye, EmptyState, EmptyStateBody, EmptyStateVariant } from '@patternfly/react-core'; import SearchIcon from '@patternfly/react-icons/dist/esm/icons/search-icon'; import { Table, Tbody, Td, Th, Thead, Tr } from '@patternfly/react-table'; @@ -13,6 +7,7 @@ import type { InstanceType as PrivateInstanceType } from '@osac/types/private'; import InstanceTypeLifecycleLabel from './InstanceTypeLifecycleLabel'; import { useTranslation } from '../../hooks/useTranslation'; import { Timestamp } from '../Primitives/Timestamp'; +import TruncatedText from '../Primitives/TruncatedText'; const INSTANCE_TYPE_DESCRIPTION_PREVIEW_LENGTH = 120; const INSTANCE_TYPE_NAME_PREVIEW_LENGTH = 32; @@ -65,12 +60,9 @@ const AdminInstanceTypeTable = ({ instanceTypes }: AdminInstanceTypeTableProps) instanceTypes.map((instanceType) => ( - @@ -83,10 +75,9 @@ const AdminInstanceTypeTable = ({ instanceTypes }: AdminInstanceTypeTableProps) {instanceType.spec?.memoryGib ?? '—'} - diff --git a/libs/ui-components/src/components/Primitives/TruncatedText.test.tsx b/libs/ui-components/src/components/Primitives/TruncatedText.test.tsx new file mode 100644 index 00000000..a977720e --- /dev/null +++ b/libs/ui-components/src/components/Primitives/TruncatedText.test.tsx @@ -0,0 +1,39 @@ +import { render, screen } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; + +import TruncatedText from './TruncatedText'; + +describe('TruncatedText', () => { + it('renders short content unmodified', () => { + render(); + + expect(screen.getByText('short text')).toBeInTheDocument(); + }); + + it('collapses internal whitespace and trims the content', () => { + render(); + + expect(screen.getByText('extra spaces here')).toBeInTheDocument(); + }); + + it('truncates content longer than maxCharsDisplayed with an omission marker', () => { + render(); + + const truncated = document.querySelector('.pf-v6-c-truncate__text'); + expect(truncated).not.toBeNull(); + expect(truncated?.textContent).toBe('a very lon'); + expect(document.querySelector('.pf-v6-c-truncate__omission')?.textContent).toBe('...'); + }); + + it('renders an em dash fallback when content is missing', () => { + render(); + + expect(screen.getByText('—')).toBeInTheDocument(); + }); + + it('renders a custom fallback when content is missing', () => { + render(); + + expect(screen.getByText('No description')).toBeInTheDocument(); + }); +}); diff --git a/libs/ui-components/src/components/Primitives/TruncatedText.tsx b/libs/ui-components/src/components/Primitives/TruncatedText.tsx new file mode 100644 index 00000000..0536404f --- /dev/null +++ b/libs/ui-components/src/components/Primitives/TruncatedText.tsx @@ -0,0 +1,31 @@ +import { Truncate } from '@patternfly/react-core'; + +export interface TruncatedTextProps { + content?: string | null; + maxCharsDisplayed: number; + fallback?: string; + omissionContent?: string; +} + +const TruncatedText = ({ + content, + maxCharsDisplayed, + fallback = '—', + omissionContent = '...', +}: TruncatedTextProps) => { + const normalized = (content || '').replace(/\s+/g, ' ').trim(); + + if (!normalized) { + return <>{fallback}; + } + + return ( + + ); +}; + +export default TruncatedText;