Attachment 75262 Details for Bug 17247 – Bug 17247: (QA follow-up) Add check in CanBookBeReserved too

[patch] Bug 17247: (QA follow-up) Add check in CanBookBeReserved too

Bug-17247-QA-follow-up-Add-check-in-CanBookBeReser.patch (text/plain), 2.48 KB, created by Marcel de Rooy on 2018-05-11 08:42:55 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2018-05-11 08:42:55 UTC

Size: 2.48 KB

patch

obsolete

>From ebd2dc6ff1d8b9ce508cde0f600b31a2101a6f7a Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Fri, 11 May 2018 10:11:35 +0200
>Subject: [PATCH] Bug 17247: (QA follow-up) Add check in CanBookBeReserved too
>Content-Type: text/plain; charset=utf-8
>
>If a patron is debarred, it is useless to do the same check over and over
>again in CanItemBeReserved. So adding an escape in CanBookBeReserved.
>Additionally, catching an invalid borrowernumber in Item.
>
>Test plan:
>Run t/db_dependent/Holds.t
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>---
> C4/Reserves.pm         | 6 +++---
> t/db_dependent/Holds.t | 7 ++++---
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index d7c4183..3a9e6cc 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -275,6 +275,7 @@ sub CanBookBeReserved{
>     foreach my $item (@$items) {
>         $canReserve = CanItemBeReserved( $borrowernumber, $item );
>         return 'OK' if $canReserve eq 'OK';
>+        return $canReserve if $canReserve eq 'debarred'; # no need to continue
>     }
>     return $canReserve;
> }
>@@ -307,11 +308,10 @@ sub CanItemBeReserved {
>     my $item       = GetItem($itemnumber);
>     my $biblio     = Koha::Biblios->find( $item->{biblionumber} );
>     my $patron = Koha::Patrons->find( $borrowernumber );
>-    my $borrower = $patron->unblessed;
>-
>-    if ($patron->is_debarred) {
>+    if( !$patron || $patron->is_debarred ) {
>         return 'debarred';
>     }
>+    my $borrower = $patron->unblessed;
> 
>     # If an item is damaged and we don't allow holds on damaged items, we can stop right here
>     return 'damaged'
>diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t
>index 2a85496..7357b5a 100755
>--- a/t/db_dependent/Holds.t
>+++ b/t/db_dependent/Holds.t
>@@ -7,7 +7,7 @@ use t::lib::TestBuilder;
> 
> use C4::Context;
> 
>-use Test::More tests => 56;
>+use Test::More tests => 57;
> use MARC::Record;
> use C4::Biblio;
> use C4::Items;
>@@ -418,8 +418,9 @@ my $debarred_borrowernumber = AddMember(
>     branchcode => $branch_1,
>     debarred => '9999-12-31',
> );
>-is(CanItemBeReserved($debarred_borrowernumber, $itemnumber),
>-    'debarred', 'Debarred patron cannot reserve');
>+is( CanBookBeReserved($debarred_borrowernumber, $bibnum), 'debarred', 'CanBookBeReserved returns debarred on debarred patron' );
>+is( CanItemBeReserved($debarred_borrowernumber, $itemnumber),
>+    'debarred', 'CanItemBeReserved returns debarred for debarred patron' );
> 
> 
> # Helper method to set up a Biblio.
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 17247: 55174 | 70684 | 72963 | 75261 | 75262 | 75270 | 89652 | 89653 | 93549 | 94037 | 94038