Bugzilla – Attachment 94851 Details for
Bug 14697
Extend and enhance "Claims returned" lost status
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 14697: (follow-up) Rely on the UNIQUE constraint and return 409 for issue_id
Bug-14697-follow-up-Rely-on-the-UNIQUE-constraint-.patch (text/plain), 4.79 KB, created by
Kyle M Hall
on 2019-10-29 19:21:04 UTC
(
hide
)
Description:
Bug 14697: (follow-up) Rely on the UNIQUE constraint and return 409 for issue_id
Filename:
MIME Type:
Creator:
Kyle M Hall
Created:
2019-10-29 19:21:04 UTC
Size:
4.79 KB
patch
obsolete
>From 2be8f9d395448cd05b9886c63ee90d706dda1647 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Mon, 28 Oct 2019 16:18:52 -0300 >Subject: [PATCH] Bug 14697: (follow-up) Rely on the UNIQUE constraint and > return 409 for issue_id > >This patch avoids querying the DB for an already existing >Koha::Checkouts::ReturnClaim with the same issue_id, now that there's a >UNIQUE constraint on it. > >Also, 409 should be returned instead. Tests added for this changes. > >To test: >- Apply this patch >- Run: > $ kshell > k$ prove t/db_dependent/api/v1/return_claims.t >=> SUCCESS: tests pass! > >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> > >Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> > >Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com> >--- > Koha/REST/V1/ReturnClaims.pm | 22 +++++++++------------- > api/v1/swagger/paths/return_claims.json | 14 ++++++++++---- > t/db_dependent/api/v1/return_claims.t | 24 +++++++++++++++--------- > 3 files changed, 34 insertions(+), 26 deletions(-) > >diff --git a/Koha/REST/V1/ReturnClaims.pm b/Koha/REST/V1/ReturnClaims.pm >index d221518650..1bed731344 100644 >--- a/Koha/REST/V1/ReturnClaims.pm >+++ b/Koha/REST/V1/ReturnClaims.pm >@@ -57,17 +57,7 @@ sub claim_returned { > status => 404 > ) unless $checkout; > >- my $claim = Koha::Checkouts::ReturnClaims->find( >- { >- issue_id => $checkout->id >- } >- ); >- return $c->render( >- openapi => { error => "Bad request - claim exists" }, >- status => 400 >- ) if $claim; >- >- $claim = $checkout->claim_returned( >+ my $claim = $checkout->claim_returned( > { > charge_lost_fee => $charge_lost_fee, > created_by => $created_by, >@@ -82,12 +72,18 @@ sub claim_returned { > ); > } > catch { >- if ( $_->isa('Koha::Exceptions::Checkouts::ReturnClaims') ) { >+ if ( $_->isa('Koha::Exceptions::Object::DuplicateID') ) { > return $c->render( >- status => 500, >+ status => 409, > openapi => { error => "$_" } > ); > } >+ elsif ( $_->isa('Koha::Exceptions::Checkouts::ReturnClaims::NoCreatedBy') ) { >+ return $c->render( >+ status => 400, >+ openapi => { error => "Mandatory attribute created_by missing" } >+ ); >+ } > else { > return $c->render( > status => 500, >diff --git a/api/v1/swagger/paths/return_claims.json b/api/v1/swagger/paths/return_claims.json >index 068176f1bb..1104bf1666 100644 >--- a/api/v1/swagger/paths/return_claims.json >+++ b/api/v1/swagger/paths/return_claims.json >@@ -67,10 +67,16 @@ > } > }, > "404": { >- "description": "Checkout not found", >- "schema": { >- "$ref": "../definitions.json#/error" >- } >+ "description": "Checkout not found", >+ "schema": { >+ "$ref": "../definitions.json#/error" >+ } >+ }, >+ "409": { >+ "description": "Conflict creating the resource", >+ "schema": { >+ "$ref": "../definitions.json#/error" >+ } > }, > "500": { > "description": "Internal server error", >diff --git a/t/db_dependent/api/v1/return_claims.t b/t/db_dependent/api/v1/return_claims.t >index 073bbd5eba..975c0ec12a 100644 >--- a/t/db_dependent/api/v1/return_claims.t >+++ b/t/db_dependent/api/v1/return_claims.t >@@ -17,9 +17,10 @@ > > use Modern::Perl; > >-use Test::More tests => 25; >+use Test::More tests => 27; > use Test::MockModule; > use Test::Mojo; >+use Test::Warn; > use t::lib::Mocks; > use t::lib::TestBuilder; > >@@ -97,18 +98,23 @@ $t->post_ok( > created_by => $librarian->id, > notes => "This is a test note." > } >-)->status_is(201); >+)->status_is(201) >+ ->header_like( Location => qr|^\/api\/v1\/return_claims/\d*|, 'SWAGGER3.4.1'); >+ > my $claim_id = $t->tx->res->json->{claim_id}; > > ## Duplicate id >-$t->post_ok( >- "//$userid:$password@/api/v1/return_claims" => json => { >- item_id => $itemnumber1, >- charge_lost_fee => Mojo::JSON->false, >- created_by => $librarian->id, >- notes => "This is a test note." >+warning_like { >+ $t->post_ok( >+ "//$userid:$password@/api/v1/return_claims" => json => { >+ item_id => $itemnumber1, >+ charge_lost_fee => Mojo::JSON->false, >+ created_by => $librarian->id, >+ notes => "This is a test note." >+ } >+ )->status_is(409) > } >-)->status_is(400); >+ qr/^DBD::mysql::st execute failed: Duplicate entry/; > > # Test editing a claim note > ## Valid claim id >-- >2.21.0 (Apple Git-122)
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 14697
:
92407
|
92408
|
92409
|
92410
|
92962
|
92963
|
92964
|
92965
|
93005
|
93006
|
93007
|
93008
|
93012
|
93013
|
93014
|
93015
|
93017
|
93038
|
93039
|
93040
|
93041
|
93042
|
93061
|
93062
|
93063
|
93064
|
93065
|
93952
|
93953
|
93954
|
93955
|
93956
|
93957
|
93959
|
94842
|
94843
|
94844
|
94845
|
94846
|
94847
|
94848
|
94849
|
94850
| 94851 |
94852
|
94853
|
94854
|
94857
|
94877
|
94878