Skip to content

Commit 9b3d6c4

Browse files
jeremylenzclaude
andcommitted
Fixes #39330 - Update tests for deprecated CV/LCE param removal
Update content_facet_ignore_update? to check content_view_environment_ids instead of the removed content_view_id/lifecycle_environment_id params. Without this, Host.new with content_facet_attributes would silently reject the nested attributes and not build the content facet. Update all test files to use content_view_environment_ids (hosts) and content_view_environment_id (hostgroups) instead of the removed params. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 3b4d4a2 commit 9b3d6c4

70 files changed

Lines changed: 826 additions & 880 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

app/assets/javascripts/katello/hosts/host_and_hostgroup_edit.js

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,16 +54,16 @@ KT.hosts.refreshContentViewEnvironments = function() {
5454
$.get(url, params, function(data) {
5555
var foundPreviousValue = false;
5656

57-
if (previousInheritText && (previousInheritText.includes('Inherit') || previousInheritText === '')) {
57+
if (inheritOption.length && (inheritOption.val() === '' || previousInheritDataId)) {
5858
var inheritOpt = $("<option />").text(previousInheritText).val('');
5959
if (previousInheritDataId) inheritOpt.attr('data-id', previousInheritDataId);
6060
select.append(inheritOpt);
6161
}
6262

63-
$.each(data.results, function(index, cve) {
64-
var label = cve.lifecycle_environment.name + ' / ' + cve.content_view.name;
65-
var option = $("<option />").val(cve.id).text(label);
66-
if (cve.id === parseInt(previousValue)) {
63+
$.each(data.results, function(index, cvEnv) {
64+
var label = cvEnv.label;
65+
var option = $("<option />").val(cvEnv.id).text(label);
66+
if (cvEnv.id === parseInt(previousValue)) {
6767
option.prop('selected', true);
6868
foundPreviousValue = true;
6969
}
@@ -99,14 +99,20 @@ KT.hosts.onKatelloHostEditLoad = function(){
9999
};
100100

101101
KT.hosts.getSelectedContentViewEnvironment = function() {
102-
var cveId = $("#hostgroup_content_view_environment_id").val();
103-
if (!cveId) {
104-
cveId = $("#host_content_view_environment_id").val();
102+
var cvEnvId = $("#hostgroup_content_view_environment_id").val();
103+
if (!cvEnvId) {
104+
cvEnvId = $("#host_content_view_environment_id").val();
105105
}
106-
if (!cveId) {
107-
cveId = $("#hostgroup_content_view_environment_id > option:selected").data("id");
106+
if (!cvEnvId) {
107+
cvEnvId = $("#hostgroup_content_view_environment_id > option:selected").data("id");
108108
}
109-
return cveId;
109+
if (!cvEnvId) {
110+
var hiddenInput = $("input[name='host[content_facet_attributes][content_view_environment_ids][]']").first();
111+
if (hiddenInput.length) {
112+
cvEnvId = hiddenInput.val();
113+
}
114+
}
115+
return cvEnvId;
110116
};
111117

112118
KT.hosts.toggle_installation_medium = function() {

app/controllers/katello/api/rhsm/candlepin_proxies_controller.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -428,13 +428,13 @@ def update_environments_from_facts(param_environments)
428428
end
429429

430430
def get_content_view_environment(key, value)
431-
cve = nil
431+
cvenv = nil
432432
if value
433-
cve = ContentViewEnvironment.where(key => value).first
434-
fail HttpErrors::NotFound, _("Couldn't find environment '%s'") % value unless cve
435-
deny_access unless cve.readable? || User.consumer?
433+
cvenv = ContentViewEnvironment.where(key => value).first
434+
fail HttpErrors::NotFound, _("Couldn't find environment '%s'") % value unless cvenv
435+
deny_access unless cvenv.readable? || User.consumer?
436436
end
437-
cve
437+
cvenv
438438
end
439439

440440
def get_content_view_environments(label = nil, organization = nil)

app/controllers/katello/api/v2/activation_keys_controller.rb

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ def index
5858
param_group :activation_key
5959
def create
6060
@activation_key = ActivationKey.new(activation_key_params) do |activation_key|
61-
activation_key.content_view_environments = @content_view_environments if update_cves?
61+
activation_key.content_view_environments = @content_view_environments if update_cvenvs?
6262
activation_key.organization = @organization
6363
activation_key.user = current_user
6464
end
@@ -73,7 +73,7 @@ def create
7373
param :id, :number, :desc => N_("ID of the activation key"), :required => true
7474
param :name, String, :desc => N_("name"), :required => false
7575
def update
76-
if @content_view_environments.present? || update_cves?
76+
if @content_view_environments.present? || update_cvenvs?
7777
@activation_key.update!(content_view_environments: @content_view_environments)
7878
end
7979
sync_task(::Actions::Katello::ActivationKey::Update, @activation_key, activation_key_params)
@@ -220,7 +220,7 @@ def index_relation
220220

221221
def find_content_view_environments
222222
@content_view_environments = []
223-
if params[:content_view_environments] || params[:content_view_environment_ids]
223+
if params_likely_not_from_angularjs? && (params[:content_view_environments] || params[:content_view_environment_ids])
224224
@content_view_environments = ::Katello::ContentViewEnvironment.fetch_content_view_environments(
225225
labels: params[:content_view_environments],
226226
ids: params[:content_view_environment_ids],
@@ -229,20 +229,36 @@ def find_content_view_environments
229229
handle_errors(labels: params[:content_view_environments],
230230
ids: params[:content_view_environment_ids])
231231
end
232+
@content_view_environments.each do |cvenv|
233+
throw_resource_not_found(name: 'content_view_environment', id: cvenv.id) unless cvenv.readable?
234+
end
232235
end
233-
handle_blank_cve_params
236+
handle_blank_cvenv_params
234237
@organization ||= @content_view_environments.first&.organization
235238
end
236239

237-
def handle_blank_cve_params
240+
def params_likely_not_from_angularjs?
241+
# AngularJS sends back the activation key's existing API response as params.
242+
# This means content_view_environments arrives as a nested hash instead of
243+
# a comma-separated string of labels, which would cause fetch_content_view_environments to fail.
244+
# We detect AngularJS by the presence of multi_content_view_environment — a computed
245+
# response-only value that a real API client would never submit.
246+
!params.key?(:multi_content_view_environment)
247+
end
248+
249+
def handle_blank_cvenv_params
250+
if params.key?(:environment) && params.key?(:content_view)
251+
return # AngularJS sends nested environment and content_view params; ignore them
252+
end
238253
if params.key?(:content_view_environments) && params[:content_view_environments].blank?
239254
@content_view_environments = []
240255
elsif params.key?(:content_view_environment_ids) && params[:content_view_environment_ids].blank?
241256
@content_view_environments = []
242257
end
243258
end
244259

245-
def update_cves?
260+
def update_cvenvs?
261+
return false unless params_likely_not_from_angularjs?
246262
params.key?(:content_view_environments) ||
247263
params.key?(:content_view_environment_ids)
248264
end

app/controllers/katello/api/v2/content_view_versions_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ def find_version_environments
229229
@composite_version_environments += lookup_all_composites(version_environment[:content_view_version]) if params[:propagate_all_composites]
230230
end
231231
end
232-
@composite_version_environments.uniq! { |cve| cve[:content_view_version] }
232+
@composite_version_environments.uniq! { |cvenv| cvenv[:content_view_version] }
233233
end
234234

235235
def lookup_all_composites(component)

app/controllers/katello/api/v2/hosts_bulk_actions_controller.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,13 +224,13 @@ def available_incremental_updates
224224
version_environments = {}
225225
content_facets = Katello::Host::ContentFacet.with_non_installable_errata(@errata, @hosts)
226226

227-
ContentViewEnvironment.for_content_facets(content_facets).each do |cve|
228-
version = cve.content_view_version
227+
ContentViewEnvironment.for_content_facets(content_facets).each do |cvenv|
228+
version = cvenv.content_view_version
229229
version_environment = version_environments[version] || {:content_view_version => version, :environments => []}
230-
version_environment[:environments] << cve.environment unless version_environment[:environments].include?(cve.environment)
230+
version_environment[:environments] << cvenv.environment unless version_environment[:environments].include?(cvenv.environment)
231231
version_environment[:next_version] ||= version.next_incremental_version
232232
version_environment[:content_host_count] ||= 0
233-
version_environment[:content_host_count] += content_facets.with_content_view_environments(cve).count
233+
version_environment[:content_host_count] += content_facets.with_content_view_environments(cvenv).count
234234

235235
if version.content_view.composite?
236236
version_environment[:components] = version.components_needing_errata(@errata)

app/controllers/katello/concerns/api/v2/hosts_controller_extensions.rb

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,15 @@ def handle_content_view_environments_for_update
6161
def validate_content_view_environment_params
6262
content_facet_attributes = params.dig(:host, :content_facet_attributes)
6363
return if content_facet_attributes.blank?
64-
return unless cve_params[:content_view_environments].present? || cve_params[:content_view_environment_ids].present?
64+
return unless cvenv_params[:content_view_environments].present? || cvenv_params[:content_view_environment_ids].present?
6565

6666
cves = ::Katello::ContentViewEnvironment.fetch_content_view_environments(
67-
labels: cve_params[:content_view_environments],
68-
ids: cve_params[:content_view_environment_ids],
67+
labels: cvenv_params[:content_view_environments],
68+
ids: cvenv_params[:content_view_environment_ids],
6969
organization: find_organization || @host&.organization)
7070
if cves.blank?
71-
handle_errors(labels: cve_params[:content_view_environments],
72-
ids: cve_params[:content_view_environment_ids])
71+
handle_errors(labels: cvenv_params[:content_view_environments],
72+
ids: cvenv_params[:content_view_environment_ids])
7373
end
7474
cves
7575
end
@@ -88,7 +88,7 @@ def set_content_view_environments(cves)
8888
end
8989
# rubocop:enable Naming/AccessorMethodName
9090

91-
def cve_params
91+
def cvenv_params
9292
params.require(:host).require(:content_facet_attributes).permit(content_view_environments: [], content_view_environment_ids: [])
9393
end
9494
end

app/controllers/katello/concerns/authorization/api/v2/content_views_controller.rb

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,15 +110,15 @@ def authorize_activation_key_environments(view, options)
110110
end
111111

112112
def authorize_hostgroup_content_view_environment(view, options)
113-
cv_env_id = options[:hostgroup_content_view_environment_id]
114-
if cv_env_id
115-
cv_env = ContentViewEnvironment.joins(:content_view, :environment)
116-
.where(id: cv_env_id)
113+
cvenv_id = options[:hostgroup_content_view_environment_id]
114+
if cvenv_id
115+
cvenv = ContentViewEnvironment.joins(:content_view, :environment)
116+
.where(id: cvenv_id)
117117
.where("#{ContentView.table_name}.organization_id" => view.organization.id)
118118
.first
119-
fail HttpErrors::NotFound, _("Couldn't find host group content view environment id '%s'") % cv_env_id unless cv_env
119+
fail HttpErrors::NotFound, _("Couldn't find host group content view environment id '%s'") % cvenv_id unless cvenv
120120
# deny if we cannot reassign hostgroups to the new content view environment
121-
return deny_access unless cv_env.content_view.readable? && cv_env.environment.readable?
121+
return deny_access unless cvenv.content_view.readable? && cvenv.environment.readable?
122122
end
123123
true
124124
end

app/helpers/katello/content_options_helper.rb

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ def content_options(host, selected_id, object_type, options = {})
2121
end
2222

2323
if orgs.count > 1
24-
all_options << %(<optgroup label="#{org.name}">#{content_object_options}</optgroup>)
24+
all_options << %(<optgroup label="#{h(org.name)}">#{content_object_options}</optgroup>)
2525
else
2626
all_options << content_object_options
2727
end
@@ -58,54 +58,54 @@ def content_view_environment_options(hostgroup, options = {})
5858
include_blank = '<option></option>' if include_blank == true
5959

6060
orgs = relevant_organizations(hostgroup)
61-
current_cve = fetch_content_view_environment(hostgroup, options)
61+
current_cvenv = fetch_content_view_environment(hostgroup, options)
6262
content_source = fetch_content_source(hostgroup, options)
6363

64-
all_options = build_cve_options_for_orgs(orgs, current_cve, content_source)
64+
all_options = build_cvenv_options_for_orgs(orgs, current_cvenv, content_source)
6565
all_options = all_options.join
6666
all_options.insert(0, include_blank) if include_blank
6767
all_options.html_safe # User content is safely escaped with h()
6868
end
6969
# rubocop:enable Rails/OutputSafety
7070

71-
def build_cve_options_for_orgs(orgs, current_cve, content_source)
71+
def build_cvenv_options_for_orgs(orgs, current_cvenv, content_source)
7272
orgs.map do |org|
73-
cves = fetch_cves_for_org(org, current_cve, content_source)
74-
cve_options = build_cve_option_tags(cves, current_cve)
73+
cvenvs = fetch_cvenvs_for_org(org, current_cvenv, content_source)
74+
cvenv_options = build_cvenv_option_tags(cvenvs, current_cvenv)
7575

7676
if orgs.count > 1
77-
%(<optgroup label="#{org.name}">#{cve_options}</optgroup>)
77+
%(<optgroup label="#{h(org.name)}">#{cvenv_options}</optgroup>)
7878
else
79-
cve_options
79+
cvenv_options
8080
end
8181
end
8282
end
8383

84-
def fetch_cves_for_org(org, current_cve, content_source)
85-
cves = Katello::ContentViewEnvironment.joins(:content_view, :environment)
84+
def fetch_cvenvs_for_org(org, current_cvenv, content_source)
85+
cvenvs = Katello::ContentViewEnvironment.joins(:content_view, :environment)
8686
.where("#{Katello::ContentView.table_name}.organization_id" => org.id)
8787
.order("#{Katello::KTEnvironment.table_name}.name", "#{Katello::ContentView.table_name}.name")
8888
.to_a
8989

90-
cves = filter_cves_by_content_source(cves, content_source, org) if content_source.present?
91-
cves |= [current_cve] if current_cve.present? && current_cve.content_view.organization_id == org.id
92-
cves
90+
cvenvs = filter_cvenvs_by_content_source(cvenvs, content_source, org) if content_source.present?
91+
cvenvs |= [current_cvenv] if current_cvenv.present? && current_cvenv.content_view.organization_id == org.id
92+
cvenvs
9393
end
9494

95-
def filter_cves_by_content_source(cves, content_source, org)
96-
return cves if content_source.pulp_primary?
95+
def filter_cvenvs_by_content_source(cvenvs, content_source, org)
96+
return cvenvs if content_source.pulp_primary?
9797

9898
available_env_ids = content_source.lifecycle_environments.where(organization_id: org.id).pluck(:id)
99-
return cves unless available_env_ids.any?
99+
return cvenvs unless available_env_ids.any?
100100

101-
cves.select { |cve| available_env_ids.include?(cve.environment_id) }
101+
cvenvs.select { |cvenv| available_env_ids.include?(cvenv.environment_id) }
102102
end
103103

104-
def build_cve_option_tags(cves, current_cve)
105-
option_tags = cves.map do |cve|
106-
selected = current_cve&.id == cve.id ? 'selected' : ''
107-
label = cve.default_environment? ? cve.environment.name : "#{cve.environment.name} / #{cve.content_view.name}"
108-
%(<option value="#{cve.id}" #{selected}>#{h(label)}</option>)
104+
def build_cvenv_option_tags(cvenvs, current_cvenv)
105+
option_tags = cvenvs.map do |cvenv|
106+
selected = current_cvenv&.id == cvenv.id ? 'selected' : ''
107+
label = cvenv.label
108+
%(<option value="#{cvenv.id}" #{selected}>#{h(label)}</option>)
109109
end
110110
option_tags.join
111111
end
@@ -160,16 +160,11 @@ def blank_or_inherit_with_id(f, attr) # f is Rails convention for form objects
160160

161161
def blank_or_inherit_cvenv(f) # f is Rails convention for form objects
162162
return true unless f.object.respond_to?(:parent_id) && f.object.parent_id
163-
parent_cve = f.object.parent&.content_view_environment
164-
inherited_value = parent_cve.try(:id) || ''
165-
166-
if parent_cve
167-
cve_name = if parent_cve.default_environment?
168-
parent_cve.environment.name
169-
else
170-
"#{parent_cve.environment.name} / #{parent_cve.content_view.name}"
171-
end
172-
%(<option data-id="#{inherited_value}" value="">#{h(_('Inherit parent (%s)') % cve_name)}</option>)
163+
parent_cvenv = f.object.parent&.content_view_environment
164+
inherited_value = parent_cvenv.try(:id) || ''
165+
166+
if parent_cvenv
167+
%(<option data-id="#{inherited_value}" value="">#{h(_('Inherit parent (%s)') % parent_cvenv.label)}</option>)
173168
else
174169
%(<option value="">#{_('Inherit parent')}</option>)
175170
end

app/helpers/katello/hosts_and_hostgroups_helper.rb

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,10 @@ def fetch_content_view(host_or_hostgroup, options = {})
7373
def fetch_content_view_environment(host_or_hostgroup, options = {})
7474
if host_or_hostgroup&.content_facet.present?
7575
if host_or_hostgroup.is_a?(::Hostgroup)
76-
return host_or_hostgroup.content_facet.content_view_environment
76+
return host_or_hostgroup.content_facet.content_view_environment if host_or_hostgroup.content_facet.content_view_environment.present?
7777
else
78-
return host_or_hostgroup.content_facet.content_view_environments.first
78+
first_cvenv = host_or_hostgroup.content_facet.content_view_environments.first
79+
return first_cvenv if first_cvenv.present?
7980
end
8081
end
8182
selected_host_group = options.fetch(:selected_host_group, nil)
@@ -135,29 +136,29 @@ def inherited_or_own_content_source_id(host_or_hostgroup, hostgroup)
135136
content_source_id
136137
end
137138

138-
def inherited_or_own_cve_id(host_or_hostgroup, hostgroup)
139-
inherited_cve_id = hostgroup.content_facet&.content_view_environment_id
140-
inherited_cve_id ||= hostgroup.send(:inherited_ancestry_attribute, :content_view_environment_id, :content_facet) if hostgroup.ancestry.present?
139+
def inherited_or_own_cvenv_id(host_or_hostgroup, hostgroup)
140+
inherited_cvenv_id = hostgroup.content_facet&.content_view_environment_id
141+
inherited_cvenv_id ||= hostgroup.send(:inherited_ancestry_attribute, :content_view_environment_id, :content_facet) if hostgroup.ancestry.present?
141142

142143
case host_or_hostgroup
143144
when ::Hostgroup
144-
own_cve_id = host_or_hostgroup.content_facet&.content_view_environment_id
145-
own_cve_id && own_cve_id != inherited_cve_id ? own_cve_id : inherited_cve_id
145+
own_cvenv_id = host_or_hostgroup.content_facet&.content_view_environment_id
146+
own_cvenv_id && own_cvenv_id != inherited_cvenv_id ? own_cvenv_id : inherited_cvenv_id
146147
when ::Host::Managed
147-
own_cve = host_or_hostgroup.content_view_environments.first
148-
own_cve && own_cve.id != inherited_cve_id ? own_cve.id : inherited_cve_id
148+
own_cvenv = host_or_hostgroup.content_view_environments.first
149+
own_cvenv && own_cvenv.id != inherited_cvenv_id ? own_cvenv.id : inherited_cvenv_id
149150
else
150-
inherited_cve_id
151+
inherited_cvenv_id
151152
end
152153
end
153154

154155
def hostgroup_content_facet(hostgroup, param_host)
155-
cve_id = inherited_or_own_cve_id(param_host, hostgroup)
156+
cvenv_id = inherited_or_own_cvenv_id(param_host, hostgroup)
156157
content_source_id = inherited_or_own_content_source_id(param_host, hostgroup)
157158
facet = ::Katello::Host::ContentFacet.new(:content_source_id => content_source_id)
158-
if cve_id
159-
cve = ::Katello::ContentViewEnvironment.find_by(id: cve_id)
160-
facet.content_view_environments = [cve] if cve
159+
if cvenv_id
160+
cvenv = ::Katello::ContentViewEnvironment.find_by(id: cvenv_id)
161+
facet.content_view_environments = [cvenv] if cvenv
161162
end
162163
facet
163164
end

0 commit comments

Comments
 (0)