CONSOLE-4976: Add ClusterExtension creation form and unit tests #15955
Conversation
- Create ClusterExtensionForm with form/YAML view toggle - Add SchemaFieldHelp component to display API schema documentation - Add TextInputModal as reusable generic modal in console-shared - Implement UI-only state for auto-generated namespace and service account names - Add comprehensive field help descriptions with semantic versioning syntax examples - Add ServiceAccountDropdown with create option - Support auto-detection of existing namespaces and service accounts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add TextInputModal.spec.tsx with 12 tests for modal validation and behavior - Add SchemaFieldHelp.spec.tsx with 5 tests for schema description rendering - Add ClusterExtensionForm.spec.tsx with 14 tests covering: - Form field rendering and initialization - Auto-generation of namespace and service account names - Radio button toggling between create new and select from cluster - Channel and catalog label management - Version and package name updates - Pencil icon editing for auto-generated names All 31 unit tests passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
openshift-ci-robot
added
the
jira/valid-reference
|
@TheRealJon: This pull request references CONSOLE-4976 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: TheRealJon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
component/olm
|
@TheRealJon: This pull request references CONSOLE-4976 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughThis pull request introduces UI components and hooks for managing ClusterExtension creation in the Operator Lifecycle Manager v1 package. New reusable components include TextInputModal for validated text input, SchemaFieldHelp for displaying property descriptions, ClusterExtensionForm for form-based resource creation, and modals for creating ServiceAccounts. The refactored CreateClusterExtension component now uses SyncedEditor to coordinate form and YAML editing. Supporting hooks enable modal launching, and localization keys are added for form fields, help text, and error messages. SyncedEditor's context prop becomes optional. Comprehensive test coverage is included. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@frontend/packages/console-shared/src/components/modals/TextInputModal.tsx`:
- Line 3: Replace the deprecated Modal import and composed-props usage with the
PatternFly v6 composable Modal API: change the import to "import { Modal,
ModalHeader, ModalBody, ModalFooter } from '@patternfly/react-core'", remove any
use of ModalVariant (e.g., ModalVariant.small) and instead use the Modal's
size/width props, and refactor the Modal JSX in TextInputModal (where title,
actions or ModalVariant are currently passed) to render
<Modal><ModalHeader>...</ModalHeader><ModalBody>...</ModalBody><ModalFooter>...</ModalFooter></Modal>
so the header, body and footer are explicit and action buttons move into
ModalFooter.
In
`@frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/ClusterExtensionForm.tsx`:
- Around line 418-423: The modal's initialValue is incorrectly using the
original name variable which discards any in-progress edits; change the
textInputModal call in ClusterExtensionForm (the invocation that passes
title/label/initialValue/onSubmit) to set initialValue to the current namespace
variable instead of name so the modal seeds with the current namespace value
(keep onSubmit as handleNamespaceChange).
- Around line 139-165: When the user edits the extension name in
handleNameChange, detect if the existing values for 'spec.namespace' and
'spec.serviceAccount.name' still match the previous auto-generated pattern
(e.g., derived from the old metadata.name); if they do, update them to the new
auto-generated values too by calling updateFormDataPath for 'spec.namespace' and
'spec.serviceAccount.name' (or invoke handleNamespaceChange /
handleServiceAccountNameChange) so the auto-generated namespace/ServiceAccount
stay in sync with the edited name; use the current form data (formData) to
compute whether the fields match the old pattern and to build the new derived
names before updating.
In
`@frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/CreateServiceAccountModal.tsx`:
- Around line 109-131: The Name field uses a nested FormGroup and neither
FormGroup includes fieldId, breaking label-input association; in
CreateServiceAccountModal update the Name block to use a single FormGroup with
fieldId="input-name" that wraps the TextInput (id="input-name") and add
fieldId="input-namespace" to the Namespace FormGroup to match the TextInput
(id="input-namespace"); ensure you leave submit, setName, namespace, and the
TextInput props unchanged while only adjusting FormGroup usage and adding the
fieldId attributes.
🧹 Nitpick comments (4)
frontend/packages/console-shared/src/components/modals/__tests__/TextInputModal.spec.tsx (1)
36-44: Solid test coverage—consider adding keyboard submission test.The suite covers the critical paths well: validation, error states, custom validators, and button interactions.
One optional enhancement: since the modal wraps inputs in a
<form>, users may expect to submit via Enter key. Consider adding a test for keyboard submission (e.g.,fireEvent.submit(form)or pressing Enter on the input).frontend/packages/console-shared/src/components/modals/TextInputModal.tsx (2)
37-55: Consider error boundary for onSubmit failures.If
onSubmitthrows synchronously, the modal won't close but also won't display the error to users. For robustness, consider wrapping in try/catch:♻️ Optional: wrap onSubmit in try/catch
+ try { onSubmit(value); closeModal(); + } catch (err) { + setErrorMessage(String(err)); + }
87-114: A11y consideration: link error to input for screen readers.The error Alert is visually associated but not programmatically linked to the input. For better screen reader support, consider using PatternFly's FormGroup
validatedandhelperTextInvalidprops, or addaria-describedbyto the input referencing the error's id.This is optional but aligns with WCAG 3.3.1 (Error Identification).
frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/ClusterExtensionForm.tsx (1)
73-86: UseCATALOG_LABEL_KEYwhen parsing selectors.Parsing uses a hard-coded label key string while updates use
CATALOG_LABEL_KEY. This risks drift if the constant changes.♻️ Consistency tweak
- const catalogLabel = selector.matchLabels?.['olm.operatorframework.io/metadata.name']; + const catalogLabel = selector.matchLabels?.[CATALOG_LABEL_KEY]; ... - expr.key === 'olm.operatorframework.io/metadata.name' && expr.operator === 'In', + expr.key === CATALOG_LABEL_KEY && expr.operator === 'In',
| @@ -0,0 +1,118 @@ | |||
| import { useState } from 'react'; | |||
| import { Button, Alert, TextInput, FormGroup } from '@patternfly/react-core'; | |||
| import { Modal, ModalVariant } from '@patternfly/react-core/deprecated'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the TextInputModal.tsx file to see the actual import and context
cat -n frontend/packages/console-shared/src/components/modals/TextInputModal.tsx | head -80Repository: openshift/console
Length of output: 2727
🏁 Script executed:
# Search for Modal imports across the codebase to understand migration patterns
rg "from ['\"]@patternfly/react-core" frontend/packages/console-shared --type ts --type tsx | head -20Repository: openshift/console
Length of output: 88
🏁 Script executed:
# Check if there are any migration tracking comments or TODO items related to PatternFly
rg "PatternFly|deprecated|v5|v6" frontend/packages/console-shared/src/components/modals/ --type ts --type tsxRepository: openshift/console
Length of output: 88
🏁 Script executed:
# Search for Modal imports across the codebase (corrected command)
rg "Modal" frontend/packages/console-shared/src/components/modals/ -A 1 | head -30Repository: openshift/console
Length of output: 2726
🏁 Script executed:
# Check how many places import from `@patternfly/react-core/deprecated`
rg "@patternfly/react-core/deprecated" frontend/packages/console-sharedRepository: openshift/console
Length of output: 673
🏁 Script executed:
# Search for Modal imports in the entire console codebase
rg "from.*@patternfly/react-core.*Modal" --max-count 30Repository: openshift/console
Length of output: 43
🌐 Web query:
PatternFly React v6 Modal component migration deprecated to non-deprecated
💡 Result:
In PatternFly React v6, the non-deprecated Modal is the “new” implementation (formerly the /next Modal). The deprecated one is kept under @patternfly/react-core/deprecated and is no longer maintained. (staging.patternfly.org)
1) Update imports
- Deprecated (old)
import { Modal } from '@patternfly/react-core/deprecated';
- Non-deprecated (v6 recommended)
import { Modal, ModalHeader, ModalBody, ModalFooter } from '@patternfly/react-core';
PatternFly’s codemods explicitly call out that the “Next” Modal was promoted to the recommended implementation. (github.com)
2) Convert from “props-driven” header/footer/actions → composable subcomponents
Before (deprecated)
Common deprecated usage relies on title, header, footer, actions, etc. (patternfly.org)
<Modal
title="My title"
isOpen={isOpen}
onClose={onClose}
actions={[
<Button key="confirm">Confirm</Button>,
<Button key="cancel" variant="link" onClick={onClose}>Cancel</Button>
]}
/>After (non-deprecated)
In the recommended Modal, you compose with <ModalHeader/>, <ModalBody/>, <ModalFooter/>. (patternfly.org)
<Modal isOpen={isOpen} onClose={onClose}>
<ModalHeader title="My title" />
<ModalBody>
Modal content…
</ModalBody>
<ModalFooter>
<Button>Confirm</Button>
<Button variant="link" onClick={onClose}>Cancel</Button>
</ModalFooter>
</Modal>3) A couple of common behavioral/API gotchas
- Close button rendering: in the recommended Modal,
onClose“needs to be passed to render the close button.” (patternfly.org) - Scrollable content: deprecated docs say to set
tabIndex={0}on<Modal>; the recommended docs say to settabIndex={0}on<ModalBody>. (patternfly.org) - Custom header: deprecated Modal used
header/footerprops; recommended Modal uses<ModalHeader>composition for custom header content. (patternfly.org)
If you paste one of your existing deprecated Modal usages (especially if you’re using header, footer, actions, or help), I can rewrite it into the v6 non-deprecated structure.
Citations:
- 1: https://staging.patternfly.org/releases/overview?utm_source=openai
- 2: https://github.com/patternfly/pf-codemods?utm_source=openai
- 3: https://www.patternfly.org/components/modal/react-deprecated
- 4: https://www.patternfly.org/components/modal
- 5: https://www.patternfly.org/components/modal
- 6: https://www.patternfly.org/components/modal/react-deprecated
- 7: https://www.patternfly.org/components/modal/react-deprecated
Migrate to non-deprecated PatternFly v6 Modal—deprecated path is no longer maintained.
The import from @patternfly/react-core/deprecated should not be used. PatternFly v6 has already been released, and the deprecated Modal is the old unmaintained implementation. The recommended Modal is available directly from @patternfly/react-core and uses a composable API with ModalHeader, ModalBody, and ModalFooter.
Update the import and refactor to use composed subcomponents instead of props like title, actions, and ModalVariant:
- Replace
import { Modal, ModalVariant } from '@patternfly/react-core/deprecated'withimport { Modal, ModalHeader, ModalBody, ModalFooter } from '@patternfly/react-core' - Restructure the Modal tree to compose the header, body, and footer explicitly
- Remove
ModalVariant.smalland use the Modal's size props instead
This pattern appears in at least 4 files across the codebase (CreateNamespaceModal, CreateProjectModal, Modal.tsx wrapper, and here). Consider a coordinated migration effort.
🤖 Prompt for AI Agents
In `@frontend/packages/console-shared/src/components/modals/TextInputModal.tsx` at
line 3, Replace the deprecated Modal import and composed-props usage with the
PatternFly v6 composable Modal API: change the import to "import { Modal,
ModalHeader, ModalBody, ModalFooter } from '@patternfly/react-core'", remove any
use of ModalVariant (e.g., ModalVariant.small) and instead use the Modal's
size/width props, and refactor the Modal JSX in TextInputModal (where title,
actions or ModalVariant are currently passed) to render
<Modal><ModalHeader>...</ModalHeader><ModalBody>...</ModalBody><ModalFooter>...</ModalFooter></Modal>
so the header, body and footer are explicit and action buttons move into
ModalFooter.
| const handleNameChange = useCallback( | ||
| (_event: any, value: string) => { | ||
| updateFormDataPath('metadata.name', value); | ||
| }, | ||
| [updateFormDataPath], | ||
| ); | ||
|
|
||
| const handlePackageNameChange = useCallback( | ||
| (_event: any, value: string) => { | ||
| updateFormDataPath('spec.source.catalog.packageName', value); | ||
| }, | ||
| [updateFormDataPath], | ||
| ); | ||
|
|
||
| const handleNamespaceChange = useCallback( | ||
| (value: string) => { | ||
| updateFormDataPath('spec.namespace', value); | ||
| }, | ||
| [updateFormDataPath], | ||
| ); | ||
|
|
||
| const handleServiceAccountNameChange = useCallback( | ||
| (value: string) => { | ||
| updateFormDataPath('spec.serviceAccount.name', value); | ||
| }, | ||
| [updateFormDataPath], | ||
| ); |
There was a problem hiding this comment.
Keep auto-generated namespace/ServiceAccount in sync with name edits.
Right now, changing the extension name does not update the auto-generated namespace/serviceAccount, so you can end up creating resources with stale names. If the current values still match the previous auto pattern, update them on name changes to preserve expected “auto” behavior.
🔧 Suggested fix
- const handleNameChange = useCallback(
- (_event: any, value: string) => {
- updateFormDataPath('metadata.name', value);
- },
- [updateFormDataPath],
- );
+ const handleNameChange = useCallback(
+ (_event: any, value: string) => {
+ const prevName = name;
+ updateFormDataPath('metadata.name', value);
+
+ if (useAutoNamespace && namespace === prevName) {
+ updateFormDataPath('spec.namespace', value);
+ }
+ if (
+ useAutoServiceAccount &&
+ serviceAccountName === `${prevName}-service-account`
+ ) {
+ updateFormDataPath('spec.serviceAccount.name', `${value}-service-account`);
+ }
+ },
+ [
+ name,
+ namespace,
+ serviceAccountName,
+ updateFormDataPath,
+ useAutoNamespace,
+ useAutoServiceAccount,
+ ],
+ );🤖 Prompt for AI Agents
In
`@frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/ClusterExtensionForm.tsx`
around lines 139 - 165, When the user edits the extension name in
handleNameChange, detect if the existing values for 'spec.namespace' and
'spec.serviceAccount.name' still match the previous auto-generated pattern
(e.g., derived from the old metadata.name); if they do, update them to the new
auto-generated values too by calling updateFormDataPath for 'spec.namespace' and
'spec.serviceAccount.name' (or invoke handleNamespaceChange /
handleServiceAccountNameChange) so the auto-generated namespace/ServiceAccount
stay in sync with the edited name; use the current form data (formData) to
compute whether the fields match the old pattern and to build the new derived
names before updating.
| textInputModal({ | ||
| title: t('olm-v1~Edit Namespace Name'), | ||
| label: t('olm-v1~Namespace'), | ||
| initialValue: name, | ||
| onSubmit: handleNamespaceChange, | ||
| }); |
There was a problem hiding this comment.
Use the current namespace as the modal’s initial value.
The edit modal seeds initialValue with name, which overwrites previously edited namespace values. Use the current namespace instead.
✅ Minimal fix
- initialValue: name,
+ initialValue: namespace,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| textInputModal({ | |
| title: t('olm-v1~Edit Namespace Name'), | |
| label: t('olm-v1~Namespace'), | |
| initialValue: name, | |
| onSubmit: handleNamespaceChange, | |
| }); | |
| textInputModal({ | |
| title: t('olm-v1~Edit Namespace Name'), | |
| label: t('olm-v1~Namespace'), | |
| initialValue: namespace, | |
| onSubmit: handleNamespaceChange, | |
| }); |
🤖 Prompt for AI Agents
In
`@frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/ClusterExtensionForm.tsx`
around lines 418 - 423, The modal's initialValue is incorrectly using the
original name variable which discards any in-progress edits; change the
textInputModal call in ClusterExtensionForm (the invocation that passes
title/label/initialValue/onSubmit) to set initialValue to the current namespace
variable instead of name so the modal seeds with the current namespace value
(keep onSubmit as handleNamespaceChange).
| <Form onSubmit={submit}> | ||
| <FormGroup> | ||
| <FormGroup label={t('olm-v1~Name')}> | ||
| <TextInput | ||
| id="input-name" | ||
| data-test="input-name" | ||
| name="name" | ||
| type="text" | ||
| value={name} | ||
| onChange={(e, value) => setName(value)} | ||
| /> | ||
| </FormGroup> | ||
| </FormGroup> | ||
| <FormGroup label={t('olm-v1~Namespace')}> | ||
| <TextInput | ||
| readOnly | ||
| id="input-namespace" | ||
| data-test="input-namespace" | ||
| name="namespace" | ||
| value={namespace} | ||
| type="text" | ||
| /> | ||
| </FormGroup> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/CreateServiceAccountModal.tsx | sed -n '100,140p'Repository: openshift/console
Length of output: 1573
🏁 Script executed:
rg "FormGroup" frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/CreateServiceAccountModal.tsx -A 5 -B 1Repository: openshift/console
Length of output: 1229
🏁 Script executed:
rg "FormGroup.*label" frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/CreateServiceAccountModal.tsxRepository: openshift/console
Length of output: 157
🌐 Web query:
PatternFly FormGroup fieldId label accessibility best practice
💡 Result:
-
Always use an explicit label–control association. In PatternFly forms, each field should have an
id, and its<label>should have a matchingforattribute (or the React equivalent:labelwired to the input’sid). This is the primary accessibility best practice for form fields. [3], [4], [6] -
FormGroup fieldIdbest practice (React):- If your
FormGroupcontains a single form control, setfieldIdto the same value as the control’sid. PatternFly explicitly calls this out. [1] - If your
FormGrouprepresents multiple related controls (e.g., a set of radios/checkboxes), usefieldIdto identify the group, and setrole="radiogroup"(for radios) orrole="group"(for other multi-control groupings). PatternFly notesfieldIdis required when using those group roles. [1], [3]
- If your
-
For helper/extra text, connect it to the input. If you render helper text for a field, give the helper text an
idand addaria-describedbyon the input pointing to it (PatternFly documents this requirement). [7] -
Don’t rely on placeholder text for essential labeling/instructions. PatternFly form guidelines call this out because placeholder text isn’t reliably announced by screen readers. [2]
Link labels to inputs with fieldId for screen-reader navigation.
The nested FormGroup structure for the Name field is non-standard, and both fields lack fieldId, breaking the programmatic label-input association required by PatternFly and WCAG. Set fieldId to match each input's id.
♿ Proposed fix
- <FormGroup>
- <FormGroup label={t('olm-v1~Name')}>
- <TextInput
- id="input-name"
- data-test="input-name"
- name="name"
- type="text"
- value={name}
- onChange={(e, value) => setName(value)}
- />
- </FormGroup>
- </FormGroup>
- <FormGroup label={t('olm-v1~Namespace')}>
- <TextInput
- readOnly
- id="input-namespace"
- data-test="input-namespace"
- name="namespace"
- value={namespace}
- type="text"
- />
- </FormGroup>
+ <FormGroup label={t('olm-v1~Name')} fieldId="input-name">
+ <TextInput
+ id="input-name"
+ data-test="input-name"
+ name="name"
+ type="text"
+ value={name}
+ onChange={(e, value) => setName(value)}
+ />
+ </FormGroup>
+ <FormGroup label={t('olm-v1~Namespace')} fieldId="input-namespace">
+ <TextInput
+ readOnly
+ id="input-namespace"
+ data-test="input-namespace"
+ name="namespace"
+ value={namespace}
+ type="text"
+ />
+ </FormGroup>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Form onSubmit={submit}> | |
| <FormGroup> | |
| <FormGroup label={t('olm-v1~Name')}> | |
| <TextInput | |
| id="input-name" | |
| data-test="input-name" | |
| name="name" | |
| type="text" | |
| value={name} | |
| onChange={(e, value) => setName(value)} | |
| /> | |
| </FormGroup> | |
| </FormGroup> | |
| <FormGroup label={t('olm-v1~Namespace')}> | |
| <TextInput | |
| readOnly | |
| id="input-namespace" | |
| data-test="input-namespace" | |
| name="namespace" | |
| value={namespace} | |
| type="text" | |
| /> | |
| </FormGroup> | |
| <Form onSubmit={submit}> | |
| <FormGroup label={t('olm-v1~Name')} fieldId="input-name"> | |
| <TextInput | |
| id="input-name" | |
| data-test="input-name" | |
| name="name" | |
| type="text" | |
| value={name} | |
| onChange={(e, value) => setName(value)} | |
| /> | |
| </FormGroup> | |
| <FormGroup label={t('olm-v1~Namespace')} fieldId="input-namespace"> | |
| <TextInput | |
| readOnly | |
| id="input-namespace" | |
| data-test="input-namespace" | |
| name="namespace" | |
| value={namespace} | |
| type="text" | |
| /> | |
| </FormGroup> |
🤖 Prompt for AI Agents
In
`@frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/CreateServiceAccountModal.tsx`
around lines 109 - 131, The Name field uses a nested FormGroup and neither
FormGroup includes fieldId, breaking label-input association; in
CreateServiceAccountModal update the Name block to use a single FormGroup with
fieldId="input-name" that wraps the TextInput (id="input-name") and add
fieldId="input-namespace" to the Namespace FormGroup to match the TextInput
(id="input-namespace"); ensure you leave submit, setName, namespace, and the
TextInput props unchanged while only adjusting FormGroup usage and adding the
fieldId attributes.
|
@TheRealJon: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Summary
Adds ClusterExtension creation form with comprehensive unit test coverage. The form supports auto-generated namespace and service account configuration, with the ability to edit names via modal, select existing resources from cluster, and configure version constraints, channels, and catalogs.
Components Added
ClusterExtensionForm: Main form component for creating ClusterExtensions
TextInputModal: Reusable modal for editing text values
SchemaFieldHelp: Popover help icon displaying API schema descriptions with fallback support
Unit Tests (31 total)
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.