Merge pull request #7579 from d-m-u/fix_nil_includes

fix error in policy condition edit: no include? for nil

app/controllers/application_controller/filter.rb

@@ -553,7 +553,7 @@ def exp_commit_field(exp)
       unless @edit[@expkey][:exp_key].include?("NULL") || @edit[@expkey][:exp_key].include?("EMPTY") # Check for "IS/IS NOT NULL/EMPTY"
         exp[@edit[@expkey][:exp_key]]["value"] = @edit[@expkey][:exp_value]                          #   else set the value
         unless exp[@edit[@expkey][:exp_key]]["value"] == :user_input
-          exp[@edit[@expkey][:exp_key]]["value"] += Expression.prefix_by_dot(@edit[@expkey].val1_suffix)
+          exp[@edit[@expkey][:exp_key]]["value"] += Expression.prefix_by_dot(@edit[@expkey].val1_suffix).to_s
         end
       end
       exp[@edit[@expkey][:exp_key]]["alias"] = @edit[@expkey][:alias] if @edit.fetch_path(@expkey, :alias)
@@ -647,7 +647,7 @@ def exp_commit_find(exp)
       exp[@edit[@expkey][:exp_key]]["search"][skey]["field"] = @edit[@expkey][:exp_field]   # Set the search field
       unless skey.include?("NULL") || skey.include?("EMPTY")                                # Check for "IS/IS NOT NULL/EMPTY"
         exp[@edit[@expkey][:exp_key]]["search"][skey]["value"] = @edit[@expkey][:exp_value] #   else set the value
-        exp[@edit[@expkey][:exp_key]]["search"][skey]["value"] += Expression.prefix_by_dot(@edit[@expkey].val1_suffix)
+        exp[@edit[@expkey][:exp_key]]["search"][skey]["value"] += Expression.prefix_by_dot(@edit[@expkey].val1_suffix).to_s
       end
       chk = @edit[@expkey][:exp_check]
       exp[@edit[@expkey][:exp_key]][chk] = {}                                               # Create the check hash
@@ -660,7 +660,7 @@ def exp_commit_find(exp)
                                                           end
       unless ckey.include?("NULL") || ckey.include?("EMPTY")                                # Check for "IS/IS NOT NULL/EMPTY"
         exp[@edit[@expkey][:exp_key]][chk][ckey]["value"] = @edit[@expkey][:exp_cvalue]     #   else set the value
-        exp[@edit[@expkey][:exp_key]][chk][ckey]["value"] += Expression.prefix_by_dot(@edit[@expkey].val2_suffix)
+        exp[@edit[@expkey][:exp_key]][chk][ckey]["value"] += Expression.prefix_by_dot(@edit[@expkey].val2_suffix).to_s
       end
       exp[@edit[@expkey][:exp_key]]["search"][skey]["alias"] = @edit[@expkey][:alias] if @edit.fetch_path(@expkey, :alias)
     end

app/controllers/application_controller/filter/expression.rb

@@ -446,14 +446,14 @@ def update_from_exp_tree(exp)
 
       if val1 && [:datetime, :date].include?(val1[:type])     # Change datetime and date field values into arrays while editing
         self.exp_value = Array.wrap(exp_value)                # Turn date/time values into an array
-        val1[:date_format] = exp_value.to_s.first.include?('/') ? 's' : 'r'
+        val1[:date_format] = exp_value.first.to_s.include?('/') ? 's' : 'r'
         if key == EXP_FROM && val1[:date_format] == 'r'
           val1[:through_choices] = Expression.through_choices(exp_value[0])
         end
       end
       if val2 && [:datetime, :date].include?(val2[:type])
         self.exp_cvalue = Array.wrap(exp_cvalue)              # Turn date/time cvalues into an array
-        val2[:date_format] = exp_cvalue.first.include?('/') ? 's' : 'r'
+        val2[:date_format] = exp_cvalue.first&.include?('/') ? 's' : 'r'
         if ckey == EXP_FROM && val2[:date_format] == 'r'
           val2[:through_choices] = Expression.through_choices(exp_cvalue[0])
         end
@@ -583,6 +583,8 @@ def val_type_for(key, field)
 
     # Return the through_choices pulldown array for FROM datetime/date operators
     def self.through_choices(from_choice)
+      return nil if from_choice.nil?
+
       tc = if ViewHelper::FROM_HOURS.include?(from_choice)
              ViewHelper::FROM_HOURS
            elsif ViewHelper::FROM_DAYS.include?(from_choice)

spec/controllers/application_controller/filter/expression_spec.rb

@@ -40,6 +40,24 @@
     end
   end
 
+  describe '#update_from_exp_tree' do
+    context "datetime with nil value" do
+      let(:expression) { ApplicationController::Filter::Expression.new }
+      let(:expr) { {"FIND" => {"search" => {"INCLUDES" => {"field" => "Vm.filesystems-name", "value" => "iptables"}}, "checkany" => {"IS NOT EMPTY" => {"field" => "Vm.filesystems-ctime", "value" => nil}}}, :token => 2} }
+      let(:expr2) { {"FIND" => {"search" => {"INCLUDES" => {"field" => "Vm.filesystems-name", "value" => "iptables"}}, "checkany" => {"FROM" => {"field" => "Vm.filesystems-ctime", "value" => nil}}}, :token => 2} }
+
+      it 'should not raise on the include' do
+        expect { expression.update_from_exp_tree(expr) }.not_to raise_error
+      end
+
+      it 'with check key FROM should set through_choices to nil' do
+        expect(ApplicationController::Filter::Expression).to receive(:through_choices).and_return(nil)
+
+        expression.update_from_exp_tree(expr2)
+      end
+    end
+  end
+
   context 'selected and last loaded filter' do
     let(:expression) { ApplicationController::Filter::Expression.new }
     let(:search) { FactoryBot.create(:miq_search) }