Bug 15533 - Allow patrons and librarians to select itemtype when placing hold
authorKyle M Hall <kyle@bywatersolutions.com>
Fri, 25 Dec 2015 12:05:57 +0000 (12:05 +0000)
committerKyle M Hall <kyle@bywatersolutions.com>
Fri, 29 Apr 2016 10:26:03 +0000 (10:26 +0000)
Some libraries would like the ability to select the itemtype to request
when placing holds. For example, if a record has 3 copies of BookA and 3
copies of BookA in large print, this feature would allow a person to
place a hold on the record, but still be able to target only the Large
Print edition so that the first Large Print copy that becomes available
is targeted, rather than forcing the patron to select a particular copy
to hold.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Create a record with items of two or more itemtypes
4) Place a record level hold on the record while choosing one particular
   itemtype
5) Check in an item from the record that is not of that itemtype
6) Notee it is not trapped for the hold
7) Check in an item from the record that does match the selected itemtype
8) Note the item is trapped for the hold

Signed-off-by: Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

14 files changed:
C4/HoldsQueue.pm
C4/Reserves.pm
Koha/Schema/Result/OldReserve.pm
Koha/Schema/Result/Reserve.pm
installer/data/mysql/atomicupdate/hold_itype.sql [new file with mode: 0644]
installer/data/mysql/kohastructure.sql
koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
opac/opac-reserve.pl
reserve/placerequest.pl
reserve/request.pl
t/db_dependent/Holds/HoldItemtypeLimit.t [new file with mode: 0644]
t/db_dependent/HoldsQueue.t

