Skip to content

ACM-23852: Add repository selected validation and fix delete button in argo create application wizard. - #5015

Merged
openshift-merge-bot[bot] merged 5 commits into
stolostron:mainfrom
nitin-dhevar:ACM-23852-validation
Sep 24, 2025
Merged

ACM-23852: Add repository selected validation and fix delete button in argo create application wizard.#5015
openshift-merge-bot[bot] merged 5 commits into
stolostron:mainfrom
nitin-dhevar:ACM-23852-validation

Conversation

@nitin-dhevar

@nitin-dhevar nitin-dhevar commented Sep 18, 2025

Copy link
Copy Markdown
Contributor

📝 Summary

Ticket Summary (Title):

ACM-23852: Add repository selected validation and fix delete button in argo create application wizard.

Ticket Link:

https://issues.redhat.com/browse/ACM-23852

Type of Change:

  • 🐞 Bug Fix
  • ✨ Feature
  • 🔧 Refactor
  • 💸 Tech Debt
  • 🧪 Test-related
  • 📄 Docs

✅ Checklist

General

  • PR title follows the convention (e.g. ACM-12340 Fix bug with...)
  • Code builds and runs locally without errors
  • No console logs, commented-out code, or unnecessary files
  • All commits are meaningful and well-labeled
  • All new display strings are externalized for localization (English only)
  • (Nice to have) JSDoc comments added for new functions and interfaces

If Feature

  • UI/UX reviewed (if applicable)
  • All acceptance criteria met
  • Unit test coverage added or updated
  • Relevant documentation or comments included

If Bugfix

  • Root cause and fix summary are documented in the ticket (for future reference / errata)
  • Fix tested thoroughly and resolves the issue
  • Test(s) added to prevent regression

🗒️ Notes for Reviewers

Signed-off-by: Nitin Dhevar <nitin.dhanabalan@gmail.com>
Signed-off-by: Kevin Cormier <kcormier@redhat.com>
Signed-off-by: Kevin Cormier <kcormier@redhat.com>

@KevinFCormier KevinFCormier left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey Nitin,

I don't think we should change the default behaviour of WizArrayInput to require there to be at least one entry. This affected the EditPolicy wizard, and then you had to update the test.

I also don't think we should change the Argo wizard to have Git initially selected. The user can get around this easily by editing the YAML.

I've opened a PR. What I propose is:

  • Marking this particular WizArrayInput as required and only removing the Delete button for cases where at least 1 is required
  • Adding a custom validation function to check if the only thing present is the placeholder empty object (from disallowEmpty)

What would still need to be done is to visually indicate when this field is required. For example, in this screenshot, you see that I cannot click Next, but there is no indication of what the validation error is. Perhaps we could indicate that where I've circled in blue?
image

This would be similar to when you've selected Git but not entered the URL. When collapsed, there is a validation error shown.
image
image

Alternatively, perhaps you could look into making WizTiles required.

nitin-dhevar and others added 2 commits September 19, 2025 11:30
Signed-off-by: Nitin Dhevar <nitin.dhanabalan@gmail.com>
<Title headingLevel="h6">{t('Repository type')}</Title>
</WizHidden>
<WizTiles path="repositoryType">
<WizTiles path="repositoryType" required>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh wow! This worked. I assumed you would need to implement this support, but glad to see it's already there.

@KevinFCormier

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm label Sep 24, 2025
@openshift-ci

openshift-ci Bot commented Sep 24, 2025

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: KevinFCormier, nitin-dhevar

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

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [KevinFCormier,nitin-dhevar]

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

@sonarqubecloud

Copy link
Copy Markdown

@openshift-merge-bot
openshift-merge-bot Bot merged commit 2b06211 into stolostron:main Sep 24, 2025
12 checks passed
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