@@ -, +, @@ home library, or the patron's pickup library for the hold holding library, or home library. --- C4/Reserves.pm | 47 +++++++- 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, 189 insertions(+), 7 deletions(-) create mode 100755 t/db_dependent/Holds/LocalHoldsPriority.t --- a/C4/Reserves.pm +++ a/C4/Reserves.pm @@ -874,7 +874,9 @@ sub CheckReserves { itemtypes.notforloan, items.notforloan AS itemnotforloan, items.itemnumber, - items.damaged + items.damaged, + items.homebranch, + items.holdingbranch FROM items LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber LEFT JOIN itemtypes ON items.itype = itemtypes.itemtype @@ -887,7 +889,9 @@ sub CheckReserves { itemtypes.notforloan, items.notforloan AS itemnotforloan, items.itemnumber, - items.damaged + items.damaged, + items.homebranch, + items.holdingbranch FROM items LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber LEFT JOIN itemtypes ON biblioitems.itemtype = itemtypes.itemtype @@ -903,14 +907,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, $damaged ) = $sth->fetchrow_array; + my ( $biblio, $bibitem, $notforloan_per_itemtype, $notforloan_per_item, $itemnumber, $damaged, $item_homebranch, $item_holdingbranch ) = $sth->fetchrow_array; return if ( $damaged && !C4::Context->preference('AllowHoldsOnDamagedItems') ); 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 @@ -922,21 +926,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; } } } --- a/installer/data/mysql/sysprefs.sql +++ a/installer/data/mysql/sysprefs.sql @@ -179,6 +179,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,'Define the contents of MARC21 authority control field 008 position 06-39','Textarea'), ('MarcFieldsToOrder','',NULL,'Set the mapping values for a new order line created from a MARC record in a staged file. In a YAML format.','textarea'), --- a/installer/data/mysql/updatedatabase.pl +++ a/installer/data/mysql/updatedatabase.pl @@ -8465,6 +8465,18 @@ if ( CheckVersion($DBversion) ) { SetVersion($DBversion); } +$DBversion = "3.17.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) --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -508,6 +508,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 --- a/t/db_dependent/Holds/LocalHoldsPriority.t +++ a/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, '' ); +} --