Attachment 61044 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.12 KB, created by Lari Taskula on 2017-03-13 17:22:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Lari Taskula

Created: 2017-03-13 17:22:33 UTC

Size: 5.12 KB

patch

obsolete

>From e580d73b044544770661ae5883eac1e8c5da1434 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
>---
> C4/Reserves.pm         | 24 +++++++++++++++++++-----
> t/db_dependent/Holds.t | 26 +++++++++++++++++++++++++-
> 2 files changed, 44 insertions(+), 6 deletions(-)
>
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index c078af1..0b914fb 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -418,7 +418,7 @@ sub GetReservesFromBorrowernumber {
> 
> =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.
>@@ -426,7 +426,7 @@ See CanItemBeReserved() for possible return values.
> =cut
> 
> sub CanBookBeReserved{
>-    my ($borrowernumber, $biblionumber) = @_;
>+    my ($borrowernumber, $biblionumber, $branchcode) = @_;
> 
>     my $items = GetItemnumbersForBiblio($biblionumber);
>     #get items linked via host records
>@@ -437,7 +437,7 @@ sub CanBookBeReserved{
> 
>     my $canReserve;
>     foreach my $item (@$items) {
>-        $canReserve = CanItemBeReserved( $borrowernumber, $item );
>+        $canReserve = CanItemBeReserved( $borrowernumber, $item, $branchcode );
>         return 'OK' if $canReserve eq 'OK';
>     }
>     return $canReserve;
>@@ -445,7 +445,7 @@ sub CanBookBeReserved{
> 
> =head2 CanItemBeReserved
> 
>-  $canReserve = &CanItemBeReserved($borrowernumber, $itemnumber)
>+  $canReserve = &CanItemBeReserved($borrowernumber, $itemnumber, $branchcode)
>   if ($canReserve eq 'OK') { #We can reserve this Item! }
> 
> @RETURNS OK,              if the Item can be reserved.
>@@ -454,11 +454,13 @@ sub CanBookBeReserved{
>          cannotReserveFromOtherBranches, if syspref 'canreservefromotherbranches' is OK.
>          tooManyReserves, if the borrower has exceeded his maximum reserve amount.
>          notReservable,   if holds on this item are not allowed
>+         libraryNotFound  if given branchcode is not an existing library
>+         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
>@@ -587,6 +589,18 @@ sub CanItemBeReserved {
>         }
>     }
> 
>+    if ($branchcode_to) {
>+        my $destination = Koha::Libraries->find({
>+            branchcode => $branchcode_to,
>+        });
>+        unless ($destination) {
>+            return 'libraryNotFound';
>+        }
>+        unless ($destination->pickup_location) {
>+            return 'libraryNotPickupLocation';
>+        }
>+    }
>+
>     return 'OK';
> }
> 
>diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t
>index 61b9e6e..cf2c1d4 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 => 61;
>+use Test::More tests => 62;
> use MARC::Record;
> use C4::Biblio;
> use C4::Items;
>@@ -16,6 +16,8 @@ use C4::Calendar;
> use Koha::Database;
> use Koha::DateUtils qw( dt_from_string output_pref );
> use Koha::Holds;
>+use Koha::Items;
>+use Koha::Libraries;
> 
> BEGIN {
>     use FindBin;
>@@ -485,7 +487,29 @@ my $res_id = AddReserve( $branch_1, $borrowernumbers[0], $bibnum, '', 1, );
> is( CanItemBeReserved( $borrowernumbers[0], $itemnumber ),
>     'tooManyReserves', 'Patron cannot reserve item with hold limit of 1, 1 bib level hold placed' );
> 
>+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');
>+};
>+my ( $bibnum, $title, $bibitemnum );
> # Helper method to set up a Biblio.
> sub create_helper_biblio {
>     my $itemtype = shift;
>-- 
>1.9.1

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