[Rails52] Use arel to fix "Dangerous query method"

There were multiple places (all in `.order` calls) that triggered the
following warning

  DEPRECATION WARNING: Dangerous query method (method whose arguments
  are used as raw SQL) called with non-attribute argument(s):
  "lower(name)".  Non-attribute arguments will be disallowed in Rails
  6.0. This method should not be called with user-provided values, such
  as request parameters or model attributes. Known-safe values can be
  passed by wrapping them in Arel.sql().

This fixes that by instead calling to `arel_table[].lower`, which does
the same thing, but uses the existing arel interface to do it, which
should be safer.  It should fail earlier for non-name columns, and
should be safer overall then using `Arel.sql()`.

app/controllers/application_controller.rb

@@ -1079,7 +1079,7 @@ def get_sort_col
   def process_saved_reports(saved_reports, task)
     success_count = 0
     failure_count = 0
-    MiqReportResult.for_user(current_user).where(:id => saved_reports).order("lower(name)").each do |rep|
+    MiqReportResult.for_user(current_user).where(:id => saved_reports).order(MiqReportResult.arel_table[:name].lower).each do |rep|
       rep.public_send(task) if rep.respond_to?(task) # Run the task
     rescue StandardError
       failure_count += 1 # Push msg and error flag

app/controllers/application_controller/ci_processing.rb

@@ -107,7 +107,7 @@ def storage_button_action
   def process_elements(elements, klass, task, display_name = nil, order_field = nil)
     order_field ||= %w[name description title].find { |field| klass.column_names.include?(field) }
 
-    order_by = order_field == "ems_id" ? order_field : "lower(#{order_field})"
+    order_by = order_field == "ems_id" ? order_field : klass.arel_table[order_field].lower
 
     Rbac.filtered(klass.where(:id => elements).order(order_by)).each do |record|
       name = record.send(order_field.to_sym)
@@ -531,15 +531,15 @@ def process_clusters(clusters, task, _ = nil)
     return if clusters.empty?
 
     if task == "destroy"
-      EmsCluster.where(:id => clusters).order("lower(name)").each do |cluster|
+      EmsCluster.where(:id => clusters).order(EmsCluster.arel_table[:name].lower).each do |cluster|
         id = cluster.id
         cluster_name = cluster.name
         audit = {:event => "ems_cluster_record_delete_initiated", :message => "[#{cluster_name}] Record delete initiated", :target_id => id, :target_class => "EmsCluster", :userid => session[:userid]}
         AuditEvent.success(audit)
       end
       EmsCluster.destroy_queue(clusters)
     else
-      EmsCluster.where(:id => clusters).order("lower(name)").each do |cluster|
+      EmsCluster.where(:id => clusters).order(EmsCluster.arel_table[:name].lower).each do |cluster|
         cluster_name = cluster.name
         begin
           cluster.send(task.to_sym) if cluster.respond_to?(task) # Run the task
@@ -561,15 +561,15 @@ def process_resourcepools(rps, task)
     return if rps.empty?
 
     if task == "destroy"
-      ResourcePool.where(:id => rps).order("lower(name)").each do |rp|
+      ResourcePool.where(:id => rps).order(ResourcePool.arel_table[:name].lower).each do |rp|
         id = rp.id
         rp_name = rp.name
         audit = {:event => "rp_record_delete_initiated", :message => "[#{rp_name}] Record delete initiated", :target_id => id, :target_class => "ResourcePool", :userid => session[:userid]}
         AuditEvent.success(audit)
       end
       ResourcePool.destroy_queue(rps)
     else
-      ResourcePool.where(:id => rps).order("lower(name)").each do |rp|
+      ResourcePool.where(:id => rps).order(ResourcePool.arel_table[:name].lower).each do |rp|
         rp_name = rp.name
         begin
           rp.send(task.to_sym) if rp.respond_to?(task) # Run the task
@@ -699,7 +699,7 @@ def process_orchestration_stacks(stacks, task, _ = nil)
     return if stacks.empty?
 
     if task == "destroy"
