OCPBUGS-72598 | [OVE] Custom manifests are broken in local Assisted UI

[asmasarw]

Summary by CodeRabbit

• Bug Fixes
  • Refined cluster wizard step navigation to prevent forced advancement away from the custom manifests step while it is actively being edited. This improves the user experience when configuring cluster settings.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: asmasarw
Once this PR has been reviewed and has the lgtm label, please assign rawagner for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[coderabbitai]

📝 Walkthrough

Walkthrough

Refines step advancement logic in the cluster wizard context provider to prevent forced navigation away from the custom-manifests step while it is actively being filled. A condition check is added alongside clarifying comments to ensure proper step progression behavior.

Changes

Cohort / File(s)	Summary
Cluster Wizard Context libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx	Refines step advancement logic to avoid auto-advancing away from the custom-manifests step while being filled, by adding a condition that checks the current step is not 'custom-manifests' before applying auto-advance when customManifestsStepNeedsToBeFilled is true.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• MGMT-22204: add new fields to above sea level ABI #3274: Modifies the same ClusterWizardContextProvider.tsx file to add disconnectedInfraEnv state management.

Suggested labels

lgtm, size/XS, jira/valid-reference

Suggested reviewers

• rawagner
• ammont82
• jgyselov

Poem

🐰 A wizard's step, once rushed with haste,
Now pauses gently, leaves no trace—
Custom manifests need their time,
To flourish fully, line by line.
Smart conditions guard the way,
Let forms be filled without delay! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title references a specific bug ticket (OCPBUGS-72598) and accurately describes the main fix: preventing broken custom manifests step navigation in the Assisted UI by refining step advancement logic.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[rawagner]

I dont understand the fix. This useEffect is triggered when user opens the wizard and settings are loaded/or settings are changed. How can this have some effect on copying content into custom manifests field ? the settings should be loaded already, right ?

That leaves out that uiSettings changed and maybe we dont calculate customManifestsStepNeedsToBeFilled properly ?

Can you explain what is the root cause that triggers the original condition which changes the current step?

[asmasarw]

@rawagner - when user paste content it changes the uisettings indirectly - see explanations below.

Why does pasting in Custom Manifests reset the wizard to Cluster Details?

1. How does pasting cause the effect to run?

Pasting does not change "wizard settings" directly. It does trigger a chain that updates
uiSettings:

1. User pastes in the CodeField → setValue(text, true) (CodeField.tsx).
2. Formik values change for the manifest YAML field.
3. CustomManifestsForm uses useFormikAutoSave() (debounced ~1s). When values change, it
   calls submitForm().
4. handleSubmit in CustomManifestsForm.tsx runs. For a new manifest (not yet saved), it:
   • Calls ClustersAPI.createCustomManifest(...)
   • Then:
     if (!uiSettings?.customManifestsAdded) { await updateUISettings({ customManifestsAdded: true }); }
5. So as a direct result of the paste → auto-save flow, we call
   updateUISettings({ customManifestsAdded: true }).
6. That updates state in useUISettings → uiSettings gets a new reference.
7. ClusterWizardContextProvider uses useUISettings() and has a useEffect that depends on
   [uiSettings, UISettingsLoading, UISettingsError].
8. So when uiSettings changes (after the paste → auto-save → updateUISettings chain), the effect
   runs.

So: settings are not “already loaded and never change.” Pasting triggers auto-save, which
explicitly updates uiSettings, and that is what triggers the effect.

---

2. What is the condition that changes the current step?

In ClusterWizardContextProvider.tsx, the effect does:

const customManifestsStepNeedsToBeFilled = !!(
  uiSettings?.addCustomManifests && !uiSettings?.customManifestsAdded
);

const requiredStepId = getClusterWizardFirstStep(
  locationState,
  staticIpInfo,
  cluster?.status,
  cluster?.hosts,
  customManifestsStepNeedsToBeFilled,
);

if (
  !currentStepId ||
  (customManifestsStepNeedsToBeFilled && isStepAfter(currentStepId, requiredStepId))
) {
  setCurrentStepId(requiredStepId);
}

So the step is changed when:

• !currentStepId (initial step), or
• customManifestsStepNeedsToBeFilled && isStepAfter(currentStepId, requiredStepId) (user is
  “ahead” of the required step and we force them back to requiredStepId).

---

[rawagner]

The AI nicely described the few lines of code we have here.. but didnt really answer anything :)

Lets take a closer look at the original condition

if (
  !currentStepId ||
  (customManifestsStepNeedsToBeFilled && isStepAfter(currentStepId, requiredStepId))
) {
  setCurrentStepId(requiredStepId);
}

the step changes if:
a) currentStepId is undefined -> in our case I believe it is it is
b) customManifestsStepNeedsToBeFilled is truthy and isStepAfter is truthy

customManifestsStepNeedsToBeFilled is

const customManifestsStepNeedsToBeFilled = !!(
  uiSettings?.addCustomManifests && !uiSettings?.customManifestsAdded
);

it seems the bug is here - how can customManifestsStepNeedsToBeFilled be true, if we just added the custom manifest ?