Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting the default_value for TagControl dialog field #276

Merged
merged 2 commits into from
Apr 4, 2018

Conversation

romanblanco
Copy link
Member

@romanblanco romanblanco commented Apr 3, 2018

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1559159

The problem is the default options <Choose> and <None> had no id, and so the default_value remained unset and so an empty option field was created. (https://stackoverflow.com/a/12654812)

   [{:id=>nil, :name=>"<Choose>", :description=>"<Choose>"},
    {:id=>10000000000035, :name=>"silver", :description=>"Silver"},
    {:id=>10000000000036, :name=>"gold", :description=>"Gold"},
    {:id=>10000000000037, :name=>"platinum", :description=>"Platinum"}],

In the fix I'm setting a value of the default options to 0, and setting them as default.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1559159

The problem is the default options Choose and None had no id, and so the default_value remained unset and so an empty option field was created. (https://stackoverflow.com/a/12654812)

In the fix I'm setting a value of the default options to 0, and setting them as default.
@romanblanco
Copy link
Member Author

@karelhala can you please take a look if the fix is OK?

@romanblanco
Copy link
Member Author

@miq-bot assign @karelhala
@miq-bot add_label bug

*/
private setDefaultValue() {
let defaultOption = _.find(this.dialogField.values, { id: null })
defaultOption.id = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if there is no defaultOption? I guess leaving blank space as was before is good isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe there always should be one for tag control element: ManageIQ/manageiq@58d78df#diff-8e5736b945bd4233c1f577172ad458b3R58

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@karelhala updated. Thanks 👍

@miq-bot
Copy link
Member

miq-bot commented Apr 4, 2018

Checked commits romanblanco/ui-components@f49f9d3~...c7ab976 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🍪

@karelhala karelhala merged commit e9a7280 into ManageIQ:master Apr 4, 2018
@romanblanco romanblanco deleted the bz1559159 branch April 5, 2018 04:40
@romanblanco romanblanco mentioned this pull request Apr 5, 2018
@simaishi
Copy link

simaishi commented May 16, 2018

@romanblanco @himdel FYI, this is marked as blocker for the next gaprindashvili release.

@simaishi
Copy link

@miq-bot add_label blocker

@himdel
Copy link
Contributor

himdel commented May 22, 2018

Backported to gaprindashvili:

commit 39fe768c2f318c3e4870895bdb6c4df20b1e0422 (HEAD -> gaprindashvili)
Author: Karel Hala <[email protected]>
Date:   Wed Apr 4 17:17:00 2018 +0200

    Merge pull request #276 from romanblanco/bz1559159
    
    Setting the default_value for TagControl dialog field
    
    (cherry picked from commit e9a728076a8b002c10d2dc121e0a8428c31d1db1)

@miq-bot add_label gaprindashvili/backported
@miq-bot remove_label gaprindashvili/yes

himdel pushed a commit that referenced this pull request May 22, 2018
Setting the default_value for TagControl dialog field

(cherry picked from commit e9a7280)
@himdel
Copy link
Contributor

himdel commented May 22, 2018

Released in 1.0.28

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants