Bugzilla – Attachment 94852 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: Make controller methods rely on the stashed user
Bug-14697-Make-controller-methods-rely-on-the-stas.patch (text/plain), 9.03 KB, created by
Kyle M Hall (khall)
on 2019-10-29 19:21:09 UTC
(
hide
)
Description:
Bug 14697: Make controller methods rely on the stashed user
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2019-10-29 19:21:09 UTC
Size:
9.03 KB
patch
obsolete
>From 17ea3d2c2fc7eef38e4173065aa5bcdb4b64839e Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Tue, 29 Oct 2019 10:27:18 -0300 >Subject: [PATCH] Bug 14697: Make controller methods rely on the stashed user > >This patch adjusts the return values and HTTP status codes, as well as >removing the use of C4::Context->userenv. It also makes the date >calculation happen on the DB engine in the case of resolving the claim. > >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 | 122 ++++++++++++------------ > api/v1/swagger/paths/return_claims.json | 2 +- > t/db_dependent/api/v1/return_claims.t | 19 ++-- > 3 files changed, 75 insertions(+), 68 deletions(-) > >diff --git a/Koha/REST/V1/ReturnClaims.pm b/Koha/REST/V1/ReturnClaims.pm >index 1bed731344..c497e01561 100644 >--- a/Koha/REST/V1/ReturnClaims.pm >+++ b/Koha/REST/V1/ReturnClaims.pm >@@ -29,7 +29,9 @@ use Koha::DateUtils qw( dt_from_string output_pref ); > > Koha::REST::V1::ReturnClaims > >-=head2 Operations >+=head1 API >+ >+=head2 Methods > > =head3 claim_returned > >@@ -53,7 +55,7 @@ sub claim_returned { > my $checkout = Koha::Checkouts->find( { itemnumber => $itemnumber } ); > > return $c->render( >- openapi => { error => "Not found - Checkout not found" }, >+ openapi => { error => "Checkout not found" }, > status => 404 > ) unless $checkout; > >@@ -100,52 +102,45 @@ Update the notes of an existing claim > =cut > > sub update_notes { >- my $c = shift->openapi->valid_input or return; >- my $input = $c->validation->output; >- my $body = $c->validation->param('body'); >+ my $c = shift->openapi->valid_input or return; >+ >+ my $claim_id = $c->validation->param('claim_id'); >+ my $body = $c->validation->param('body'); >+ >+ my $claim = Koha::Checkouts::ReturnClaims->find( $claim_id ); >+ >+ return $c->render( >+ status => 404, >+ openapi => { >+ error => "Claim not found" >+ } >+ ) unless $claim; > > return try { >- my $id = $input->{claim_id}; > my $updated_by = $body->{updated_by}; > my $notes = $body->{notes}; > >- $updated_by ||= >- C4::Context->userenv ? C4::Context->userenv->{number} : undef; >- >- my $claim = Koha::Checkouts::ReturnClaims->find($id); >- >- return $c->render( >- openapi => { error => "Not found - Claim not found" }, >- status => 404 >- ) unless $claim; >+ my $user = $c->stash('koha.user'); >+ $updated_by //= $user->borrowernumber; > > $claim->set( > { > notes => $notes, >- updated_by => $updated_by, >- updated_on => dt_from_string(), >+ updated_by => $updated_by > } >- ); >- $claim->store(); >- >- my $data = $claim->unblessed; >- >- my $c_dt = dt_from_string( $data->{created_on} ); >- my $u_dt = dt_from_string( $data->{updated_on} ); >- >- $data->{created_on_formatted} = output_pref( { dt => $c_dt } ); >- $data->{updated_on_formatted} = output_pref( { dt => $u_dt } ); >+ )->store; >+ $claim->discard_changes; > >- $data->{created_on} = $c_dt->iso8601; >- $data->{updated_on} = $u_dt->iso8601; >- >- return $c->render( openapi => $data, status => 200 ); >+ return $c->render( >+ status => 200, >+ openapi => $claim->to_api >+ ); > } > catch { >- if ( $_->isa('DBIx::Class::Exception') ) { >+ if ( $_->isa('Koha::Exceptions::Exception') ) { > return $c->render( > status => 500, >- openapi => { error => $_->{msg} } >+ openapi => { error => "$_" } > ); > } > else { >@@ -165,40 +160,46 @@ Marks a claim as resolved > > sub resolve_claim { > my $c = shift->openapi->valid_input or return; >- my $input = $c->validation->output; >- my $body = $c->validation->param('body'); >+ >+ my $claim_id = $c->validation->param('claim_id'); >+ my $body = $c->validation->param('body'); >+ >+ my $claim = Koha::Checkouts::ReturnClaims->find($claim_id); >+ >+ return $c->render( >+ status => 404, >+ openapi => { >+ error => "Claim not found" >+ } >+ ) unless $claim; > > return try { >- my $id = $input->{claim_id}; >+ > my $resolved_by = $body->{updated_by}; > my $resolution = $body->{resolution}; > >- $resolved_by ||= >- C4::Context->userenv ? C4::Context->userenv->{number} : undef; >- >- my $claim = Koha::Checkouts::ReturnClaims->find($id); >- >- return $c->render( >- openapi => { error => "Not found - Claim not found" }, >- status => 404 >- ) unless $claim; >+ my $user = $c->stash('koha.user'); >+ $resolved_by //= $user->borrowernumber; > > $claim->set( > { > resolution => $resolution, > resolved_by => $resolved_by, >- resolved_on => dt_from_string(), >+ resolved_on => \'NOW()', > } >- ); >- $claim->store(); >+ )->store; >+ $claim->discard_changes; > >- return $c->render( openapi => $claim, status => 200 ); >+ return $c->render( >+ status => 200, >+ openapi => $claim->to_api >+ ); > } > catch { >- if ( $_->isa('DBIx::Class::Exception') ) { >+ if ( $_->isa('Koha::Exceptions::Exception') ) { > return $c->render( > status => 500, >- openapi => { error => $_->{msg} } >+ openapi => { error => "$_" } > ); > } > else { >@@ -217,28 +218,29 @@ Deletes the claim from the database > =cut > > sub delete_claim { >- my $c = shift->openapi->valid_input or return; >- my $input = $c->validation->output; >+ my $c = shift->openapi->valid_input or return; > > return try { >- my $id = $input->{claim_id}; > >- my $claim = Koha::Checkouts::ReturnClaims->find($id); >+ my $claim = Koha::Checkouts::ReturnClaims->find( $c->validation->param('claim_id') ); > > return $c->render( >- openapi => { error => "Not found - Claim not found" }, >- status => 404 >+ status => 404, >+ openapi => { error => "Claim not found" } > ) unless $claim; > > $claim->delete(); > >- return $c->render( openapi => $claim, status => 200 ); >+ return $c->render( >+ status => 204, >+ openapi => {} >+ ); > } > catch { >- if ( $_->isa('DBIx::Class::Exception') ) { >+ if ( $_->isa('Koha::Exceptions::Exception') ) { > return $c->render( > status => 500, >- openapi => { error => $_->{msg} } >+ openapi => { error => "$_" } > ); > } > else { >diff --git a/api/v1/swagger/paths/return_claims.json b/api/v1/swagger/paths/return_claims.json >index 1104bf1666..b711f957bc 100644 >--- a/api/v1/swagger/paths/return_claims.json >+++ b/api/v1/swagger/paths/return_claims.json >@@ -215,7 +215,7 @@ > "application/json" > ], > "responses": { >- "200": { >+ "204": { > "description": "Claim deleted", > "schema": { > "$ref": "../definitions.json#/return_claim" >diff --git a/t/db_dependent/api/v1/return_claims.t b/t/db_dependent/api/v1/return_claims.t >index 975c0ec12a..986ff512af 100644 >--- a/t/db_dependent/api/v1/return_claims.t >+++ b/t/db_dependent/api/v1/return_claims.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 27; >+use Test::More tests => 32; > use Test::MockModule; > use Test::Mojo; > use Test::Warn; >@@ -88,7 +88,8 @@ $t->post_ok( > created_by => $librarian->id, > notes => "This is a test note." > } >-)->status_is(404); >+)->status_is(404) >+ ->json_is( '/error' => 'Checkout not found' ); > > ## Valid id > $t->post_ok( >@@ -135,7 +136,8 @@ $t->put_ok( > notes => "This is a different test note.", > updated_by => $librarian->id, > } >-)->status_is(404); >+)->status_is(404) >+ ->json_is( '/error' => 'Claim not found' ); > > # Resolve a claim > ## Valid claim id >@@ -156,13 +158,16 @@ $t->put_ok( > resolved_by => $librarian->id, > resolution => "FOUNDINLIB", > } >-)->status_is(404); >+)->status_is(404) >+ ->json_is( '/error' => 'Claim not found' ); > > # Test deleting a return claim >-$t = $t->delete_ok("//$userid:$password@/api/v1/return_claims/$claim_id") >- ->status_is(200); >+$t->delete_ok("//$userid:$password@/api/v1/return_claims/$claim_id") >+ ->status_is( 204, 'SWAGGER3.2.4' ) >+ ->content_is( '', 'SWAGGER3.3.4' ); > $claim = Koha::Checkouts::ReturnClaims->find($claim_id); > isnt( $claim, "Return claim was deleted" ); > > $t->delete_ok("//$userid:$password@/api/v1/return_claims/$claim_id") >- ->status_is(404); >+ ->status_is(404) >+ ->json_is( '/error' => 'Claim not found' ); >-- >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