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

Allow partial POST edits on miq policy REST #14518

Merged
merged 1 commit into from
Mar 29, 2017

Conversation

moolitayer
Copy link

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

policies API (MiqPolicy) should allow partial POTS edits
e.g

POST localhost:3000/api/policies/1000000000007
{
  "action": "edit",
  "description": "Desc bar"
}

In addition the error message is wrong as it is about creation not edit:

{
  "error": {
    "kind": "bad_request",
    "message": "Resource conditions_ids, policy_contents needs be specified for creating a new policies",
    "klass": "Api::BadRequestError"
  }
}

@moolitayer moolitayer changed the title Allow partial POST edits on miq policy rest Allow partial POST edits on miq policy REST Mar 27, 2017
@moolitayer
Copy link
Author

@miq-bot add_label bug, api, control

@moolitayer
Copy link
Author

@dkorn can you please review?

Copy link
Contributor

@dkorn dkorn left a comment

Choose a reason for hiding this comment

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

LGTM :)

policy = resource_search(id, type, collection_class(:policies))
begin
add_policies_content(data, policy)
add_policies_content(data, policy) if data["policy_contents"]
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@moolitayer
Copy link
Author

@abellotti please review

@abellotti
Copy link
Member

I think this is fine, could you add a test for editing a policy without those previously required attributes, maybe editing name or description ? Thanks.

@abellotti abellotti self-assigned this Mar 27, 2017
Fix https://bugzilla.redhat.com/show_bug.cgi?id=1435777

policies API (MiqPolicy) should allow partial POST edits
e.g

POST localhost:3000/api/policies/1000000000007
{
  "action": "edit",
  "description": "Desc bar"
}
In addition the current error message is wrong as it is about creation not edit:

{
  "error": {
    "kind": "bad_request",
    "message": "Resource conditions_ids, policy_contents needs be specified for creating a new policies",
    "klass": "Api::BadRequestError"
  }
}
@miq-bot
Copy link
Member

miq-bot commented Mar 28, 2017

Checked commit moolitayer@8cb8017 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. 🍰

@moolitayer
Copy link
Author

@abellotti please review

@abellotti
Copy link
Member

Thanks for adding the test, LGTM!! Is this for fix needed for 5.7 (fine release) too, I see the BZ for 5.8.

@moolitayer
Copy link
Author

We have no specific need for it in 5.7 (euwe?) ATM as we are not using this API outside yet. Thanks

@abellotti
Copy link
Member

sorry my confusion, I see the ticket is 5.8.

@abellotti abellotti merged commit 6872ded into ManageIQ:master Mar 29, 2017
@abellotti abellotti added this to the Sprint 58 Ending Apr 10, 2017 milestone Mar 29, 2017
simaishi pushed a commit that referenced this pull request Mar 31, 2017
Allow partial POST edits on miq policy REST
(cherry picked from commit 6872ded)

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

Fine backport details:

$ git log -1
commit b402b859f454656abb2dd2d61f1dbb392e6aedf1
Author: Alberto Bellotti <abellotti@users.noreply.github.com>
Date:   Wed Mar 29 10:19:20 2017 -0400

    Merge pull request #14518 from moolitayer/policy_edit
    
    Allow partial POST edits on miq policy REST
    (cherry picked from commit 6872ded18179536a845e2e3b26658d1f5a8eb7ad)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1435777

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.

5 participants