From 4499f1d46948f0556051271bce22eba0ead46ef5 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Wed, 23 Oct 2013 11:58:34 -0400 Subject: [PATCH] Bug 11126 - Make the holds system optionally give precedence to local holds This feature will allow libraries to specify that, when an item is returned, a local hold may be given priority for fulfillment even though it is of lower priority in the list of unfilled holds. This feature has three settings: * LocalHoldsPriority, which enables the feature * LocalHoldsPriorityPatronControl, which selects for either tha patron's home library, or the patron's pickup library for the hold * LocalHoldsPriorityItemControl, which selects for either the item's holding library, or home library. So, this feature can "give priority for filling holds to patrons whose (home library|pickup library) matches the item's (home library|holding library)" Test Plan: 1) Apply this patch 2) Run t/db_dependent/Holds/LocalHoldsPriority.t --- C4/Reserves.pm | 49 +++++++- installer/data/mysql/sysprefs.sql | 3 + installer/data/mysql/updatedatabase.pl | 12 ++ .../en/modules/admin/preferences/circulation.pref | 15 +++ t/db_dependent/Holds/LocalHoldsPriority.t | 119 ++++++++++++++++++++ 5 files changed, 191 insertions(+), 7 deletions(-) create mode 100755 t/db_dependent/Holds/LocalHoldsPriority.t diff --git a/C4/Reserves.pm b/C4/Reserves.pm index f8666a3..927fc3f 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -847,7 +847,9 @@ sub CheckReserves { items.biblioitemnumber, itemtypes.notforloan, items.notforloan AS itemnotforloan, - items.itemnumber + items.itemnumber, + items.homebranch, + items.holdingbranch FROM items LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber LEFT JOIN itemtypes ON items.itype = itemtypes.itemtype @@ -859,7 +861,9 @@ sub CheckReserves { items.biblioitemnumber, itemtypes.notforloan, items.notforloan AS itemnotforloan, - items.itemnumber + items.itemnumber, + items.homebranch, + items.holdingbranch FROM items LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber LEFT JOIN itemtypes ON biblioitems.itemtype = itemtypes.itemtype @@ -875,12 +879,14 @@ sub CheckReserves { $sth->execute($barcode); } # note: we get the itemnumber because we might have started w/ just the barcode. Now we know for sure we have it. - my ( $biblio, $bibitem, $notforloan_per_itemtype, $notforloan_per_item, $itemnumber ) = $sth->fetchrow_array; + my ( $biblio, $bibitem, $notforloan_per_itemtype, $notforloan_per_item, + $itemnumber, $item_homebranch, $item_holdingbranch ) + = $sth->fetchrow_array; return ( '' ) unless $itemnumber; # bail if we got nothing. # if item is not for loan it cannot be reserved either..... - # execpt where items.notforloan < 0 : This indicates the item is holdable. + # except where items.notforloan < 0 : This indicates the item is holdable. return ( '' ) if ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype; # Find this item in the reserves @@ -892,21 +898,50 @@ sub CheckReserves { # $highest is the most important item we've seen so far. my $highest; if (scalar @reserves) { + my $LocalHoldsPriority = C4::Context->preference('LocalHoldsPriority'); + my $LocalHoldsPriorityPatronControl = C4::Context->preference('LocalHoldsPriorityPatronControl'); + my $LocalHoldsPriorityItemControl = C4::Context->preference('LocalHoldsPriorityItemControl'); + my $priority = 10000000; foreach my $res (@reserves) { if ( $res->{'itemnumber'} == $itemnumber && $res->{'priority'} == 0) { return ( "Waiting", $res, \@reserves ); # Found it } else { + # Lazy fetch for borrower and item. We only need to know about the patron and item + # each and every time if we are using LocalHoldsPriority. This is a great place to + # leverage the inherent lazy fetching of DBIx::Class. + my $borrowerinfo; + my $iteminfo; + + my $local_hold_match; + if ($LocalHoldsPriority) { + $borrowerinfo = C4::Members::GetMember( borrowernumber => $res->{'borrowernumber'} ); + $iteminfo = C4::Items::GetItem($itemnumber); + + my $local_holds_priority_item_branchcode = + $iteminfo->{$LocalHoldsPriorityItemControl}; + my $local_holds_priority_patron_branchcode = + ( $LocalHoldsPriorityPatronControl eq 'PickupLibrary' ) + ? $res->{branchcode} + : ( $LocalHoldsPriorityPatronControl eq 'HomeLibrary' ) + ? $borrowerinfo->{branchcode} + : undef; + $local_hold_match = + $local_holds_priority_item_branchcode eq + $local_holds_priority_patron_branchcode; + } + # See if this item is more important than what we've got so far - if ( $res->{'priority'} && $res->{'priority'} < $priority ) { - my $borrowerinfo=C4::Members::GetMember(borrowernumber => $res->{'borrowernumber'}); - my $iteminfo=C4::Items::GetItem($itemnumber); + if ( ( $res->{'priority'} && $res->{'priority'} < $priority ) || $local_hold_match ) { + $borrowerinfo ||= C4::Members::GetMember( borrowernumber => $res->{'borrowernumber'} ); + $iteminfo ||= C4::Items::GetItem($itemnumber); my $branch = GetReservesControlBranch( $iteminfo, $borrowerinfo ); my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$iteminfo->{'itype'}); next if ($branchitemrule->{'holdallowed'} == 0); next if (($branchitemrule->{'holdallowed'} == 1) && ($branch ne $borrowerinfo->{'branchcode'})); $priority = $res->{'priority'}; $highest = $res; + last if $local_hold_match; } } } diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index 71513ce..0e7f772 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -170,6 +170,9 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('LinkerOptions','','','A pipe-separated list of options for the linker.','free'), ('LinkerRelink','1',NULL,'If ON the authority linker will relink headings that have previously been linked every time it runs.','YesNo'), ('LocalCoverImages','0','1','Display local cover images on intranet details pages.','YesNo'), +('LocalHoldsPriority', '0', NULL, 'Enables the LocalHoldsPriority feature', 'YesNo'), +('LocalHoldsPriorityItemControl', 'holdingbranch', 'holdingbranch|homebranch', 'decides if the feature operates using the item''s home or holding library.', 'Choice'), +('LocalHoldsPriorityPatronControl', 'PickupLibrary', 'HomeLibrary|PickupLibrary', 'decides if the feature operates using the library set as the patron''s home library, or the library set as the pickup library for the given hold.', 'Choice'), ('ManInvInNoissuesCharge','1',NULL,'MANUAL_INV charges block checkouts (added to noissuescharge).','YesNo'), ('MARCAuthorityControlField008','|| aca||aabn | a|a d',NULL,NULL,'Textarea'), ('MARCOrgCode','OSt','','Define MARC Organization Code - http://www.loc.gov/marc/organizations/orgshome.html','free'), diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index b031ed8..1925a8b 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -7330,6 +7330,18 @@ if ( CheckVersion($DBversion) ) { SetVersion($DBversion); } +$DBversion = "3.13.00.XXX"; +if ( CheckVersion($DBversion) ) { + $dbh->do(q{ + INSERT INTO systempreferences ( variable, value, options, explanation, type ) VALUES + ('LocalHoldsPriority', '0', NULL, 'Enables the LocalHoldsPriority feature', 'YesNo'), + ('LocalHoldsPriorityItemControl', 'holdingbranch', 'holdingbranch|homebranch', 'decides if the feature operates using the item''s home or holding library.', 'Choice'), + ('LocalHoldsPriorityPatronControl', 'PickupLibrary', 'HomeLibrary|PickupLibrary', 'decides if the feature operates using the library set as the patron''s home library, or the library set as the pickup library for the given hold.', 'Choice') + }); + print "Upgrade to $DBversion done (Bug 11126 - Make the holds system optionally give precedence to local holds)\n"; + SetVersion($DBversion); +} + =head1 FUNCTIONS =head2 TableExists($table) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref index a8f4e43..a2627c6 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -489,6 +489,21 @@ Circulation: yes: Allow no: "Don't allow" - a patron to place a hold on a record where the patron already has one or more items attached to that record checked out. + - + - pref: LocalHoldsPriority + choices: + yes: Give + no: "Don't give" + - priority for filling holds to patrons whose + - pref: LocalHoldsPriorityPatronControl + choices: + PickupLibrary: "pickup library" + HomeLibrary: "home library" + - matches the item's + - pref: LocalHoldsPriorityItemControl + choices: + homebranch: "home library" + holdingbranch: "holding library" Fines Policy: - - Calculate fines based on days overdue diff --git a/t/db_dependent/Holds/LocalHoldsPriority.t b/t/db_dependent/Holds/LocalHoldsPriority.t new file mode 100755 index 0000000..2e092b2 --- /dev/null +++ b/t/db_dependent/Holds/LocalHoldsPriority.t @@ -0,0 +1,119 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use t::lib::Mocks; +use C4::Context; +use C4::Branch; + +use Test::More tests => 6; +use MARC::Record; +use C4::Biblio; +use C4::Items; +use C4::Members; + +BEGIN { + use FindBin; + use lib $FindBin::Bin; + use_ok('C4::Reserves'); +} + +my $dbh = C4::Context->dbh; + +# Start transaction +$dbh->{AutoCommit} = 0; +$dbh->{RaiseError} = 1; + +my $borrowers_count = 5; + +# Setup Test------------------------ +# Helper biblio. +diag("Creating biblio instance for testing."); +my ( $bibnum, $title, $bibitemnum ) = create_helper_biblio(); + +# Helper item for that biblio. +diag("Creating item instance for testing."); +my ( $item_bibnum, $item_bibitemnum, $itemnumber ) = + AddItem( { homebranch => 'MPL', holdingbranch => 'CPL' }, $bibnum ); + +my @branchcodes = qw{XXX RPL CPL MPL CPL MPL}; + +# Create some borrowers +diag("Creating borrowers."); +my @borrowernumbers; +foreach ( 1 .. $borrowers_count ) { + my $borrowernumber = AddMember( + firstname => 'my firstname', + surname => 'my surname ' . $_, + categorycode => 'S', + branchcode => $branchcodes[$_], + ); + push @borrowernumbers, $borrowernumber; +} + +my $biblionumber = $bibnum; + +my @branches = GetBranchesLoop(); +my $branch = $branches[0][0]{value}; + +# Create five item level holds +diag("Creating holds."); +my $i = 1; +foreach my $borrowernumber (@borrowernumbers) { + AddReserve( + $branchcodes[$i], + $borrowernumber, + $biblionumber, + my $constraint = 'a', + my $bibitems = q{}, + my $priority = $i, + my $resdate, + my $expdate, + my $notes = q{}, + $title, + my $checkitem, + my $found, + ); + + $i++; +} + +my ($status, $reserve, $all_reserves); + +t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 0 ); +($status, $reserve, $all_reserves) = CheckReserves($itemnumber); +ok( $reserve->{borrowernumber} eq $borrowernumbers[0], "Recieved expected results with LocalHoldsPriority disabled" ); + +t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 1 ); + +t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'PickupLibrary' ); +t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'homebranch' ); +($status, $reserve, $all_reserves) = CheckReserves($itemnumber); +ok( $reserve->{borrowernumber} eq $borrowernumbers[2], "Received expected results with PickupLibrary/homebranch" ); + +t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'PickupLibrary' ); +t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'holdingbranch' ); +($status, $reserve, $all_reserves) = CheckReserves($itemnumber); +ok( $reserve->{borrowernumber} eq $borrowernumbers[1], "Received expected results with PickupLibrary/holdingbranch" ); + +t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'HomeLibrary' ); +t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'holdingbranch' ); +($status, $reserve, $all_reserves) = CheckReserves($itemnumber); +ok( $reserve->{borrowernumber} eq $borrowernumbers[1], "Received expected results with HomeLibrary/holdingbranch" ); + +t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'HomeLibrary' ); +t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'homebranch' ); +($status, $reserve, $all_reserves) = CheckReserves($itemnumber); +ok( $reserve->{borrowernumber} eq $borrowernumbers[2], "Received expected results with HomeLibrary/homebranch" ); + +# Helper method to set up a Biblio. +sub create_helper_biblio { + my $bib = MARC::Record->new(); + my $title = 'Silence in the library'; + $bib->append_fields( + MARC::Field->new( '100', ' ', ' ', a => 'Moffat, Steven' ), + MARC::Field->new( '245', ' ', ' ', a => $title ), + ); + return ( $bibnum, $title, $bibitemnum ) = AddBiblio( $bib, '' ); +} -- 1.7.2.5