Merge pull request #23617 from kbrock/aaar_scope

Clean up ActsAsArScope and ActsAsArModel for recent rbac changes

Author: Fryguy

app/models/chargeback.rb

@@ -1,3 +1,4 @@
+# NOTE: Does not implement any search functionality. Implements ActAsArModel to be useable for reports
 class Chargeback < ActsAsArModel
   set_columns_hash( # Fields common to any chargeback type
     :start_date             => :datetime,

app/models/manageiq/providers/cloud_manager/vm_or_template.rb

@@ -1,7 +1,6 @@
 class ManageIQ::Providers::CloudManager::VmOrTemplate < ActsAsArScope
   class << self
     delegate :orphaned, :archived, :to => :aar_scope
-    delegate :klass, :to => :aar_scope, :prefix => true
   end
 
   def self.aar_scope

app/models/manageiq/providers/infra_manager/vm_or_template.rb

@@ -1,7 +1,6 @@
 class ManageIQ::Providers::InfraManager::VmOrTemplate < ActsAsArScope
   class << self
     delegate :orphaned, :archived, :to => :aar_scope
-    delegate :klass, :to => :aar_scope, :prefix => true
   end
 
   def self.aar_scope

app/models/vim_performance_trend.rb

@@ -1,3 +1,4 @@
+# NOTE: Does not implement any search functionality. Implements ActAsArModel to be useable for reports
 class VimPerformanceTrend < ActsAsArModel
   set_columns_hash(
     :resource_name      => :string,

lib/acts_as_ar_scope.rb

@@ -1,25 +1,19 @@
-# this is an extension to act as ar_model
-# this allows a developer to formulate results as a scope
-# all other behavior is handled from there
+# This is a form of an AR Model. But the distinction is this is a model based upon an ActiveRecord scope (aar_scope)
+# Essentially, this delegates all calls to the scope.
+# It may be easier to understand by looking at an implementation like InfraManager::VmOrTemplate
 class ActsAsArScope < ActsAsArModel
-  # user required to add aar_scope
   class << self
     # Standard query methods are delegated to the core active record classes
     delegate :includes, :references, :eager_load, :preload, :limit, :order, :offset, :select, :where, :to => :aar_scope
-    delegate :find, :first, :last, :find_by_id, :find_by, :count, :to => :aar_scope
+    delegate :find, :first, :last, :find_by_id, :find_by, :count, :all, :to => :aar_scope
 
     delegate :klass, :to => :aar_scope, :prefix => true
     delegate :table_name, :reflections, :to => :aar_scope_klass
-    delegate :_virtual_columns_hash, :virtual_reflections, :to => :aar_scope_klass
+    delegate :virtual_reflections, :to => :aar_scope_klass
   end
 
-  def self.all(*args)
-    if args.empty? || args.size == 1 && args.first.respond_to?(:empty?) && args.first.empty?
-      # avoid warnings
-      aar_scope
-    else
-      aar_scope.all(*args)
-    end
+  def self.aar_scope
+    raise NotImplementedError, _("find must be implemented in a subclass")
   end
 
   # TODO: goal is for this to be false

spec/models/manageiq/providers/infra_manager/vm_or_template_spec.rb

@@ -25,4 +25,25 @@
       expect(described_class.archived).to match_array([vm, t])
     end
   end
+
+  describe ".aar_scope_klass" do
+    it "returns the class of the aar_scope" do
+      expect(described_class.aar_scope_klass).to eq(::VmOrTemplate)
+    end
+  end
+
+  describe ".orphaned" do
+    it "delegates" do
+      # orphaned adds a where clause. lets make sure it works well
+      # VmOrTemplate is here to tack on a "type" - so we ignore that
+      expect(described_class.orphaned.where_values_hash.except("type")).to eq(::VmOrTemplate.orphaned.where_values_hash.except("type"))
+    end
+  end
+
+  describe ".archived" do
+    it "delegates" do
+      # VmOrTemplate is here to tack on a "type" - so we ignore that
+      expect(described_class.archived.where_values_hash.except("type")).to eq(::VmOrTemplate.archived.where_values_hash.except("type"))
+    end
+  end
 end