-
Notifications
You must be signed in to change notification settings - Fork 900
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
[FINE] Union edit service_dialog API call with other calls - removal of cont… #16747
Conversation
…ent key Edit call needed payload: `{..., content: {dialog_tabs: [...]}}`, but all other calls needs `{..., dialog_tabs: [...]}`, just one level removed. This commit unions that, with respect to backward compatibility.
It'll be needed to update documentation as well |
Problem is described at bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1532200 |
Checked commits paulslaby/manageiq@e824f85~...edadb37 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0 |
@paulslaby Is there a 'master' PR for this change? |
I haven’t done it. Should I do it?
12. 1. 2018 v 16:14, Satoe Imaishi <notifications@github.com>:
… @paulslaby Is there a 'master' PR for this change?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
If 'master' is broken, please open a PR for 'master' branch and indicate which branch(es) the PR should be backported by adding |
service_dialog.update!(data.except('content')) | ||
service_dialog.update_tabs(data['content']['dialog_tabs']) if data['content'] | ||
service_dialog.update!(data.except('dialog_tabs', 'content')) | ||
service_dialog.update_tabs(data['dialog_tabs'] || data['content']['dialog_tabs']) if data['dialog_tabs'] || data['content'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if both are specified, we're looking at dialog_tags instead of the current content, is that the expectation ?
also, we need specs updates to test both cases.
/cc @jntullo
I'm closing this PR as 'master' PR is now created: ManageIQ/manageiq-api#285 |
Edit call needed payload:
{..., content: {dialog_tabs: [...]}}
, but all other calls needs{..., dialog_tabs: [...]}
, just one level removed. This commit unions that, with respect to backward compatibility.Bugzilla link: TBD