Merge pull request ManageIQ#20052 from kbrock/miq_expression_merge

delete MiqExpression#merge_where_clauses

app/models/miq_report/generator.rb

@@ -324,7 +324,8 @@ def generate_basic_results(options = {})
     targets = db_class.find_entries(ext_options) if targets.respond_to?(:find_entries)
     # TODO: add once only_cols is fixed
     # targets = targets.select(only_cols)
-    where_clause = MiqExpression.merge_where_clauses(self.where_clause, options[:where_clause])
+    targets = targets.where(where_clause) if where_clause
+    targets = targets.where(options[:where_clause]) if options[:where_clause]
 
     # Remove custom_attributes as part of the `includes` if all of them exist
     # in the select statement
@@ -337,7 +338,6 @@ def generate_basic_results(options = {})
       :filter           => conditions,
       :include_for_find => get_include_for_find_rbac,
       :references       => get_include_rbac,
-      :where_clause     => where_clause,
       :skip_counts      => true
     )
 

lib/miq_expression.rb

@@ -411,29 +411,6 @@ def includes_for_sql
     col_details.values.each_with_object({}) { |v, result| result.deep_merge!(v[:include]) }
   end
 
-  def self.expand_conditional_clause(klass, cond)
-    return klass.send(:sanitize_sql_for_conditions, cond) unless cond.kind_of?(Hash)
-
-    cond = klass.predicate_builder.resolve_column_aliases(cond)
-    cond = klass.send(:expand_hash_conditions_for_aggregates, cond)
-
-    klass.predicate_builder.build_from_hash(cond).map { |b| klass.connection.visitor.compile(b) }.join(' AND ')
-  end
-
-  def self.merge_where_clauses(*list)
-    list = list.compact.collect do |s|
-      expand_conditional_clause(MiqReport, s)
-    end.compact
-
-    if list.empty?
-      nil
-    elsif list.size == 1
-      list.first
-    else
-      "(#{list.join(") AND (")})"
-    end
-  end
-
   def self.get_cols_from_expression(exp, options = {})
     result = {}
     if exp.kind_of?(Hash)

spec/lib/miq_expression_spec.rb

@@ -2441,45 +2441,6 @@
     end
   end
 
-  describe ".merge_where_clauses" do
-    it "returns nil for nil" do
-      expect(MiqExpression.merge_where_clauses(nil)).to be_nil
-    end
-
-    it "returns nil for blank" do
-      expect(MiqExpression.merge_where_clauses("")).to be_nil
-    end
-
-    it "returns nil for multiple empty arrays" do
-      expect(MiqExpression.merge_where_clauses([],[])).to be_nil
-    end
-
-    it "returns same string single results" do
-      expect(MiqExpression.merge_where_clauses("a=5")).to eq("a=5")
-    end
-
-    it "returns same string when concatinating blank results" do
-      expect(MiqExpression.merge_where_clauses("a=5", [])).to eq("a=5")
-    end
-
-    # would be nice if we returned a hash
-    it "returns a string if the only argument is a hash" do
-      expect(MiqExpression.merge_where_clauses({"vms.id" => 5})).to eq("\"vms\".\"id\" = 5")
-    end
-
-    it "concatinates 2 arrays" do
-      expect(MiqExpression.merge_where_clauses(["a=?",5], ["b=?",5])).to eq("(a=5) AND (b=5)")
-    end
-
-    it "concatinates 2 string" do
-      expect(MiqExpression.merge_where_clauses("a=5", "b=5")).to eq("(a=5) AND (b=5)")
-    end
-
-    it "concatinates a string and a hash" do
-      expect(MiqExpression.merge_where_clauses("a=5", {"vms.id" => 5})).to eq("(a=5) AND (\"vms\".\"id\" = 5)")
-    end
-  end
-
   describe ".parse_field_or_tag" do
     subject { described_class.parse_field_or_tag(@field).try(:column_type) }
     let(:string_custom_attribute) do

spec/models/miq_report/generator_spec.rb

@@ -67,6 +67,23 @@
         expect(@miq_report_profile_all.table.data[0].data).to include("min_trend_value" => 400,
                                                                       "max_trend_value" => 700)
       end
+
+      it "handles merging WHERE clauses from MiqReport#where_clause and options[:where_clause]" do
+        FactoryBot.create(:vm)       # filtered out by option[:where_clause]
+        FactoryBot.create(:template) # filtered out by report.where_clause
+        vm = FactoryBot.create(:vm, :vendor => "redhat")
+
+        rpt = FactoryBot.create(
+          :miq_report,
+          :db           => "VmOrTemplate",
+          :where_clause => ["vms.type = ?", "Vm"],
+          :col_order    => %w[id name host.name vendor]
+        )
+        rpt.generate_table(:userid => @user.userid, :where_clause => {"vms.vendor" => "redhat"})
+
+        expect(rpt.table.size).to eq(1)
+        expect(rpt.table.first.id.to_s).to eq(vm.id.to_s)
+      end
     end
   end