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-1769] Add Project field to migration plan/provider wizards #1409

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

jpuzz0
Copy link
Contributor

@jpuzz0 jpuzz0 commented Dec 13, 2024

https://issues.redhat.com/browse/MTV-1769

Summary

Updated the Provider & Plan create wizards to include a "Project" name typeahead select, where in the create plan wizard the options for this typeahead select are limited by the namespaces of the possible providers to choose from, and the create provider wizard simply gets all namespaces.

On submission of both wizards, the global (active) namespace is toggled to the selected "Project" namespace from the form (which could differ from the global/active namespace that was originally chosen upon entry of either wizard).

Further details of the logic implemented for the few fields re-arranged/in-play here are described in the acceptance criteria of this design story:
https://issues.redhat.com/browse/HPUX-259

Added a new TypeaheadSelect component that uses PFv5 components to the common internal library.

Create plan wizard

image

Create provider wizard

image

@jpuzz0 jpuzz0 force-pushed the MTV-1769-project-field branch from 87f0ce2 to bb71fbb Compare December 13, 2024 00:06
@codecov-commenter
Copy link

codecov-commenter commented Dec 13, 2024

⚠️ 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.21%. Comparing base (13484d0) to head (640ac2d).
Report is 156 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    #1409      +/-   ##
==========================================
- Coverage   36.81%   36.21%   -0.60%     
==========================================
  Files         158      159       +1     
  Lines        2548     2579      +31     
  Branches      599      615      +16     
==========================================
- Hits          938      934       -4     
- Misses       1428     1450      +22     
- Partials      182      195      +13     

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

@jpuzz0 jpuzz0 force-pushed the MTV-1769-project-field branch 2 times, most recently from e8703d2 to e7084ff Compare December 16, 2024 18:06
@jpuzz0 jpuzz0 force-pushed the MTV-1769-project-field branch from e7084ff to 640ac2d Compare December 16, 2024 18:28
@yaacov yaacov merged commit 9d1d32c into kubev2v:main Dec 18, 2024
11 checks passed
@sgratch
Copy link
Collaborator

sgratch commented Dec 18, 2024

@jpuzz0 @yaacov Hi,

Few comments:

  1. It doesn't work on my local environment (mtv installed on kind, logged as an admin). The projects array is fetched empty and therefore the Projects TypeaheadSelect field is always disabled (for both plans and providers):
    Screenshot from 2024-12-18 17-24-35

    Once I try to create a provider, I get the following error:
    Screenshot from 2024-12-18 17-29-05

    Note: I posted a fix for this issue Fix the behaviour of the new project field for Kind installation #1417

  2. Why do we display both the standard console's Project ListDropdown field and the new ProjectNameSelect field? Isn't it confusing to display both on the same page? I think that the standard console's Project ListDropdown (on the page's header) can be omitted while creating a plan/provider and re-displayed once we exit the page.

    Screenshot from 2024-12-18 21-41-06

  3. I tried creating a provider in a project B different than the current project A displayed in the page's header, but it was not created there, it was still created under the current project A. What am I missing?

  4. If I create a plan, the new ProjectNameSelect field displays the current project only as the selected option to choose from, although there are providers existed under other projects as well.
    In the example below, I have providers and plans created under another project, but it is not displayed for me as an option for creating the new plan:

    Screenshot from 2024-12-18 21-57-23

  5. for the plan/provider details dialogs, the field name is still displayed as namespace and not Project. If we change the field name within the provider/plan creation page, shouldn't we change the terminology within the details pages as well?

    Screenshot from 2024-12-18 22-04-20

  6. ProjectNameSelect component is used by both providers and plans so please put it in a shared place (e.g. common) instead of putting it under modules/plans

  7. Please see few more comments below.

const defaultNamespace = process?.env?.DEFAULT_NAMESPACE || 'default';
const projectName = activeNamespace === '#ALL_NS#' ? 'openshift-mtv' : activeNamespace;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Default namespace for u/s is : 'konveyor-forklift'

let validationError: ValidationMsg = { type: 'default' };

if (!value) {
validationError = { type: 'error', msg: 'Missing project name' };
Copy link
Collaborator

@sgratch sgratch Dec 19, 2024

Choose a reason for hiding this comment

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

In case of an empty project list, the field is not enforced as mandatory and the error message is not displayed as required:

Screenshot from 2024-12-19 14-09-32

I guess the same is relevant for the plan creation page as well.

@@ -39,24 +41,26 @@ export const ProvidersCreatePage: React.FC<{
const { t } = useForkliftTranslation();
const history = useHistory();
const [isLoading, toggleIsLoading] = useToggle();

const [activeNamespace, setActiveNamespace] = useActiveNamespace();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need both activeNamespace and namespace?

sgratch added a commit to sgratch/forklift-console-plugin that referenced this pull request Dec 19, 2024
Reference:
kubev2v#1409 (comment)
- comment num 1.

- Enable the ProjectNameSelect component to run on Kind (non OCP)
installaiton, the same as supported for the console namespaces menu.

-Fix the namespace default value for upstream.

Signed-off-by: Sharon Gratch <[email protected]>
sgratch added a commit to sgratch/forklift-console-plugin that referenced this pull request Dec 19, 2024
Reference:
kubev2v#1409 (comment)
- comment num 1.

- Enable the ProjectNameSelect component to run on Kind (non OCP)
installaiton, the same as supported for the console namespaces menu.

-Fix the namespace default value for upstream.

Signed-off-by: Sharon Gratch <[email protected]>
sgratch added a commit to sgratch/forklift-console-plugin that referenced this pull request Dec 19, 2024
Reference:
kubev2v#1409 (comment)
- comment num 1.

- Enable the ProjectNameSelect component to run on Kind (non OCP)
installaiton, the same as supported for the console namespaces menu.

-Fix the namespace default value for upstream.

Signed-off-by: Sharon Gratch <[email protected]>
sgratch added a commit to sgratch/forklift-console-plugin that referenced this pull request Dec 19, 2024
Reference:
kubev2v#1409 (comment)
- comment num 1.

- Enable the ProjectNameSelect component to run on Kind (non OCP)
installaiton, the same as supported for the console namespaces menu.

-Fix the namespace default value for upstream.

Signed-off-by: Sharon Gratch <[email protected]>
sgratch added a commit to sgratch/forklift-console-plugin that referenced this pull request Dec 19, 2024
Reference:
kubev2v#1409 (comment)
- comment num 1.

- Enable the ProjectNameSelect component to run on Kind (non OCP)
installaiton, the same as supported for the console namespaces menu.

-Fix the namespace default value for upstream.

Signed-off-by: Sharon Gratch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants