Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 11126 - Make the holds system optionally give precedence to local holds

Browse source 
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

Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
This commit is contained in:
Kyle Hall 2013-10-23 11:58:34 -04:00 committed by Tomas Cohen Arazi
parent 3cbf754702
commit 6b2d2abb19
5 changed files with 189 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

47
C4/Reserves.pm
View file

@ -915,7 +915,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
@ -928,7 +930,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
@ -944,14 +948,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
@ -963,21 +967,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;
}
}
}

3
installer/data/mysql/sysprefs.sql
View file

@ -185,6 +185,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'),

12
installer/data/mysql/updatedatabase.pl
View file

@ -8974,6 +8974,18 @@ if ( C4::Context->preference("Version") < TransformToNum($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)

15
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
View file

@ -546,6 +546,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

119
t/db_dependent/Holds/LocalHoldsPriority.t Executable file
View file

@ -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, '' );
}