-
Notifications
You must be signed in to change notification settings - Fork 143
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
transformation mappings edit
action
#467
transformation mappings edit
action
#467
Conversation
@abellotti Can you review this? |
f043c1b
to
44a24c7
Compare
transformation_mapping = fetch_transformation_mapping(type, id) | ||
|
||
updated_data = data.except("transformation_mapping_items") | ||
transformation_mapping.update_attributes!(updated_data) if data.present? |
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.
shouldn't this be if updated_data.present ?
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.
Yes, correct.
I have fixed that.
@@ -49,5 +63,9 @@ def fetch_mapping_resource(href) | |||
raise "Invalid source or destination type #{resource_href.subject}" unless collection_config.collection?(resource_href.subject) | |||
resource_search(resource_href.subject_id, resource_href.subject, collection_class(resource_href.subject.to_sym)) | |||
end | |||
|
|||
def fetch_transformation_mapping(type, id) | |||
resource_search(id, type, collection_class(type)) |
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.
I'd drop this method since it's a one-liner and just call resource_search.
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.
Ok, makes sense
api_basic_authorize(action_identifier(:transformation_mappings, :edit, :resource_actions)) | ||
|
||
request = { | ||
'action' => 'edit', |
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.
maybe use " for consistency with the rest of the hash.
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.
Sure
44a24c7
to
6cbadf2
Compare
Checked commits AparnaKarve/manageiq-api@3e579e4~...6cbadf2 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
@abellotti I have made the suggested changes. Thanks! |
@abellotti This should be GTG -- seems to work OK from the UI |
Thanks @AparnaKarve for the API Enhancement. This LGTM!! 👍 |
EDIT API for transformation_mappings