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

Fix for incorrect key sent back for dynamic non sorted items #18650

Merged

Conversation

eclarizio
Copy link
Member

@eclarizio eclarizio commented Apr 11, 2019

As described in the below linked BZ, there was an issue when verifying the original fix which can be found here on the ui-components repo.

Basically, for dynamic dialogs, we were returning the dynamically computed result in a key called "values". But, this does not really make sense for text boxes, check boxes, or date/times because those fields can only have one value, not a list of values. So, this PR changes that so that we can still continue to use ng-model=vm.dialogField.default_value which makes the most sense while also passing through the correctly computed default_value from the automate side for those various types of dialog fields.

This will also need this other ui-components PR to revert the ng-model changes made in 381.

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

/cc @tinaafitz @d-m-u @himdel @gmcculloug

@eclarizio eclarizio force-pushed the fix_for_dynamic_fields_not_being_updated branch 5 times, most recently from c1bba2c to 03e58f4 Compare April 11, 2019 20:05
@eclarizio eclarizio force-pushed the fix_for_dynamic_fields_not_being_updated branch from 03e58f4 to b542dad Compare April 11, 2019 21:27
@miq-bot
Copy link
Member

miq-bot commented Apr 11, 2019

Checked commit eclarizio@b542dad with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

@h-kataria h-kataria self-assigned this Apr 11, 2019
@h-kataria h-kataria added this to the Sprint 109 Ending Apr 15, 2019 milestone Apr 11, 2019
@h-kataria h-kataria merged commit 8cf922f into ManageIQ:master Apr 11, 2019
@JPrause
Copy link
Member

JPrause commented Apr 12, 2019

@eclarizio if #18652 is hammer specific, then it appears we can remove the hammer/yes label on this PR. Correct?

@JPrause
Copy link
Member

JPrause commented Apr 12, 2019

@miq-bot remove_label hammer/yes

@simaishi
Copy link
Contributor

Backported to Hammer via #18652

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.

6 participants