Merge pull request ManageIQ#14259 from eclarizio/Backport_for_BZ1428133

[EUWE] Backport for Fix for Service Dialog not saving default value <None> for drop down or radio button

app/helpers/application_helper/dialogs.rb

@@ -1,11 +1,6 @@
 module ApplicationHelper::Dialogs
-  def dialog_dropdown_select_values(field, selected_value, category_tags = nil)
+  def dialog_dropdown_select_values(field, _selected_value, category_tags = nil)
     values = []
-    if !field.required
-      values.push(["<None>", nil])
-    elsif selected_value.blank?
-      values.push(["<Choose>", nil])
-    end
     if field.type.include?("DropDown")
       values += field.values.collect(&:reverse)
     elsif field.type.include?("TagControl")
@@ -110,6 +105,19 @@ def radio_options(field, url, value, selected_value)
     add_options_unless_read_only({}, tag_options, field)
   end
 
+  def default_value_form_options(field_type, field_values, field_default_value)
+    no_default_value = [["<#{_('None')}>", nil]]
+    if field_values.empty?
+      values = no_default_value
+    else
+      values = field_values.collect(&:reverse)
+      values = no_default_value + values if field_type == "DialogFieldRadioButton"
+    end
+
+    selected = field_default_value || nil
+    options_for_select(values, selected)
+  end
+
   def build_auto_refreshable_field_indicies(workflow)
     auto_refreshable_field_indicies = []
 

app/models/dialog_field_radio_button.rb

@@ -4,6 +4,19 @@ def show_refresh_button?
   end
 
   def initial_values
-    [["", "<None>"]]
+    [[nil, "<None>"]]
+  end
+
+  private
+
+  def raw_values
+    @raw_values ||= dynamic ? values_from_automate : static_raw_values
+    self.value ||= default_value if default_value_included_in_raw_values?
+
+    @raw_values
+  end
+
+  def static_raw_values
+    self[:values].to_miq_a
   end
 end

app/models/dialog_field_sorted_item.rb

@@ -91,15 +91,26 @@ def sort_data(data_to_sort)
   end
 
   def raw_values
-    @raw_values ||= dynamic ? values_from_automate : self[:values].to_miq_a
-    unless @raw_values.collect { |value_pair| value_pair[0] }.include?(default_value)
-      self.default_value = sort_data(@raw_values).first.try(:first)
-    end
+    @raw_values ||= dynamic ? values_from_automate : static_raw_values
+    use_first_value_as_default unless default_value_included_in_raw_values?
     self.value ||= default_value
 
     @raw_values
   end
 
+  def use_first_value_as_default
+    self.default_value = sort_data(@raw_values).first.try(:first)
+  end
+
+  def default_value_included_in_raw_values?
+    @raw_values.collect { |value_pair| value_pair[0] }.include?(default_value)
+  end
+
+  def static_raw_values
+    first_values = required? ? [[nil, "<Choose>"]] : initial_values
+    first_values + self[:values].to_miq_a.reject { |value| value[0].nil? }
+  end
+
   def initial_values
     [[nil, "<None>"]]
   end

app/views/miq_ae_customization/_dialog_field_form_non_dynamic_options.html.haml

@@ -114,12 +114,9 @@
     %label.col-md-2.control-label
       = _('Default Value')
     .col-md-8
-      - none = [["<#{_('None')}>", nil]]
-      - values = @edit[:field_values].empty? ? none : none + @edit[:field_values].collect(&:reverse)
-      - selected = @edit[:field_default_value] || nil
       = select_tag("field_default_value",
-                    options_for_select(values, selected),
-                    :class    => "selectpicker")
+                   default_value_form_options(@edit[:field_typ], @edit[:field_values], @edit[:field_default_value]),
+                   :class    => "selectpicker")
       :javascript
         miqSelectPickerEvent("field_default_value", "#{url}")
   .form-group

app/views/miq_ae_customization/_dialog_sample.html.haml

@@ -86,22 +86,15 @@
                                     options_for_select(val.collect(&:reverse), field.default_value))
                                 - else
                                   = select_tag('field.id',
-                                              options_for_select([["<#{_('Choose')}>", nil]] + val.collect(&:reverse),
-                                                                  field.default_value))
+                                              options_for_select(val.collect(&:reverse), field.default_value))
 
                               - else
                                 -# NOT REQUIRED
                                 = select_tag('field.id',
-                                              options_for_select([["#{_('None')}", nil]] + val.collect(&:reverse),
-                                                                  field.default_value))
+                                              options_for_select(val.collect(&:reverse), field.default_value))
 
                             - else
                               - val.each_with_index do |rb, i|
-                                - unless field.required || i != 0
-                                  %input{:type => "radio", :disabled => true, :checked => field.default_value.nil?,
-                                    :id => "None", :value => "nil", :name => "None"}
-                                  &nbsp; None &nbsp;
-
                                 %input{:type => "radio", :disabled => true, :checked => (field.default_value == rb[0]),
                                   :id => rb[0], :value => rb[1], :name => rb[0]}
                                 &nbsp;

app/views/miq_ae_customization/_field_values.html.haml

@@ -17,7 +17,8 @@
         - else
           = render :partial => "field_value_entry", :locals => {:entry => "new", :edit => false}
         - @edit[:field_values].each_with_index do |e, i|
-          - if !entry.nil? && entry != "new" && entry.to_i == i
-            = render :partial => "field_value_entry", :locals => {:entry => e, :idx => i, :edit => true}
-          - else
-            = render :partial => "field_value_entry", :locals => {:entry => e, :idx => i, :edit => false}
+          - unless e[0].nil?
+            - if !entry.nil? && entry != "new" && entry.to_i == i
+              = render :partial => "field_value_entry", :locals => {:entry => e, :idx => i, :edit => true}
+            - else
+              = render :partial => "field_value_entry", :locals => {:entry => e, :idx => i, :edit => false}

app/views/shared/dialogs/_dialog_field_radio_button.html.haml

@@ -1,9 +1,7 @@
 - if edit
   - if field.values.length > 1
     %span{:id => "dynamic-radio-#{field.id}"}
-      - values = field.values
-      - values = [['', "<#{_('None')}>"]] + values if !field.required && !field.dynamic
-      - values.each do |rb|
+      - field.values.each do |rb|
 
         %input{radio_options(field, url, rb[0], wf.value(field.name))}
         %label.dynamic-radio-label= rb[1]

spec/helpers/application_helper/dialogs_spec.rb

@@ -10,25 +10,23 @@
   let(:trigger_auto_refresh) { nil }
 
   describe "#dialog_dropdown_select_values" do
-    before do
-      val_array = [["cat", "Cat"], ["dog", "Dog"]]
-      @val_array_reversed = val_array.collect(&:reverse)
-      @field = DialogFieldDropDownList.new(:values => val_array)
-    end
+    let(:dialog_field) { instance_double("DialogFieldDropDownList", :values => values, :type => type) }
+    let(:values) { [%w(cat Cat), %w(dog Dog)] }
 
-    it "not required" do
-      @field.required = false
-      expect(helper.dialog_dropdown_select_values(@field, nil)).to eq([["<None>", nil]] + @val_array_reversed)
-    end
+    context "when the field type includes drop down" do
+      let(:type) { "BananaDropDown" }
 
-    it "required, nil selected" do
-      @field.required = true
-      expect(helper.dialog_dropdown_select_values(@field, nil)).to eq([["<Choose>", nil]] + @val_array_reversed)
+      it "returns the values collected and reversed" do
+        expect(helper.dialog_dropdown_select_values(dialog_field, nil)).to eq(values.collect(&:reverse))
+      end
     end
 
-    it "required, non-nil selected" do
-      @field.required = true
-      expect(helper.dialog_dropdown_select_values(@field, "cat")).to eq(@val_array_reversed)
+    context "when the field type includes tag control" do
+      let(:type) { "BananaTagControl" }
+
+      it "returns the values with the passed in category tags" do
+        expect(helper.dialog_dropdown_select_values(dialog_field, nil, %w(category tags))).to eq(%w(category tags))
+      end
     end
   end
 
@@ -506,4 +504,104 @@
       expect(helper.auto_refresh_listening_options(options, true)).to eq(:trigger => true)
     end
   end
+
+  describe "#default_value_form_options" do
+    let(:subject) { helper.default_value_form_options(field_type, field_values, field_default_value) }
+
+    context "when the field values are empty" do
+      let(:field_values) { [] }
+
+      context "when the field type is a DialogFieldDropDownList" do
+        let(:field_type) { "DialogFieldDropDownList" }
+
+        context "when the field default value is set" do
+          let(:field_default_value) { "some default" }
+
+          it "adds the ability to select no default value" do
+            expect(subject).to eq("<option value=\"\">&lt;None&gt;</option>")
+          end
+        end
+
+        context "when the field default value is not set" do
+          let(:field_default_value) { nil }
+
+          it "adds the ability to select no default value" do
+            expect(subject).to eq("<option value=\"\">&lt;None&gt;</option>")
+          end
+        end
+      end
+
+      context "when the field type is a DialogFieldRadioButton" do
+        let(:field_type) { "DialogFieldRadioButton" }
+
+        context "when the field default value is set" do
+          let(:field_default_value) { "some default" }
+
+          it "adds the ability to select no default value" do
+            expect(subject).to eq("<option value=\"\">&lt;None&gt;</option>")
+          end
+        end
+
+        context "when the field default value is not set" do
+          let(:field_default_value) { nil }
+
+          it "adds the ability to select no default value" do
+            expect(subject).to eq("<option value=\"\">&lt;None&gt;</option>")
+          end
+        end
+      end
+    end
+
+    context "when the field values are not empty" do
+      let(:field_values) { [%w(123 456)] }
+
+      context "when the field type is a DialogFieldDropDownList" do
+        let(:field_type) { "DialogFieldDropDownList" }
+
+        context "when the field default value is set" do
+          let(:field_default_value) { "123" }
+
+          it "selects the default value and does not add any" do
+            expect(subject).to eq("<option selected=\"selected\" value=\"123\">456</option>")
+          end
+        end
+
+        context "when the field default value is not set" do
+          let(:field_default_value) { nil }
+
+          it "does not explicitly select anything" do
+            expect(subject).to eq("<option value=\"123\">456</option>")
+          end
+        end
+      end
+
+      context "when the field type is a DialogFieldRadioButton" do
+        let(:field_type) { "DialogFieldRadioButton" }
+
+        context "when the field default value is set" do
+          let(:field_default_value) { "123" }
+
+          it "adds the ability to select no default value but selects the default" do
+            expected_html = <<-HTML
+<option value=\"\">&lt;None&gt;</option>
+<option selected=\"selected\" value=\"123\">456</option>
+            HTML
+            expect(subject).to eq(expected_html.chomp)
+          end
+        end
+
+        context "when the field default value is not set" do
+          let(:field_default_value) { nil }
+
+          it "adds the ability to select no default value" do
+            expected_html = <<-HTML
+<option value=\"\">&lt;None&gt;</option>
+<option value=\"123\">456</option>
+            HTML
+            expect(subject).to eq(expected_html.chomp)
+          end
+        end
+      end
+    end
+  end
 end