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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ def find_content_view_environments
ids: params[:content_view_environment_ids],
organization: @organization || @activation_key&.organization)
if @content_view_environments.blank?
handle_errors(candlepin_names: params[:content_view_environments],
handle_errors(labels: params[:content_view_environments],
ids: params[:content_view_environment_ids])
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def set_content_view_environments
if cves.present?
@host.content_facet.content_view_environments = cves
else
handle_errors(candlepin_names: cve_params[:content_view_environments],
handle_errors(labels: cve_params[:content_view_environments],
ids: cve_params[:content_view_environment_ids])
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ module Api::V2::MultiCVParamsHandling
extend ActiveSupport::Concern
include ::Katello::Api::V2::ErrorHandling

def handle_errors(candlepin_names: [], ids: [])
if candlepin_names.present?
fail HttpErrors::UnprocessableEntity, "No content view environments found with names: #{candlepin_names.join(',')}"
def handle_errors(labels: [], ids: [])
if labels.present?
fail HttpErrors::UnprocessableEntity, "No content view environments found with names: #{labels.join(',')}"
elsif ids.present?
fail HttpErrors::UnprocessableEntity, "No content view environments found with ids: #{ids}"
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.


lces = KTEnvironment.readable
.where(organization_id: registration_params[:organization_id])
Expand Down
10 changes: 3 additions & 7 deletions app/models/katello/content_view_environment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ def self.for_content_facets(content_facets)
joins(:content_view_environment_content_facets, :content_facets).where("#{Katello::ContentViewEnvironmentContentFacet.table_name}.content_facet_id" => content_facets).uniq
end

def self.with_candlepin_name(cp_name, organization: Organization.current)
joins(:environment, :content_view).where("#{Katello::KTEnvironment.table_name}.organization_id" => organization, label: cp_name).first
def self.with_label_and_org(label, organization: Organization.current)
joins(:environment, :content_view).where("#{Katello::KTEnvironment.table_name}.organization_id" => organization, label: label).first
end

# retrieve the owning environment for this content view environment.
Expand All @@ -56,10 +56,6 @@ def default_environment?
content_view.default? && environment.library?
end

def candlepin_name
label
end

def priority(content_object)
if content_object.is_a? Katello::ActivationKey
content_view_environment_activation_keys.find_by(:activation_key_id => content_object.id).try(:priority)
Expand All @@ -86,7 +82,7 @@ def self.fetch_content_view_environments(labels: [], ids: [], organization:)
elsif labels.present?
environment_names = labels.map(&:strip)
environment_names.each do |name|
cve = with_candlepin_name(name, organization: organization)
cve = with_label_and_org(name, organization: organization)
if cve.blank?
label_errors << name
else
Expand Down
5 changes: 0 additions & 5 deletions app/models/katello/host/content_facet.rb
Original file line number Diff line number Diff line change
Expand Up @@ -359,11 +359,6 @@ def update_errata_status
host.refresh_global_status!
end

# TODO: uncomment when we need to display multiple CVE names in the UI
# def content_view_environment_names
# content_view_environments.map(&:candlepin_name).join(', ')
# end

def self.joins_installable_relation(content_model, facet_join_model)
facet_repository = Katello::ContentFacetRepository.table_name
content_table = content_model.table_name
Expand Down
2 changes: 1 addition & 1 deletion app/views/katello/api/v2/activation_keys/base.json.rabl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ child :content_view_environments => :content_view_environments do
}
end
node :label do |cve|
cve.candlepin_name
cve.label
end
end

Expand Down
4 changes: 2 additions & 2 deletions app/views/katello/api/v2/content_facet/base.json.rabl
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ child :content_view_environments => :content_view_environments do
lifecycle_environment_library: cve.lifecycle_environment&.library?
}
end
node :candlepin_name do |cve|
cve.candlepin_name
node :label do |cve|
cve.label
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/katello/plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ def katello_template_setting_values(name)
type: :boolean,
default: false,
full_name: N_('Allow multiple content views'),
description: N_("Allow a host to be assigned to multiple content view environments with 'subscription-manager register --environments' or 'subscription-manager environments --set'.") # TODO: update this description when AKs support this setting as well
description: N_("Allow hosts or activation keys to be associated with multiple content view environments")

setting 'content_default_http_proxy',
type: :string,
Expand Down
2 changes: 1 addition & 1 deletion test/controllers/api/v2/activation_keys_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ def test_create_with_content_view_environments_param
content_view_environments = ['published_dev_view_library']
ActivationKey.any_instance.expects(:reload)
assert_sync_task(::Actions::Katello::ActivationKey::Create) do |activation_key|
assert_equal content_view_environments, activation_key.content_view_environments.map(&:candlepin_name), [cve.candlepin_name]
assert_equal content_view_environments, activation_key.content_view_environments.map(&:label), [cve.label]
assert_valid activation_key
end

Expand Down
8 changes: 4 additions & 4 deletions test/models/content_view_environment_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ def test_hosts
assert_includes cve.hosts, host
end

def test_with_candlepin_name
def test_with_label_and_org
dev = katello_environments(:dev)
view = katello_content_views(:library_dev_view)
cve = Katello::ContentViewEnvironment.where(:environment_id => dev, :content_view_id => view).first
assert_equal cve, ContentViewEnvironment.with_candlepin_name('published_dev_view_dev', organization: dev.organization)
assert_equal cve, ContentViewEnvironment.with_label_and_org('published_dev_view_dev', organization: dev.organization)
end

def test_fetch_content_view_environments_candlepin_names
def test_fetch_content_view_environments_labels
dev = katello_environments(:dev)
view = katello_content_views(:library_dev_view)
cve = Katello::ContentViewEnvironment.where(:environment_id => dev, :content_view_id => view).first
Expand All @@ -60,7 +60,7 @@ def test_fetch_content_view_environments_invalid_ids_does_not_mutate_array
assert_equal [0, 999], input_ids # should not have a map! which mutates the input array
end

def test_fetch_content_view_environments_mixed_validity_candlepin_names
def test_fetch_content_view_environments_mixed_validity_labels
dev = katello_environments(:dev)
assert_raises(HttpErrors::UnprocessableEntity) do
ContentViewEnvironment.fetch_content_view_environments(labels: ['published_dev_view_dev, bogus'], organization: dev.organization)
Expand Down
2 changes: 1 addition & 1 deletion webpack/ForemanColumnExtensions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ const hostsIndexColumnExtensions = [
}
>
<FlexItem>
{truncate(contentViewEnvironments.map(cve => cve.candlepin_name).join(', '), 35)}
{truncate(contentViewEnvironments.map(cve => cve.label).join(', '), 35)}
</FlexItem>
</Popover>
</Flex>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ const RepositorySetsTab = () => {
const { name: lifecycleEnvironmentName } = lifecycleEnvironment ?? {};
const multiEnvHost = contentViewEnvironments.length > 1;
const contentViewEnvironmentNames =
contentViewEnvironments.map(({ candlepin_name: candlepinName }) => candlepinName).join(', ');
contentViewEnvironments.map(({ label }) => label).join(', ');
const nonLibraryHost = contentViewDefault === false ||
lifecycleEnvironmentLibrary === false;
const [isBulkActionOpen, setIsBulkActionOpen] = useState(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ const ActivationKeys = ({
<SelectOption
key={ack.name}
value={ack.name}
description={ack?.lce ? ack.lce : __('No environment')}
description={ack?.cves ? ack.cves : __('No content view environments')}
/>
))}
</Select>
Expand Down
Loading