From 841a40e6618bd2345bb23a5e4f7624ab3a9df236 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Fri, 1 Jan 2016 11:33:49 +0000 Subject: [PATCH] Bug 15534 - Add the ability to prevent a patron from placing a hold on a record with available items Some libraries would like to prevent patrons from placing holds on records/items where there are other items available for the patron to check out. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Browse to the circulation rules 4) Note the new rule "Allow hold if others available" 5) Set the rule to "No" 6) Find a patron/branch/itemtype applicable to this rule 7) Ensure at least one item on the record is available for the patron to check out 8) Attempt to place a hold for the patron 9) Note you cannot place the hold 10) Check the available item out to another patron 11) Note you can now place a hold for the first patron --- C4/Members.pm | 7 +- C4/Reserves.pm | 71 +++++++----- Koha/Schema/Result/Issuingrule.pm | 12 +- admin/smart-rules.pl | 2 + .../mysql/atomicupdate/on_shelf_non_available.sql | 1 + installer/data/mysql/kohastructure.sql | 1 + .../prog/en/modules/admin/smart-rules.tt | 9 ++ t/db_dependent/Holds/AllowHoldIfItemsAvailable.t | 123 +++++++++++++++++++++ 8 files changed, 198 insertions(+), 28 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/on_shelf_non_available.sql create mode 100755 t/db_dependent/Holds/AllowHoldIfItemsAvailable.t diff --git a/C4/Members.pm b/C4/Members.pm index 4ba1f19..d12aa03 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -452,7 +452,12 @@ sub GetMember { #FIXME interface to this routine now allows generation of a result set #so whole array should be returned but bowhere in the current code expects this if (@{$data} ) { - return $data->[0]; + my $patron = $data->[0]; + + my $flags = patronflags( $patron ); + $patron->{flags} = $flags; + + return $patron; } return; diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 4d20c98..2b4edad 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -29,6 +29,7 @@ use C4::Members; use C4::Items; use C4::Circulation; use C4::Accounts; +use C4::Circulation qw(CanBookBeIssued); # for _koha_notify_reserve use C4::Members::Messaging; @@ -41,6 +42,7 @@ use Koha::Calendar; use Koha::Database; use Koha::Hold; use Koha::Holds; +use Koha::Items; use List::MoreUtils qw( firstidx any ); use Carp; @@ -421,19 +423,20 @@ See CanItemBeReserved() for possible return values. =cut -sub CanBookBeReserved{ - my ($borrowernumber, $biblionumber) = @_; +sub CanBookBeReserved { + my ( $borrowernumber, $biblionumber ) = @_; my $items = GetItemnumbersForBiblio($biblionumber); + #get items linked via host records my @hostitems = get_hostitemnumbers_of($biblionumber); - if (@hostitems){ - push (@$items,@hostitems); + if (@hostitems) { + push( @$items, @hostitems ); } my $canReserve; - foreach my $item (@$items) { - $canReserve = CanItemBeReserved( $borrowernumber, $item ); + foreach my $itemnumber (@$items) { + $canReserve = CanItemBeReserved( $borrowernumber, $itemnumber ); return 'OK' if $canReserve eq 'OK'; } return $canReserve; @@ -459,7 +462,7 @@ sub CanItemBeReserved{ my $dbh = C4::Context->dbh; my $ruleitemtype; # itemtype of the matching issuing rule my $allowedreserves = 0; - + # we retrieve borrowers and items informations # # item->{itype} will come for biblioitems if necessery my $item = GetItem($itemnumber); @@ -476,7 +479,7 @@ sub CanItemBeReserved{ my $controlbranch = C4::Context->preference('ReservesControlBranch'); # we retrieve user rights on this itemtype and branchcode - my $sth = $dbh->prepare("SELECT categorycode, itemtype, branchcode, reservesallowed + my $sth = $dbh->prepare("SELECT categorycode, itemtype, branchcode, reservesallowed, allow_hold_if_items_available FROM issuingrules WHERE (categorycode in (?,'*') ) AND (itemtype IN (?,'*')) @@ -495,8 +498,8 @@ sub CanItemBeReserved{ LEFT JOIN borrowers USING (borrowernumber) WHERE borrowernumber = ? "; - - + + my $branchcode = ""; my $branchfield = "reserves.branchcode"; @@ -507,20 +510,24 @@ sub CanItemBeReserved{ $branchfield = "borrowers.branchcode"; $branchcode = $borrower->{branchcode}; } - - # we retrieve rights + + my $allow_hold_if_items_available; + + # we retrieve rights $sth->execute($borrower->{'categorycode'}, $item->{'itype'}, $branchcode); - if(my $rights = $sth->fetchrow_hashref()){ - $ruleitemtype = $rights->{itemtype}; - $allowedreserves = $rights->{reservesallowed}; - }else{ + if ( my $rights = $sth->fetchrow_hashref() ) { + $ruleitemtype = $rights->{itemtype}; + $allowedreserves = $rights->{reservesallowed}; + $allow_hold_if_items_available = $rights->{allow_hold_if_items_available}; + } + else { $ruleitemtype = '*'; } # we retrieve count $querycount .= "AND $branchfield = ?"; - + # If using item-level itypes, fall back to the record # level itemtype if the hold has no associated item $querycount .= @@ -530,7 +537,7 @@ sub CanItemBeReserved{ if ( $ruleitemtype ne "*" ); my $sthcount = $dbh->prepare($querycount); - + if($ruleitemtype eq "*"){ $sthcount->execute($borrowernumber, $branchcode); }else{ @@ -572,6 +579,20 @@ sub CanItemBeReserved{ } } + unless ( $allow_hold_if_items_available ) { + my $item = Koha::Items->find($itemnumber); + my @items = Koha::Items->search({ biblionumber => $item->biblionumber }); + foreach $item (@items) { + my ( $issuingimpossible, $needsconfirmation ) = C4::Circulation::CanBookBeIssued( + $borrower, $item->barcode, my $duedate, + my $inprocess, + my $ignore_reserves = 1 + ); + + return 'itemsAvailable' unless ( keys %$issuingimpossible || keys %$needsconfirmation ); + } + } + return 'OK'; } @@ -744,8 +765,8 @@ sub GetReservesToBranch { my $dbh = C4::Context->dbh; my $sth = $dbh->prepare( "SELECT reserve_id,borrowernumber,reservedate,itemnumber,timestamp - FROM reserves - WHERE priority='0' + FROM reserves + WHERE priority='0' AND branchcode=?" ); $sth->execute( $frombranch ); @@ -770,7 +791,7 @@ sub GetReservesForBranch { my $query = " SELECT reserve_id,borrowernumber,reservedate,itemnumber,waitingdate - FROM reserves + FROM reserves WHERE priority='0' AND found='W' "; @@ -1389,7 +1410,7 @@ sub ModReserveMinusPriority { my $dbh = C4::Context->dbh; my $query = " UPDATE reserves - SET priority = 0 , itemnumber = ? + SET priority = 0 , itemnumber = ? WHERE reserve_id = ? "; my $sth_upd = $dbh->prepare($query); @@ -1604,7 +1625,7 @@ sub ToggleLowestPriority { my $sth = $dbh->prepare( "UPDATE reserves SET lowestPriority = NOT lowestPriority WHERE reserve_id = ?"); $sth->execute( $reserve_id ); - + _FixPriority({ reserve_id => $reserve_id, rank => '999999' }); } @@ -1814,7 +1835,7 @@ sub _FixPriority { $priority[$j]->{'reserve_id'} ); } - + $sth = $dbh->prepare( "SELECT reserve_id FROM reserves WHERE lowestPriority = 1 ORDER BY priority" ); $sth->execute(); @@ -2052,7 +2073,7 @@ sub _koha_notify_reserve { if (! $notification_sent) { &$send_notification('print', 'HOLD'); } - + } =head2 _ShiftPriorityByDateAndPriority diff --git a/Koha/Schema/Result/Issuingrule.pm b/Koha/Schema/Result/Issuingrule.pm index f782456..1f43373 100644 --- a/Koha/Schema/Result/Issuingrule.pm +++ b/Koha/Schema/Result/Issuingrule.pm @@ -191,6 +191,12 @@ __PACKAGE__->table("issuingrules"); is_nullable: 0 size: 1 +=head2 allow_hold_if_items_available + + data_type: 'tinyint' + default_value: 1 + is_nullable: 0 + =cut __PACKAGE__->add_columns( @@ -257,6 +263,8 @@ __PACKAGE__->add_columns( { data_type => "tinyint", default_value => 0, is_nullable => 0 }, "opacitemholds", { data_type => "char", default_value => "N", is_nullable => 0, size => 1 }, + "allow_hold_if_items_available", + { data_type => "tinyint", default_value => 1, is_nullable => 0 }, ); =head1 PRIMARY KEY @@ -276,8 +284,8 @@ __PACKAGE__->add_columns( __PACKAGE__->set_primary_key("branchcode", "categorycode", "itemtype"); -# Created by DBIx::Class::Schema::Loader v0.07042 @ 2015-12-31 15:26:16 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:K/8SKpDjba5CM4+WPZtWPw +# Created by DBIx::Class::Schema::Loader v0.07042 @ 2016-01-02 12:22:52 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:P6v2zqse5sAEUI5qHdFOHQ # You can replace this text with custom content, and it will be preserved on regeneration diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index 8e17802..f21968d 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -144,6 +144,7 @@ elsif ($op eq 'add') { my $hardduedatecompare = $input->param('hardduedatecompare'); my $rentaldiscount = $input->param('rentaldiscount'); my $opacitemholds = $input->param('opacitemholds') || 0; + my $allow_hold_if_items_available = $input->param('allow_hold_if_items_available') || 0; my $overduefinescap = $input->param('overduefinescap') || undef; my $cap_fine_to_replacement_price = $input->param('cap_fine_to_replacement_price') eq 'on'; $debug and warn "Adding $br, $bor, $itemtype, $fine, $maxissueqty, $maxonsiteissueqty, $cap_fine_to_replacement_price"; @@ -173,6 +174,7 @@ elsif ($op eq 'add') { onshelfholds => $onshelfholds, opacitemholds => $opacitemholds, overduefinescap => $overduefinescap, + allow_hold_if_items_available => $allow_hold_if_items_available, cap_fine_to_replacement_price => $cap_fine_to_replacement_price, }; diff --git a/installer/data/mysql/atomicupdate/on_shelf_non_available.sql b/installer/data/mysql/atomicupdate/on_shelf_non_available.sql new file mode 100644 index 0000000..015a214 --- /dev/null +++ b/installer/data/mysql/atomicupdate/on_shelf_non_available.sql @@ -0,0 +1 @@ +ALTER TABLE issuingrules ADD COLUMN allow_hold_if_items_available TINYINT(1) NOT NULL DEFAULT 1 AFTER opacitemholds; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index bc3c3b1..ec33194 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1198,6 +1198,7 @@ CREATE TABLE `issuingrules` ( -- circulation and fine rules cap_fine_to_replacement_price BOOLEAN NOT NULL DEFAULT '0', -- cap the fine based on item's replacement price onshelfholds tinyint(1) NOT NULL default 0, -- allow holds for items that are on shelf opacitemholds char(1) NOT NULL default 'N', -- allow opac users to place specific items on hold + allow_hold_if_items_available TINYINT(1) NOT NULL DEFAULT 1, -- allow holds or not if other items available PRIMARY KEY (`branchcode`,`categorycode`,`itemtype`), KEY `categorycode` (`categorycode`), KEY `itemtype` (`itemtype`) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt index 52f749c..3b6ac51 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt @@ -172,6 +172,7 @@ for="tobranch">Clone these rules to:   @@ -250,6 +251,7 @@ for="tobranch">Clone these rules to: Edit @@ -328,6 +330,12 @@ for="tobranch">Clone these rules to: Force + + + @@ -359,6 +367,7 @@ for="tobranch">Clone these rules to:   diff --git a/t/db_dependent/Holds/AllowHoldIfItemsAvailable.t b/t/db_dependent/Holds/AllowHoldIfItemsAvailable.t new file mode 100755 index 0000000..caaa46a --- /dev/null +++ b/t/db_dependent/Holds/AllowHoldIfItemsAvailable.t @@ -0,0 +1,123 @@ +#!/usr/bin/perl + +use Modern::Perl; + +use C4::Context; +use C4::Items; +use C4::Circulation; +use Koha::IssuingRule; + +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 $library1 = $builder->build({ + source => 'Branch', +}); + +# Now, set a userenv +C4::Context->_new_userenv('xxx'); +C4::Context->set_userenv(0,0,0,'firstname','surname', $library1->{branchcode}, 'Midway Public Library', '', '', ''); + +my $bib_title = "Test Title"; + +my $borrower1 = $builder->build({ + source => 'Borrower', + value => { + categorycode => 'S', + branchcode => $library1->{branchcode}, + dateexpiry => '3000-01-01', + } +}); + +my $borrower2 = $builder->build({ + source => 'Borrower', + value => { + categorycode => 'S', + branchcode => $library1->{branchcode}, + dateexpiry => '3000-01-01', + } +}); + +# Test hold_fulfillment_policy +my ( $itemtype ) = @{ $dbh->selectrow_arrayref("SELECT itemtype FROM itemtypes LIMIT 1") }; +my $borrowernumber1 = $borrower1->{borrowernumber}; +my $borrowernumber2 = $borrower2->{borrowernumber}; +my $library_A = $library1->{branchcode}; + +$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, '', '$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 (barcode, biblionumber, biblioitemnumber, homebranch, holdingbranch, notforloan, damaged, itemlost, withdrawn, onloan, itype) + VALUES ('AllowHoldIf1', $biblionumber, $biblioitemnumber, '$library_A', '$library_A', 0, 0, 0, 0, NULL, '$itemtype') +"); + +my $itemnumber1 = + $dbh->selectrow_array("SELECT itemnumber FROM items WHERE biblionumber = $biblionumber") + or BAIL_OUT("Cannot find newly created item"); + +my $item1 = GetItem( $itemnumber1 ); + +$dbh->do(" + INSERT INTO items (barcode, biblionumber, biblioitemnumber, homebranch, holdingbranch, notforloan, damaged, itemlost, withdrawn, onloan, itype) + VALUES ('AllowHoldIf2', $biblionumber, $biblioitemnumber, '$library_A', '$library_A', 0, 0, 0, 0, NULL, '$itemtype') +"); + +my $itemnumber2 = + $dbh->selectrow_array("SELECT itemnumber FROM items WHERE biblionumber = $biblionumber ORDER BY itemnumber DESC") + or BAIL_OUT("Cannot find newly created item"); + +my $item2 = GetItem( $itemnumber2 ); + +$dbh->do("DELETE FROM issuingrules"); +my $rule = Koha::IssuingRule->new( + { + categorycode => '*', + itemtype => '*', + branchcode => '*', + maxissueqty => 99, + issuelength => 7, + lengthunit => 8, + reservesallowed => 99, + onshelfholds => 1, + allow_hold_if_items_available => 0, + } +); +$rule->store(); + +my $can = CanItemBeReserved( $borrowernumber1, $itemnumber1); +is( $can, 'itemsAvailable', "Item can be held" ); + +AddIssue( $borrower2, $item1->{barcode} ); + +$can = CanItemBeReserved( $borrowernumber1, $itemnumber1); +is( $can, 'itemsAvailable', "Item can be held" ); + +AddIssue( $borrower2, $item2->{barcode} ); + +$can = CanItemBeReserved( $borrowernumber1, $itemnumber1); +is( $can, 'OK', "Item can be held" ); + +# Cleanup +$schema->storage->txn_rollback; -- 2.1.4