Merge pull request ManageIQ#13079 from lpichler/add_validation_for_ch…

…arts_with_values

Add validation for charts with values

app/controllers/report_controller/reports/editor.rb

@@ -234,7 +234,7 @@ def build_edit_screen
       if options.empty?
         @edit[:new][:chart_column] = nil
       else
-        options[0][1] unless options.detect { |_, v| v == @edit[:new][:chart_column] }
+        @edit[:new][:chart_column] = options[0][1] unless options.detect { |_, v| v == @edit[:new][:chart_column] }
       end
 
     when "6"  # Timeline
@@ -856,6 +856,9 @@ def move_cols_left
             @edit[:new][:sortby2] = NOTHING_STRING
           end
 
+          # Clear out selected chart data column
+          @edit[:new][:chart_column] = nil if @edit[:new][:chart_column] == nf.last
+
           @edit[:new][:col_options].delete(field_to_col(nf.last)) # Remove this column from the col_options hash
         end
       end
@@ -1623,6 +1626,8 @@ def valid_report?(rpt)
       end
     end
 
+    active_tab = 'edit_5' unless valid_chart_data_column?
+
     # Validate column styles
     unless rpt.col_options.blank? || @edit[:new][:field_order].nil?
       @edit[:new][:field_order].each do |f| # Go thru all of the cols in order
@@ -1662,6 +1667,14 @@ def valid_report?(rpt)
     @flash_array.nil?
   end
 
+  def valid_chart_data_column?
+    is_valid = !(@edit[:new][:graph_type] && @edit[:new][:chart_mode] == 'values' && @edit[:new][:chart_column].blank?)
+
+    add_flash(_('Data column must be selected when chart mode is set to "Values"'), :error) unless is_valid
+
+    is_valid
+  end
+
   # Check for valid report configuration in @edit[:new]
   # Check if chargeback field is valid
   def valid_chargeback_fields
@@ -1747,6 +1760,8 @@ def check_tabs
       elsif Chargeback.db_is_chargeback?(@edit[:new][:model]) && !valid_chargeback_fields
         add_flash(_('Preview tab is not available until Chargeback Filters has been configured'), :error)
         active_tab = 'edit_3'
+      elsif !valid_chart_data_column?
+        active_tab = 'edit_5'
       end
     when '9'
       if @edit[:new][:fields].empty?

app/helpers/report_helper.rb

@@ -27,29 +27,30 @@ def visibility_options(widget)
   end
 
   def chart_fields_options
-    if @edit[:new][:group] != 'No'
-      groupings = @edit[:new][:col_options].find_all do |_field, col_options|
-        col_options[:grouping].present? && !col_options[:grouping].empty?
-      end
-      groupings.each_with_object([]) do |(field, col_options), options|
-        model = @edit[:new][:model]
-        col_options[:grouping].each do |fun|
-          field_key = if field =~ /\./
-                        f = field.sub('.', '-')
-                        "#{model}.#{f}"
-                      else
-                        "#{model}-#{field}"
-                      end
-          field_label = @edit[:new][:headers][field_key]
-          options << ["#{field_label} (#{fun.to_s.titleize})", "#{model}-#{field}:#{fun}"]
-        end
-      end
-    else
-      @edit[:new][:field_order].find_all do |f|
-        ci = MiqReport.get_col_info(f.last.split("__").first)
-        ci[:numeric]
-      end
-    end
+    chart_data_columns = if @edit[:new][:group] != 'No'
+                           groupings = @edit[:new][:col_options].find_all do |_field, col_options|
+                             col_options[:grouping].present? && !col_options[:grouping].empty?
+                           end
+                           groupings.each_with_object([]) do |(field, col_options), options|
+                             model = @edit[:new][:model]
+                             col_options[:grouping].each do |fun|
+                               field_key = if field =~ /\./
+                                             f = field.sub('.', '-')
+                                             "#{model}.#{f}"
+                                           else
+                                             "#{model}-#{field}"
+                                           end
+                               field_label = @edit[:new][:headers][field_key]
+                               options << ["#{field_label} (#{fun.to_s.titleize})", "#{model}-#{field}:#{fun}"]
+                             end
+                           end
+                         else
+                           @edit[:new][:field_order].find_all do |f|
+                             ci = MiqReport.get_col_info(f.last.split("__").first)
+                             ci[:numeric]
+                           end
+                         end
+    [[_("Nothing selected"), nil]] + chart_data_columns
   end
 
   def filter_performance_start_options

spec/helpers/report_helper_spec.rb

@@ -31,8 +31,13 @@ def create_and_generate_report_for_user(report_name, user_id)
       }
 
       options = chart_fields_options
+      expected_array = [
+        ["Nothing selected", nil],
+        ["Memory (Total)", "Vm-mem_cpu:total"],
+        ["Allocated Disk Storage (Total)", "Vm-allocated_disk_storage:total"]
+      ]
 
-      expect(options).to eq([["Memory (Total)", "Vm-mem_cpu:total"], ["Allocated Disk Storage (Total)", "Vm-allocated_disk_storage:total"]])
+      expect(options).to eq(expected_array)
     end
 
     it 'should return numeric fields from report with models when "Show Sort Breaks" is "No"' do
@@ -51,7 +56,13 @@ def create_and_generate_report_for_user(report_name, user_id)
 
       options = chart_fields_options
 
-      expect(options).to eq([[" Memory", "Vm-mem_cpu"], [" Allocated Disk Storage", "Vm-allocated_disk_storage"]])
+      expected_array = [
+        ["Nothing selected", nil],
+        [" Memory", "Vm-mem_cpu"],
+        [" Allocated Disk Storage", "Vm-allocated_disk_storage"]
+      ]
+
+      expect(options).to eq(expected_array)
     end
   end
 end