From c57905fe0927d6e65cc47a35ef2c4b97927ac0c7 Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Tue, 24 Sep 2019 14:05:04 -0400 Subject: [PATCH 1/2] Add MiqServer#server_role_names= There is a `#role` and a `#role=` method that takes a comma separated list, then a `#server_role_names` which takes an array but no `#server_roles_names=`. --- app/models/miq_server/role_management.rb | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/app/models/miq_server/role_management.rb b/app/models/miq_server/role_management.rb index 155cf9b730c..b4d1acae3d1 100644 --- a/app/models/miq_server/role_management.rb +++ b/app/models/miq_server/role_management.rb @@ -134,19 +134,12 @@ def server_role_names alias_method :my_roles, :server_role_names alias_method :assigned_role_names, :server_role_names - def role - server_role_names.join(',') - end - alias_method :my_role, :role - alias_method :assigned_role, :role - - def role=(val) + def server_role_names=(roles) zone.lock do - val = val.to_s.strip.downcase - if val.blank? + if roles.blank? server_roles.delete_all else - desired = (val == "*" ? ServerRole.all_names : val.split(",").collect { |v| v.strip.downcase }.sort) + desired = (roles == "*" ? ServerRole.all_names : roles.map { |role| role.strip.downcase }.sort) current = server_role_names # MiqServer#server_role_names may include database scoped roles, which are managed elsewhere, @@ -165,6 +158,17 @@ def role=(val) end end + roles + end + + def role + server_role_names.join(',') + end + alias_method :my_role, :role + alias_method :assigned_role, :role + + def role=(val) + self.server_role_names = val == "*" ? val : val.split(",") role end From c686ae46c0ec7e6284a38402241e40e3a5c288a5 Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Wed, 25 Sep 2019 08:44:21 -0400 Subject: [PATCH 2/2] Add test coverage for server_role_names= --- .../models/miq_server/role_management_spec.rb | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/spec/models/miq_server/role_management_spec.rb b/spec/models/miq_server/role_management_spec.rb index 4201a495893..c6d225cb77c 100644 --- a/spec/models/miq_server/role_management_spec.rb +++ b/spec/models/miq_server/role_management_spec.rb @@ -63,6 +63,33 @@ end end + context "server_role_names=" do + it "normal case" do + @miq_server.assign_role('ems_operations', 1) + expect(@miq_server.server_role_names).to eq(['ems_operations']) + + desired = %w[event scheduler user_interface] + @miq_server.server_role_names = desired + expect(@miq_server.server_role_names).to eq(desired) + end + + it "with a duplicate existing role" do + @miq_server.assign_role('ems_operations', 1) + + desired = %w[ems_operations ems_operations scheduler] + @miq_server.server_role_names = desired + expect(@miq_server.server_role_names).to eq(%w[ems_operations scheduler]) + end + + it "with duplicate new roles" do + @miq_server.assign_role('event', 1) + + desired = %w[ems_operations scheduler scheduler] + @miq_server.server_role_names = desired + expect(@miq_server.server_role_names).to eq(%w[ems_operations scheduler]) + end + end + it "should assign role properly when requested" do @roles = [['ems_operations', 1], ['event', 2], ['ems_metrics_coordinator', 1], ['scheduler', 1], ['reporting', 1]] @roles.each do |role, priority|