Add multiselect option to dropdowns #10270

d-m-u · 2016-08-04T20:18:12Z

Adds multiselect flag to dropdowns much like the existing single option for tag controls.

@miq-bot add_label ui
@miq-bot assign @gmcculloug
@miq-bot add_label wip

gmcculloug · 2016-08-08T14:18:43Z

@d-m-u Changes look good. You still need to handle processing an array of values when passing to automate. You can see an example of that here: app/models/dialog_field_tag_control.rb#L70

I did not test with a data_type for the field set to integer. It would be good to test that use case and make sure it is passing integers to automate.

Also, it would be good if the service dialog preview screen worked as a mult-select control.

eclarizio · 2016-08-08T14:32:27Z

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

@@ -31,7 +31,15 @@
      = render(:partial => "shared/dialogs/dialog_field_radio_button", :locals => locals)

    - when "DialogFieldDropDownList"
-      = render(:partial => "shared/dialogs/dialog_field_drop_down_list", :locals => locals)
+      - category_tags = field.values.collect(&:reverse)


category_tags is probably not an accurate description of the values being collected here.

Also, is there a reason we're blindly reversing the collection? Shouldn't field.values already return the values in the correct order based on what the sort_order is set to?

eclarizio · 2016-08-08T15:21:33Z

I know the methods you added code to in app/controllers/miq_ae_customization_controller/dialogs.rb are pretty huge, so it may be difficult to add specs, but if possible it would be nice.

app/models/dialog_field_drop_down_list.rb however shouldn't be too bad to add specs for.

gmcculloug · 2016-08-11T15:53:32Z

app/models/dialog_field_drop_down_list.rb

+  end
+
+  def automate_key_name
+    MiqAeEngine.create_automation_attribute_array_key(super)


This needs the same return super unless multi_value? as the automate_output_value call.

K, does it also need the def value_from_dialog_fields(dialog_values) that's in tagControl?

Yes, I believe it does. Hoping @eclarizio can help you with that.

It's already in the parent DialogField class, so I don't think we need def value_from_dialog_fields in this file. The one that is in the tag control does something special with classifications.

d-m-u · 2016-08-22T23:39:29Z

@miq-bot remove_label wip

Ladas · 2016-09-01T12:10:53Z

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

@@ -1,7 +1,13 @@
 - if edit
-  - select_values = field.values.collect(&:reverse)
+  - select_values = field.values.collect


the (&:reverse) seems to be important, without that, the names and ids in the selectboxes have switched

d-m-u · 2016-09-01T15:02:05Z

@miq-bot add_label wip

Ladas · 2016-09-01T16:38:54Z

@d-m-u could you chrry-pick this commit Ladas@8c88baa

I made it on top of your commits https://github.com/Ladas/manageiq/commits/multiselect

These are the fixes I needed, to make multiselectboxes work. When have multiple dynamic multiselectboxes, it throws various failures, without that commit.

After that, it works nicely. :-)

gmcculloug · 2017-02-10T17:52:57Z

app/models/dialog_field_drop_down_list.rb

+          @value.blank? ? [] : @value.chomp.split(',')
+        end
+    automate_values = a.first.kind_of?(Integer) ? a.map(&:to_i) : a
+    MiqAeEngine.create_automation_attribute_array_value(automate_values)


@mkanoor Can you verify that this is going to pass data to the automate engine as expect? This is the last part before merging this feature.

@gmcculloug @d-m-u
MiqAeEngine.create_automation_attribute_array_value([1,2,3]) => gives you back 1,2,3

For Automate to use it as an array the keyname should be prefixed with Array::

We have a helper method that does that
MiqAeEngine.create_automation_attribute_array_key("my_key")
which returns Array::my_key

You have to use the key and value together

In https://github.com/ManageIQ/manageiq/blob/master/app/models/dialog.rb#L50
We would have to detect that if it is a multi value field it should use

result[MiqAeEngine.create_automation_attribute_array_key(df.automate_key_name)] = df.automate_output_value

I fixed this Sunday at 8:05 Mahwah time.

@d-m-u Along with overriding the automate_output_value you need to override automate_key_name like we do here app/models/dialog_field_tag_control.rb#L78.

You need the same check for force_multi_value as above.

Something like this:

def automate_key_name return super unless force_multi_value MiqAeEngine.create_automation_attribute_array_key(super) end

d-m-u · 2017-02-13T01:04:31Z

@miq-bot remove_label wip

mkanoor · 2017-02-13T15:30:08Z

app/models/dialog.rb

@@ -47,7 +47,9 @@ def dialog_resources
  end

  def automate_values_hash
-    dialog_fields.each_with_object({}) { |df, result| result[df.automate_key_name] = df.automate_output_value }
+    dialog_fields.each_with_object({}) do |df, result|
+      result[MiqAeEngine.create_automation_attribute_array_key(df.automate_key_name)] = df.automate_output_value


@d-m-u Not every attribute is going to be an array type should this be done only for array types?

miq-bot · 2017-02-15T23:42:30Z

Checked commits d-m-u/manageiq@ab3412b~...390f8fa with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks good. 🍪

gmcculloug · 2017-02-16T13:59:22Z

