From 337f5d71e2a3c2ed4bae02caafe9d97665c5c5ec Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Fri, 25 Dec 2015 12:05:57 +0000 Subject: [PATCH] Bug 15533 - Allow patrons and librarians to select itemtype when placing hold 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 --- C4/HoldsQueue.pm | 23 ++++- C4/Reserves.pm | 27 +++-- Koha/Schema/Result/OldReserve.pm | 33 +++++- Koha/Schema/Result/Reserve.pm | 33 +++++- installer/data/mysql/atomicupdate/hold_itype.sql | 7 ++ installer/data/mysql/kohastructure.sql | 9 +- .../prog/en/modules/reserve/request.tt | 19 +++- .../opac-tmpl/bootstrap/en/modules/opac-reserve.tt | 13 +++ .../opac-tmpl/bootstrap/en/modules/opac-user.tt | 7 +- opac/opac-reserve.pl | 19 ++-- reserve/placerequest.pl | 28 +++--- reserve/request.pl | 7 ++ t/db_dependent/Holds/HoldItemtypeLimit.t | 112 +++++++++++++++++++++ t/db_dependent/HoldsQueue.t | 64 +++++++++++- 14 files changed, 364 insertions(+), 37 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/hold_itype.sql create mode 100755 t/db_dependent/Holds/HoldItemtypeLimit.t diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm index e34df3a..55761c9 100755 --- a/C4/HoldsQueue.pm +++ b/C4/HoldsQueue.pm @@ -266,7 +266,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 = ? @@ -356,6 +356,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; @@ -388,6 +389,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} ) ) ) @@ -439,6 +442,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}; @@ -460,6 +465,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; } @@ -492,6 +501,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; @@ -507,6 +520,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 } @@ -527,6 +544,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; diff --git a/C4/Reserves.pm b/C4/Reserves.pm index df05f82..a4a768e 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -163,9 +163,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 ) { @@ -193,6 +193,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( { @@ -205,7 +208,8 @@ sub AddReserve { itemnumber => $checkitem, found => $found, waitingdate => $waitingdate, - expirationdate => $expdate + expirationdate => $expdate, + itemtype => $itemtype, } )->store(); my $reserve_id = $hold->id(); @@ -312,7 +316,8 @@ sub GetReservesFromBiblionumber { expirationdate, lowestPriority, suspend, - suspend_until + suspend_until, + itemtype FROM reserves WHERE biblionumber = ? "; push( @params, $biblionumber ); @@ -948,8 +953,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); @@ -1835,7 +1841,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) @@ -1869,7 +1876,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) @@ -1902,7 +1910,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 = ?) diff --git a/Koha/Schema/Result/OldReserve.pm b/Koha/Schema/Result/OldReserve.pm index 2f7100a..f253a86 100644 --- a/Koha/Schema/Result/OldReserve.pm +++ b/Koha/Schema/Result/OldReserve.pm @@ -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 + +=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 diff --git a/Koha/Schema/Result/Reserve.pm b/Koha/Schema/Result/Reserve.pm index f87fc22..4d56006 100644 --- a/Koha/Schema/Result/Reserve.pm +++ b/Koha/Schema/Result/Reserve.pm @@ -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 + +=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 index 0000000..200dcfe --- /dev/null +++ b/installer/data/mysql/atomicupdate/hold_itype.sql @@ -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; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 852b411..db28815 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1704,16 +1704,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; @@ -1881,16 +1885,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 `itemtype` (`itemtype`) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci; -- diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt index 85f687a..ae5d256 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -1,5 +1,6 @@ [% USE Koha %] [% USE KohaDates %] +[% USE ItemTypes %] [% INCLUDE 'doc-head-open.inc' %] [% UNLESS ( multi_hold ) %] Koha › Circulation › Holds › Place a hold on [% title |html %] @@ -410,6 +411,18 @@ function checkMultiHold() { + [% UNLESS ( multi_hold ) %] +
  • + + +
  • + [% END %] + [% IF ( reserve_in_future ) %]
  • @@ -798,7 +811,11 @@ function checkMultiHold() { [% ELSE %] - Next available + [% IF reserveloo.itemtype %] + Next available [% ItemTypes.GetDescription( reserveloo.itemtype ) %] item + [% ELSE %] + Next available + [% END %] [% END %] [% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt index de7e3e8..ec8c7d2 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt @@ -1,6 +1,7 @@ [% USE Koha %] [% USE KohaDates %] [% USE Price %] +[% USE ItemTypes %] [% INCLUDE 'doc-head-open.inc' %] [% IF ( LibraryNameTitle ) %][% LibraryNameTitle %][% ELSE %]Koha online[% END %] catalog › Placing a hold [% INCLUDE 'doc-head-close.inc' %] @@ -248,6 +249,18 @@ [% INCLUDE 'date-format.inc' %]
  • + [% UNLESS ( multi_hold ) %] +
  • + + +
  • + [% END %] + [% IF ( OpacHoldNotes ) %]
  • diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt index 3a8b016..22b0686 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt @@ -1,5 +1,6 @@ [% USE Koha %] [% USE KohaDates %] +[% USE ItemTypes %] [% INCLUDE 'doc-head-open.inc' %] [% IF ( LibraryNameTitle ) %][% LibraryNameTitle %][% ELSE %]Koha online[% END %] catalog › Your library home @@ -607,7 +608,11 @@ [% ELSIF ( RESERVE.suspend ) %] Suspended [% IF ( RESERVE.suspend_until ) %] until [% RESERVE.suspend_until %] [% END %] [% ELSE %] - Pending + [% IF RESERVE.itemtype %] + Pending for next available [% ItemTypes.GetDescription( RESERVE.itemtype ) %] item + [% ELSE %] + Pending + [% END %] [% END %] [% END %] diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index 277b2fe..bf660cb 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -36,6 +36,7 @@ use C4::Debug; use Koha::DateUtils; use Koha::Borrower::Debarments qw(IsDebarred); use Date::Calc qw/Today Date_to_Days/; +use List::MoreUtils qw/uniq/; # use Data::Dumper; 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 ); } diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index 28e8868..1f26b3e 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -37,19 +37,21 @@ my $input = CGI->new(); checkauth($input, 0, { reserveforothers => 'place_holds' }, 'intranet'); -my @bibitems=$input->param('biblioitem'); -my @reqbib=$input->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->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->param('biblioitem'); +my @reqbib = $input->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->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); } } diff --git a/reserve/request.pl b/reserve/request.pl index dfdd6e1..c39c16f 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -320,6 +320,7 @@ foreach my $biblionumber (@biblionumbers) { my @bibitemloop; + my @available_itemtypes; foreach my $biblioitemnumber (@biblioitemnumbers) { my $biblioitem = $biblioiteminfos_of->{$biblioitemnumber}; my $num_available = 0; @@ -453,6 +454,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 @@ -482,6 +485,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' } ); @@ -562,6 +568,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'} = [ GetBranchDetail( $res->branchcode() ) ]; diff --git a/t/db_dependent/Holds/HoldItemtypeLimit.t b/t/db_dependent/Holds/HoldItemtypeLimit.t new file mode 100755 index 0000000..dcd7932 --- /dev/null +++ b/t/db_dependent/Holds/HoldItemtypeLimit.t @@ -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; diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t index 271d760..8746089 100755 --- a/t/db_dependent/HoldsQueue.t +++ b/t/db_dependent/HoldsQueue.t @@ -12,7 +12,7 @@ use C4::Context; use Data::Dumper; -use Test::More tests => 32; +use Test::More tests => 35; use C4::Branch; use C4::Members; @@ -525,6 +525,68 @@ CancelReserve( { reserve_id => $reserve_id } ); # End testing hold_fulfillment_policy +# Test hold itemtype limit +C4::Context->set_preference( "UseTransportCostMatrix", 0 ); +my $wrong_itemtype = $item_types[0]->{itemtype}; +my $right_itemtype = $item_types[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; -- 2.1.4