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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,32 @@ describe('AdditionalInitializers', () => {
expect(screen.getByText('tags (list[str], optional)')).toBeInTheDocument()
})

it('should disable Edit while the initializer catalog is unavailable', () => {
render(
<TestWrapper>
<AdditionalInitializers {...defaultProps} catalogAvailable={false} />
</TestWrapper>,
)

const editButtons = screen.getAllByRole('button', { name: 'Edit' })
expect(editButtons.length).toBeGreaterThan(0)
editButtons.forEach((button) => expect(button).toBeDisabled())
// The row explains why editing is unavailable instead of failing silently on save.
expect(screen.getAllByText(/editing is disabled until it reloads/i).length).toBeGreaterThan(0)
})

it('should keep Apply and Remove usable while the initializer catalog is unavailable', () => {
render(
<TestWrapper>
<AdditionalInitializers {...defaultProps} catalogAvailable={false} />
</TestWrapper>,
)

const row = screen.getByTestId('initializer-row-additional-2')
expect(within(row).getByRole('button', { name: 'Apply now' })).toBeEnabled()
expect(within(row).getByRole('button', { name: 'Remove' })).toBeEnabled()
})

it('should show the saved parameters read-only without an inline editor', () => {
render(
<TestWrapper>
Expand Down
14 changes: 12 additions & 2 deletions frontend/src/components/Initializers/AdditionalInitializers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import ConfirmDialog from '../ConfirmDialog'
interface AdditionalInitializersProps {
items: AdditionalInitializerSetting[]
registeredInitializers: RegisteredInitializer[]
catalogAvailable?: boolean

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m thinking we could replace catalogAvailable with a catalog status union such as:

type CatalogStatus = 'loading' | 'loaded' | 'error'

We could then keep the catalog entries separately and have the lookup return RegisteredInitializer | undefined instead of constructing synthetic RegisteredInitializer objects:

const initializer = registeredInitializers.find(
  (item) => item.initializer_name === initializerName,
)

The renderers could handle each state explicitly: error shows the unavailable message without rendering catalog-derived metadata, loaded with no match shows “no longer registered,” and loaded with a match renders the real metadata. Browse, Add, and Edit could require loaded, while Apply and Remove remain available.

This would avoid interpreting placeholder empty arrays as “Required env vars: None” or “No declared parameters,” and would also prevent cached catalog data from enabling controls after a failed refresh. Could we also cover initial failure, recovery, and loaded-then-failed-refresh transitions?

creating: boolean
savingInitializerId?: string | null
saveErrors?: Record<string, string>
Expand All @@ -40,6 +41,7 @@ interface AdditionalInitializersProps {
interface AdditionalInitializerCardProps {
item: AdditionalInitializerSetting
initializer: RegisteredInitializer
catalogAvailable: boolean
isSaving: boolean
isApplying: boolean
isDeleting: boolean
Expand All @@ -53,6 +55,7 @@ interface AdditionalInitializerCardProps {
function AdditionalInitializerCard({
item,
initializer,
catalogAvailable,
isSaving,
isApplying,
isDeleting,
Expand Down Expand Up @@ -93,6 +96,11 @@ function AdditionalInitializerCard({
Required env vars: {initializer.required_env_vars.join(', ')}
</Text>
)}
{!catalogAvailable && (
<Text className={styles.envVarText}>
Initializer catalog is unavailable; editing is disabled until it reloads.
</Text>
)}
</div>
</div>

Expand All @@ -110,7 +118,7 @@ function AdditionalInitializerCard({
</div>

<div className={styles.actionsRow}>
<Button appearance="primary" onClick={() => setEditOpen(true)} disabled={isBusy}>
<Button appearance="primary" onClick={() => setEditOpen(true)} disabled={isBusy || !catalogAvailable}>
Edit
</Button>
<Button
Expand Down Expand Up @@ -157,6 +165,7 @@ function AdditionalInitializerCard({
export default function AdditionalInitializers({
items,
registeredInitializers,
catalogAvailable = true,
creating,
savingInitializerId = null,
saveErrors = {},
Expand Down Expand Up @@ -237,7 +246,8 @@ export default function AdditionalInitializers({
<AdditionalInitializerCard
key={`${item.id}:${formatInitializerParameters(item.parameters)}:${item.order_index ?? ''}`}
item={item}
initializer={resolveRegisteredInitializer(item.initializer_name, registeredInitializers)}
initializer={resolveRegisteredInitializer(item.initializer_name, registeredInitializers, catalogAvailable)}
catalogAvailable={catalogAvailable}
isSaving={savingInitializerId === item.id}
isApplying={applyingInitializerId === item.id}
isDeleting={deletingInitializerId === item.id}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import { useInitializersStyles } from './Initializers.styles'
interface BaselineInitializersProps {
items: BaselineInitializerSetting[]
registeredInitializers: RegisteredInitializer[]
catalogAvailable?: boolean
}

export default function BaselineInitializers({
items,
registeredInitializers,
catalogAvailable = true,
}: BaselineInitializersProps) {
const styles = useInitializersStyles()

Expand All @@ -32,7 +34,7 @@ export default function BaselineInitializers({
) : (
<div className={styles.baselineGroup} role="list" aria-label="Baseline initializers">
{items.map((item: BaselineInitializerSetting) => {
const initializer = resolveRegisteredInitializer(item.initializer_name, registeredInitializers)
const initializer = resolveRegisteredInitializer(item.initializer_name, registeredInitializers, catalogAvailable)
return (
<div
key={`${item.initializer_name}:${item.order_index}`}
Expand Down
25 changes: 25 additions & 0 deletions frontend/src/components/Initializers/Initializers.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,31 @@ describe('Initializers', () => {
expect(screen.getByText('Service Unavailable')).toBeInTheDocument()
})

it('should describe rows as temporarily unavailable, not unregistered, when the catalog fails', async () => {
mockedInitializersApi.listRegistered.mockRejectedValue(new Error('Service Unavailable'))

renderInitializers()

const row = await screen.findByTestId('baseline-initializer-row-target')
// A metadata outage must not be presented as a configuration problem.
expect(within(row).getByText(/temporarily unavailable/i)).toBeInTheDocument()
expect(within(row).queryByText(/no longer registered/i)).not.toBeInTheDocument()

mockedInitializersApi.listRegistered.mockResolvedValue({
items: [targetInitializer],
pagination: { limit: 200, has_more: false },
})
fireEvent.click(screen.getByRole('button', { name: 'Refresh' }))

// Refresh re-runs the load effect (and remounts rows via the loading
// state), so assert on a freshly queried node.
await waitFor(async () => {
expect(screen.getByTestId('baseline-initializer-row-target')).toHaveTextContent(
'Registers targets.',
)
})
})

it('should remove an additional initializer and show success feedback', async () => {
const user = userEvent.setup()
renderInitializers()
Expand Down
8 changes: 8 additions & 0 deletions frontend/src/components/Initializers/Initializers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ export default function Initializers() {
const styles = useInitializersStyles()
const [settings, setSettings] = useState<InitializerSettingsResponse>(EMPTY_SETTINGS)
const [registeredInitializers, setRegisteredInitializers] = useState<RegisteredInitializer[]>([])
// A failed catalog request leaves registration state unknown; rows must
// then say "temporarily unavailable" instead of "no longer registered",
// which would present a metadata outage as a configuration problem.
const [catalogAvailable, setCatalogAvailable] = useState(true)
const [loading, setLoading] = useState(true)
const [statusMessage, setStatusMessage] = useState<StatusMessage | null>(null)
const [refetchCount, setRefetchCount] = useState(0)
Expand Down Expand Up @@ -55,7 +59,9 @@ export default function Initializers() {

if (registeredResult.status === 'fulfilled') {
setRegisteredInitializers(registeredResult.value.items)
setCatalogAvailable(true)
} else {
setCatalogAvailable(false)
const catalogError = toApiError(registeredResult.reason).detail
setStatusMessage((current: StatusMessage | null) =>
current
Expand Down Expand Up @@ -207,10 +213,12 @@ export default function Initializers() {
<BaselineInitializers
items={settings.baseline}
registeredInitializers={registeredInitializers}
catalogAvailable={catalogAvailable}
/>
<AdditionalInitializers
items={settings.additional}
registeredInitializers={registeredInitializers}
catalogAvailable={catalogAvailable}
creating={creating}
savingInitializerId={savingInitializerId}
saveErrors={saveErrors}
Expand Down
21 changes: 21 additions & 0 deletions frontend/src/components/Initializers/initializerLookup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,24 @@ describe('resolveRegisteredInitializer', () => {
expect(result.initializer_type).toBe('UnknownInitializer')
})
})

describe('resolveRegisteredInitializer with catalog availability', () => {
it('reports temporary unavailability instead of unregistered when the catalog request failed', () => {
const result = resolveRegisteredInitializer('target', [], false)

expect(result).toEqual({
initializer_name: 'target',
initializer_type: 'UnverifiedInitializer',
description: expect.stringContaining('temporarily unavailable'),
required_env_vars: [],
supported_parameters: [],
})
expect(result.description).not.toContain('no longer registered')
})

it('still resolves matches from stale-but-loaded catalog data when available flag is true', () => {
const result = resolveRegisteredInitializer('target', registered, true)

expect(result).toBe(registered[0])
})
})
22 changes: 21 additions & 1 deletion frontend/src/components/Initializers/initializerLookup.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,36 @@
import type { RegisteredInitializer } from '@/types'

export const UNREGISTERED_INITIALIZER_DESCRIPTION = 'Initializer is no longer registered.'
export const CATALOG_UNAVAILABLE_DESCRIPTION =
'Initializer catalog is temporarily unavailable; registration state cannot be confirmed.'

/**
* Resolve a settings entry's `initializer_name` to its catalog definition.
*
* Settings reference an initializer by name; the catalog (from `listRegistered`)
* is the single source of truth for display metadata. When a persisted name is no
* longer registered, return a placeholder so the row still renders.
*
* `catalogAvailable` distinguishes the two ways a name can fail to resolve:
* a successful catalog response that lacks the name means the entry is truly
* unregistered, while a failed catalog request only means registration state
* is unknown and must not be reported as a configuration problem.
*/
export function resolveRegisteredInitializer(
initializerName: string,
registeredInitializers: RegisteredInitializer[],
catalogAvailable: boolean = true,
): RegisteredInitializer {
if (!catalogAvailable) {
return {
initializer_name: initializerName,
initializer_type: 'UnverifiedInitializer',
description: CATALOG_UNAVAILABLE_DESCRIPTION,
required_env_vars: [],
supported_parameters: [],
}
}

const match = registeredInitializers.find((item) => item.initializer_name === initializerName)
if (match) {
return match
Expand All @@ -19,7 +39,7 @@ export function resolveRegisteredInitializer(
return {
initializer_name: initializerName,
initializer_type: 'UnknownInitializer',
description: 'Initializer is no longer registered.',
description: UNREGISTERED_INITIALIZER_DESCRIPTION,
required_env_vars: [],
supported_parameters: [],
}
Expand Down