-
Notifications
You must be signed in to change notification settings - Fork 65
OSAC-2934: [UI] Catalog item detail page #106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ElayAharoni
wants to merge
22
commits into
osac-project:main
Choose a base branch
from
ElayAharoni:OSAC-2934-catalog-item-detail-page
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
a2a9ba4
OSAC-2934: extract escapeCelStringLiteral into shared api/cel module
ElayAharoni 134e0b1
OSAC-2934: add paginated provisioned-resources hooks per catalog item…
ElayAharoni ded0ad2
OSAC-2934: add missing single-item catalog item fetch hooks
ElayAharoni 6ba2231
OSAC-2934: add SanitizedMarkdown component for catalog item descriptions
ElayAharoni 190a6b1
OSAC-2934: add validation constraints summary formatter
ElayAharoni 9e646be
OSAC-2934: add CatalogItemOverviewTab
ElayAharoni d74a365
OSAC-2934: add CatalogItemFieldDefinitionsTab
ElayAharoni 2ec35e8
OSAC-2934: add CatalogItemProvisionedResourcesTab with server-side pa…
ElayAharoni 4332bef
OSAC-2934: add CatalogItemDetailActionButtons and CatalogItemPublishT…
ElayAharoni dd6ea48
OSAC-2934: add catalog item detail pages and wire admin routing
ElayAharoni 626a88c
OSAC-2934: sync i18n translations for detail page tab labels
ElayAharoni 354417f
OSAC-2934: address code review findings — i18n, test coverage, securi…
ElayAharoni 7ba674c
OSAC-2934: disable Delete and publish toggle with a tooltip until OSA…
ElayAharoni 7990260
OSAC-2934: resolve template names for ComputeInstance and BareMetalIn…
ElayAharoni 43b21ca
OSAC-2934: extract shared loading/error/not-found shell for detail pages
ElayAharoni efbdd1c
OSAC-2934: split provisioned resources tab per kind, derive kind inte…
ElayAharoni 67ebcbe
OSAC-2934: extract WithTooltip to remove repeated disabled-tooltip te…
ElayAharoni f557254
OSAC-2934: trim id before gating and querying in template hooks
ElayAharoni 489917d
OSAC-2934: keep whole-number hint alongside integer min/max bounds
ElayAharoni a9d97e8
OSAC-2934: use full Metadata fixtures instead of 'as never' casts in …
ElayAharoni b11719f
OSAC-2934: preserve Markdown-significant whitespace in item description
ElayAharoni 00485b8
OSAC-2934: extract shared useCatalogItemDetailData hook
ElayAharoni File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import { Route, Routes } from 'react-router-dom'; | ||
| import { screen } from '@testing-library/react'; | ||
| import { describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import { renderWithProviders } from '@osac/ui-components/test-utils/TestProviders'; | ||
|
|
||
| import { AdminCatalogRoutes } from './AdminCatalogRoutes'; | ||
|
|
||
| vi.mock('@osac/ui-components/pages/admin/CatalogManagementListPage', () => ({ | ||
| default: () => <div>list-page</div>, | ||
| })); | ||
| vi.mock('@osac/ui-components/pages/admin/cluster/ClusterCatalogItemDetailPage', () => ({ | ||
| default: () => <div>cluster-detail-page</div>, | ||
| })); | ||
| vi.mock( | ||
| '@osac/ui-components/pages/admin/compute-instance/ComputeInstanceCatalogItemDetailPage', | ||
| () => ({ | ||
| default: () => <div>compute-instance-detail-page</div>, | ||
| }), | ||
| ); | ||
| vi.mock( | ||
| '@osac/ui-components/pages/admin/baremetal-instance/BareMetalInstanceCatalogItemDetailPage', | ||
| () => ({ | ||
| default: () => <div>baremetal-instance-detail-page</div>, | ||
| }), | ||
| ); | ||
|
|
||
| // Mirrors AppShell.tsx's real mount point (`/admin/catalog/*`) — required so the component's | ||
| // internal `<Navigate to="/admin/catalog" />` redirect resolves to a route that actually exists. | ||
| const renderAt = (path: string) => | ||
| renderWithProviders( | ||
| <Routes> | ||
| <Route path="/admin/catalog/*" element={<AdminCatalogRoutes />} /> | ||
| </Routes>, | ||
| { routerEntries: [`/admin/catalog${path}`] }, | ||
| ); | ||
|
|
||
| describe('AdminCatalogRoutes', () => { | ||
| it('renders the list page at the index route', () => { | ||
| renderAt('/'); | ||
| expect(screen.getByText('list-page')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('dispatches :type/:id to the cluster detail page for type=cluster', () => { | ||
| renderAt('/cluster/catalog-1'); | ||
| expect(screen.getByText('cluster-detail-page')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('dispatches :type/:id to the compute-instance detail page for type=compute-instance', () => { | ||
| renderAt('/compute-instance/catalog-1'); | ||
| expect(screen.getByText('compute-instance-detail-page')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('dispatches :type/:id to the baremetal-instance detail page for type=baremetal-instance', () => { | ||
| renderAt('/baremetal-instance/catalog-1'); | ||
| expect(screen.getByText('baremetal-instance-detail-page')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('redirects to the list page for an unknown type', () => { | ||
| renderAt('/unknown-type/catalog-1'); | ||
| expect(screen.getByText('list-page')).toBeInTheDocument(); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| import { catalogItemProvisionedResourcesFilter, escapeCelStringLiteral } from './cel'; | ||
|
|
||
| describe('escapeCelStringLiteral', () => { | ||
| it('escapes embedded quotes for CEL string literals', () => { | ||
| expect(escapeCelStringLiteral('say "hello"')).toBe('say \\"hello\\"'); | ||
| }); | ||
|
|
||
| it('escapes backslashes for CEL string literals', () => { | ||
| expect(escapeCelStringLiteral('path\\to\\thing')).toBe('path\\\\to\\\\thing'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('catalogItemProvisionedResourcesFilter', () => { | ||
| it('filters resources by catalog item id', () => { | ||
| expect(catalogItemProvisionedResourcesFilter('catalog-1')).toBe( | ||
| 'this.spec.catalog_item == "catalog-1"', | ||
| ); | ||
| }); | ||
|
|
||
| it('escapes CEL injection characters in the catalog item id', () => { | ||
| expect(catalogItemProvisionedResourcesFilter(`"'] || true || this.id in ['`)).toBe( | ||
| `this.spec.catalog_item == "\\"'] || true || this.id in ['"`, | ||
| ); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| export const escapeCelStringLiteral = (value: string): string => | ||
| value.replaceAll('\\', '\\\\').replaceAll('"', '\\"'); | ||
|
|
||
| export const catalogItemProvisionedResourcesFilter = (catalogItemId: string): string => | ||
| `this.spec.catalog_item == "${escapeCelStringLiteral(catalogItemId)}"`; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
70 changes: 70 additions & 0 deletions
70
libs/ui-components/src/api/v1/baremetal-instance-templates.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| import { createRouterTransport } from '@connectrpc/connect'; | ||
| import { waitFor } from '@testing-library/react'; | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| import type { BareMetalInstanceTemplate } from '@osac/types'; | ||
| import { BareMetalInstanceTemplates } from '@osac/types'; | ||
|
|
||
| import { useBareMetalInstanceTemplate } from './baremetal-instance-templates'; | ||
| import { renderHookWithProviders } from '../../test-utils/TestProviders'; | ||
|
|
||
| const template: BareMetalInstanceTemplate = { | ||
| $typeName: 'osac.public.v1.BareMetalInstanceTemplate', | ||
| id: 'tpl-bm-worker', | ||
| title: 'Bare Metal Worker', | ||
| description: '', | ||
| parameters: [], | ||
| }; | ||
|
|
||
| describe('useBareMetalInstanceTemplate', () => { | ||
| const createTestTransport = (onGet?: (req: unknown) => void) => | ||
| createRouterTransport((router) => { | ||
| router.service(BareMetalInstanceTemplates, { | ||
| get: (req) => { | ||
| onGet?.(req); | ||
| return { object: template }; | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| it('fetches a single template by id from the Get endpoint', async () => { | ||
| const transport = createTestTransport(); | ||
| const { result } = renderHookWithProviders( | ||
| () => useBareMetalInstanceTemplate('tpl-bm-worker'), | ||
| { role: 'tenantAdmin', transport }, | ||
| ); | ||
|
|
||
| await waitFor(() => expect(result.current.isSuccess).toBe(true)); | ||
| expect(result.current.data).toMatchObject(template); | ||
| }); | ||
|
|
||
| it('does not fetch when id is undefined', async () => { | ||
| let getCalled = false; | ||
| const transport = createTestTransport(() => { | ||
| getCalled = true; | ||
| }); | ||
|
|
||
| renderHookWithProviders(() => useBareMetalInstanceTemplate(undefined), { | ||
| role: 'tenantAdmin', | ||
| transport, | ||
| }); | ||
|
|
||
| await new Promise((resolve) => setTimeout(resolve, 10)); | ||
| expect(getCalled).toBe(false); | ||
| }); | ||
|
|
||
| it('does not fetch when id is whitespace-only', async () => { | ||
| let getCalled = false; | ||
| const transport = createTestTransport(() => { | ||
| getCalled = true; | ||
| }); | ||
|
|
||
| renderHookWithProviders(() => useBareMetalInstanceTemplate(' '), { | ||
| role: 'tenantAdmin', | ||
| transport, | ||
| }); | ||
|
|
||
| await new Promise((resolve) => setTimeout(resolve, 10)); | ||
| expect(getCalled).toBe(false); | ||
| }); | ||
| }); |
16 changes: 16 additions & 0 deletions
16
libs/ui-components/src/api/v1/baremetal-instance-templates.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import { BareMetalInstanceTemplates } from '@osac/types'; | ||
|
|
||
| import { useApiFetch } from '../api-context'; | ||
| import { apiQueryKey } from '../types'; | ||
| import { useApiQuery } from '../use-api-query'; | ||
|
|
||
| export const useBareMetalInstanceTemplate = (id: string | undefined) => { | ||
| const client = useApiFetch(BareMetalInstanceTemplates); | ||
| const trimmedId = id?.trim() ?? ''; | ||
| return useApiQuery({ | ||
| queryKey: apiQueryKey('v1/baremetal_instance_templates', trimmedId ? [trimmedId] : undefined), | ||
| queryFn: () => client.get({ id: trimmedId }), | ||
| select: (data) => data.object, | ||
| enabled: Boolean(trimmedId), | ||
| }); | ||
| }; | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.