Wow, a labor of love. Tested last night after the key changes and data is being passed to the automate method as expected. Thanks @d-m-u! 🥇

Add multiselect option to dropdowns (cherry picked from commit fd5e8ac) https://bugzilla.redhat.com/show_bug.cgi?id=1430542

simaishi · 2017-03-20T16:58:43Z

Euwe backport details:

$ git log -1
commit a21cb2147654f231b25046c14f37137472c81a0d
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Thu Feb 16 08:59:29 2017 -0500

    Merge pull request #10270 from d-m-u/multiselect
    
    Add multiselect option to dropdowns
    (cherry picked from commit fd5e8acce71d53b28360eeb0167eed0f151e7df3)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1430542

miq-bot added refactoring ui labels Aug 4, 2016

miq-bot assigned gmcculloug Aug 4, 2016

gmcculloug added enhancement darga/no and removed refactoring labels Aug 5, 2016

gmcculloug assigned eclarizio and unassigned gmcculloug Aug 5, 2016

eclarizio reviewed Aug 8, 2016
View reviewed changes

d-m-u changed the title ~~Add multiselect option to dropdowns~~ [WIP] Add multiselect option to dropdowns Aug 8, 2016

chessbyte added the wip label Aug 8, 2016

gmcculloug reviewed Aug 11, 2016
View reviewed changes

d-m-u changed the title ~~[WIP] Add multiselect option to dropdowns~~ Add multiselect option to dropdowns Aug 22, 2016

miq-bot removed the wip label Aug 22, 2016

Ladas reviewed Sep 1, 2016
View reviewed changes

miq-bot added the wip label Sep 1, 2016

d-m-u changed the title ~~Add multiselect option to dropdowns~~ [WIP] Add multiselect option to dropdowns Sep 1, 2016

fabiendupont mentioned this pull request Feb 6, 2017

Allow multi-select on Drop Down List elements ManageIQ/manageiq-ui-classic#303

Closed

gmcculloug reviewed Feb 10, 2017

View reviewed changes

miq-bot added the ui label Feb 10, 2017

miq-bot assigned gmcculloug and unassigned eclarizio Feb 10, 2017

miq-bot added the wip label Feb 10, 2017

miq-bot changed the title ~~Add multiselect option to dropdowns~~ [WIP] Add multiselect option to dropdowns Feb 10, 2017

miq-bot added wip and removed wip labels Feb 10, 2017

miq-bot changed the title ~~[WIP] Add multiselect option to dropdowns~~ Add multiselect option to dropdowns Feb 13, 2017

miq-bot removed the wip label Feb 13, 2017

mkanoor reviewed Feb 13, 2017

View reviewed changes

d-m-u and others added 7 commits February 15, 2017 16:14

Removes files split off in classic ui

ab3412b

Add tests for model change

ee9df1e

fix merge conflicts

d9837b7

Removes file split into classic ui repo

f13906f

Fix tests per Erik's comment

594d0e7

Fixes failing test

0309a17

Removes extraneous method that's already in superclass

390f8fa

gmcculloug merged commit fd5e8ac into ManageIQ:master Feb 16, 2017

gmcculloug added this to the Sprint 55 Ending Feb 27, 2017 milestone Feb 16, 2017

simaishi pushed a commit that referenced this pull request Mar 20, 2017

Merge pull request #10270 from d-m-u/multiselect

a21cb21

Add multiselect option to dropdowns (cherry picked from commit fd5e8ac) https://bugzilla.redhat.com/show_bug.cgi?id=1430542

simaishi added euwe/backported and removed euwe/yes labels Mar 20, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add multiselect option to dropdowns #10270

Add multiselect option to dropdowns #10270

d-m-u commented Aug 4, 2016 •

edited by miq-bot

Loading

gmcculloug commented Aug 8, 2016

eclarizio Aug 8, 2016

eclarizio commented Aug 8, 2016

gmcculloug Aug 11, 2016

d-m-u Aug 11, 2016

gmcculloug Aug 11, 2016

eclarizio Aug 11, 2016

d-m-u commented Aug 22, 2016

Ladas Sep 1, 2016

d-m-u commented Sep 1, 2016

Ladas commented Sep 1, 2016

gmcculloug Feb 10, 2017

mkanoor Feb 10, 2017

d-m-u Feb 13, 2017

gmcculloug Feb 15, 2017

d-m-u commented Feb 13, 2017

mkanoor Feb 13, 2017

miq-bot commented Feb 15, 2017

gmcculloug commented Feb 16, 2017

simaishi commented Mar 20, 2017

Add multiselect option to dropdowns #10270

Add multiselect option to dropdowns #10270

Conversation

d-m-u commented Aug 4, 2016 • edited by miq-bot Loading

gmcculloug commented Aug 8, 2016

Choose a reason for hiding this comment

eclarizio commented Aug 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d-m-u commented Aug 22, 2016

Choose a reason for hiding this comment

d-m-u commented Sep 1, 2016

Ladas commented Sep 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d-m-u commented Feb 13, 2017

Choose a reason for hiding this comment

miq-bot commented Feb 15, 2017

gmcculloug commented Feb 16, 2017

simaishi commented Mar 20, 2017

d-m-u commented Aug 4, 2016 •

edited by miq-bot

Loading