Skip to content

Conversation

@sjd78
Copy link
Member

@sjd78 sjd78 commented Nov 11, 2025

Resolves: #2726
Requires: konveyor/operator#491

Add minItems: 1 for organizations filter prop to the Cloud Foundry discovery filter form.

Summary by CodeRabbit

  • Improvements
    • CloudFoundry filter now requires at least one organization to be specified.
    • Validation strengthened to enforce non-empty organization entries.
    • UI messaging updated to indicate "at least one is required" for organizations and mark that group as required.
    • Spaces and names remain optional (no change to their required status).

Resolves: konveyor#2726
Requires: konveyor/operator#491

Add `minItems: 1` for organizations filter prop to the Cloud
Foundry discovery filter form.

Signed-off-by: Scott J Dickerson <[email protected]>
@coderabbitai
Copy link

coderabbitai bot commented Nov 11, 2025

Walkthrough

CloudFoundry discovery validation and UI were updated so the organizations field requires at least one entry. The form UI now marks organizations required and shows an updated empty message; the JSON schema enforces minItems: 1 for organizations. Spaces and names retain non-required behavior.

Changes

Cohort / File(s) Summary
CloudFoundry Filter UI
client/src/app/components/discover-import-wizard/filter-input-cloudfoundry.tsx
Organizations field marked required; empty message updated to indicate "at least one is required"; StringFieldsGroup usage for organizations now enforces requirement. Spaces and names no longer pass isRequired={false} (defaults to non-required).
CloudFoundry Schema Validation
client/src/app/components/discover-import-wizard/validate-cloudfoundry-schema.tsx
Added minItems: 1 to the organizations array schema (items still require non-empty strings). No array-level minItems added for spaces or names.

Sequence Diagram(s)

sequenceDiagram
  participant UI as Discover Wizard UI
  participant Validator as CloudFoundry Schema Validator
  participant API as Discovery Controller

  UI->>Validator: Submit form (organizations, spaces, names)
  alt organizations array empty
    Validator-->>UI: Validation error: "at least one is required"
    UI-->>User: Show required message on Organizations field
  else validation passes
    Validator-->>API: Validated filter payload
    API-->>UI: Start discovery / success response
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Review focus: ensure UI messaging matches schema constraint and that the minItems: 1 is correctly enforced at runtime.
  • Pay extra attention to: filter-input-cloudfoundry.tsx (UI/props change) and validate-cloudfoundry-schema.tsx (schema constraint).

Possibly related PRs

Suggested reviewers

  • ibolton336
  • rszwajko

Poem

🐰 Hop, hop, the forms align,

One org is needed this time.
Schema and UI in tune,
Discovery hums—no more gloom. ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Title uses the correct 🐛 prefix and clearly describes the change as updating the Cloud Foundry discovery filter form validation.
Description check ✅ Passed Description links the resolved issue and provides context for the change, though it lacks detailed explanation of the UI/validation updates.
Linked Issues check ✅ Passed Changes implement the core requirement from #2726: adding validation to require at least one organization entry, preventing discovery from proceeding without organization specified.
Out of Scope Changes check ✅ Passed All changes directly address #2726 requirements: adding minItems constraint to organizations array and updating UI messaging to indicate the field is required.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9dffc7a and 6d9b977.

📒 Files selected for processing (1)
  • client/src/app/components/discover-import-wizard/validate-cloudfoundry-schema.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: sjd78
Repo: konveyor/tackle2-ui PR: 2718
File: client/src/app/components/discover-import-wizard/validate-cloudfoundry-schema.tsx:16-41
Timestamp: 2025-11-07T22:56:18.638Z
Learning: In the Cloud Foundry discover applications filter schema (client/src/app/components/discover-import-wizard/validate-cloudfoundry-schema.tsx), the organizations field does not require any items and can be empty (no minItems constraint). Users can provide zero or more organization names.
📚 Learning: 2025-11-07T22:56:18.638Z
Learnt from: sjd78
Repo: konveyor/tackle2-ui PR: 2718
File: client/src/app/components/discover-import-wizard/validate-cloudfoundry-schema.tsx:16-41
Timestamp: 2025-11-07T22:56:18.638Z
Learning: In the Cloud Foundry discover applications filter schema (client/src/app/components/discover-import-wizard/validate-cloudfoundry-schema.tsx), the organizations field does not require any items and can be empty (no minItems constraint). Users can provide zero or more organization names.

Applied to files:

  • client/src/app/components/discover-import-wizard/validate-cloudfoundry-schema.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: unit-test
  • GitHub Check: build-and-upload-for-global-ci
🔇 Additional comments (2)
client/src/app/components/discover-import-wizard/validate-cloudfoundry-schema.tsx (2)

16-24: LGTM! Schema change correctly enforces the organization requirement.

The addition of minItems: 1 on line 23 properly requires at least one organization name for Cloud Foundry discovery, directly addressing issue #2726. Combined with the existing minLength: 1 constraint on items, this ensures both a non-empty array and non-empty organization names.

Note: This change supersedes the retrieved learning from November 7, 2025, which stated that organizations could be empty. The new behavior is intentional and necessary to prevent discovery failures.

Based on learnings


13-43: Verify backend validation for the organizations field and consider adding required to the schema for defense-in-depth.

The original review comment correctly identifies that the JSON schema lacks a required array at the object level, meaning an empty object {} would technically pass schema validation. However, the UI layer provides strong protection:

  1. The form component has isRequired prop enabled (line 153)
  2. Yup validation enforces .min(1, "At least one organization is required") (lines 92-94)
  3. Empty state message prevents submission without organizations (line 152)

Remaining gap: Backend validation for organizations presence could not be confirmed from the UI codebase alone (operator PR #491 not referenced here). Please verify that:

  • The backend/operator also enforces organizations presence when processing discovery data
  • Whether adding required: ["organizations"] to the schema would be appropriate for additional safety

Note: The retrieved learning stating "no minItems constraint" is incorrect; the actual schema has minItems: 1.


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

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

@sjd78 sjd78 added this to the v0.8.1 milestone Nov 12, 2025
@sjd78 sjd78 added the cherry-pick/release-0.8 This PR should be cherry-picked to release-0.8 branch label Nov 12, 2025
Signed-off-by: Scott J Dickerson <[email protected]>
@sjd78 sjd78 requested a review from jortel November 12, 2025 15:03
Copy link
Contributor

@jortel jortel left a comment

Choose a reason for hiding this comment

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

lgtm

@sjd78 sjd78 merged commit d2dddeb into konveyor:main Nov 12, 2025
12 checks passed
@sjd78 sjd78 deleted the discovery_orgs_required branch November 12, 2025 15:33
github-actions bot pushed a commit that referenced this pull request Nov 12, 2025
Resolves: #2726
Requires: konveyor/operator#491

Add `minItems: 1` for organizations filter prop to the Cloud Foundry
discovery filter form.

---------

Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Cherry Picker <[email protected]>
sjd78 added a commit that referenced this pull request Nov 12, 2025
Resolves: #2726
Requires: konveyor/operator#491

Add `minItems: 1` for organizations filter prop to the Cloud Foundry
discovery filter form.

---------

Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Cherry Picker <[email protected]>
Co-authored-by: Scott Dickerson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-pick/release-0.8 This PR should be cherry-picked to release-0.8 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] [PA&AG] CF application discovery should ony proceed if organization name is specified on the 'Discover application' wizard

2 participants