From fc81ee50040076ba6417cd047ea72c9ced7a1414 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 MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Benjamin Rokseth Signed-off-by: Katrin Fischer Signed-off-by: Kyle M Hall --- C4/HoldsQueue.pm | 23 +++- C4/Reserves.pm | 27 +++-- Koha/Schema/Result/OldReserve.pm | 33 +++++- Koha/Schema/Result/Reserve.pm | 33 +++++- .../data/mysql/atomicupdate/hold_itype.sql | 7 ++ installer/data/mysql/kohastructure.sql | 9 +- .../prog/en/modules/reserve/request.tt | 19 ++- .../bootstrap/en/modules/opac-reserve.tt | 13 ++ .../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 | 66 ++++++++++- 14 files changed, 365 insertions(+), 38 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/hold_itype.sql create mode 100644 t/db_dependent/Holds/HoldItemtypeLimit.t diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm index 7cadfecc91..e523f16ab5 100755 --- a/C4/HoldsQueue.pm +++ b/C4/HoldsQueue.pm @@ -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; diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 44e384c272..55ec002b27 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -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 = ?) diff --git a/Koha/Schema/Result/OldReserve.pm b/Koha/Schema/Result/OldReserve.pm index 2f7100a0dc..f253a86cc9 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 f87fc220b9..4d560069ac 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 0000000000..200dcfe133 --- /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 3bec3e5678..ca73390068 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -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; -- 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 473cfb4f9d..d2482065e8 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 516ada50d9..01032ccf8f 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 d45222e0c9..f31f47e0a3 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt @@ -1,6 +1,7 @@ [% USE Koha %] [% USE KohaDates %] [% USE Branches %] +[% USE ItemTypes %] [% INCLUDE 'doc-head-open.inc' %] [% IF ( LibraryNameTitle ) %][% LibraryNameTitle %][% ELSE %]Koha online[% END %] catalog › Your library home @@ -608,7 +609,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 0f753d8dde..687a663ecb 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -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 ); } diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index c8d15d76e5..f48103b530 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->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); } } diff --git a/reserve/request.pl b/reserve/request.pl index 23ff0f3237..1e8ff184df 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -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 index 0000000000..dcd79323af --- /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 18049bb960..135c062fc7 100755 --- a/t/db_dependent/HoldsQueue.t +++ b/t/db_dependent/HoldsQueue.t @@ -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; -- 2.39.5