-      OrchestrationStack.where(:id => stacks).order("lower(name)").each do |stack|
+      OrchestrationStack.where(:id => stacks).order(OrchestrationStack.arel_table[:name].lower).each do |stack|
         id = stack.id
         stack_name = stack.name
         audit = {:event        => "stack_record_delete_initiated",
@@ -719,7 +719,8 @@ def process_configuration_jobs(stacks, task, _ = nil)
     return if stacks.empty?
 
     if task == "destroy"
-      ManageIQ::Providers::AnsibleTower::AutomationManager::Job.where(:id => stacks).order("lower(name)").each do |stack|
+      model = ManageIQ::Providers::AnsibleTower::AutomationManager::Job
+      model.where(:id => stacks).order(model.arel_table[:name].lower).each do |stack|
         id = stack.id
         stack_name = stack.name
         audit = {:event        => "stack_record_delete_initiated",
@@ -729,7 +730,7 @@ def process_configuration_jobs(stacks, task, _ = nil)
                  :userid       => session[:userid]}
         AuditEvent.success(audit)
       end
-      ManageIQ::Providers::AnsibleTower::AutomationManager::Job.destroy_queue(stacks)
+      model.destroy_queue(stacks)
     end
   end
 
@@ -738,7 +739,7 @@ def process_storage(storages, task, _ = nil)
     return if storages.empty?
 
     if task == "destroy"
-      Storage.where(:id => storages).order("lower(name)").each do |storage|
+      Storage.where(:id => storages).order(Storage.arel_table[:name].lower).each do |storage|
         id = storage.id
         storage_name = storage.name
         audit = {:event => "storage_record_delete_initiated", :message => "[#{storage_name}] Record delete initiated", :target_id => id, :target_class => "Storage", :userid => session[:userid]}
@@ -748,7 +749,7 @@ def process_storage(storages, task, _ = nil)
       add_flash(n_("Delete initiated for Datastore from the %{product} Database",
                    "Delete initiated for Datastores from the %{product} Database", storages.length) % {:product => Vmdb::Appliance.PRODUCT_NAME})
     else
-      Storage.where(:id => storages).order("lower(name)").each do |storage|
+      Storage.where(:id => storages).order(Storage.arel_table[:name].lower).each do |storage|
         storage_name = storage.name
         begin
           if task == "scan"

app/controllers/catalog_controller.rb

@@ -1150,7 +1150,7 @@ def set_form_vars
   end
 
   def fetch_zones
-    @zones = Zone.visible.in_my_region.order('lower(description)').pluck(:description, :id)
+    @zones = Zone.visible.in_my_region.order(Zone.arel_table[:description].lower).pluck(:description, :id)
   end
 
   def st_set_form_vars

app/controllers/mixins/actions/host_actions/misc.rb

@@ -5,7 +5,7 @@ module Misc
         # Common handling of misc Host buttons
 
         def each_host(host_ids, task_name)
-          Host.where(:id => host_ids).order('lower(name)').each do |host|
+          Host.where(:id => host_ids).order(Host.arel_table[:name].lower).each do |host|
             yield host
           rescue StandardError => err
             add_flash(

app/controllers/mixins/containers_common_mixin.rb

@@ -122,7 +122,7 @@ def check_compliance(model)
   end
 
   def process_scan_images(ids)
-    ContainerImage.where(:id => ids).order("lower(name)").each do |image|
+    ContainerImage.where(:id => ids).order(ContainerImage.arel_table[:name].lower).each do |image|
       image_name = image.name
       begin
         image.scan
@@ -138,7 +138,7 @@ def process_scan_images(ids)
   end
 
   def process_check_compliance(model, ids)
-    model.where(:id => ids).order("lower(name)").each do |entity|
+    model.where(:id => ids).order(model.arel_table[:name].lower).each do |entity|
       entity.check_compliance
     rescue => bang
       add_flash(_("%{model} \"%{name}\": Error during 'Check Compliance': %{error}") %

app/controllers/mixins/ems_common.rb

@@ -496,7 +496,7 @@ def check_compliance_nested(model)
     end
 
     def process_check_compliance(model, ids)
-      model.where(:id => ids).order("lower(name)").each do |entity|
+      model.where(:id => ids).order(model.arel_table[:name].lower).each do |entity|
         entity.check_compliance
       rescue => bang
         add_flash(_("%{model} \"%{name}\": Error during 'Check Compliance': %{error}") %
@@ -517,7 +517,7 @@ def set_form_vars
 
     def form_instance_vars
       @server_zones = []
-      zones = Zone.visible.order('lower(description)')
+      zones = Zone.visible.order(Zone.arel_table[:name].lower)
       zones.each do |zone|
         @server_zones.push([zone.description, zone.name])
       end

app/controllers/mixins/ems_common/core.rb

@@ -2,7 +2,7 @@ module Mixins
   module EmsCommon
     module Core
       def process_emss_task_destroy(emss)
-        model.where(:id => emss).order("lower(name)").each do |ems|
+        model.where(:id => emss).order(model.arel_table[:name].lower).each do |ems|
           audit = {:event        => "ems_record_delete_initiated",
                    :message      => "[#{ems.name}] Record delete initiated",
                    :target_id    => ems.id,
@@ -23,7 +23,7 @@ def process_emss_task_destroy(emss)
 
       def process_emss_task_pause_resume(emss, task)
         action = task.split("_").first
-        model.where(:id => emss).order("lower(name)").each do |ems|
+        model.where(:id => emss).order(model.arel_table[:name].lower).each do |ems|
           audit = {:event        => "ems_record_#{action}_initiated",
                    :message      => "[#{ems.name}] Record #{action} initiated",
                    :target_id    => ems.id,
@@ -37,7 +37,7 @@ def process_emss_task_pause_resume(emss, task)
       end
 
       def process_emss_task_other(emss, task)
-        model.where(:id => emss).order("lower(name)").each do |ems|
+        model.where(:id => emss).order(model.arel_table[:name].lower).each do |ems|
           ems.send(task.to_sym) if ems.respond_to?(task) # Run the task
         rescue => bang
           add_flash(_("%{model} \"%{name}\": Error during '%{task}': %{error_message}") %

app/controllers/mixins/manager_controller_mixin.rb

@@ -208,13 +208,13 @@ def accordion_select
     def new
       assert_privileges("#{privilege_prefix}_add_provider")
       @provider_manager = concrete_model.new
-      @server_zones = Zone.visible.in_my_region.order('lower(description)').pluck(:description, :name)
+      @server_zones = Zone.visible.in_my_region.order(Zone.arel_table[:name].lower).pluck(:description, :name)
       @sb[:action] = params[:action]
       render_form
     end
 
     def edit
-      @server_zones = Zone.visible.in_my_region.order('lower(description)').pluck(:description, :name)
+      @server_zones = Zone.visible.in_my_region.order(Zone.arel_table[:name].lower).pluck(:description, :name)
       case params[:button]
       when "cancel"
         cancel_provider

app/presenters/tree_builder_automation_manager_configuration_scripts.rb

@@ -17,8 +17,10 @@ def root_options
 
   # Get root nodes count/array for explorer tree
   def x_get_tree_roots
-    objects = []
-    templates = Rbac.filtered(ManageIQ::Providers::AnsibleTower::AutomationManager.order("lower(name)"), :match_via_descendants => ConfigurationScript)
+    objects    = []
+    model      = ManageIQ::Providers::AnsibleTower::AutomationManager
+    rbac_scope = model.order(model.arel_table[:name].lower)
+    templates  = Rbac.filtered(rbac_scope, :match_via_descendants => ConfigurationScript)
 
     templates.each do |temp|
       objects.push(temp)

app/presenters/tree_builder_infra_networking.rb

@@ -22,7 +22,8 @@ def root_options
   end
 
   def x_get_tree_roots
-    objects = Rbac.filtered(ManageIQ::Providers::Vmware::InfraManager.order("lower(name)"))
+    model   = ManageIQ::Providers::Vmware::InfraManager
+    objects = Rbac.filtered(model.order(model.arel_table[:name].lower))
     count_only_or_objects(false, objects)
   end
 

spec/presenters/tree_builder_spec.rb

@@ -162,7 +162,7 @@ def root_options
     end
 
     it 'counts collections with order' do
-      expect(builder.count_only_or_objects(true, VmOrTemplate.distinct.order("lower(vms.name)"))).to eq(0)
+      expect(builder.count_only_or_objects(true, VmOrTemplate.distinct.order(VmOrTemplate.arel_table[:name].lower))).to eq(0)
     end
   end