Skip to content

fix(orchestrator-form): Review step crash when form data is empty#2834

Merged
karthikjeeyar merged 1 commit intoredhat-developer:mainfrom
lokanandaprabhu:fix/orchestrator-review-empty-formdata
Apr 20, 2026
Merged

fix(orchestrator-form): Review step crash when form data is empty#2834
karthikjeeyar merged 1 commit intoredhat-developer:mainfrom
lokanandaprabhu:fix/orchestrator-review-empty-formdata

Conversation

@lokanandaprabhu
Copy link
Copy Markdown
Member

Hey, I just made a Pull Request!

PR description

Problem

On workflow execute, moving to the Review step could throw when formData was effectively empty—for example input schemas with only display-only ActiveText (no onChange, so nothing is submitted) or when every visible value is filtered out for review (e.g. ui:hidden). generateReviewTableData could return undefined, and NestedReviewTable called Object.entries on that value.

Solution

generateReviewTableData: always return a root payload object; use {} when the processed root is missing.
NestedReviewTable: treat missing data as {} so the table never receives undefined.


-----BEFORE-----

Screenshot 2026-04-20 at 5 05 50 PM

----AFTER-----

Screenshot 2026-04-20 at 3 51 49 PM

Use below schema for testing

{
  "$id": "classpath:/schemas/test-activetext-only-review.input-schema.json",
  "title": "IT Cluster Platform Access Request Input Schema",
  "$schema": "http://json-schema.org/draft-07/schema#",
  "type": "object",
  "properties": {
    "infoMessage": {
      "type": "string",
      "ui:widget": "ActiveText",
      "ui:props": {
        "ui:text": "User **$${{identityApi.displayName}}** will be added to the IT Cluster Platform users rover group and thus granted permissions to use the IT Cluster Platform plugin.\n\n**Note:** Group synchronization within IT Developer Hub can take up to an hour. If you experience permission issues after the workflow completes, please allow time for synchronization to complete."
      }
    }
  }
}

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

Empty pruned form data (e.g. display-only ActiveText) made generateReviewTableData return undefined and NestedReviewTable throw. Return {} from generateReviewTableData and guard NestedReviewTable. Add regression test and changeset.

Made-with: Cursor
@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge Bot commented Apr 20, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Advisory comments

1. Prop type/usage mismatch 🐞 Bug ⚙ Maintainability
Description
NestedReviewTableProps declares data as required (JsonObject), but the component now
defensively treats it as nullable (data ?? {}), creating an inconsistent contract that can confuse
future callers and may trip strict lint rules.
Code

workspaces/orchestrator/plugins/orchestrator-form-react/src/components/NestedReviewTable.tsx[R126-131]

+  const safeData = data ?? {};

  // Group top-level sections
-  const sections: [string, JsonValue][] = Object.entries(data).filter(
+  const sections: [string, JsonValue][] = Object.entries(safeData).filter(
    ([_, value]) => value !== undefined,
  ) as [string, JsonValue][];
Relevance

⭐⭐⭐ High

Code now null-guards required data; team typically aligns types with runtime behavior to avoid
confusion.

PR-#2759
PR-#2597
PR-#2499

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The prop interface requires data: JsonObject, yet the implementation assumes it might be nullish
and introduces safeData. If data truly can be undefined at runtime, the type should reflect
that; if it cannot, the defensive fallback is redundant and should be removed.

workspaces/orchestrator/plugins/orchestrator-form-react/src/components/NestedReviewTable.tsx[65-68]
workspaces/orchestrator/plugins/orchestrator-form-react/src/components/NestedReviewTable.tsx[122-131]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`NestedReviewTableProps` says `data` is always provided (`JsonObject`), but the component treats it as optional (`data ?? {}`). This is an inconsistent API contract.

### Issue Context
The PR adds the defensive fallback to prevent crashes when upstream returns/forwards `undefined`. Now that `generateReviewTableData` returns `{}` instead of `undefined`, the defensive fallback may be redundant; alternatively, if other callers might still pass `undefined`, the prop type should allow it.

### Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator-form-react/src/components/NestedReviewTable.tsx[65-68]
- workspaces/orchestrator/plugins/orchestrator-form-react/src/components/NestedReviewTable.tsx[122-131]

### Suggested fix
Choose one:
1) Make the contract explicit: change prop to `data?: JsonObject` (or `data: JsonObject | undefined`) and keep `safeData`.
2) If `data` is guaranteed non-null now, remove `safeData` and use `Object.entries(data)` directly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@rhdh-gh-app
Copy link
Copy Markdown

rhdh-gh-app Bot commented Apr 20, 2026

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-orchestrator-form-react workspaces/orchestrator/plugins/orchestrator-form-react patch v2.8.1

@sonarqubecloud
Copy link
Copy Markdown

