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

Use '.name' instead of '.id' for refresh field list #228

Merged
merged 1 commit into from Jan 8, 2018

Conversation

eclarizio
Copy link
Member

@eclarizio eclarizio commented Jan 5, 2018

The problem is that when creating or editing dialogs, if you add a new field, the field is not saved before the associations are created, and so there is no id to send up. Therefore we have to send up the name and have the backend handle the association creation by name instead.

This will need to be combined with this main repo PR in order to fully fix the bug.

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

@miq-bot add_label gaprindashvili/yes, bug

@romanblanco @himdel Please review

@himdel
Copy link
Contributor

himdel commented Jan 5, 2018

All those <dialog-editor-modal-field-template...> need a closing tag..

EDIT: fixed 👍

@himdel
Copy link
Contributor

himdel commented Jan 5, 2018

Nice DRYing up 👍
Looks like what actually changes is..

-ng-options="dynamicField.id as dynamicField.label for dynamicField in vm.modalData.dynamicFieldList"
+ng-options="dynamicField.name as dynamicField.label for dynamicField in vm.modalData.dynamicFieldList"

@miq-bot
Copy link
Member

miq-bot commented Jan 5, 2018

Checked commit eclarizio@15d190e with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. ⭐

Copy link
Member

@romanblanco romanblanco left a comment

Choose a reason for hiding this comment

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

@eclarizio The fix does not work for me. I'm getting error messages in console after trying to edit an existing dialog:

TypeError
columnNumber: 13
fileName: "http://localhost:3000/assets/manageiq-ui-components/dist/js/ui-components.self-b957ce520f3f74eea649dcfe3bfa869caca663ad12dd05297d9a5edc6f9d7234.js?body=1"
lineNumber: 2094
message: "field.values is undefined"

EDIT: my mistake, works good

Copy link
Member

@romanblanco romanblanco left a comment

Choose a reason for hiding this comment

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

I've found problem on my site. Works good 👍

@himdel himdel self-assigned this Jan 8, 2018
@himdel himdel added this to the Sprint 77 Ending Jan 15, 2018 milestone Jan 8, 2018
@himdel himdel merged commit 22a72f6 into ManageIQ:master Jan 8, 2018
@himdel
Copy link
Contributor

himdel commented Jan 9, 2018

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

This was released in manageiq-ui-components 1.0.5, backported to gaprindashvili via ManageIQ/manageiq-ui-classic#3203 .

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.

None yet

4 participants