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.

Sign up for GitHub

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

Jump to bottom

Edit VMs API #14623

Merged
merged 2 commits into from
Apr 5, 2017
Merged

Edit VMs API #14623

merged 2 commits into from
Apr 5, 2017

Conversation

jntullo
Copy link

@jntullo jntullo commented Apr 3, 2017 •
edited by abellotti
Loading

Edit API to update basic attributes (custom_1, description) and parent, child relationships. These attributes align with what is currently allowed when editing a VM in the OpsUI.

{
   "custom_1": "new_custom_identifier",
   "description": "new_description",
   "parent_resource": { "href": <parent url> },
   "child_resources": [ {"href":<child url>}, {"href": <child url>} ] 
}

@miq-bot add_label enhancement, wip, api
@miq-bot assign @abellotti

@jntullo jntullo changed the title [WIP] Edit VMs API Edit VMs API Apr 4, 2017
@jntullo
Copy link
Author

jntullo commented Apr 4, 2017

@miq-bot remove_label wip

@miq-bot miq-bot removed the wip label Apr 4, 2017
@jntullo jntullo mentioned this pull request Apr 4, 2017
Merged
abellotti
abellotti reviewed Apr 5, 2017
View reviewed changes
Copy link
Member

@abellotti abellotti left a comment

Choose a reason for hiding this comment

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

Looks good @jntullo minor change to naming.

app/controllers/api/vms_controller.rb Outdated
@@ -8,6 +8,9 @@ class VmsController < BaseController
include Subcollections::Software
include Subcollections::Snapshots

VALID_EDIT_ATTRS = %w(description child_relationships parent_relationship).freeze
Copy link
Member

Choose a reason for hiding this comment

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

would prefer child_resources and parent_resource for consistency with other _resource exposure in API calls.

Copy link
Author

Choose a reason for hiding this comment

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

@abellotti will do! I'll update ##14639 as well

app/controllers/api/vms_controller.rb

def fetch_relationship(href)
collection, id = parse_href(href)
raise "Invalid relationship type #{collection}" unless RELATIONSHIP_COLLECTIONS.include?(collection)
Copy link
Member

Choose a reason for hiding this comment

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

👍 this way we control what gets assigned.

@miq-bot
Copy link
Member

miq-bot commented Apr 5, 2017

Checked commits jntullo/manageiq@ec696a2~...619e277 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. ⭐

@abellotti abellotti added this to the Sprint 58 Ending Apr 10, 2017 milestone Apr 5, 2017
@abellotti
Copy link
Member

LGTM!! Thanks @jntullo

@abellotti abellotti merged commit ef497d0 into ManageIQ:master Apr 5, 2017
@abellotti
Copy link
Member

abellotti commented Apr 18, 2017 •
edited
Loading

@jntullo let me know of the BZ for 4.2.3, we can then set the euwe/yes, Thanks.

simaishi pushed a commit that referenced this pull request Apr 20, 2017
@abellotti @simaishi
Merge pull request #14623 from jntullo/enhancement/edit_vm
cc10b25 
Edit VMs API
(cherry picked from commit ef497d0)
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit cc10b255bd08a8aaa1991ee0caed053c06b3bca9
Author: Alberto Bellotti <[email protected]>
Date:   Wed Apr 5 16:55:58 2017 -0400

    Merge pull request #14623 from jntullo/enhancement/edit_vm
    
    Edit VMs API
    (cherry picked from commit ef497d01cd21d0da36636366a0c6c5aeecc2e2c5)

@simaishi
Copy link
Contributor

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

@jntullo
Copy link
Author

jntullo commented May 26, 2017

This should be able to be backported to euwe
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1445356

@miq-bot add_label euwe/yes

simaishi pushed a commit that referenced this pull request May 26, 2017
@abellotti @simaishi
Merge pull request #14623 from jntullo/enhancement/edit_vm
a62bb53 
Edit VMs API
(cherry picked from commit ef497d0)

https://bugzilla.redhat.com/show_bug.cgi?id=1445356
@simaishi
Copy link
Contributor

Euwe backport details:

$ git log -1
commit a62bb533be6ca328b0637264b3a7646ec14af03f
Author: Alberto Bellotti <[email protected]>
Date:   Wed Apr 5 16:55:58 2017 -0400

    Merge pull request #14623 from jntullo/enhancement/edit_vm
    
    Edit VMs API
    (cherry picked from commit ef497d01cd21d0da36636366a0c6c5aeecc2e2c5)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1445356

@jntullo jntullo deleted the enhancement/edit_vm branch November 28, 2017 19:41
@himdel himdel mentioned this pull request Mar 9, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abellotti abellotti abellotti left review comments

Assignees

@abellotti abellotti

Labels
api enhancement euwe/backported fine/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jntullo @miq-bot @abellotti @simaishi