@rhdh-qodo-merge
Copy link
Copy Markdown

Review Summary by Qodo

Fix Review step crash with empty form data

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Prevent Review step crash when form data is empty
• Return empty object instead of undefined from generateReviewTableData
• Guard NestedReviewTable against undefined data with fallback
• Add regression test for display-only ActiveText scenarios
Diagram
flowchart LR
  A["Empty Form Data<br/>e.g. display-only ActiveText"] -->|"processSchema returns undefined"| B["generateReviewTableData"]
  B -->|"Before: undefined"| C["NestedReviewTable crashes"]
  B -->|"After: empty object"| D["NestedReviewTable handles safely"]
  D -->|"safeData = data ?? {}"| E["Review step renders"]
Loading

Grey Divider

File Changes

1. workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/generateReviewTableData.ts 🐞 Bug fix +2/-1

Return empty object instead of undefined

• Modified return statement to handle undefined root object
• Returns empty object {} when result[''] is undefined
• Prevents undefined from propagating to NestedReviewTable

workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/generateReviewTableData.ts


2. workspaces/orchestrator/plugins/orchestrator-form-react/src/components/NestedReviewTable.tsx 🐞 Bug fix +2/-1

Guard component against undefined data

• Added safeData variable with nullish coalescing fallback
• Guards against undefined data by defaulting to empty object
• Uses safeData in Object.entries() call for safety

workspaces/orchestrator/plugins/orchestrator-form-react/src/components/NestedReviewTable.tsx


3. workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/generateReviewTableData.test.ts 🧪 Tests +17/-0

Add regression test for empty form data

• Added regression test for empty form data scenario
• Tests display-only ActiveText widget with no submitted values
• Verifies generateReviewTableData returns empty object

workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/generateReviewTableData.test.ts


View more (1)
4. workspaces/orchestrator/.changeset/fix-review-empty-formdata.md 📝 Documentation +5/-0

Document patch release for bug fix

• Created changeset documenting the bug fix
• Marks package as patch version bump
• Describes issue and solution for empty form data scenarios

workspaces/orchestrator/.changeset/fix-review-empty-formdata.md


Grey Divider

Qodo Logo

Copy link
Copy Markdown
Member

@karthikjeeyar karthikjeeyar left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@openshift-ci openshift-ci Bot added the lgtm label Apr 20, 2026
@karthikjeeyar karthikjeeyar merged commit 652fac0 into redhat-developer:main Apr 20, 2026
12 checks passed
lokanandaprabhu added a commit to lokanandaprabhu/rhdh-plugins that referenced this pull request Apr 22, 2026
…loper#2834)

Empty pruned form data (e.g. display-only ActiveText) made generateReviewTableData return undefined and NestedReviewTable throw. Return {} from generateReviewTableData and guard NestedReviewTable. Add regression test and changeset.

Made-with: Cursor
karthikjeeyar added a commit that referenced this pull request Apr 22, 2026
* fix(orchestrator): preserve nested extraErrors and async validation errors (#2818)

* fix(orchestrator-form): Review step with empty form data (#2834)

Empty pruned form data (e.g. display-only ActiveText) made generateReviewTableData return undefined and NestedReviewTable throw. Return {} from generateReviewTableData and guard NestedReviewTable. Add regression test and changeset.

Made-with: Cursor

* fix(orchestrator-form-react): fix ui styling issues in workflow results page (#2808)

---------

Co-authored-by: Karthik Jeeyar <karthik@redhat.com>
karthikjeeyar added a commit that referenced this pull request Apr 22, 2026
* fix(orchestrator): preserve nested extraErrors and async validation errors (#2818)

* fix(orchestrator-form): Review step with empty form data (#2834)

Empty pruned form data (e.g. display-only ActiveText) made generateReviewTableData return undefined and NestedReviewTable throw. Return {} from generateReviewTableData and guard NestedReviewTable. Add regression test and changeset.

Made-with: Cursor

* fix(orchestrator-form-react): fix ui styling issues in workflow results page (#2808)

---------

Co-authored-by: Karthik Jeeyar <karthik@redhat.com>
karthikjeeyar added a commit that referenced this pull request Apr 22, 2026
* fix(orchestrator): preserve nested extraErrors and async validation errors (#2818)

* fix(orchestrator-form): Review step with empty form data (#2834)

Empty pruned form data (e.g. display-only ActiveText) made generateReviewTableData return undefined and NestedReviewTable throw. Return {} from generateReviewTableData and guard NestedReviewTable. Add regression test and changeset.

Made-with: Cursor

* fix(orchestrator-form-react): fix ui styling issues in workflow results page (#2808)

---------

Co-authored-by: Lokananda Prabhu <102503482+lokanandaprabhu@users.noreply.github.com>
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.

2 participants