Specify a target and resource when refreshing a dialog field #233
Conversation
eclarizio
force-pushed
the
dialog_refresh_targets
branch
from
2aa886a
to
8c28972
Compare
eclarizio
force-pushed
the
dialog_refresh_targets
branch
from
8c28972
to
0cbec91
Compare
eclarizio
changed the title
| data ||= {} | ||
| dialog_fields = Hash(data["dialog_fields"]) | ||
| refresh_fields = data["fields"] | ||
| return action_result(false, "Must specify fields to refresh") if refresh_fields.blank? | ||
|
|
||
| define_service_dialog_fields(service_dialog, dialog_fields) | ||
| service_dialog = define_service_dialog(dialog_fields, data) |
There was a problem hiding this comment.
with the service_dialog no longer passed in as parameter (which was RBAC checked earlier for the user via resource_search), is it possible we'll be updating a different service_dialog (data driven and not URL) ?
There was a problem hiding this comment.
I thought about that too, but the way the classic-ui was doing the refresh before was pulling the dialog off of the resource_action, since it belongs to it. So I guess I could envision a scenario where someone could call, say, /api/service_dialogs/123 with action set to refresh, and pass in a bogus resource_action_id that doesn't correspond to the dialog with the id of 123. We could compare ids, and return an error if they don't match, maybe?
There was a problem hiding this comment.
maybe pass the service_dialog to refresh_dialog_fields_service_dialog and have it check there against the one defined in data and raise an error accordingly, (with added test for that). Thanks.
|
@abellotti, addressed your concerns, added specs. It definitely would have been possible (although very unlikely) that the resource action id could have pointed to a different dialog, but when trying to update the fields, unless there was a field of the same name to attempt to be updated in the different dialog, the API would have returned an error as well. This way, though, we should be safe if somehow that does happen. |
|
Thanks @eclarizio just the picky rubocop ^ |
eclarizio
force-pushed
the
dialog_refresh_targets
branch
from
98b24e8
to
574e63d
Compare
|
Checked commits eclarizio/manageiq-api@0cbec91~...574e63d with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0 |
|
@eclarizio thanks for the update. @eclarizio @gmcculloug Is this g/yes ? |
|
@miq-bot add_label gaprindashvili/yes |
Specify a target and resource when refreshing a dialog field (cherry picked from commit d56b2a6) https://bugzilla.redhat.com/show_bug.cgi?id=1520678
|
Gaprindashvili backport details: |
Related to ManageIQ/manageiq-ui-classic#2885, this needs to be merged first so that the API then supports the parameters that are being passed in for refresh.
https://bugzilla.redhat.com/show_bug.cgi?id=1518390