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

Return Not Found on Snapshots Delete actions #15489

Merged
merged 1 commit into from
Jul 5, 2017
Merged

Return Not Found on Snapshots Delete actions #15489

merged 1 commit into from
Jul 5, 2017

Conversation

jntullo
Copy link

@jntullo jntullo commented Jun 30, 2017

Currently the resource search for a snapshot happens within the rescue block, which means any resource search errors will be returned as a 200 with a "success":false rather than the expected 400 Not Found. This PR moves the resource search out of the rescue block.

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

@miq-bot add_label api, bug, fine/yes
cc: @imtayadeway

@@ -262,6 +279,29 @@
expect(response.parsed_body).to include(expected)
expect(response).to have_http_status(:ok)
end

it "raises a 404 with proper message if a resource isn't found" do
api_basic_authorize(action_identifier(:vms, :delete, :snapshots_subresource_actions, :delete))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you want the :post action here, and :snaphots_subcollection_actions (looks like this might be wrong in some of the examples above)

@miq-bot
Copy link
Member

miq-bot commented Jun 30, 2017

Checked commit jntullo@3ec4914 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@abellotti
Copy link
Member

LGTM @jntullo Thanks for fixing this. 😍

@abellotti abellotti merged commit 0a3088e into ManageIQ:master Jul 5, 2017
@abellotti abellotti added this to the Sprint 64 Ending Jul 10, 2017 milestone Jul 5, 2017
simaishi pushed a commit that referenced this pull request Aug 4, 2017
…e_snapshot

Return Not Found on Snapshots Delete actions
(cherry picked from commit 0a3088e)

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

simaishi commented Aug 4, 2017

Fine backport details:

$ git log -1
commit db4182ec83c9048aa2783c01fc653d5af8bbbe56
Author: Alberto Bellotti <[email protected]>
Date:   Wed Jul 5 12:10:41 2017 -0400

    Merge pull request #15489 from jntullo/bz/inconsistent_response_delete_snapshot
    
    Return Not Found on Snapshots Delete actions
    (cherry picked from commit 0a3088e9f403bbe7432f4e6691319af88001dbcb)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1478506

@jntullo jntullo deleted the bz/inconsistent_response_delete_snapshot branch November 28, 2017 19:42
d-m-u pushed a commit to d-m-u/manageiq that referenced this pull request Jun 6, 2018
…se_delete_snapshot

Return Not Found on Snapshots Delete actions
(cherry picked from commit 0a3088e)

https://bugzilla.redhat.com/show_bug.cgi?id=1478506
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