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

Fixes #35559 - Change Content Source LCE dropdown should not show multiple Library entries #10457

Merged

Conversation

lfu
Copy link
Member

@lfu lfu commented Feb 17, 2023

What are the changes introduced in this pull request?

There should be only one 'Library' selection in the dropdown. Which Library it is depends on the organization selector.

Considerations taken when implementing this change?

What are the testing steps for this pull request?

  1. Create at least one extra org
  2. Create at least one content view in Default Organization (this will make the problem easier to see)
  3. Register a host to Default Organization
  4. Go to change that host's content source (host details > kebab > Change Content Source)
  5. Select the Satellite as the content source
  6. Observe the selections for Lifecycle Environment

@theforeman-bot
Copy link

Issues: #35559

@jeremylenz
Copy link
Member

jeremylenz commented Feb 17, 2023

@lfu I thought we were going to move to the new CV/LCE selector? (See the Redmine issue)

@chris1984
Copy link
Member

@jeremylenz do you want me to review this so you can focus on other things?

@jeremylenz
Copy link
Member

@chris1984 feel free to review as you like! But I definitely want to see the results of this, since it will become even more important once the multi-CV work starts up again :)

@lfu
Copy link
Member Author

lfu commented Feb 23, 2023

Updated.

@jeremylenz
Copy link
Member

jeremylenz commented Feb 27, 2023

Change-host-content-source

Can the environment paths be left-aligned? Kinda looks odd to have them centered

Copy link
Member

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

We've already fixed the bug - confirmed that the environment paths (including Library) are only shown for the current organization context.

This is looking fine once the code is cleaned up; see below :)

Copy link
Member

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

Looking much better!!

Copy link
Member

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

Code looks better now, thanks :)

Couple more things:

  1. After the update process is complete, I am able to re-select a different content source and env/CV, even though the Update button remains disabled. Can we just disable everything after the update is complete?
  2. If no content views are available for the selected environment, can we add the banner like we have in the ChangeHostCVModal?

Copy link
Member

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

thanks @sjha4 @lfu!

LGTM 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants