Skip to content

NO-ISSUE: Replace LabeledResourceRef with plain values in Select/Mult… - #123

Merged
openshift-merge-bot[bot] merged 1 commit into
osac-project:mainfrom
rawagner:remove_labeled_ref
Aug 6, 2026
Merged

NO-ISSUE: Replace LabeledResourceRef with plain values in Select/Mult…#123
openshift-merge-bot[bot] merged 1 commit into
osac-project:mainfrom
rawagner:remove_labeled_ref

Conversation

@rawagner

@rawagner rawagner commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

…iSelect fields

SelectField and MultiSelectField stored { value, label } objects (LabeledResourceRef) in Formik state. This coupled display concerns into form values, complicated payloads, and required unwrapping everywhere. Simplify to store plain string/number values instead and derive display labels from the options list at render time.

  • Remove labeledResourceRef.ts and labeledResourceRefSchema.ts
  • Update SelectField/MultiSelectField to use primitive Formik values
  • Move review step rendering into per-adapter components (VmReviewStep, ClusterReviewStep, BareMetalReviewStep) that fetch display data directly, replacing the generic ReviewStep that relied on labeled refs
  • Simplify form schemas and payload builders to work with plain values
  • Add get handlers to mock transport for InstanceTypes, VirtualNetworks, and Subnets to support review step data fetching in tests

Assisted-by: Claude Code noreply@anthropic.com

Summary by CodeRabbit

  • New Features

    • Added detailed review screens for virtual machine, cluster, and bare-metal provisioning.
    • Review summaries display selected resources, configuration, networking, storage, credentials, and user data.
    • Added translations for boot disk, VM image, user data, and fetch-error messages.
  • Improvements

    • Simplified form selections across provisioning and networking workflows.
    • Updated security group and external IP forms for more reliable selection and submission.
  • Bug Fixes

    • Added loading states, inline errors, and fallbacks when review details are unavailable.

@openshift-ci-robot

Copy link
Copy Markdown

@rawagner: This pull request explicitly references no jira issue.

Details

In response to this:

…iSelect fields

SelectField and MultiSelectField stored { value, label } objects (LabeledResourceRef) in Formik state. This coupled display concerns into form values, complicated payloads, and required unwrapping everywhere. Simplify to store plain string/number values instead and derive display labels from the options list at render time.

  • Remove labeledResourceRef.ts and labeledResourceRefSchema.ts
  • Update SelectField/MultiSelectField to use primitive Formik values
  • Move review step rendering into per-adapter components (VmReviewStep, ClusterReviewStep, BareMetalReviewStep) that fetch display data directly, replacing the generic ReviewStep that relied on labeled refs
  • Simplify form schemas and payload builders to work with plain values
  • Add get handlers to mock transport for InstanceTypes, VirtualNetworks, and Subnets to support review step data fetching in tests

Assisted-by: Claude Code noreply@anthropic.com

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.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The PR replaces labeled resource-reference objects with primitive form values across shared fields, catalog provisioning, networking forms, payloads, and schemas. It also replaces generic catalog review sections with adapter-specific review components that resolve resource metadata.

Changes

Catalog provisioning form and review flow

