Fix for <Choose> found as option in drop down service dialogs #15456
Conversation
eclarizio
force-pushed
the
BZ1462375
branch
3 times, most recently
from
5ae9d58
to
d22138e
Compare
|
This pull request is not mergeable. Please rebase and repush. |
|
|
||
| context "when the field is not required" do | ||
| it "returns the values with a nil 'None' option" do | ||
| expect(dialog_field.values).to eq([[nil, "<None>"], %w(test test), %w(abc abc)]) |
There was a problem hiding this comment.
@eclarizio - I'm assuming this part ( that the [nil, '<None>'] as the first option is the bit that resolves the main description of BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1462375
| context "when there is a default value that matches a value in the values list" do | ||
| let(:default_value) { "2" } | ||
| let(:values) { [%w(1 1), %w(2 2), %w(3 3)] } | ||
| it "returns the values with the first option being a nil 'None' option" do |
There was a problem hiding this comment.
@eclarizio - same here - I like that the title makes note that it is the first option.
| if !required? | ||
| initial_values.flatten | ||
| elsif default_value.blank? || !default_value_included?(self[:values]) | ||
| [nil, "<Choose>"] |
There was a problem hiding this comment.
@eclarizio - For consistency - possibly move this out into a method like initial_values - Or bring both back into nil_option
|
Checked commit eclarizio@b42c110 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
Fix for <Choose> found as option in drop down service dialogs (cherry picked from commit 0cbb33d) https://bugzilla.redhat.com/show_bug.cgi?id=1472806
|
Euwe backport details: |
|
@eclarizio Travis is failing in Euwe branch now. Can you take a look and see if backport of this PR is causing the failure? |
|
Yes, it does appear to be because of the backport. It is safe to simply remove the It will also fail on I'll email you a patch file. |
|
Can you please create a PR? |
Fix for <Choose> found as option in drop down service dialogs (cherry picked from commit 0cbb33d) https://bugzilla.redhat.com/show_bug.cgi?id=1478435
|
Fine backport details: |
|
@eclarizio Looks like this is causing Travis failure in Fine branch too: https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/261064934 Can you please create a PR to fix it? |
|
Sure, here it is: ManageIQ/manageiq-ui-classic#1837 |
Fix for <Choose> found as option in drop down service dialogs (cherry picked from commit 0cbb33d) https://bugzilla.redhat.com/show_bug.cgi?id=1478435
For all static fields, if the default value is not chosen, the first item in the list will be a 'nil' option with a description of either
<None>or<Choose>, depending on if the field is required or not. However, if a default value is chosen, this doesn't really make sense for required fields because a default value is already selected and there is no reason someone should go back to a nil value. For non-required fields, however, it makes sense to continue to have the<None>option even if a default value is already set.This PR enhances the logic of when and when not to add a 'nil' option for sorted item fields.
https://bugzilla.redhat.com/show_bug.cgi?id=1462375
@miq-bot add_label bug, fine/yes, euwe/yes
@miq-bot assign @gmcculloug
/cc @dclarizio Since you assigned this BZ to me I wanted to tag you for visibility.
@syncrou Can you review for me? I'm not super convinced that the way I handled the
nil_optionlogic is optimal, but I feel like it's at least contained in a small enough method that it will be easier to make changes to if we need to down the road.