Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MTV-1880] Fix 'Project' name select issues #1433

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jpuzz0
Copy link
Contributor

@jpuzz0 jpuzz0 commented Jan 13, 2025

Issue: https://issues.redhat.com/browse/MTV-1880
Design issue: https://issues.redhat.com/browse/HPUX-259

Summary

  1. Fixed validation of create plan wizard for Project select so that it is required before proceeding to the next step
  2. Updated behavior of project select in the create plan wizard so that all projects (namespaces) are shown instead of filtering them, and instead, the provider cards are filtered based on the Project selection made - only providers with namespaces matching the selected Project will be rendered. Added empty state view for when a Project is selected without any associated providers.

NOTE: All comments made here were addressed with the exception of #5 to rename the details "Namespace" label (was suggested to update to be named "Project). This is still being discussed as to what we might change the tooltip text to, since there are many references to the term "Namespace".

Demo
https://github.com/user-attachments/assets/7c95fbeb-1018-45aa-8984-0b79f3a12bce

Empty provider state
image

@jpuzz0 jpuzz0 added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 13, 2025
@jpuzz0 jpuzz0 force-pushed the MTV-1880-project-field-updates branch from c663a63 to d93736c Compare January 13, 2025 21:09
@codecov-commenter
Copy link

codecov-commenter commented Jan 13, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 36.20%. Comparing base (13484d0) to head (1dd376a).
Report is 184 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1433      +/-   ##
==========================================
- Coverage   36.81%   36.20%   -0.62%     
==========================================
  Files         158      159       +1     
  Lines        2548     2580      +32     
  Branches      599      618      +19     
==========================================
- Hits          938      934       -4     
- Misses       1428     1464      +36     
  Partials      182      182              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jpuzz0 jpuzz0 force-pushed the MTV-1880-project-field-updates branch from d93736c to 5b5f5d5 Compare January 13, 2025 21:19
@jpuzz0 jpuzz0 requested a review from sgratch January 13, 2025 21:32
@jpuzz0 jpuzz0 marked this pull request as draft January 13, 2025 21:39
@jpuzz0 jpuzz0 changed the title Draft [MTV-1880] Fix 'Project' select issues, add 'Plan name' back to step 1 of migration plan wizard [MTV-1880] Fix 'Project' select issues, add 'Plan name' back to step 1 of migration plan wizard Jan 13, 2025
@jpuzz0 jpuzz0 force-pushed the MTV-1880-project-field-updates branch from 5b5f5d5 to 1dd376a Compare January 14, 2025 17:12
@metalice
Copy link
Collaborator

Hey @jpuzz0

Do you know if we have a JIRA ticket for the MTV project?
You missed our last conversation about PR's associated to JIRA ticket on MTV - each PR should have a reference in MTV project with ACK's. I would be happy to explain more on Slack.

@sgratch
Copy link
Collaborator

sgratch commented Jan 14, 2025

@jpuzz0 Can you please also breakdown this PR into small PRs? Much easy to review and verify by QE when the PR is small and related to one issue...
At least it will be great if you can divide this into two PRs: one for the 'project field' feature fixes and the other for the 'plan name' feature.

Thanks!

@jpuzz0
Copy link
Contributor Author

jpuzz0 commented Jan 14, 2025

Hey @jpuzz0

Do you know if we have a JIRA ticket for the MTV project? You missed our last conversation about PR's associated to JIRA ticket on MTV - each PR should have a reference in MTV project with ACK's. I would be happy to explain more on Slack.

@metalice
Yes, I had one open, and the ID is in the title/comment of the PR/branch, but forgot to link it. The JIRA link is in the description now.

@jpuzz0
Copy link
Contributor Author

jpuzz0 commented Jan 14, 2025

@jpuzz0 Can you please also breakdown this PR into small PRs? Much easy to review and verify by QE when the PR is small and related to one issue... At least it will be great if you can divide this into two PRs: one for the 'project field' feature fixes and the other for the 'plan name' feature.

Thanks!

@sgratch Sure, I'll break it down into 2 PRs. I'll keep this one just for "Project name" fixes and break out the "Plan name" feature.

@jpuzz0 jpuzz0 changed the title [MTV-1880] Fix 'Project' select issues, add 'Plan name' back to step 1 of migration plan wizard [MTV-1880] Fix 'Project' name select issues Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants