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

raise not found error on DELETE of authentications #15736

Closed
wants to merge 1 commit into from
Closed

raise not found error on DELETE of authentications #15736

wants to merge 1 commit into from

Conversation

jntullo
Copy link

@jntullo jntullo commented Aug 4, 2017

DELETE /api/authentications/:id should raise a not found for invalid authentications. It is currently sending back a 204 as if it's a success due to the catching of the exception and returning it as an action result.

This is inconsistent with other areas of the API where DELETE returns a not found for invalid records.

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

@miq-bot add_label bug, api
@miq-bot assign @abellotti

@miq-bot
Copy link
Member

miq-bot commented Aug 4, 2017

Checked commit jntullo@050aaf9 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

hmm, for action returns signatures, don't we always return that signature ? i.e. if the 4th out of 10 resources in a bulk operation does not exist do we return a not found for the whole lot or just the 4th resource (i.e. as it is done now)

@jntullo
Copy link
Author

jntullo commented Aug 7, 2017

@abellotti this isn't for bulk operations, it's just for DELETE. Bulk operations will remain the same, which is just returning the action response. But for DELETE it is acting as if the resource was found due to the action response, which it is not, and this change corrects that.

@miq-bot
Copy link
Member

miq-bot commented Aug 7, 2017

This pull request is not mergeable. Please rebase and repush.

@jntullo jntullo closed this Aug 10, 2017
@jntullo jntullo deleted the bz/raise_not_found_authentication branch November 28, 2017 19:41
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.

3 participants