Store role ids for custom buttons, not role names

Author: jrafanie

app/controllers/application_controller/buttons.rb

@@ -852,12 +852,7 @@ def button_set_record_vars(button)
     end
     button.visibility ||= {}
     if @edit[:new][:visibility_typ] == "role"
-      roles = []
-      @edit[:new][:roles].each do |r|
-        role = MiqUserRole.find_by(:id => r)
-        roles.push(role.name) if role
-      end
-      button.visibility[:roles] = roles
+      button.visibility[:roles] = @edit[:new][:roles]
     else
       button.visibility[:roles] = ["_ALL_"]
     end
@@ -1026,13 +1021,9 @@ def button_set_form_vars
       if @custom_button.visibility[:roles][0] == "_ALL_"
         @edit[:new][:roles] = ["_ALL_"]
       else
-        @edit[:new][:roles] ||= []
-        @custom_button.visibility[:roles].each do |r|
-          role = MiqUserRole.find_by(:name => r)
-          @edit[:new][:roles].push(role.id) if role
-        end
+        @edit[:new][:roles] = @custom_button.visibility[:roles]
+        @edit[:new][:roles].sort! if @edit[:new][:roles].present?
       end
-      @edit[:new][:roles].sort! if @edit[:new][:roles].present?
     end
 
     @edit[:sorted_user_roles] = []
@@ -1096,9 +1087,8 @@ def buttons_get_node_info(node)
       end
       @sb[:user_roles] = []
       if @custom_button.visibility && @custom_button.visibility[:roles] && @custom_button.visibility[:roles][0] != "_ALL_"
-        MiqUserRole.all.sort_by(&:name).each do |r|
-          @sb[:user_roles].push(r.name) if @custom_button.visibility[:roles].include?(r.name)
-        end
+        role_ids = @custom_button.visibility[:roles]
+        @sb[:user_roles] = MiqUserRole.where(:id => role_ids).order(:name).pluck(:name)
       end
       @resolve[:new][:target_class] = "ServiceTemplate"
       dialog_id = @custom_button.resource_action.dialog_id

app/controllers/miq_ae_customization_controller/custom_buttons.rb

@@ -76,9 +76,8 @@ def ab_get_node_info(node)
       end
       @sb[:user_roles] = []
       if @custom_button.visibility && @custom_button.visibility[:roles] && @custom_button.visibility[:roles][0] != "_ALL_"
-        MiqUserRole.all.sort_by(&:name).each do |r|
-          @sb[:user_roles].push(r.name) if @custom_button.visibility[:roles].include?(r.name)
-        end
+        role_ids = @custom_button.visibility[:roles]
+        @sb[:user_roles] = MiqUserRole.where(:id => role_ids).order(:name).pluck(:name)
       end
       dialog_id = @custom_button.resource_action.dialog_id
       @sb[:dialog_label] = dialog_id ? Dialog.find(dialog_id).label : ""

spec/controllers/application_controller/buttons_spec.rb

@@ -325,7 +325,7 @@
                 :object_request => 'request',
                 :open_url       => true,
                 :visibility_typ => 'role',
-                :roles          => [role.id.to_s],
+                :roles          => [role.id],
                 :display_for    => :list}
       }
     }
@@ -340,13 +340,13 @@
 
     it "sets new role visibility for custom button" do
       controller.send(:button_set_record_vars, custom_button)
-      expect(custom_button.visibility[:roles]).to eq([role.name])
+      expect(custom_button.visibility[:roles]).to eq([role.id])
     end
 
     it "sets new role and preserves old role for custom button" do
       edit[:new][:roles] = [old_role.id, role.id.to_s] # old roles are represented by int and new ones by string
       controller.send(:button_set_record_vars, custom_button)
-      expect(custom_button.visibility[:roles]).to eq([old_role.name, role.name])
+      expect(custom_button.visibility[:roles]).to eq([old_role.id, role.id.to_s])
     end
   end