Wizard: always enable Create/Save buttons on Review step (HMS-10658)

[mgold1234]

Keep Create and Save buttons enabled on the Review step instead of disabling them when validation errors exist.
On click, navigate back to the step with errors instead of blocking.
Refactor useIsBlueprintValid into useBlueprintValidation to expose firstErrorStepId.
Fix edit mode save being blocked while the async name uniqueness check is still in flight.
Follows up on the "always enable Next/Review buttons" commit — same pattern applied to the Review step.

Note: Depends on the activation key loading fix in #4622.

JIRA: HMS-10658

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• The ValidationContext sets forceShowErrors to true and never resets it, which may cause errors to keep showing on subsequent wizard runs; consider exposing a reset/clear method or tying it to wizard lifecycle.
• scrollToFirstError relies on a global .pf-m-error query, which could match unintended elements or break if PatternFly class names change; scoping the selector to the wizard container or a more specific error marker would make this more robust.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `ValidationContext` sets `forceShowErrors` to `true` and never resets it, which may cause errors to keep showing on subsequent wizard runs; consider exposing a reset/clear method or tying it to wizard lifecycle.
- `scrollToFirstError` relies on a global `.pf-m-error` query, which could match unintended elements or break if PatternFly class names change; scoping the selector to the wizard container or a more specific error marker would make this more robust.

## Individual Comments