index 7cadfec..e523f16 100755 (executable)
@@ -278,7 +278,7 @@ sub GetPendingHoldRequestsForBib {
     my $dbh = C4::Context->dbh;
 
     my $request_query = "SELECT biblionumber, borrowernumber, itemnumber, priority, reserves.branchcode,
-                                reservedate, reservenotes, borrowers.branchcode AS borrowerbranch
+                                reservedate, reservenotes, borrowers.branchcode AS borrowerbranch, itemtype
                          FROM reserves
                          JOIN borrowers USING (borrowernumber)
                          WHERE biblionumber = ?
@@ -368,6 +368,7 @@ sub GetItemsAvailableToFillHoldRequestsForBib {
 sub MapItemsToHoldRequests {
     my ($hold_requests, $available_items, $branches_to_use, $transport_cost_matrix) = @_;
 
+
     # handle trival cases
     return unless scalar(@$hold_requests) > 0;
     return unless scalar(@$available_items) > 0;
@@ -400,6 +401,8 @@ sub MapItemsToHoldRequests {
                 and ( # Don't fill item level holds that contravene the hold pickup policy at this time
                     ( $items_by_itemnumber{ $request->{itemnumber} }->{hold_fulfillment_policy} eq 'any' )
                     || ( $request->{branchcode} eq $items_by_itemnumber{ $request->{itemnumber} }->{ $items_by_itemnumber{ $request->{itemnumber} }->{hold_fulfillment_policy} }  )
+                and ( !$request->{itemtype} # If hold itemtype is set, item's itemtype must match
+                    || $items_by_itemnumber{ $request->{itemnumber} }->{itype} eq $request->{itemtype} )
                 )
 
               )
@@ -451,6 +454,8 @@ sub MapItemsToHoldRequests {
                     $request->{borrowerbranch} eq $item->{homebranch}
                     && ( ( $item->{hold_fulfillment_policy} eq 'any' ) # Don't fill item level holds that contravene the hold pickup policy at this time
                         || $request->{branchcode} eq $item->{ $item->{hold_fulfillment_policy} } )
+                    && ( !$request->{itemtype} # If hold itemtype is set, item's itemtype must match
+                        || $items_by_itemnumber{ $request->{itemnumber} }->{itype} eq $request->{itemtype} )
                   )
                 {
                     $itemnumber = $item->{itemnumber};
@@ -472,6 +477,10 @@ sub MapItemsToHoldRequests {
                     next unless $item->{hold_fulfillment_policy} eq 'any'
                         || $request->{branchcode} eq $item->{ $item->{hold_fulfillment_policy} };
 
+                    # If hold itemtype is set, item's itemtype must match
+                    next unless ( !$request->{itemtype}
+                        || $item->{itype} eq $request->{itemtype} );
+
                     $itemnumber = $item->{itemnumber};
                     last;
                 }
@@ -504,6 +513,10 @@ sub MapItemsToHoldRequests {
                     next unless $item->{hold_fulfillment_policy} eq 'any'
                         || $request->{branchcode} eq $item->{ $item->{hold_fulfillment_policy} };
 
+                    # If hold itemtype is set, item's itemtype must match
+                    next unless ( !$request->{itemtype}
+                        || $item->{itype} eq $request->{itemtype} );
+
                     $itemnumber = $item->{itemnumber};
                     $holdingbranch = $branch;
                     last PULL_BRANCHES;
@@ -519,6 +532,10 @@ sub MapItemsToHoldRequests {
                         next unless $current_item->{hold_fulfillment_policy} eq 'any'
                             || $request->{branchcode} eq $current_item->{ $current_item->{hold_fulfillment_policy} };
 
+                        # If hold itemtype is set, item's itemtype must match
+                        next unless ( !$request->{itemtype}
+                            || $current_item->{itype} eq $request->{itemtype} );
+
                         $itemnumber = $current_item->{itemnumber};
                         last; # quit this loop as soon as we have a suitable item
                     }
@@ -539,6 +556,10 @@ sub MapItemsToHoldRequests {
                         next unless $item->{hold_fulfillment_policy} eq 'any'
                             || $request->{branchcode} eq $item->{ $item->{hold_fulfillment_policy} };
 
+                        # If hold itemtype is set, item's itemtype must match
+                        next unless ( !$request->{itemtype}
+                            || $item->{itype} eq $request->{itemtype} );
+
                         $itemnumber = $item->{itemnumber};
                         $holdingbranch = $branch;
                         last PULL_BRANCHES2;
index 44e384c..55ec002 100644 (file)
@@ -161,9 +161,9 @@ The following tables are available witin the HOLDPLACED message:
 
 sub AddReserve {
     my (
-        $branch,    $borrowernumber, $biblionumber,
-        $bibitems,  $priority, $resdate, $expdate, $notes,
-        $title,      $checkitem, $found
+        $branch,   $borrowernumber, $biblionumber, $bibitems,
+        $priority, $resdate,        $expdate,      $notes,
+        $title,    $checkitem,      $found,        $itemtype
     ) = @_;
 
     if ( Koha::Holds->search( { borrowernumber => $borrowernumber, biblionumber => $biblionumber } )->count() > 0 ) {
@@ -191,6 +191,9 @@ sub AddReserve {
         $waitingdate = $resdate;
     }
 
+    # Don't add itemtype limit if specific item is selected
+    $itemtype = undef if $checkitem;
+
     # updates take place here
     my $hold = Koha::Hold->new(
         {
@@ -203,7 +206,8 @@ sub AddReserve {
             itemnumber     => $checkitem,
             found          => $found,
             waitingdate    => $waitingdate,
-            expirationdate => $expdate
+            expirationdate => $expdate,
+            itemtype       => $itemtype,
         }
     )->store();
     my $reserve_id = $hold->id();
@@ -310,7 +314,8 @@ sub GetReservesFromBiblionumber {
                 expirationdate,
                 lowestPriority,
                 suspend,
-                suspend_until
+                suspend_until,
+                itemtype
         FROM     reserves
         WHERE biblionumber = ? ";
     push( @params, $biblionumber );
@@ -946,8 +951,9 @@ sub CheckReserves {
 
                 # See if this item is more important than what we've got so far
                 if ( ( $res->{'priority'} && $res->{'priority'} < $priority ) || $local_hold_match ) {
-                    $borrowerinfo ||= C4::Members::GetMember( borrowernumber => $res->{'borrowernumber'} );
                     $iteminfo ||= C4::Items::GetItem($itemnumber);
+                    next if $res->{itemtype} && $res->{itemtype} ne _get_itype( $iteminfo );
+                    $borrowerinfo ||= C4::Members::GetMember( borrowernumber => $res->{'borrowernumber'} );
                     my $branch = GetReservesControlBranch( $iteminfo, $borrowerinfo );
                     my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$iteminfo->{'itype'});
                     next if ($branchitemrule->{'holdallowed'} == 0);
@@ -1833,7 +1839,8 @@ sub _Findgroupreserve {
                reserves.timestamp           AS timestamp,
                biblioitems.biblioitemnumber AS biblioitemnumber,
                reserves.itemnumber          AS itemnumber,
-               reserves.reserve_id          AS reserve_id
+               reserves.reserve_id          AS reserve_id,
+               reserves.itemtype            AS itemtype
         FROM reserves
         JOIN biblioitems USING (biblionumber)
         JOIN hold_fill_targets USING (biblionumber, borrowernumber, itemnumber)
@@ -1867,7 +1874,8 @@ sub _Findgroupreserve {
                reserves.timestamp           AS timestamp,
                biblioitems.biblioitemnumber AS biblioitemnumber,
                reserves.itemnumber          AS itemnumber,
-               reserves.reserve_id          AS reserve_id
+               reserves.reserve_id          AS reserve_id,
+               reserves.itemtype            AS itemtype
         FROM reserves
         JOIN biblioitems USING (biblionumber)
         JOIN hold_fill_targets USING (biblionumber, borrowernumber)
@@ -1900,7 +1908,8 @@ sub _Findgroupreserve {
                reserves.priority                   AS priority,
                reserves.timestamp                  AS timestamp,
                reserves.itemnumber                 AS itemnumber,
-               reserves.reserve_id                 AS reserve_id
+               reserves.reserve_id                 AS reserve_id,
+               reserves.itemtype                   AS itemtype
         FROM reserves
         WHERE reserves.biblionumber = ?
           AND (reserves.itemnumber IS NULL OR reserves.itemnumber = ?)
index 2f7100a..f253a86 100644 (file)
@@ -129,6 +129,13 @@ __PACKAGE__->table("old_reserves");
   datetime_undef_if_invalid: 1
   is_nullable: 1
 
+=head2 itemtype
+
+  data_type: 'varchar'
+  is_foreign_key: 1
+  is_nullable: 1
+  size: 10
+
 =cut
 
 __PACKAGE__->add_columns(
@@ -177,6 +184,8 @@ __PACKAGE__->add_columns(
     datetime_undef_if_invalid => 1,
     is_nullable => 1,
   },
+  "itemtype",
+  { data_type => "varchar", is_foreign_key => 1, is_nullable => 1, size => 10 },
 );
 
 =head1 PRIMARY KEY
@@ -253,9 +262,29 @@ __PACKAGE__->belongs_to(
   },
 );
 
+=head2 itemtype
+
+Type: belongs_to
+
+Related object: L<Koha::Schema::Result::Itemtype>
+
+=cut
+
+__PACKAGE__->belongs_to(
+  "itemtype",
+  "Koha::Schema::Result::Itemtype",
+  { itemtype => "itemtype" },
+  {
+    is_deferrable => 1,
+    join_type     => "LEFT",
+    on_delete     => "CASCADE",
+    on_update     => "CASCADE",
+  },
+);
+
 
-# Created by DBIx::Class::Schema::Loader v0.07040 @ 2015-06-30 08:51:40
-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:wGi7SO5Sz+IwuvqaAyQDbg
+# Created by DBIx::Class::Schema::Loader v0.07042 @ 2015-12-26 12:22:09
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:34jZVAc6xF4/49hChXdSEg
 
 
 # You can replace this text with custom content, and it will be preserved on regeneration
index f87fc22..4d56006 100644 (file)
@@ -133,6 +133,13 @@ __PACKAGE__->table("reserves");
   datetime_undef_if_invalid: 1
   is_nullable: 1
 
+=head2 itemtype
+
+  data_type: 'varchar'
+  is_foreign_key: 1
+  is_nullable: 1
+  size: 10
+
 =cut
 
 __PACKAGE__->add_columns(
@@ -191,6 +198,8 @@ __PACKAGE__->add_columns(
     datetime_undef_if_invalid => 1,
     is_nullable => 1,
   },
+  "itemtype",
+  { data_type => "varchar", is_foreign_key => 1, is_nullable => 1, size => 10 },
 );
 
 =head1 PRIMARY KEY
@@ -277,9 +286,29 @@ __PACKAGE__->belongs_to(
   },
 );
 
+=head2 itemtype
+
+Type: belongs_to
+
+Related object: L<Koha::Schema::Result::Itemtype>
+
+=cut
+
+__PACKAGE__->belongs_to(
+  "itemtype",
+  "Koha::Schema::Result::Itemtype",
+  { itemtype => "itemtype" },
+  {
+    is_deferrable => 1,
+    join_type     => "LEFT",
+    on_delete     => "CASCADE",
+    on_update     => "CASCADE",
+  },
+);
+
 
-# Created by DBIx::Class::Schema::Loader v0.07040 @ 2015-06-30 08:51:40
-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:uHHqseJ56g3nDyKnNncyUA
+# Created by DBIx::Class::Schema::Loader v0.07042 @ 2015-12-26 12:22:09
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:v9+wPKUT381CLNlusQ4LMA
 
 __PACKAGE__->belongs_to(
   "item",
diff --git a/installer/data/mysql/atomicupdate/hold_itype.sql b/installer/data/mysql/atomicupdate/hold_itype.sql
new file mode 100644 (file)
index 0000000..200dcfe
--- /dev/null
@@ -0,0 +1,7 @@
+ALTER TABLE reserves ADD COLUMN itemtype VARCHAR(10) NULL DEFAULT NULL AFTER suspend_until;
+ALTER TABLE reserves ADD KEY `itemtype` (`itemtype`);
+ALTER TABLE reserves ADD CONSTRAINT `reserves_ibfk_5` FOREIGN KEY (`itemtype`) REFERENCES `itemtypes` (`itemtype`) ON DELETE CASCADE ON UPDATE CASCADE;
+
+ALTER TABLE old_reserves ADD COLUMN itemtype VARCHAR(10) NULL DEFAULT NULL AFTER suspend_until;
+ALTER TABLE old_reserves ADD KEY `itemtype` (`itemtype`);
+ALTER TABLE old_reserves ADD CONSTRAINT `old_reserves_ibfk_4` FOREIGN KEY (`itemtype`) REFERENCES `itemtypes` (`itemtype`) ON DELETE CASCADE ON UPDATE CASCADE;
index 3bec3e5..ca73390 100644 (file)
@@ -1705,16 +1705,20 @@ CREATE TABLE `old_reserves` ( -- this table holds all holds/reserves that have b
   `lowestPriority` tinyint(1) NOT NULL, -- has this hold been pinned to the lowest priority in the holds queue (1 for yes, 0 for no)
   `suspend` BOOLEAN NOT NULL DEFAULT 0, -- in this hold suspended (1 for yes, 0 for no)
   `suspend_until` DATETIME NULL DEFAULT NULL, -- the date this hold is suspended until (NULL for infinitely)
+  `itemtype` VARCHAR(10) NULL DEFAULT NULL, -- If record level hold, the optional itemtype of the item the patron is requesting
   PRIMARY KEY (`reserve_id`),
   KEY `old_reserves_borrowernumber` (`borrowernumber`),
   KEY `old_reserves_biblionumber` (`biblionumber`),
   KEY `old_reserves_itemnumber` (`itemnumber`),
   KEY `old_reserves_branchcode` (`branchcode`),
+  KEY `old_reserves_itemtype` (`itemtype`),
   CONSTRAINT `old_reserves_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`)
     ON DELETE SET NULL ON UPDATE SET NULL,
   CONSTRAINT `old_reserves_ibfk_2` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`)
     ON DELETE SET NULL ON UPDATE SET NULL,
   CONSTRAINT `old_reserves_ibfk_3` FOREIGN KEY (`itemnumber`) REFERENCES `items` (`itemnumber`)
+    ON DELETE SET NULL ON UPDATE SET NULL,
+  CONSTRAINT `old_reserves_ibfk_4` FOREIGN KEY (`itemtype`) REFERENCES `itemtypes` (`itemtype`)
     ON DELETE SET NULL ON UPDATE SET NULL
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
@@ -1882,16 +1886,19 @@ CREATE TABLE `reserves` ( -- information related to holds/reserves in Koha
   `lowestPriority` tinyint(1) NOT NULL,
   `suspend` BOOLEAN NOT NULL DEFAULT 0,
   `suspend_until` DATETIME NULL DEFAULT NULL,
+  `itemtype` VARCHAR(10) NULL DEFAULT NULL, -- If record level hold, the optional itemtype of the item the patron is requesting
   PRIMARY KEY (`reserve_id`),
   KEY priorityfoundidx (priority,found),
   KEY `borrowernumber` (`borrowernumber`),
   KEY `biblionumber` (`biblionumber`),
   KEY `itemnumber` (`itemnumber`),
   KEY `branchcode` (`branchcode`),
+  KEY `itemtype` (`itemtype`),
   CONSTRAINT `reserves_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE,
   CONSTRAINT `reserves_ibfk_2` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) ON DELETE CASCADE ON UPDATE CASCADE,
   CONSTRAINT `reserves_ibfk_3` FOREIGN KEY (`itemnumber`) REFERENCES `items` (`itemnumber`) ON DELETE CASCADE ON UPDATE CASCADE,
-  CONSTRAINT `reserves_ibfk_4` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON UPDATE CASCADE
+  CONSTRAINT `reserves_ibfk_4` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON UPDATE CASCADE,
+  CONSTRAINT `reserves_ibfk_5` FOREIGN KEY (`itemtype`) REFERENCES `itemtypes` (`itemtype`) ON DELETE CASCADE ON UPDATE CASCADE
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 --
index 473cfb4..d248206 100644 (file)
@@ -1,5 +1,6 @@
 [% USE Koha %]
 [% USE KohaDates %]
+[% USE ItemTypes %]
 [% INCLUDE 'doc-head-open.inc' %]
 [% UNLESS ( multi_hold ) %]
     <title>Koha &rsaquo; Circulation &rsaquo; Holds &rsaquo; Place a hold on [% title |html %]</title>
@@ -410,6 +411,18 @@ function checkMultiHold() {
             </select>
         </li>
 
+        [% UNLESS ( multi_hold ) %]
+            <li>
+                <label for="itemtype">Request specific item type:</label>
+                <select name="itemtype" size="1" id="itemtype">
+                    <option value="">Any item type</option>
+                    [%- FOREACH itemtype IN available_itemtypes %]
+                        <option value="[% itemtype %]">[% ItemTypes.GetDescription( itemtype ) %]</option>
+                    [%- END %]
+                </select>
+            </li>
+        [% END %]
+
        [% IF ( reserve_in_future ) %]
        <li>
         <label for="from">Hold starts on date:</label>
@@ -798,7 +811,11 @@ function checkMultiHold() {
                 </a>
                 </i>
             [% ELSE %]
-                <i>Next available</i>
+                [% IF reserveloo.itemtype %]
+                    <i>Next available <b>[% ItemTypes.GetDescription( reserveloo.itemtype ) %]</b> item</i>
+                [% ELSE %]
+                    <i>Next available</i>
+                [% END %]
                  <input type="hidden" name="itemnumber" value="" />
             [% END %]
     [% END %]
index 516ada5..01032cc 100644 (file)
@@ -1,6 +1,7 @@
 [% USE Koha %]
 [% USE KohaDates %]
 [% USE Price %]
+[% USE ItemTypes %]
 [% INCLUDE 'doc-head-open.inc' %]
 <title>[% IF ( LibraryNameTitle ) %][% LibraryNameTitle %][% ELSE %]Koha online[% END %] catalog &rsaquo; Placing a hold</title>
 [% INCLUDE 'doc-head-close.inc' %]
                                                             <span class="date-format to" data-biblionumber="[% bibitemloo.biblionumber %]">[% INCLUDE 'date-format.inc' %]</span>
                                                         </li>
 
+                                                        [% UNLESS ( multi_hold ) %]
+                                                            <li>
+                                                                <label for="itemtype">Request specific item type:</label>
+                                                                <select name="itemtype" size="1" id="itemtype">
+                                                                    <option value="">Any item type</option>
+                                                                    [%- FOREACH itemtype IN available_itemtypes %]
+                                                                        <option value="[% itemtype %]">[% ItemTypes.GetDescription( itemtype ) %]</option>
+                                                                    [%- END %]
+                                                                </select>
+                                                            </li>
+                                                        [% END %]
+
                                                         [% IF ( OpacHoldNotes ) %]
                                                             <li>
                                                                 <div class="notesrow" id="notesrow_[% bibitemloo.biblionumber %]">
index d45222e..f31f47e 100644 (file)
@@ -1,6 +1,7 @@
 [% USE Koha %]
 [% USE KohaDates %]
 [% USE Branches %]
+[% USE ItemTypes %]
 
 [% INCLUDE 'doc-head-open.inc' %]
 <title>[% IF ( LibraryNameTitle ) %][% LibraryNameTitle %][% ELSE %]Koha online[% END %] catalog &rsaquo; Your library home</title>
                                                     [% ELSIF ( RESERVE.suspend ) %]
                                                         Suspended [% IF ( RESERVE.suspend_until ) %] until [% RESERVE.suspend_until %] [% END %]
                                                     [% ELSE %]
-                                                        Pending
+                                                        [% IF RESERVE.itemtype %]
+                                                            Pending for next available <i>[% ItemTypes.GetDescription( RESERVE.itemtype ) %]</i> item
+                                                        [% ELSE %]
+                                                            Pending
+                                                        [% END %]
                                                     [% END %]
                                                 [% END %]
                                             </td>
index 0f753d8..687a663 100755 (executable)
@@ -37,6 +37,7 @@ use Koha::DateUtils;
 use Koha::Libraries;
 use Koha::Patron::Debarments qw(IsDebarred);
 use Date::Calc qw/Today Date_to_Days/;
+use List::MoreUtils qw/uniq/;
 
 my $maxreserves = C4::Context->preference("maxreserves");
 
@@ -281,15 +282,16 @@ if ( $query->param('place_reserve') ) {
             $canreserve = 0;
         }
 
+        my $itemtype = $query->param('itemtype') || undef;
+        $itemtype = undef if $itemNum;
+
         # Here we actually do the reserveration. Stage 3.
         if ($canreserve) {
             my $reserve_id = AddReserve(
-                $branch,      $borrowernumber,
-                $biblioNum,
-                [$biblioNum], $rank,
-                $startdate,   $expiration_date,
-                $notes,       $biblioData->{title},
-                $itemNum,     $found
+                $branch,          $borrowernumber, $biblioNum,
+                [$biblioNum],     $rank,           $startdate,
+                $expiration_date, $notes,          $biblioData->{title},
+                $itemNum,         $found,          $itemtype,
             );
             $failed_holds++ unless $reserve_id;
             ++$reserve_cnt;
@@ -381,6 +383,7 @@ unless ($noreserves) {
 #
 my $notforloan_label_of = get_notforloan_label_of();
 
+my @available_itemtypes;
 my $biblioLoop = [];
 my $numBibsAvailable = 0;
 my $itemdata_enumchron = 0;
@@ -534,6 +537,7 @@ foreach my $biblioNum (@biblionumbers) {
                 $itemLoopIter->{available} = 1;
                 $numCopiesOPACAvailable++;
                 $biblioLoopIter{force_hold} = 1 if $hold_allowed eq 'F';
+                push( @available_itemtypes, $itemInfo->{itype} );
             }
             $numCopiesAvailable++;
         }
@@ -571,6 +575,9 @@ foreach my $biblioNum (@biblionumbers) {
     $anyholdable = 1 if $biblioLoopIter{holdable};
 }
 
+@available_itemtypes = uniq( @available_itemtypes );
+$template->param( available_itemtypes => \@available_itemtypes );
+
 if ( $numBibsAvailable == 0 || $anyholdable == 0) {
     $template->param( none_available => 1 );
 }
index c8d15d7..f48103b 100755 (executable)
@@ -37,19 +37,21 @@ my $input = CGI->new();
 
 checkauth($input, 0, { reserveforothers => 'place_holds' }, 'intranet');
 
-my @bibitems=$input->multi_param('biblioitem');
-my @reqbib=$input->multi_param('reqbib');
-my $biblionumber=$input->param('biblionumber');
-my $borrowernumber=$input->param('borrowernumber');
-my $notes=$input->param('notes');
-my $branch=$input->param('pickup');
-my $startdate=$input->param('reserve_date') || '';
-my @rank=$input->multi_param('rank-request');
-my $type=$input->param('type');
-my $title=$input->param('title');
-my $borrower=GetMember('borrowernumber'=>$borrowernumber);
-my $checkitem=$input->param('checkitem');
+my @bibitems       = $input->multi_param('biblioitem');
+my @reqbib         = $input->multi_param('reqbib');
+my $biblionumber   = $input->param('biblionumber');
+my $borrowernumber = $input->param('borrowernumber');
+my $notes          = $input->param('notes');
+my $branch         = $input->param('pickup');
+my $startdate      = $input->param('reserve_date') || '';
+my @rank           = $input->multi_param('rank-request');
+my $type           = $input->param('type');
+my $title          = $input->param('title');
+my $checkitem      = $input->param('checkitem');
 my $expirationdate = $input->param('expiration_date');
+my $itemtype       = $input->param('itemtype') || undef;
+
+my $borrower = GetMember( 'borrowernumber' => $borrowernumber );
 
 my $multi_hold = $input->param('multi_hold');
 my $biblionumbers = $multi_hold ? $input->param('biblionumbers') : ($biblionumber . '/');
@@ -108,7 +110,7 @@ if ($type eq 'str8' && $borrower){
                        $bibinfo->{rank},$startdate,$expirationdate,$notes,$bibinfo->{title},$checkitem,$found);
         } else {
             # place a request on 1st available
-            AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem,$found);
+            AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem,$found, $itemtype);
         }
     }
 
index 23ff0f3..1e8ff18 100755 (executable)
@@ -321,6 +321,7 @@ foreach my $biblionumber (@biblionumbers) {
 
     my @bibitemloop;
 
+    my @available_itemtypes;
     foreach my $biblioitemnumber (@biblioitemnumbers) {
         my $biblioitem = $biblioiteminfos_of->{$biblioitemnumber};
         my $num_available = 0;
@@ -454,6 +455,8 @@ foreach my $biblionumber (@biblionumbers) {
             {
                 $item->{available} = 1;
                 $num_available++;
+
+                push( @available_itemtypes, $item->{itype} );
             }
             elsif ( C4::Context->preference('AllowHoldPolicyOverride') ) {
                 # If AllowHoldPolicyOverride is set, it should override EVERY restriction, not just branch item rules
@@ -483,6 +486,9 @@ foreach my $biblionumber (@biblionumbers) {
         push @bibitemloop, $biblioitem;
     }
 
+    @available_itemtypes = uniq( @available_itemtypes );
+    $template->param( available_itemtypes => \@available_itemtypes );
+
     # existingreserves building
     my @reserveloop;
     my @reserves = Koha::Holds->search( { biblionumber => $biblionumber }, { order_by => 'priority' } );
@@ -561,6 +567,7 @@ foreach my $biblionumber (@biblionumbers) {
         $reserve{'suspend'}        = $res->suspend();
         $reserve{'suspend_until'}  = $res->suspend_until();
         $reserve{'reserve_id'}     = $res->reserve_id();
+        $reserve{itemtype}         = $res->{itemtype};
 
         if ( C4::Context->preference('IndependentBranches') && $flags->{'superlibrarian'} != 1 ) {
             $reserve{'branchloop'} = [ Koha::Libraries->find( $res->branchcode() ) ];
diff --git a/t/db_dependent/Holds/HoldItemtypeLimit.t b/t/db_dependent/Holds/HoldItemtypeLimit.t
new file mode 100644 (file)
index 0000000..dcd7932
--- /dev/null
@@ -0,0 +1,112 @@
+#!/usr/bin/perl
+
+use Modern::Perl;
+
+use C4::Context;
+
+use Test::More tests => 4;
+
+use t::lib::TestBuilder;
+
+BEGIN {
+    use FindBin;
+    use lib $FindBin::Bin;
+    use_ok('C4::Reserves');
+}
+
+my $schema = Koha::Database->schema;
+$schema->storage->txn_begin;
+my $dbh = C4::Context->dbh;
+
+my $builder = t::lib::TestBuilder->new;
+
+my $library = $builder->build({
+    source => 'Branch',
+});
+
+my $bib_title = "Test Title";
+
+my $borrower = $builder->build({
+    source => 'Borrower',
+    value => {
+        categorycode => 'S',
+        branchcode => $library->{branchcode},
+    }
+});
+
+my $itemtype1 = $builder->build({
+    source => 'Itemtype',
+    value => {
+        notforloan => 0
+    }
+});
+
+my $itemtype2 = $builder->build({
+    source => 'Itemtype',
+    value => {
+        notforloan => 0
+    }
+});
+
+
+# Test hold_fulfillment_policy
+my $right_itemtype = $itemtype1->{itemtype};
+my $wrong_itemtype = $itemtype2->{itemtype};
+my $borrowernumber = $borrower->{borrowernumber};
+my $branchcode = $library->{branchcode};
+$dbh->do("DELETE FROM reserves");
+$dbh->do("DELETE FROM issues");
+$dbh->do("DELETE FROM items");
+$dbh->do("DELETE FROM biblio");
+$dbh->do("DELETE FROM biblioitems");
+$dbh->do("DELETE FROM transport_cost");
+$dbh->do("DELETE FROM tmp_holdsqueue");
+$dbh->do("DELETE FROM hold_fill_targets");
+$dbh->do("DELETE FROM default_branch_circ_rules");
+$dbh->do("DELETE FROM default_branch_item_rules");
+$dbh->do("DELETE FROM default_circ_rules");
+$dbh->do("DELETE FROM branch_item_rules");
+
+$dbh->do("INSERT INTO biblio (frameworkcode, author, title, datecreated) VALUES ('', 'Koha test', '$bib_title', '2011-02-01')");
+
+my $biblionumber = $dbh->selectrow_array("SELECT biblionumber FROM biblio WHERE title = '$bib_title'")
+  or BAIL_OUT("Cannot find newly created biblio record");
+
+$dbh->do("INSERT INTO biblioitems (biblionumber, marcxml, itemtype) VALUES ($biblionumber, '', '$right_itemtype')");
+
+my $biblioitemnumber =
+  $dbh->selectrow_array("SELECT biblioitemnumber FROM biblioitems WHERE biblionumber = $biblionumber")
+  or BAIL_OUT("Cannot find newly created biblioitems record");
+
+$dbh->do("
+    INSERT INTO items (biblionumber, biblioitemnumber, homebranch, holdingbranch, notforloan, damaged, itemlost, withdrawn, onloan, itype)
+    VALUES ($biblionumber, $biblioitemnumber, '$branchcode', '$branchcode', 0, 0, 0, 0, NULL, '$right_itemtype')
+");
+
+my $itemnumber =
+  $dbh->selectrow_array("SELECT itemnumber FROM items WHERE biblionumber = $biblionumber")
+  or BAIL_OUT("Cannot find newly created item");
+
+$dbh->do("DELETE FROM default_circ_rules");
+$dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy ) VALUES ( 2, 'any' )");
+
+# Itemtypes match
+my $reserve_id = AddReserve( $branchcode, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $right_itemtype );
+my ( $status ) = CheckReserves($itemnumber);
+is( $status, 'Reserved', "Hold where itemtype matches item's itemtype targed" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# Itemtypes don't match
+$reserve_id = AddReserve( $branchcode, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $wrong_itemtype );
+( $status ) = CheckReserves($itemnumber);
+is($status, q{}, "Hold where itemtype does not match item's itemtype not targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# No itemtype set
+$reserve_id = AddReserve( $branchcode, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, undef );
+( $status ) = CheckReserves($itemnumber);
+is( $status, 'Reserved', "Item targeted with no hold itemtype set" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# Cleanup
+$schema->storage->txn_rollback;
index 18049bb..135c062 100755 (executable)
@@ -8,10 +8,9 @@
 
 use Modern::Perl;
 
-use Test::More tests => 35;
+use Test::More tests => 38;
 use Data::Dumper;
 
-
 use C4::Branch;
 use C4::Calendar;
 use C4::Context;
@@ -555,6 +554,69 @@ CancelReserve( { reserve_id => $reserve_id } );
 
 # End testing hold_fulfillment_policy
 
+# Test hold itemtype limit
+C4::Context->set_preference( "UseTransportCostMatrix", 0 );
+my @itemtypes = Koha::ItemTypes->search();
+my $wrong_itemtype = $itemtypes[0]->itemtype;
+my $right_itemtype = $itemtypes[1]->itemtype;
+$borrowernumber = $borrower3->{borrowernumber};
+my $branchcode = $library1->{branchcode};
+$dbh->do("DELETE FROM reserves");
+$dbh->do("DELETE FROM issues");
+$dbh->do("DELETE FROM items");
+$dbh->do("DELETE FROM biblio");
+$dbh->do("DELETE FROM biblioitems");
+$dbh->do("DELETE FROM transport_cost");
+$dbh->do("DELETE FROM tmp_holdsqueue");
+$dbh->do("DELETE FROM hold_fill_targets");
+$dbh->do("DELETE FROM default_branch_circ_rules");
+$dbh->do("DELETE FROM default_branch_item_rules");
+$dbh->do("DELETE FROM default_circ_rules");
+$dbh->do("DELETE FROM branch_item_rules");
+
+$dbh->do("INSERT INTO biblio (frameworkcode, author, title, datecreated) VALUES ('', 'Koha test', '$TITLE', '2011-02-01')");
+
+$biblionumber = $dbh->selectrow_array("SELECT biblionumber FROM biblio WHERE title = '$TITLE'")
+  or BAIL_OUT("Cannot find newly created biblio record");
+
+$dbh->do("INSERT INTO biblioitems (biblionumber, marcxml, itemtype) VALUES ($biblionumber, '', '$itemtype')");
+
+$biblioitemnumber =
+  $dbh->selectrow_array("SELECT biblioitemnumber FROM biblioitems WHERE biblionumber = $biblionumber")
+  or BAIL_OUT("Cannot find newly created biblioitems record");
+
+$dbh->do("
+    INSERT INTO items (biblionumber, biblioitemnumber, homebranch, holdingbranch, notforloan, damaged, itemlost, withdrawn, onloan, itype)
+    VALUES ($biblionumber, $biblioitemnumber, '$library_A', '$library_B', 0, 0, 0, 0, NULL, '$right_itemtype')
+");
+
+# With hold_fulfillment_policy = homebranch, hold should only be picked up if pickup branch = homebranch
+$dbh->do("DELETE FROM default_circ_rules");
+$dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy ) VALUES ( 2, 'any' )");
+
+# Home branch matches pickup branch
+$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $wrong_itemtype );
+C4::HoldsQueue::CreateQueue();
+$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
+is( @$holds_queue, 0, "Item with incorrect itemtype not targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# Holding branch matches pickup branch
+$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $right_itemtype );
+C4::HoldsQueue::CreateQueue();
+$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
+is( @$holds_queue, 1, "Item with matching itemtype is targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# Neither branch matches pickup branch
+$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, undef );
+C4::HoldsQueue::CreateQueue();
+$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
+is( @$holds_queue, 1, "Item targeted when hold itemtype is not set" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# End testing hold itemtype limit
+
 # Cleanup
 $schema->storage->txn_rollback;