Revert abi abov the sea new flow#3517
Merged
openshift-merge-bot[bot] merged 12 commits intoopenshift-assisted:masterfrom Mar 30, 2026
Merged
Revert abi abov the sea new flow#3517openshift-merge-bot[bot] merged 12 commits intoopenshift-assisted:masterfrom
openshift-merge-bot[bot] merged 12 commits intoopenshift-assisted:masterfrom
Conversation
…assisted#3413)" This reverts commit dc463bb.
This reverts commit 2dd860c.
This reverts commit c2499bc.
…ions page (openshift-assisted#3402)" This reverts commit a5428af.
…hift-assisted#3401)" This reverts commit 52c39a9.
…ssisted#3383)" This reverts commit f61e1b1.
…message (openshift-assisted#3376)" This reverts commit 142fdc5.
…assisted#3337)" This reverts commit 2283bcd.
…hnology Preview spinner (openshift-assisted#3341)" This reverts commit d47ab5f.
This reverts commit 41a3210.
Import isThirdPartyCNI from common network types (not ocm utils). Review step: drop stale disconnectedInfraEnv reference; arch matches static ISO URL.
openshift-ci
bot
added
the
size/XXL
📝 WalkthroughWalkthroughRemoves disconnected-cluster installation support across the UI library: deletes disconnected image/type entries, removes disconnected wizard steps and context/state, strips related API/service methods and translations, and simplifies multiple validation and form flows. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
libs/ui-lib/lib/common/components/ui/WizardFooter.tsx (1)
68-73:⚠️ Potential issue | 🟠 MajorKeep the shared submit guard on
Next.Line 72 drops
isSubmittingfrom the disabled state, but callers likelibs/ui-lib/lib/ocm/components/clusterWizard/Storage.tsxandlibs/ui-lib/lib/ocm/components/clusterWizard/Operators.tsxstill rely on the footer button itself to block re-entry and do not guardonNext. Since they also don't passisNextButtonLoading, this can makeNextclickable again during an in-flight submit.🔧 Suggested fix
- isDisabled={isNextDisabled} + isDisabled={isNextDisabled || isSubmitting}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/common/components/ui/WizardFooter.tsx` around lines 68 - 73, WizardFooter's Next button dropped isSubmitting from its disabled state which allows callers that don't guard onNext or set isNextButtonLoading (e.g., Storage.tsx, Operators.tsx) to re-enter during an in-flight submit; update the Button props inside WizardFooter (the Button with name="next" and onClick={onNext}) to include isSubmitting in its disabled logic (e.g., combine isNextDisabled || isSubmitting) and also reflect isSubmitting in the loading state (combine isNextButtonLoading || isSubmitting) so the footer itself continues to block re-entry.libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmDiscoveryImageConfigForm.tsx (1)
97-109: 🛠️ Refactor suggestion | 🟠 MajorChange the type assertion to
DiscoveryImageTypeto match the actual possible values.
imageTypeValuecan be'discovery-image-ipxe', which is not included in theImageTypeunion ('full-iso' | 'minimal-iso'). The current assertion on Line 109 masks this type incompatibility.Fix
- imageType: imageTypeValue as ImageType, + imageType: imageTypeValue as DiscoveryImageType,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmDiscoveryImageConfigForm.tsx` around lines 97 - 109, The code asserts imageTypeValue as ImageType but imageTypeValue can be 'discovery-image-ipxe' which belongs to DiscoveryImageType; update the type assertion on imageType in the initialValues object to use DiscoveryImageType instead of ImageType (or update OcmDiscoveryImageFormValues to accept DiscoveryImageType), making the change near the imageTypeValue and initialValues declarations (symbols: imageTypeValue, initialValues, OcmDiscoveryImageFormValues, ImageType, DiscoveryImageType).
🧹 Nitpick comments (2)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx (1)
163-165: Throw an Error object instead of a string literal.Throwing a string makes stack traces harder to debug and is inconsistent with standard error handling patterns.
Suggested fix
if (!staticIpInfo) { - throw `Wizard step is currently ${currentStepId}, but no static ip info is defined`; + throw new Error(`Wizard step is currently ${currentStepId}, but no static ip info is defined`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx` around lines 163 - 165, Replace the string literal throw in the ClusterWizardContextProvider (the block that checks staticIpInfo and references currentStepId) with throwing an Error object; e.g., construct a new Error containing the same message ("Wizard step is currently ${currentStepId}, but no static ip info is defined") so the thrown value is an Error instance (use the existing staticIpInfo check and currentStepId variables).libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx (1)
24-29: Empty Formik wrapper appears vestigial.The Formik wrapper now has empty
initialValuesand a no-oponSubmit. If no form validation or state management is needed in this simplified flow, consider removing the Formik wrapper entirely and renderingClusterWizardStepdirectly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx` around lines 24 - 29, The Formik wrapper in BasicStep.tsx (the Formik component around ClusterWizardStep) is vestigial—remove the Formik element and its empty initialValues/onSubmit and render ClusterWizardStep directly in its place (ensure any props passed through Formik are forwarded to ClusterWizardStep if needed), then delete the unused Formik import to avoid dead code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/types/assisted-installer-service.d.ts`:
- Line 1809: The declared union type ImageType in
assisted-installer-service.d.ts currently omits 'disconnected-iso' while the
file's doc comments still reference it; regenerate the auto-generated types so
code and docs match by running the generator for the assisted-installer API
(yarn workspace `@openshift-assisted/types` run update:assisted-installer-service)
and commit the updated libs/types/assisted-installer-service.d.ts; if the
backend intentionally removed 'disconnected-iso', ensure the doc comments in the
regenerated file no longer mention it, otherwise confirm the API spec includes
'disconnected-iso' so the regenerated ImageType union contains
'disconnected-iso'.
In `@libs/ui-lib/lib/common/components/clusterWizard/clusterDetailsValidation.ts`:
- Around line 110-113: The previous fullClusterAddressTest (which validated the
concatenated name.baseDnsDomain as the full DNS name including total length and
overall DNS rules) was removed and only baseDnsDomain is being validated via
baseDomainValidationSchema/dnsNameValidationSchema, so reinstate a validation
that checks the combined full cluster FQDN (name + '.' + baseDnsDomain) —
reintroduce fullClusterAddressTest or an equivalent check in the same validation
schema that builds the fullName from name and baseDnsDomain and enforces overall
DNS rules (including the 253-character total limit and label rules); update the
schema around baseDnsDomain/name references and use fullClusterAddressTest to
return a validation error when name + baseDnsDomain produce an invalid full DNS
name.
In `@libs/ui-lib/lib/common/components/ui/OpenShiftVersionModal.tsx`:
- Around line 53-56: The onClick handler currently calls
setFieldValue('openshiftVersion', values.customOpenshiftSelect) which can assign
null when values.customOpenshiftSelect is null; restore a null-guard or prevent
the click when nothing is chosen: either check values.customOpenshiftSelect !==
null before calling setFieldValue (keep existing onClose behavior) or disable
the "Select" button unless values.customOpenshiftSelect has a value; refer to
the onClick handler, setFieldValue('openshiftVersion', ...),
values.customOpenshiftSelect and onClose to locate the change.
In `@libs/ui-lib/lib/common/validationSchemas/addressValidation.tsx`:
- Around line 31-34: The Yup.string().matches(MAC_REGEX, { message: (value) =>
t('ai:Value "{{value}}" is not valid MAC address.', { value }), ... }) callback
is using the wrong parameter; matches passes a TestContext, not the raw value,
so the error shows [object Object]. Update the message callback for matches to
accept the context and extract the actual value (e.g., message: ({ value }) =>
t('ai:Value "{{value}}" is not valid MAC address.', { value })) while keeping
MAC_REGEX and excludeEmptyString unchanged so the real MAC string is
interpolated into the translation.
In `@libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx`:
- Around line 57-59: The label prop on StaticTextField for the cpuArchitecture
field is hardcoded ("CPU architecture"); update it to use the translation helper
by replacing the label value with t('ai:CPU architecture') (or the agreed
translation key) inside BasicStep.tsx so the StaticTextField
name="cpuArchitecture" receives a translated label; ensure you
import/availability of t where BasicStep.tsx defines or uses StaticTextField.
- Line 37: The heading literal "Basic information" in BasicStep.tsx is missing
translation; replace the hardcoded string in the JSX Content component (Content
component="h2") with the i18n call using the required syntax, e.g. t('ai:Basic
information') (or an agreed key like t('ai:clusterWizard.basicInformation')) and
ensure the file imports and uses the translation function referenced in this
repository (the same t used elsewhere in libs/ui-lib).
In `@libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx`:
- Around line 33-34: The downloadUrl constant in ReviewStep.tsx points at a
moving "/latest/" mirror but the component still displays fixed metadata
(release, arch, size); update ReviewStep so the downloadUrl and the displayed
metadata are sourced from the same generated state (or both pinned to a specific
release). Concretely: replace the hardcoded 'downloadUrl' and any hardcoded
metadata shown in the component with a single source-of-truth (e.g., an
artifactInfo object or props returned by the image generation call) and use that
object for the URL, releaseVersion, architecture, and size; alternatively, if
you intend to pin to a release, change the URL to a release-specific path and
update the displayed values to the matching pinned values so both URL and
metadata always match. Ensure you update references to downloadUrl and the
metadata rendering inside the ReviewStep component to use that unified source.
In `@libs/ui-lib/lib/ocm/hooks/useInfraEnvId.ts`:
- Around line 57-63: The effect in useInfraEnvId incorrectly sets an error when
clusterId is missing even though the single-cluster path can look up the
infraEnvId; change the logic so you only call setError('Missing clusterId...')
when clusterId is missing AND isSingleClusterFeatureEnabled is false, otherwise
allow void findInfraEnvId() to run when infraEnvId is undefined; update the
React.useEffect in useInfraEnvId to reference isSingleClusterFeatureEnabled,
preserve the findInfraEnvId call, and only setError when both clusterId is falsy
and single-cluster fallback is not enabled.
In `@libs/ui-lib/lib/ocm/services/InfraEnvsService.ts`:
- Around line 21-26: The current code calls InfraEnvCache.updateInfraEnvs and
InfraEnvCache.getInfraEnvId even when clusterId is falsy in the single-cluster
flow, but the cache intentionally skips falsy clusterIds and getInfraEnvId
returns Error, causing callers to fall through and later call create() without a
clusterId. Change InfraEnvsService so that after calling InfraEnvsAPI.list(...)
you do not touch InfraEnvCache when isSingleClusterFeatureEnabled (or clusterId
is falsy); instead, if infraEnvs.length > 0 derive infraEnvId directly from the
returned infraEnvs by matching cpuArchitecture (without using
InfraEnvCache.updateInfraEnvs or InfraEnvCache.getInfraEnvId), and only call
create() or any code that requires clusterId when clusterId is present. Ensure
references: InfraEnvsAPI.list, InfraEnvCache.updateInfraEnvs,
InfraEnvCache.getInfraEnvId, and create() are updated accordingly.
---
Outside diff comments:
In `@libs/ui-lib/lib/common/components/ui/WizardFooter.tsx`:
- Around line 68-73: WizardFooter's Next button dropped isSubmitting from its
disabled state which allows callers that don't guard onNext or set
isNextButtonLoading (e.g., Storage.tsx, Operators.tsx) to re-enter during an
in-flight submit; update the Button props inside WizardFooter (the Button with
name="next" and onClick={onNext}) to include isSubmitting in its disabled logic
(e.g., combine isNextDisabled || isSubmitting) and also reflect isSubmitting in
the loading state (combine isNextButtonLoading || isSubmitting) so the footer
itself continues to block re-entry.
In
`@libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmDiscoveryImageConfigForm.tsx`:
- Around line 97-109: The code asserts imageTypeValue as ImageType but
imageTypeValue can be 'discovery-image-ipxe' which belongs to
DiscoveryImageType; update the type assertion on imageType in the initialValues
object to use DiscoveryImageType instead of ImageType (or update
OcmDiscoveryImageFormValues to accept DiscoveryImageType), making the change
near the imageTypeValue and initialValues declarations (symbols: imageTypeValue,
initialValues, OcmDiscoveryImageFormValues, ImageType, DiscoveryImageType).
---
Nitpick comments:
In
`@libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx`:
- Around line 163-165: Replace the string literal throw in the
ClusterWizardContextProvider (the block that checks staticIpInfo and references
currentStepId) with throwing an Error object; e.g., construct a new Error
containing the same message ("Wizard step is currently ${currentStepId}, but no
static ip info is defined") so the thrown value is an Error instance (use the
existing staticIpInfo check and currentStepId variables).
In `@libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx`:
- Around line 24-29: The Formik wrapper in BasicStep.tsx (the Formik component
around ClusterWizardStep) is vestigial—remove the Formik element and its empty
initialValues/onSubmit and render ClusterWizardStep directly in its place
(ensure any props passed through Formik are forwarded to ClusterWizardStep if
needed), then delete the unused Formik import to avoid dead code.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: dbdf9295-1bf0-412c-b990-10c42c3e5a7e
📒 Files selected for processing (31)
libs/locales/lib/en/translation.jsonlibs/types/assisted-installer-service.d.tslibs/ui-lib/lib/common/api/assisted-service/ClustersAPI.tslibs/ui-lib/lib/common/components/clusterWizard/clusterDetailsValidation.tslibs/ui-lib/lib/common/components/ui/OpenShiftVersionModal.tsxlibs/ui-lib/lib/common/components/ui/WizardFooter.tsxlibs/ui-lib/lib/common/components/ui/formik/PullSecretField.tsxlibs/ui-lib/lib/common/config/constants.tslibs/ui-lib/lib/common/validationSchemas/addressValidation.tsxlibs/ui-lib/lib/common/validationSchemas/clusterDetailsValidation.tsxlibs/ui-lib/lib/ocm/components/Routes.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/DiscoveryImageTypeDropdown.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/OcmDiscoveryImageConfigForm.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/commonValidationSchemas.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/components/FormViewHosts/FormViewHostsFields.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/components/FormViewNetworkWide/FormViewNetworkWide.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/components/FormViewNetworkWide/formViewNetworkWideValidationSchema.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/NewClusterWizard.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/constants.tslibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/DisconnectedStaticIp.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/index.tslibs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.tslibs/ui-lib/lib/ocm/hooks/useInfraEnvId.tslibs/ui-lib/lib/ocm/services/ClustersService.tslibs/ui-lib/lib/ocm/services/InfraEnvsService.ts
💤 Files with no reviewable changes (12)
- libs/ui-lib/lib/common/components/ui/formik/PullSecretField.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/components/FormViewHosts/FormViewHostsFields.tsx
- libs/ui-lib/lib/ocm/components/clusterWizard/index.ts
- libs/ui-lib/lib/common/api/assisted-service/ClustersAPI.ts
- libs/ui-lib/lib/ocm/components/clusterConfiguration/DiscoveryImageTypeDropdown.tsx
- libs/ui-lib/lib/ocm/services/ClustersService.ts
- libs/ui-lib/lib/ocm/components/clusterWizard/constants.ts
- libs/locales/lib/en/translation.json
- libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/DisconnectedStaticIp.tsx
- libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx
- libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx
- libs/ui-lib/lib/common/config/constants.ts
libs/ui-lib/lib/common/components/clusterWizard/clusterDetailsValidation.ts
Show resolved
Hide resolved
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx
Show resolved
Hide resolved
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
ElayAharoni
force-pushed
the
revert-ABI-abov-the-sea-new-flow
branch
from
ba78e9c to
222d9ea
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx (1)
24-29: Consider removing the unused Formik wrapper.The Formik form wrapper has empty
initialValuesand a no-oponSubmit, providing no functionality. If form management is no longer needed for this step, consider removing the Formik wrapper entirely to reduce complexity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx` around lines 24 - 29, The Formik wrapper around the UI in BasicStep is unused (empty initialValues and noop onSubmit); remove the <Formik> wrapper and its closing tag in the BasicStep component so the step renders its children directly, and delete the Formik import (and any unused Formik props like initialValues/onSubmit) to avoid dead code and unused imports; ensure the JSX that was inside Formik is returned by the component unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx`:
- Around line 24-29: The Formik wrapper around the UI in BasicStep is unused
(empty initialValues and noop onSubmit); remove the <Formik> wrapper and its
closing tag in the BasicStep component so the step renders its children
directly, and delete the Formik import (and any unused Formik props like
initialValues/onSubmit) to avoid dead code and unused imports; ensure the JSX
that was inside Formik is returned by the component unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ef012caa-f560-4acd-879b-43fc19fda3c6
📒 Files selected for processing (2)
libs/locales/lib/en/translation.jsonlibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx
💤 Files with no reviewable changes (1)
- libs/locales/lib/en/translation.json
rawagner
approved these changes
Mar 30, 2026
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
openshift-merge-bot
bot
merged commit 4c6d658
into
openshift-assisted:master
15 checks passed
ElayAharoni
added a commit
to ElayAharoni/assisted-installer-ui
that referenced
this pull request
Mar 30, 2026
This reverts commit 4c6d658.
ElayAharoni
added a commit
to ElayAharoni/assisted-installer-ui
that referenced
this pull request
Mar 31, 2026
This reverts commit 4c6d658.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary by CodeRabbit
Removed Features
Refactor