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 #37881 - Update global reg form for multi-env AKs and clean up loose ends #11169

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jeremylenz
Copy link
Member

@jeremylenz jeremylenz commented Oct 4, 2024

What are the changes introduced in this pull request?

image

  1. Update the global registration form to display content view environment labels, including for multi-env AKs, and not just lifecycle environments.
  2. Rename candlepin_name and candlepin_names methods and method arguments to labels throughout the Ruby codebase.
  3. Update the description of the allow_multiple_content_views Setting

Considerations taken when implementing this change?

This resolves the outstanding issues mentioned in #11161 (comment)

It also cleans up the loose end with the setting description, and eliminates a TODO comment.

What are the testing steps for this pull request?

This should affect these places in the UI - make sure everything works ok:

  1. Global Registration form - Activation Keys dropdown
  2. Host details Repository Sets tab - banner text when your host is multi-environment

also, the method name changes affect multi-env hosts and AKs, so test some updates and make sure everything still works.

@@ -7,7 +7,7 @@ def plugin_data
aks = ActivationKey.authorized(:view_activation_keys)
.where(organization_id: registration_params[:organization_id])
.order(:name)
.map { |ak| { name: ak.name, lce: ak.environment&.name } }
.map { |ak| { name: ak.name, cves: ak.content_view_environments.map(&:label).join(', ') } }
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the space after comma necessary ? does it work ?

Copy link
Member Author

Choose a reason for hiding this comment

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

If you were copy/pasting to hammer with the spaces you'd need quotes. But this is just for displaying in the web UI, so I thought the spaces just looked nicer.

@@ -26,8 +26,8 @@ child :content_view_environments => :content_view_environments do
lifecycle_environment_library: cve.lifecycle_environment&.library?
}
end
node :candlepin_name do |cve|
Copy link
Contributor

Choose a reason for hiding this comment

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

I 'd be nervous to do this for an api response. Can we may be have both ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The code that introduced this API response node was just merged today IIRC, so I don't think anyone relies on it yet. Thoughts?

@jeremylenz
Copy link
Member Author

Also updated the setting description since multiCV works with both hosts and AKs now.

@jeremylenz jeremylenz changed the title Fixes #37881 - Update global reg form for multi-env AKs Fixes #37881 - Update global reg form for multi-env AKs and clean up loose ends Oct 8, 2024
@jeremylenz
Copy link
Member Author

rebased

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.

2 participants