Bugzilla – Attachment 154429 Details for
Bug 24401
REST API: Check-in
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 24401: (QA follow-up) Tidy Checkouts.pm
Bug-24401-QA-follow-up-Tidy-Checkoutspm.patch (text/plain), 11.73 KB, created by
Martin Renvoize (ashimema)
on 2023-08-15 12:17:23 UTC
(
hide
)
Description:
Bug 24401: (QA follow-up) Tidy Checkouts.pm
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2023-08-15 12:17:23 UTC
Size:
11.73 KB
patch
obsolete
>From 5e23a9e0c816569785e081bf349a3688ebc93403 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Tue, 15 Aug 2023 13:06:38 +0100 >Subject: [PATCH] Bug 24401: (QA follow-up) Tidy Checkouts.pm > >This just tidies the API controller to fit current best practice and >make the QA script happy. >--- > Koha/REST/V1/Checkouts.pm | 122 +++++++++++++++++--------------------- > 1 file changed, 54 insertions(+), 68 deletions(-) > >diff --git a/Koha/REST/V1/Checkouts.pm b/Koha/REST/V1/Checkouts.pm >index 179e5d2573..8ac46fe909 100644 >--- a/Koha/REST/V1/Checkouts.pm >+++ b/Koha/REST/V1/Checkouts.pm >@@ -52,13 +52,13 @@ sub list { > try { > my $checkouts_set; > >- if ( $checked_in ) { >+ if ($checked_in) { > $checkouts_set = Koha::Old::Checkouts->new; > } else { > $checkouts_set = Koha::Checkouts->new; > } > >- my $checkouts = $c->objects->search( $checkouts_set ); >+ my $checkouts = $c->objects->search($checkouts_set); > > return $c->render( > status => 200, >@@ -79,13 +79,13 @@ sub get { > my $c = shift->openapi->valid_input or return; > > my $checkout_id = $c->param('checkout_id'); >- my $checkout = Koha::Checkouts->find( $checkout_id ); >- $checkout = Koha::Old::Checkouts->find( $checkout_id ) >+ my $checkout = Koha::Checkouts->find($checkout_id); >+ $checkout = Koha::Old::Checkouts->find($checkout_id) > unless ($checkout); > > unless ($checkout) { > return $c->render( >- status => 404, >+ status => 404, > openapi => { error => "Checkout doesn't exist" } > ); > } >@@ -95,8 +95,7 @@ sub get { > status => 200, > openapi => $checkout->to_api > ); >- } >- catch { >+ } catch { > $c->unhandled_exception($_); > }; > } >@@ -115,15 +114,16 @@ sub _check_availability { > my $ignore_reserves = 0; # Don't ignore reserves > my $params = { item => $item }; > >- my ( $impossible, $confirmation, $alerts, $messages ) = >- C4::Circulation::CanBookBeIssued( $patron, undef, undef, $inprocess, >- $ignore_reserves, $params ); >+ my ( $impossible, $confirmation, $alerts, $messages ) = C4::Circulation::CanBookBeIssued( >+ $patron, undef, undef, $inprocess, >+ $ignore_reserves, $params >+ ); > > if ( $c->stash('is_public') ) { > > # Upgrade some confirmations to blockers > my @should_block = >- qw/TOO_MANY ISSUED_TO_ANOTHER RESERVED RESERVED_WAITING TRANSFERRED PROCESSING AGE_RESTRICTION/; >+ qw/TOO_MANY ISSUED_TO_ANOTHER RESERVED RESERVED_WAITING TRANSFERRED PROCESSING AGE_RESTRICTION/; > for my $block (@should_block) { > if ( exists( $confirmation->{$block} ) ) { > $impossible->{$block} = $confirmation->{$block}; >@@ -133,7 +133,7 @@ sub _check_availability { > > # Remove any non-public info that's returned by CanBookBeIssued > my @restricted_keys = >- qw/issued_borrowernumber issued_cardnumber issued_firstname issued_surname resborrowernumber resbranchcode rescardnumber reserve_id resfirstname resreservedate ressurname item_notforloan/; >+ qw/issued_borrowernumber issued_cardnumber issued_firstname issued_surname resborrowernumber resbranchcode rescardnumber reserve_id resfirstname resreservedate ressurname item_notforloan/; > for my $key (@restricted_keys) { > delete $confirmation->{$key}; > delete $impossible->{$key}; >@@ -152,19 +152,18 @@ Controller function that handles retrieval of Checkout availability > =cut > > sub get_availability { >- my $c = shift->openapi->valid_input or return; >+ my $c = shift->openapi->valid_input or return; > my $user = $c->stash('koha.user'); > > my $patron = Koha::Patrons->find( $c->param('patron_id') ); > my $item = Koha::Items->find( $c->param('item_id') ); > >- my ( $impossible, $confirmation, $warnings ) = >- $c->_check_availability( $patron, $item ); >+ my ( $impossible, $confirmation, $warnings ) = $c->_check_availability( $patron, $item ); > > my @confirm_keys = sort keys %{$confirmation}; > unshift @confirm_keys, $item->id; > unshift @confirm_keys, $user->id >- if $user; >+ if $user; > > my $token = Koha::Token->new->generate_jwt( { id => join( ':', @confirm_keys ) } ); > >@@ -185,7 +184,7 @@ Add a new checkout > =cut > > sub add { >- my $c = shift->openapi->valid_input or return; >+ my $c = shift->openapi->valid_input or return; > my $user = $c->stash('koha.user'); > > my $body = $c->req->json; >@@ -228,8 +227,7 @@ sub add { > ); > } > >- my ( $impossible, $confirmation, $warnings ) = >- $c->_check_availability( $patron, $item ); >+ my ( $impossible, $confirmation, $warnings ) = $c->_check_availability( $patron, $item ); > > # * Fail for blockers - render 403 > if ( keys %{$impossible} ) { >@@ -250,8 +248,7 @@ sub add { > if ( my $token = $c->param('confirmation') ) { > my $confirm_keys = join( ":", sort keys %{$confirmation} ); > $confirm_keys = $user->id . ":" . $item->id . ":" . $confirm_keys; >- $confirmed = Koha::Token->new->check_jwt( >- { id => $confirm_keys, token => $token } ); >+ $confirmed = Koha::Token->new->check_jwt( { id => $confirm_keys, token => $token } ); > } > > unless ($confirmed) { >@@ -265,21 +262,18 @@ sub add { > # Call 'AddIssue' > my $checkout = AddIssue( $patron->unblessed, $item->barcode ); > if ($checkout) { >- $c->res->headers->location( >- $c->req->url->to_string . '/' . $checkout->id ); >+ $c->res->headers->location( $c->req->url->to_string . '/' . $checkout->id ); > return $c->render( > status => 201, > openapi => $checkout->to_api > ); >- } >- else { >+ } else { > return $c->render( > status => 500, > openapi => { error => 'Unknown error during checkout' } > ); > } >- } >- catch { >+ } catch { > $c->unhandled_exception($_); > }; > } >@@ -295,39 +289,37 @@ sub delete { > > my $c = shift->openapi->valid_input or return; > >- my $body = $c->validation->param('body'); >- my $item_id = $body->{item_id}; >+ my $body = $c->validation->param('body'); >+ my $item_id = $body->{item_id}; > > return try { > my $item = Koha::Items->find($item_id); > unless ($item) { > return $c->render( > status => 409, >- openapi => { >- error => 'Item not found' >- } >+ openapi => { error => 'Item not found' } > ); > } >- my $checkout = Koha::Checkouts->find({ itemnumber => $item_id }); >- unless ( $checkout ) { >+ my $checkout = Koha::Checkouts->find( { itemnumber => $item_id } ); >+ unless ($checkout) { > return $c->render( >- status => 400, >+ status => 400, > openapi => { error => "Item not checked out" } > ); > } > my $library_id = $c->validation->output->{library_id}; >- unless ( $library_id ) { >- $library_id = $checkout->patron->branchcode; >+ unless ($library_id) { >+ $library_id = $checkout->patron->branchcode; > } >- my $library = Koha::Libraries->find( $library_id ); >+ my $library = Koha::Libraries->find($library_id); > > try { > my $barcode = $item->barcode; >- my ( $returned ) = C4::Circulation::AddReturn( $barcode, $library_id ); >+ my ($returned) = C4::Circulation::AddReturn( $barcode, $library_id ); > >- unless ( $returned ) { >+ unless ($returned) { > return $c->render( >- status => 403, >+ status => 403, > openapi => { error => "Checkin not allowed" } > ); > } >@@ -337,13 +329,11 @@ sub delete { > status => 201, > openapi => $item->to_api > ); >- } >- catch { >+ } catch { > $c->unhandled_exception($_); > }; > } >- } >- >+} > > =head3 get_renewals > >@@ -358,7 +348,7 @@ sub get_renewals { > my $checkout_id = $c->param('checkout_id'); > my $checkout = Koha::Checkouts->find($checkout_id); > $checkout = Koha::Old::Checkouts->find($checkout_id) >- unless ($checkout); >+ unless ($checkout); > > unless ($checkout) { > return $c->render( >@@ -368,19 +358,17 @@ sub get_renewals { > } > > my $renewals_rs = $checkout->renewals; >- my $renewals = $c->objects->search( $renewals_rs ); >+ my $renewals = $c->objects->search($renewals_rs); > > return $c->render( > status => 200, > openapi => $renewals > ); >- } >- catch { >+ } catch { > $c->unhandled_exception($_); > }; > } > >- > =head3 renew > > Renew a checkout >@@ -391,22 +379,22 @@ sub renew { > my $c = shift->openapi->valid_input or return; > > my $checkout_id = $c->param('checkout_id'); >- my $seen = $c->param('seen') || 1; >- my $checkout = Koha::Checkouts->find( $checkout_id ); >+ my $seen = $c->param('seen') || 1; >+ my $checkout = Koha::Checkouts->find($checkout_id); > > unless ($checkout) { > return $c->render( >- status => 404, >+ status => 404, > openapi => { error => "Checkout doesn't exist" } > ); > } > > return try { >- my ($can_renew, $error) = CanBookBeRenewed($checkout->patron, $checkout); >+ my ( $can_renew, $error ) = CanBookBeRenewed( $checkout->patron, $checkout ); > >- if (!$can_renew) { >+ if ( !$can_renew ) { > return $c->render( >- status => 403, >+ status => 403, > openapi => { error => "Renewal not authorized ($error)" } > ); > } >@@ -426,8 +414,7 @@ sub renew { > status => 201, > openapi => $checkout->to_api > ); >- } >- catch { >+ } catch { > $c->unhandled_exception($_); > }; > } >@@ -442,17 +429,17 @@ sub allows_renewal { > my $c = shift->openapi->valid_input or return; > > my $checkout_id = $c->param('checkout_id'); >- my $checkout = Koha::Checkouts->find( $checkout_id ); >+ my $checkout = Koha::Checkouts->find($checkout_id); > > unless ($checkout) { > return $c->render( >- status => 404, >+ status => 404, > openapi => { error => "Checkout doesn't exist" } > ); > } > > return try { >- my ($can_renew, $error) = CanBookBeRenewed($checkout->patron, $checkout); >+ my ( $can_renew, $error ) = CanBookBeRenewed( $checkout->patron, $checkout ); > > my $renewable = Mojo::JSON->false; > $renewable = Mojo::JSON->true if $can_renew; >@@ -466,17 +453,16 @@ sub allows_renewal { > } > ); > return $c->render( >- status => 200, >+ status => 200, > openapi => { >- allows_renewal => $renewable, >- max_renewals => $rule->rule_value, >+ allows_renewal => $renewable, >+ max_renewals => $rule->rule_value, > current_renewals => $checkout->renewals_count, >- unseen_renewals => $checkout->unseen_renewals, >- error => $error >+ unseen_renewals => $checkout->unseen_renewals, >+ error => $error > } > ); >- } >- catch { >+ } catch { > $c->unhandled_exception($_); > }; > } >-- >2.41.0
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 24401
:
151109
|
151181
|
151182
|
151183
|
151184
|
152717
|
154428
|
154429
|
154430
|
154431
|
156884
|
156885
|
168337
|
172764
|
173189
|
173194
|
176190
|
176191