Attachment 78483 Details for Bug 7534 – Bug 7534: Validate pickup location in CanBook/ItemBeReserved

[patch] Bug 7534: Validate pickup location in CanBook/ItemBeReserved

Bug-7534-Validate-pickup-location-in-CanBookItemBe.patch (text/plain), 5.32 KB, created by Kyle M Hall (khall) on 2018-09-06 13:47:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2018-09-06 13:47:05 UTC

Size: 5.32 KB

patch

obsolete

>From cc2af8c9f880d5e3cac9b5aba3770680e4fbf870 Mon Sep 17 00:00:00 2001
>From: Lari Taskula <lari.taskula@jns.fi>
>Date: Tue, 7 Feb 2017 17:51:18 +0200
>Subject: [PATCH] Bug 7534: Validate pickup location in CanBook/ItemBeReserved
>
>This patch adds $branchcode_to parameter to CanBookBeReserved and
>CanItemBeReserved. It represents the pickup location for the hold.
>
>To test:
>1. prove t/db_dependent/Holds.t
>
>Signed-off-by: Koha Team AMU <axelle.clarisse@univ-amu.fr>
>---
> C4/Reserves.pm         | 24 +++++++++++++++++++-----
> t/db_dependent/Holds.t | 27 ++++++++++++++++++++++++++-
> 2 files changed, 45 insertions(+), 6 deletions(-)
>
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index 8e8bea3349..d7b9c9a1af 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -265,7 +265,7 @@ sub AddReserve {
> 
> =head2 CanBookBeReserved
> 
>-  $canReserve = &CanBookBeReserved($borrowernumber, $biblionumber)
>+  $canReserve = &CanBookBeReserved($borrowernumber, $biblionumber, $branchcode)
>   if ($canReserve eq 'OK') { #We can reserve this Item! }
> 
> See CanItemBeReserved() for possible return values.
>@@ -273,7 +273,7 @@ See CanItemBeReserved() for possible return values.
> =cut
> 
> sub CanBookBeReserved{
>-    my ($borrowernumber, $biblionumber) = @_;
>+    my ($borrowernumber, $biblionumber, $branchcode) = @_;
> 
>     my @itemnumbers = Koha::Items->search({ biblionumber => $biblionumber})->get_column("itemnumber");
>     #get items linked via host records
>@@ -284,7 +284,7 @@ sub CanBookBeReserved{
> 
>     my $canReserve;
>     foreach my $itemnumber (@itemnumbers) {
>-        $canReserve = CanItemBeReserved( $borrowernumber, $itemnumber );
>+        $canReserve = CanItemBeReserved( $borrowernumber, $itemnumber, $branchcode );
>         return $canReserve if $canReserve->{status} eq 'OK';
>     }
>     return $canReserve;
>@@ -292,7 +292,7 @@ sub CanBookBeReserved{
> 
> =head2 CanItemBeReserved
> 
>-  $canReserve = &CanItemBeReserved($borrowernumber, $itemnumber)
>+  $canReserve = &CanItemBeReserved($borrowernumber, $itemnumber, $branchcode)
>   if ($canReserve->{status} eq 'OK') { #We can reserve this Item! }
> 
> @RETURNS { status => OK },              if the Item can be reserved.
>@@ -301,11 +301,13 @@ sub CanBookBeReserved{
>          { status => cannotReserveFromOtherBranches }, if syspref 'canreservefromotherbranches' is OK.
>          { status => tooManyReserves, limit => $limit }, if the borrower has exceeded their maximum reserve amount.
>          { status => notReservable },   if holds on this item are not allowed
>+         { status => libraryNotFound },   if given branchcode is not an existing library
>+         { status => libraryNotPickupLocation },   if given branchcode is not configured to be a pickup location
> 
> =cut
> 
> sub CanItemBeReserved {
>-    my ( $borrowernumber, $itemnumber ) = @_;
>+    my ( $borrowernumber, $itemnumber, $branchcode_to ) = @_;
> 
>     my $dbh = C4::Context->dbh;
>     my $ruleitemtype;    # itemtype of the matching issuing rule
>@@ -458,6 +460,18 @@ sub CanItemBeReserved {
>         }
>     }
> 
>+    if ($branchcode_to) {
>+        my $destination = Koha::Libraries->find({
>+            branchcode => $branchcode_to,
>+        });
>+        unless ($destination) {
>+            return { status => 'libraryNotFound' };
>+        }
>+        unless ($destination->pickup_location) {
>+            return { status => 'libraryNotPickupLocation' };
>+        }
>+    }
>+
>     return { status => 'OK' };
> }
> 
>diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t
>index 27151ae84f..2350bb6e80 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 => 55;
>+use Test::More tests => 62;
> use MARC::Record;
> use Koha::Patrons;
> use C4::Items;
>@@ -19,6 +19,8 @@ use Koha::Database;
> use Koha::DateUtils qw( dt_from_string output_pref );
> use Koha::Biblios;
> use Koha::Holds;
>+use Koha::Items;
>+use Koha::Libraries;
> 
> BEGIN {
>     use FindBin;
>@@ -479,6 +481,29 @@ subtest 'Test max_holds per library/patron category' => sub {
>     is( $ret->{status}, 'OK', 'Patron can place hold with branch/category rule of 5, category rule of 5' );
> };
> 
>+subtest 'Pickup location availability tests' => sub {
>+    plan tests => 3;
>+
>+    my ( $bibnum, $title, $bibitemnum ) = create_helper_biblio('ONLY1');
>+    my ( $item_bibnum, $item_bibitemnum, $itemnumber )
>+    = AddItem( { homebranch => $branch_1, holdingbranch => $branch_1 }, $bibnum );
>+    my $item = Koha::Items->find($itemnumber);
>+    my $branch_to = $builder->build({ source => 'Branch' })->{ branchcode };
>+    my $library = Koha::Libraries->find($branch_to);
>+    my $patron = $builder->build({ source => 'Borrower' })->{ borrowernumber };
>+
>+    t::lib::Mocks::mock_preference('UseBranchTransferLimits', 1);
>+    t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'itemtype');
>+    $library->pickup_location('1')->store;
>+    is(CanItemBeReserved($patron, $item->itemnumber, $branch_to),
>+       'OK', 'Library is a pickup location');
>+    $library->pickup_location('0')->store;
>+    is(CanItemBeReserved($patron, $item->itemnumber, $branch_to),
>+       'libraryNotPickupLocation', 'Library is not a pickup location');
>+    is(CanItemBeReserved($patron, $item->itemnumber, 'nonexistent'),
>+       'libraryNotFound', 'Cannot set unknown library as pickup location');
>+};
>+
> # Helper method to set up a Biblio.
> sub create_helper_biblio {
>     my $itemtype = shift;
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 7534: 60014 | 60015 | 60016 | 60017 | 60018 | 60022 | 60023 | 60781 | 60794 | 60795 | 60796 | 60797 | 60798 | 60799 | 60878 | 60879 | 60880 | 60881 | 60882 | 60883 | 61044 | 61045 | 61046 | 61047 | 61273 | 61274 | 61275 | 61276 | 61277 | 61278 | 61279 | 61280 | 77847 | 77848 | 77849 | 77850 | 77851 | 77852 | 77853 | 77854 | 77855 | 77856 | 78160 | 78161 | 78162 | 78163 | 78164 | 78165 | 78166 | 78167 | 78168 | 78169 | 78170 | 78171 | 78477 | 78478 | 78479 | 78480 | 78481 | 78482 | 78483 | 78484 | 78485 | 78486 | 78487 | 78488 | 78489 | 78495 | 78540 | 78548 | 84852