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> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This commit is contained in:
parent
40e841da98
commit
9dbf0ff0e0
|
|
@ -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
|
||||
}
|
||||
)->store;
|
||||
$claim->discard_changes;
|
||||
|
||||
return $c->render(
|
||||
status => 200,
|
||||
openapi => $claim->to_api
|
||||
);
|
||||
$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 } );
|
||||
|
||||
$data->{created_on} = $c_dt->iso8601;
|
||||
$data->{updated_on} = $u_dt->iso8601;
|
||||
|
||||
return $c->render( openapi => $data, status => 200 );
|
||||
}
|
||||
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 {
|
||||
|
|
|
|||
|
|
@ -215,7 +215,7 @@
|
|||
"application/json"
|
||||
],
|
||||
"responses": {
|
||||
"200": {
|
||||
"204": {
|
||||
"description": "Claim deleted",
|
||||
"schema": {
|
||||
"$ref": "../definitions.json#/return_claim"
|
||||
|
|
|
|||
|
|
@ -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' );
|
||||
|
|
|
|||
Loading…
Reference in New Issue