### Comment 1
<location path="src/Components/CreateImageWizard/utilities/ValidationContext.tsx" line_range="20-23" />
<code_context>
+}: {
+  children: React.ReactNode;
+}) => {
+  const [forceShowErrors, setForceShow] = useState(false);
+
+  const setForceShowErrors = useCallback(() => {
+    setForceShow(true);
+  }, []);
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider a way to reset `forceShowErrors` so the flag doesn't stay permanently true for the provider lifetime.

With the current implementation, once `setForceShowErrors` is called, `forceShowErrors` stays `true` for the entire lifetime of the provider. Any consumers relying on this flag will keep showing validation errors even after issues are fixed or the user leaves the flow, as long as the same provider instance is reused. If this is intended as a transient "attempted submit" indicator, consider adding a reset (e.g., on successful save, wizard exit, or when errors are resolved) or deriving it from existing state instead of persisting it indefinitely.

Suggested implementation:

```typescript
type ValidationContextType = {
  forceShowErrors: boolean;
  setForceShowErrors: () => void;
  resetForceShowErrors: () => void;
};

```

```typescript
const ValidationContext = createContext<ValidationContextType>({
  forceShowErrors: false,
  setForceShowErrors: () => {},
  resetForceShowErrors: () => {},
});

```

```typescript
  const [forceShowErrors, setForceShow] = useState(false);

  const setForceShowErrors = useCallback(() => {
    setForceShow(true);
  }, []);

  const resetForceShowErrors = useCallback(() => {
    setForceShow(false);
  }, []);

```

1. In the `ValidationProvider` JSX where `ValidationContext.Provider` is returned, update the `value` prop to include `resetForceShowErrors`, for example:
   `value={{ forceShowErrors, setForceShowErrors, resetForceShowErrors }}`.
2. Any consumers of `ValidationContextType` should be updated (if needed) to handle the new `resetForceShowErrors` function, e.g., calling it on successful save, wizard exit, or when validation errors are resolved.
</issue_to_address>

### Comment 2
<location path="src/Components/CreateImageWizard/utilities/scrollToFirstError.ts" line_range="2" />
<code_context>
+export const scrollToFirstError = () => {
+  const errorElement = document.querySelector('.pf-m-error');
+  if (errorElement) {
+    errorElement.scrollIntoView({ behavior: 'smooth', block: 'center' });
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using a generic `.pf-m-error` selector may scroll to an unintended element or a prior step’s error.

Since `.pf-m-error` can occur outside the current step (e.g., global/header errors or stale errors from previous steps), `document.querySelector('.pf-m-error')` may scroll to the wrong element. To avoid confusing scroll behavior, scope the query to the current wizard step container or a more specific form-field selector so you reliably target the first relevant error in this step.

Suggested implementation:

```typescript
export const scrollToFirstError = (container?: HTMLElement) => {
  // Scope the search to the current step/container if provided, otherwise fall back to the whole document.
  const root: ParentNode = container ?? document;

  // Restrict to typical PatternFly form error elements within the current step
  const errorElement = root.querySelector(
    '.pf-c-form__group.pf-m-error, .pf-c-form__helper-text.pf-m-error'
  );

  if (errorElement instanceof HTMLElement) {
    errorElement.scrollIntoView({ behavior: 'smooth', block: 'center' });
  }
};

```

1. Update all call sites of `scrollToFirstError()` to pass the current wizard step container (e.g., the DOM element for the active step). For example: `scrollToFirstError(activeStepElement)`.
2. If there is a specific wrapper element for the step’s form fields (e.g., a class like `.create-image-wizard__step`), obtain that element via `document.querySelector` or a ref and pass it as the `container` parameter.
3. If your error markup uses different selectors than `.pf-c-form__group.pf-m-error` or `.pf-c-form__helper-text.pf-m-error`, adjust the selector string in `querySelector` to match the concrete error elements used in this wizard.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[codecov]

Codecov Report

❌ Patch coverage is 67.12329% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.66%. Comparing base (4ec9568) to head (66783a2).

Files with missing lines	Patch %	Lines
...reateImageWizard/components/ReviewWizardFooter.tsx	28.57%	14 Missing and 1 partial ⚠️
...ImageWizard/steps/Review/Footer/CreateDropdown.tsx	25.00%	2 Missing and 1 partial ⚠️
...teImageWizard/steps/Review/Footer/EditDropdown.tsx	25.00%	2 Missing and 1 partial ⚠️
.../CreateImageWizard/utilities/scrollToFirstError.ts	0.00%	2 Missing ⚠️
...ents/CreateImageWizard/utilities/useValidation.tsx	96.96%	1 Missing ⚠️

@@            Coverage Diff             @@
##             main    #4627      +/-   ##
==========================================
+ Coverage   72.90%   77.66%   +4.75%     
==========================================
  Files         261      265       +4     
  Lines        7043     7114      +71     
  Branches     2596     2580      -16     
==========================================
+ Hits         5135     5525     +390     
+ Misses       1880     1492     -388     
- Partials       28       97      +69     

Flag	Coverage Δ
playwright	59.77% <48.97%> (?)
vitest	72.72% <63.01%> (-0.19%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...Components/CreateImageWizard/CreateImageWizard.tsx	84.33% <100.00%> (+5.79%)	⬆️
...reateImageWizard/components/CustomWizardFooter.tsx	55.55% <100.00%> (+13.88%)	⬆️
...eWizard/steps/Review/Footer/shouldDisableAction.ts	100.00% <100.00%> (ø)
...teps/Review/components/shared/ReviewCardHeader.tsx	55.55% <ø> (ø)
...ents/CreateImageWizard/utilities/useValidation.tsx	88.81% <96.96%> (+2.68%)	⬆️
.../CreateImageWizard/utilities/scrollToFirstError.ts	10.00% <0.00%> (-1.12%)	⬇️
...ImageWizard/steps/Review/Footer/CreateDropdown.tsx	49.18% <25.00%> (+43.91%)	⬆️
...teImageWizard/steps/Review/Footer/EditDropdown.tsx	40.90% <25.00%> (+10.90%)	⬆️
...reateImageWizard/components/ReviewWizardFooter.tsx	57.44% <28.57%> (-6.84%)	⬇️

... and 67 files with indirect coverage changes

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ec9568...66783a2. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sourcery-ai]

Hey - I've left some high level feedback:

• The ValidationContext default value uses no-op setters, which can hide cases where components are mounted outside the provider; consider throwing or logging in the default context to surface misconfiguration earlier.
• The WizardStepId union is hard-coded in useBlueprintValidation, which risks diverging from the actual wizard configuration; consider sourcing these IDs from a shared type or constants used by the wizard steps.
• The isDisabled={!onBeforeAction && isDisabled} pattern in the dropdown buttons is a bit non-obvious; consider extracting this into a helper or renaming onBeforeAction to clarify that it gatekeeps actions while the visual disabled state remains driven by validation.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The ValidationContext default value uses no-op setters, which can hide cases where components are mounted outside the provider; consider throwing or logging in the default context to surface misconfiguration earlier.
- The `WizardStepId` union is hard-coded in `useBlueprintValidation`, which risks diverging from the actual wizard configuration; consider sourcing these IDs from a shared type or constants used by the wizard steps.
- The `isDisabled={!onBeforeAction && isDisabled}` pattern in the dropdown buttons is a bit non-obvious; consider extracting this into a helper or renaming `onBeforeAction` to clarify that it gatekeeps actions while the visual disabled state remains driven by validation.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[regexowl]

Can you please rebase and fix the prettier issues with npm run format so the diff gets cleaned up from the unrelated prettier connected changes?