Layer / File(s) Summary
Primitive form field values
libs/ui-components/src/components/Form/*, libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/*
Select, multi-select, compute-instance, and related tests now store primitive values directly in Formik.
Cluster node-set values
libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/*
Cluster host types now use string identifiers in fields, validation, payloads, and fixtures.
Adapter-specific review rendering
libs/ui-components/src/components/catalogProvision/CatalogProvisionWizard.tsx, libs/ui-components/src/components/catalogProvision/wizard/adapters/*
The wizard renders adapter-provided review components for bare-metal, cluster, and VM provisioning.
Networking form values
libs/ui-components/src/components/networking/*, libs/ui-components/src/components/vm/DetailsPage/AttachExternalIpModal.tsx
Networking forms use primitive IDs and numeric protocol values for validation and submission.
Review labels and lookup fixtures
libs/i18n/locales/en/translation.json, libs/ui-components/src/components/catalogProvision/wizard/stepIds.ts, libs/ui-components/src/test-utils/createMockConnectTransport.ts
Review labels and translated wizard step labels are updated. Mock handlers resolve selected resources by ID.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CatalogProvisionWizard
  participant AdapterReviewStep
  participant Formik
  participant ResourceAPI
  CatalogProvisionWizard->>AdapterReviewStep: render selected catalog item
  AdapterReviewStep->>Formik: read wizard values
  AdapterReviewStep->>ResourceAPI: fetch selected resource metadata
  ResourceAPI-->>AdapterReviewStep: return resource details
  AdapterReviewStep-->>CatalogProvisionWizard: render formatted review fields
Loading

Possibly related PRs

Suggested labels: lgtm

Suggested reviewers: liatb-rh, elayaharoni, batzionb, larsks

🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the core replacement of LabeledResourceRef values in Select and MultiSelect components, although it does not cover the broader review-step changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No-Hardcoded-Secrets ✅ Passed The diff adds no API keys, tokens, passwords, private keys, embedded credentials, recognizable vendor tokens, JWTs, or long encoded secrets; pull-secret data is runtime-only.
No-Weak-Crypto ✅ Passed The PR adds no MD5, SHA-1, DES, RC4, Blowfish, ECB, custom crypto, or secret/token comparisons; its only crypto API, crypto.randomUUID(), is pre-existing.
No-Injection-Vectors ✅ Passed Changed files contain no SQL, shell, eval/exec, pickle, unsafe YAML, os.system, or dangerouslySetInnerHTML usage.
Container-Privileges ✅ Passed No privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation settings exist; Containerfile root users are build stages and production runs as UID 1001.
No-Sensitive-Data-In-Logs ✅ Passed No console or logger calls appear in the changed files; review components mask pull secrets and user data is rendered in UI, not logged.
Ai-Attribution ✅ Passed The PR has one commit with Assisted-by: Claude Code <noreply@anthropic.com> and no Co-Authored-By trailer.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterReviewStep.tsx`:
- Around line 43-45: Validate host-type IDs in ClusterReviewStep.tsx lines 43-45
and security-group IDs in VmReviewStep.tsx lines 40-42 using the shared
filter-literal encoder or an allow-list before constructing the API predicates;
preserve only validated IDs in the generated filters.

In
`@libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/payload.ts`:
- Line 40: Update the hostTypeId assignment in the payload builder to trim
row.hostType before using it for nodeSets and hostType. Add a payload.test.ts
case with a padded host-type ID and verify the generated request uses the
normalized value.

In `@libs/ui-components/src/components/Form/SelectField.tsx`:
- Around line 56-74: Update the empty-value checks in the auto-selection
useEffect and toggleLabel useMemo to distinguish only nullish/empty values,
preserving numeric 0 as selected. Ensure auto-selection does not overwrite an
existing 0 value and toggleLabel resolves and displays the option label for 0,
then add a regression test covering an option with value 0.
- Around line 76-80: Update the onSelect callback in SelectField to accept an
optional string or number value, and return early when value is undefined before
invoking helpers.setValue. Preserve the existing behavior for defined values.

In `@libs/ui-components/src/components/networking/SecurityGroupRuleModal.tsx`:
- Line 27: Update the protocol-dependent Yup.when predicates in
SecurityGroupRuleModal to compare the numeric protocol directly with
Protocol.TCP and Protocol.UDP instead of reading protocol.value, ensuring
TCP/UDP port validation is applied and missing ports are rejected. Add coverage
for submitting a TCP rule with missing ports.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8abe2600-5d4e-4dda-92fe-888842728ac2

📥 Commits

Reviewing files that changed from the base of the PR and between 752f695 and 1d4bf8b.

📒 Files selected for processing (35)
  • libs/i18n/locales/en/translation.json
  • libs/ui-components/src/components/Form/MultiSelectField.tsx
  • libs/ui-components/src/components/Form/SelectField.test.tsx
  • libs/ui-components/src/components/Form/SelectField.tsx
  • libs/ui-components/src/components/Form/labeledResourceRef.ts
  • libs/ui-components/src/components/Form/labeledResourceRefSchema.ts
  • libs/ui-components/src/components/catalogProvision/CatalogProvisionWizard.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/bareMetalInstance/BareMetalReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/bareMetalInstanceAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterConfigurationStep.test.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterNodeSetsArrayField.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/fields.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/payload.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/payload.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/schemas.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/schemas.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/clusterAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/VmNetworkingStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/VmReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/fields.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/generalFields.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/payload.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/schemas.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/schemas.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstanceAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/types.ts
  • libs/ui-components/src/components/catalogProvision/wizard/catalogOverlay.ts
  • libs/ui-components/src/components/catalogProvision/wizard/steps/ReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/steps/WizardSteps.tsx
  • libs/ui-components/src/components/networking/SecurityGroupCreateModal.tsx
  • libs/ui-components/src/components/networking/SecurityGroupRuleForm.tsx
  • libs/ui-components/src/components/networking/SecurityGroupRuleModal.tsx
  • libs/ui-components/src/components/vm/DetailsPage/AttachExternalIpModal.tsx
  • libs/ui-components/src/test-utils/createMockConnectTransport.ts
💤 Files with no reviewable changes (5)
  • libs/ui-components/src/components/catalogProvision/wizard/steps/ReviewStep.tsx
  • libs/ui-components/src/components/Form/labeledResourceRefSchema.ts
  • libs/ui-components/src/components/catalogProvision/wizard/steps/WizardSteps.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/catalogOverlay.ts
  • libs/ui-components/src/components/Form/labeledResourceRef.ts

Comment thread libs/ui-components/src/components/Form/SelectField.tsx
Comment thread libs/ui-components/src/components/Form/SelectField.tsx Outdated
@rawagner
rawagner force-pushed the remove_labeled_ref branch from 1d4bf8b to e9c7642 Compare August 6, 2026 09:58
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@libs/ui-components/src/components/catalogProvision/wizard/adapters/clusterAdapter.ts`:
- Line 50: Update ClusterReviewStep’s filter construction to prevent hostType
values from injecting CEL expressions. Escape each hostType as a CEL string
literal or validate it against an explicit allow-list before interpolation,
while preserving the intended filtering behavior.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0b01216e-0e77-462e-bde8-a2ec1c13fc47

📥 Commits

Reviewing files that changed from the base of the PR and between f490b4f and e9c7642.

📒 Files selected for processing (35)
  • libs/i18n/locales/en/translation.json
  • libs/ui-components/src/components/Form/MultiSelectField.tsx
  • libs/ui-components/src/components/Form/SelectField.test.tsx
  • libs/ui-components/src/components/Form/SelectField.tsx
  • libs/ui-components/src/components/Form/labeledResourceRef.ts
  • libs/ui-components/src/components/Form/labeledResourceRefSchema.ts
  • libs/ui-components/src/components/catalogProvision/CatalogProvisionWizard.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/bareMetalInstance/BareMetalReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/bareMetalInstanceAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterConfigurationStep.test.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterNodeSetsArrayField.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/fields.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/payload.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/payload.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/schemas.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/schemas.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/clusterAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/VmNetworkingStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/VmReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/fields.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/generalFields.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/payload.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/schemas.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/schemas.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstanceAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/types.ts
  • libs/ui-components/src/components/catalogProvision/wizard/catalogOverlay.ts
  • libs/ui-components/src/components/catalogProvision/wizard/steps/ReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/steps/WizardSteps.tsx
  • libs/ui-components/src/components/networking/SecurityGroupCreateModal.tsx
  • libs/ui-components/src/components/networking/SecurityGroupRuleForm.tsx
  • libs/ui-components/src/components/networking/SecurityGroupRuleModal.tsx
  • libs/ui-components/src/components/vm/DetailsPage/AttachExternalIpModal.tsx
  • libs/ui-components/src/test-utils/createMockConnectTransport.ts
💤 Files with no reviewable changes (5)
  • libs/ui-components/src/components/catalogProvision/wizard/steps/ReviewStep.tsx
  • libs/ui-components/src/components/Form/labeledResourceRefSchema.ts
  • libs/ui-components/src/components/catalogProvision/wizard/catalogOverlay.ts
  • libs/ui-components/src/components/Form/labeledResourceRef.ts
  • libs/ui-components/src/components/catalogProvision/wizard/steps/WizardSteps.tsx
🚧 Files skipped from review as they are similar to previous changes (28)
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/payload.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterConfigurationStep.test.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/schemas.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/payload.ts
  • libs/ui-components/src/components/catalogProvision/CatalogProvisionWizard.tsx
  • libs/ui-components/src/test-utils/createMockConnectTransport.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterNodeSetsArrayField.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/types.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/fields.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/bareMetalInstanceAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/schemas.ts
  • libs/ui-components/src/components/networking/SecurityGroupRuleForm.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/VmNetworkingStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/generalFields.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/fields.ts
  • libs/ui-components/src/components/networking/SecurityGroupCreateModal.tsx
  • libs/ui-components/src/components/networking/SecurityGroupRuleModal.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/schemas.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstanceAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/payload.test.ts
  • libs/ui-components/src/components/vm/DetailsPage/AttachExternalIpModal.tsx
  • libs/ui-components/src/components/Form/SelectField.test.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/schemas.test.ts
  • libs/ui-components/src/components/Form/MultiSelectField.tsx
  • libs/i18n/locales/en/translation.json
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/VmReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterReviewStep.tsx
  • libs/ui-components/src/components/Form/SelectField.tsx

@rawagner
rawagner force-pushed the remove_labeled_ref branch from e9c7642 to 2c93f56 Compare August 6, 2026 10:07
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
libs/ui-components/src/components/catalogProvision/wizard/stepIds.ts (1)

15-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Rename STEP_LABEL_KEYS to match its return value.

The export now returns translated labels, not translation keys. Rename it to getWizardStepLabels and update libs/ui-components/src/components/catalogProvision/CatalogProvisionWizard.tsx. This prevents future callers from treating the returned labels as i18next keys.

Suggested rename
-export const STEP_LABEL_KEYS = ...
+export const getWizardStepLabels = ...
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@libs/ui-components/src/components/catalogProvision/wizard/stepIds.ts` around
lines 15 - 21, Rename the exported STEP_LABEL_KEYS function to
getWizardStepLabels, preserving its translated-label mapping, and update all
references in CatalogProvisionWizard.tsx to use the new name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@libs/ui-components/src/components/catalogProvision/wizard/stepIds.ts`:
- Around line 15-21: Rename the exported STEP_LABEL_KEYS function to
getWizardStepLabels, preserving its translated-label mapping, and update all
references in CatalogProvisionWizard.tsx to use the new name.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d1bab872-e2cc-4790-8b88-6f4789054bb1

📥 Commits

Reviewing files that changed from the base of the PR and between f490b4f and 2c93f56.

📒 Files selected for processing (36)
  • libs/i18n/locales/en/translation.json
  • libs/ui-components/src/components/Form/MultiSelectField.tsx
  • libs/ui-components/src/components/Form/SelectField.test.tsx
  • libs/ui-components/src/components/Form/SelectField.tsx
  • libs/ui-components/src/components/Form/labeledResourceRef.ts
  • libs/ui-components/src/components/Form/labeledResourceRefSchema.ts
  • libs/ui-components/src/components/catalogProvision/CatalogProvisionWizard.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/bareMetalInstance/BareMetalReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/bareMetalInstanceAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterConfigurationStep.test.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterNodeSetsArrayField.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/fields.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/payload.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/payload.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/schemas.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/schemas.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/clusterAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/VmNetworkingStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/VmReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/fields.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/generalFields.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/payload.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/schemas.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/schemas.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstanceAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/types.ts
  • libs/ui-components/src/components/catalogProvision/wizard/catalogOverlay.ts
  • libs/ui-components/src/components/catalogProvision/wizard/stepIds.ts
  • libs/ui-components/src/components/catalogProvision/wizard/steps/ReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/steps/WizardSteps.tsx
  • libs/ui-components/src/components/networking/SecurityGroupCreateModal.tsx
  • libs/ui-components/src/components/networking/SecurityGroupRuleForm.tsx
  • libs/ui-components/src/components/networking/SecurityGroupRuleModal.tsx
  • libs/ui-components/src/components/vm/DetailsPage/AttachExternalIpModal.tsx
  • libs/ui-components/src/test-utils/createMockConnectTransport.ts
💤 Files with no reviewable changes (5)
  • libs/ui-components/src/components/catalogProvision/wizard/steps/WizardSteps.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/catalogOverlay.ts
  • libs/ui-components/src/components/catalogProvision/wizard/steps/ReviewStep.tsx
  • libs/ui-components/src/components/Form/labeledResourceRefSchema.ts
  • libs/ui-components/src/components/Form/labeledResourceRef.ts
🚧 Files skipped from review as they are similar to previous changes (30)
  • libs/ui-components/src/test-utils/createMockConnectTransport.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/payload.ts
  • libs/ui-components/src/components/networking/SecurityGroupCreateModal.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterConfigurationStep.test.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/schemas.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/payload.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/schemas.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/clusterAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/schemas.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/fields.ts
  • libs/ui-components/src/components/networking/SecurityGroupRuleForm.tsx
  • libs/ui-components/src/components/catalogProvision/CatalogProvisionWizard.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/VmNetworkingStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/schemas.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterNodeSetsArrayField.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/VmReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/types.ts
  • libs/ui-components/src/components/Form/MultiSelectField.tsx
  • libs/ui-components/src/components/vm/DetailsPage/AttachExternalIpModal.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/generalFields.test.ts
  • libs/ui-components/src/components/Form/SelectField.test.tsx
  • libs/ui-components/src/components/networking/SecurityGroupRuleModal.tsx
  • libs/ui-components/src/components/Form/SelectField.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstanceAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/bareMetalInstance/BareMetalReviewStep.tsx
  • libs/i18n/locales/en/translation.json
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/payload.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/bareMetalInstanceAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/fields.ts

@rawagner
rawagner force-pushed the remove_labeled_ref branch from 2c93f56 to 2e628d5 Compare August 6, 2026 10:16
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@ElayAharoni ElayAharoni left a comment

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.

the 3 new review files has some duplications in them, the DescriptionLIst, loading spinner, and hte catalog item related fields.
maybe it worth trying to create a reusable component for those?

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@rawagner

rawagner commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

I dont see a reason to have another abstraction on top of DescriptionList - there is no logic to abstract. It is just rendering.

we may want to consider loading state abstraction - but in a separate PR where we change the wizard from adapter pattern to jsx composition.

the changes to review pages are done only to align with changes to labeled ref. The previous, pure function didnt allow for fetching resources.

…iSelect fields

SelectField and MultiSelectField stored { value, label } objects (LabeledResourceRef)
in Formik state. This coupled display concerns into form values, complicated payloads,
and required unwrapping everywhere. Simplify to store plain string/number values instead
and derive display labels from the options list at render time.

- Remove labeledResourceRef.ts and labeledResourceRefSchema.ts
- Update SelectField/MultiSelectField to use primitive Formik values
- Move review step rendering into per-adapter components (VmReviewStep,
  ClusterReviewStep, BareMetalReviewStep) that fetch display data directly,
  replacing the generic ReviewStep that relied on labeled refs
- Simplify form schemas and payload builders to work with plain values
- Add get handlers to mock transport for InstanceTypes, VirtualNetworks,
  and Subnets to support review step data fetching in tests

Assisted-by: Claude Code <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@ElayAharoni
ElayAharoni self-requested a review August 6, 2026 12:37
@rawagner

rawagner commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai resume

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
✅ Action performed

Reviews resumed.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@libs/ui-components/src/components/catalogProvision/wizard/stepIds.ts`:
- Around line 1-15: Add the required commit trailers with Red Hat
identification: include a single Red Hat AI-assistance trailer using the
expected Assisted-by format, remove duplicate Assisted-by: Claude Code trailers,
and add the required Co-Authored-By trailer. Keep the implementation unchanged.
- Around line 5-7: Restore literal step ID typing by declaring WIZARD_STEP_IDS
with a const assertion, so WizardStepId remains the union of known IDs and
downstream labels/schema APIs reject arbitrary strings. Update isWizardStepId to
use some with an equality comparison if includes no longer accepts its string
parameter.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b8e7ad66-6ef9-461d-882c-7ea3bac1ee33

📥 Commits

Reviewing files that changed from the base of the PR and between f490b4f and b604e37.

📒 Files selected for processing (36)
  • libs/i18n/locales/en/translation.json
  • libs/ui-components/src/components/Form/MultiSelectField.tsx
  • libs/ui-components/src/components/Form/SelectField.test.tsx
  • libs/ui-components/src/components/Form/SelectField.tsx
  • libs/ui-components/src/components/Form/labeledResourceRef.ts
  • libs/ui-components/src/components/Form/labeledResourceRefSchema.ts
  • libs/ui-components/src/components/catalogProvision/CatalogProvisionWizard.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/bareMetalInstance/BareMetalReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/bareMetalInstanceAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterConfigurationStep.test.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterNodeSetsArrayField.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/fields.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/payload.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/payload.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/schemas.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/schemas.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/clusterAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/VmNetworkingStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/VmReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/fields.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/generalFields.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/payload.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/schemas.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/schemas.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstanceAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/types.ts
  • libs/ui-components/src/components/catalogProvision/wizard/catalogOverlay.ts
  • libs/ui-components/src/components/catalogProvision/wizard/stepIds.ts
  • libs/ui-components/src/components/catalogProvision/wizard/steps/ReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/steps/WizardSteps.tsx
  • libs/ui-components/src/components/networking/SecurityGroupCreateModal.tsx
  • libs/ui-components/src/components/networking/SecurityGroupRuleForm.tsx
  • libs/ui-components/src/components/networking/SecurityGroupRuleModal.tsx
  • libs/ui-components/src/components/vm/DetailsPage/AttachExternalIpModal.tsx
  • libs/ui-components/src/test-utils/createMockConnectTransport.ts
💤 Files with no reviewable changes (5)
  • libs/ui-components/src/components/catalogProvision/wizard/steps/WizardSteps.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/steps/ReviewStep.tsx
  • libs/ui-components/src/components/Form/labeledResourceRefSchema.ts
  • libs/ui-components/src/components/Form/labeledResourceRef.ts
  • libs/ui-components/src/components/catalogProvision/wizard/catalogOverlay.ts
🚧 Files skipped from review as they are similar to previous changes (30)
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/payload.test.ts
  • libs/ui-components/src/components/networking/SecurityGroupRuleForm.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/payload.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterConfigurationStep.test.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/payload.ts
  • libs/ui-components/src/test-utils/createMockConnectTransport.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstanceAdapter.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/schemas.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/schemas.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/bareMetalInstanceAdapter.ts
  • libs/ui-components/src/components/vm/DetailsPage/AttachExternalIpModal.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterNodeSetsArrayField.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/VmNetworkingStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/bareMetalInstance/BareMetalReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/schemas.test.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/VmReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/CatalogProvisionWizard.tsx
  • libs/ui-components/src/components/networking/SecurityGroupRuleModal.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/ClusterReviewStep.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/schemas.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/clusterAdapter.ts
  • libs/ui-components/src/components/Form/SelectField.test.tsx
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/cluster/fields.ts
  • libs/ui-components/src/components/networking/SecurityGroupCreateModal.tsx
  • libs/i18n/locales/en/translation.json
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/fields.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/types.ts
  • libs/ui-components/src/components/catalogProvision/wizard/adapters/computeInstance/generalFields.test.ts
  • libs/ui-components/src/components/Form/MultiSelectField.tsx
  • libs/ui-components/src/components/Form/SelectField.tsx

@openshift-ci openshift-ci Bot added the lgtm label Aug 6, 2026
@openshift-ci

openshift-ci Bot commented Aug 6, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ElayAharoni, rawagner

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [ElayAharoni,rawagner]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot
openshift-merge-bot Bot merged commit 813f7e3 into osac-project:main Aug 6, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants