[Koha-patches] [PATCH] Allow SIP checkout to pre-empt unfilled holds.
Galen Charlton
galen.charlton at liblime.com
Tue Nov 4 21:34:53 CET 2008
From: Joe Atzberger (siptest <atz4sip at arwen.metavore.com>
This affects transactions on items that may be covered by
a title or item-level hold, but have not yet been retrieved
from the stacks (i.e. "confirmed") by the librarian. This does
not affect waiting holds (found="W"), so they will still block
transaction unless they belong to the operating patron.
Also I cleanup any hold the patron has on a biblio/item when
they are allowed to checkout the item.
Here we are filling the hold because the checkout is allowed, regardless
of of the queue position. This is different from AddIssue's CancelReserve, that
only fills the hold if it is next in line, but in the future AddIssue should
adopt a similar logic.
Signed-off-by: Galen Charlton <galen.charlton at liblime.com>
---
C4/SIP/ILS.pm | 5 +-
C4/SIP/ILS/Item.pm | 49 ++++++++++++++++++-----
C4/SIP/ILS/Patron.pm | 3 +-
C4/SIP/ILS/Transaction/Checkout.pm | 76 +++++++++++++++++++++++-------------
4 files changed, 93 insertions(+), 40 deletions(-)
diff --git a/C4/SIP/ILS.pm b/C4/SIP/ILS.pm
index 1a4fc54..2dcc616 100644
--- a/C4/SIP/ILS.pm
+++ b/C4/SIP/ILS.pm
@@ -138,8 +138,9 @@ sub checkout {
$circ->screen_msg("Patron Blocked");
} elsif (!$item) {
$circ->screen_msg("Invalid Item");
- } elsif ($item->hold_queue && @{$item->hold_queue} && ! $item->barcode_is_borrowernumber($patron_id, $item->hold_queue->[0]->{borrowernumber})) {
- $circ->screen_msg("Item on Hold for Another User");
+ # holds checked inside do_checkout
+ # } elsif ($item->hold_queue && @{$item->hold_queue} && ! $item->barcode_is_borrowernumber($patron_id, $item->hold_queue->[0]->{borrowernumber})) {
+ # $circ->screen_msg("Item on Hold for Another User");
} elsif ($item->{patron} && ($item->{patron} ne $patron_id)) {
# I can't deal with this right now
$circ->screen_msg("Item checked out to another patron");
diff --git a/C4/SIP/ILS/Item.pm b/C4/SIP/ILS/Item.pm
index 2a4f850..b32ea92 100644
--- a/C4/SIP/ILS/Item.pm
+++ b/C4/SIP/ILS/Item.pm
@@ -81,13 +81,14 @@ sub new {
my ($class, $item_id) = @_;
my $type = ref($class) || $class;
my $self;
- my $item = GetBiblioFromItemNumber( GetItemnumberFromBarcode($item_id) );
-
+ my $itemnumber = GetItemnumberFromBarcode($item_id);
+ my $item = GetBiblioFromItemNumber($itemnumber);
if (! $item) {
syslog("LOG_DEBUG", "new ILS::Item('%s'): not found", $item_id);
warn "new ILS::Item($item_id) : No item '$item_id'.";
return undef;
}
+ $item->{itemnumber} = $itemnumber;
$item->{'id'} = $item->{'barcode'};
# check if its on issue and if so get the borrower
my $issue = GetItemIssue($item->{'itemnumber'});
@@ -95,7 +96,8 @@ sub new {
$item->{patron} = $borrower->{'cardnumber'};
my @reserves = (@{ GetReservesFromBiblionumber($item->{biblionumber}) });
$item->{hold_queue} = [ sort priority_sort @reserves ];
- # $item->{joetest} = 111;
+ $item->{hold_shelf} = [( grep { defined $_->{found} and $_->{found} eq 'W' } @{$item->{hold_queue}} )];
+ $item->{pending_queue} = [( grep {(! defined $_->{found}) or ($_->{found} ne 'W')} @{$item->{hold_queue}} )];
$self = $item;
bless $self, $type;
@@ -178,6 +180,16 @@ sub hold_queue {
(defined $self->{hold_queue}) or return [];
return $self->{hold_queue};
}
+sub pending_queue {
+ my $self = shift;
+ (defined $self->{pending_queue}) or return [];
+ return $self->{pending_queue};
+}
+sub hold_shelf {
+ my $self = shift;
+ (defined $self->{hold_shelf}) or return [];
+ return $self->{hold_shelf};
+}
sub hold_queue_position {
my ($self, $patron_id) = @_;
@@ -216,20 +228,29 @@ sub print_line {
# This is a partial check of "availability". It is not supposed to check everything here.
# An item is available for a patron if it is:
-# 1) checked out to the same patron and there's no hold queue
+# 1) checked out to the same patron
+# AND no pending (i.e. non-W) hold queue
# OR
-# 2) not checked out and (there's no hold queue OR patron
-# is at the front of the queue)
+# 2) not checked out
+# AND (not on hold_shelf OR is on hold_shelf for patron)
+#
+# What this means is we are consciously allowing the patron to checkout (but not renew) an item that DOES
+# have non-W holds on it, but has not been "picked" from the stacks. That is to say, the
+# patron has retrieved the item before the librarian.
+#
+# We don't check if the patron is at the front of the pending queue in the first case, because
+# they should not be able to place a hold on an item they already have.
+
sub available {
my ($self, $for_patron) = @_;
- my $count = (defined $self->{hold_queue}) ? scalar @{$self->{hold_queue}} : 0;
- $debug and print STDERR "availability check: hold_queue size $count\n";
+ my $count = (defined $self->{pending_queue}) ? scalar @{$self->{pending_queue}} : 0;
+ my $count2 = (defined $self->{hold_shelf} ) ? scalar @{$self->{hold_shelf} } : 0;
+ $debug and print STDERR "availability check: pending_queue size $count, hold_shelf size $count2\n";
if (defined($self->{patron_id})) {
($self->{patron_id} eq $for_patron) or return 0;
return ($count ? 0 : 1);
} else { # not checked out
- ($count) or return 1;
- ($self->barcode_is_borrowernumber($for_patron, $self->{hold_queue}[0]->{borrowernumber})) and return 1;
+ ($count2) and return $self->barcode_is_borrowernumber($for_patron, $self->{hold_shelf}[0]->{borrowernumber});
}
return 0;
}
@@ -248,6 +269,14 @@ sub barcode_is_borrowernumber ($$$) { # because hold_queue only has borrowern
my $converted = _barcode_to_borrowernumber($barcode) or return undef;
return ($number eq $converted); # even though both *should* be numbers, eq is safer.
}
+sub fill_reserve ($$) {
+ my $self = shift;
+ my $hold = shift or return undef;
+ foreach (qw(biblionumber borrowernumber reservedate)) {
+ $hold->{$_} or return undef;
+ }
+ return ModReserveFill($hold);
+}
1;
__END__
diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm
index 257f9e2..2252be5 100644
--- a/C4/SIP/ILS/Patron.pm
+++ b/C4/SIP/ILS/Patron.pm
@@ -65,6 +65,7 @@ sub new {
ptype => $kp->{categorycode}, # 'A'dult. Whatever.
birthdate => $kp->{dateofbirth}, ##$dob,
branchcode => $kp->{branchcode},
+ borrowernumber => $kp->{borrowernumber},
address => $adr,
home_phone => $kp->{phone},
email_addr => $kp->{email},
@@ -99,7 +100,7 @@ sub new {
}
}
- # FIXME: populate items fine_items recall_items
+ # FIXME: populate fine_items recall_items
# $ilspatron{hold_items} = (GetReservesFromBorrowernumber($kp->{borrowernumber},'F'));
$ilspatron{unavail_holds} = [(GetReservesFromBorrowernumber($kp->{borrowernumber}))];
my ($count,$issues) = GetPendingIssues($kp->{borrowernumber});
diff --git a/C4/SIP/ILS/Transaction/Checkout.pm b/C4/SIP/ILS/Transaction/Checkout.pm
index f4f7ff8..8a14877 100644
--- a/C4/SIP/ILS/Transaction/Checkout.pm
+++ b/C4/SIP/ILS/Transaction/Checkout.pm
@@ -18,6 +18,7 @@ use ILS::Transaction;
use C4::Context;
use C4::Circulation;
use C4::Members;
+use C4::Reserves qw(ModReserveFill);
use C4::Debug;
use vars qw($VERSION @ISA $debug);
@@ -50,51 +51,72 @@ sub new {
sub do_checkout {
my $self = shift;
syslog('LOG_DEBUG', "ILS::Transaction::Checkout performing checkout...");
+ my $pending = $self->{item}->pending_queue;
+ my $shelf = $self->{item}->hold_shelf;
my $barcode = $self->{item}->id;
my $patron_barcode = $self->{patron}->id;
$debug and warn "do_checkout: patron (" . $patron_barcode . ")";
- # my $borrower = GetMember( $patron_barcode, 'cardnumber' );
- # my $borrower = $self->{patron};
- # my $borrower = GetMemberDetails(undef, $patron_barcode);
my $borrower = $self->{patron}->getmemberdetails_object();
$debug and warn "do_checkout borrower: . " . Dumper $borrower;
my ($issuingimpossible,$needsconfirmation) = CanBookBeIssued( $borrower, $barcode );
my $noerror=1;
- foreach ( keys %$issuingimpossible ) {
- # do something here so we pass these errors
- $self->screen_msg($_ . ': ' . $issuingimpossible->{$_});
- $noerror = 0;
- }
- foreach my $confirmation ( keys %$needsconfirmation ) {
- if ($confirmation eq 'RENEW_ISSUE'){
- $self->screen_msg("Item already checked out to you: renewing item.");
- } elsif ($confirmation eq 'RESERVED' or $confirmation eq 'RESERVE_WAITING') {
- my $x = $self->{item}->available($patron_barcode);
- if ($x) {
- $self->screen_msg("Item was reserved for you.");
+ if (scalar keys %$issuingimpossible) {
+ foreach (keys %$issuingimpossible) {
+ # do something here so we pass these errors
+ $self->screen_msg($_ . ': ' . $issuingimpossible->{$_});
+ $noerror = 0;
+ }
+ } else {
+ foreach my $confirmation (keys %$needsconfirmation) {
+ if ($confirmation eq 'RENEW_ISSUE'){
+ $self->screen_msg("Item already checked out to you: renewing item.");
+ } elsif ($confirmation eq 'RESERVED' or $confirmation eq 'RESERVE_WAITING') {
+ my $x = $self->{item}->available($patron_barcode);
+ if ($x) {
+ $self->screen_msg("Item was reserved for you.");
+ } else {
+ $self->screen_msg("Item is reserved for another patron upon return.");
+ # $noerror = 0;
+ }
+ } elsif ($confirmation eq 'ISSUED_TO_ANOTHER') {
+ $self->screen_msg("Item already checked out to another patron. Please return item for check-in.");
+ $noerror = 0;
+ } elsif ($confirmation eq 'DEBT') { # don't do anything, it's the minor debt, and alarms fire elsewhere
} else {
- $self->screen_msg("Item is reserved for another patron.");
+ $self->screen_msg($needsconfirmation->{$confirmation});
$noerror = 0;
}
- } elsif ($confirmation eq 'ISSUED_TO_ANOTHER') {
- $self->screen_msg("Item already checked out to another patron. Please return item for check-in.");
- $noerror = 0;
- } elsif ($confirmation eq 'DEBT') { # don't do anything, it's the minor debt, and alarms fire elsewhere
- } else {
- $self->screen_msg($needsconfirmation->{$confirmation});
- $noerror = 0;
- }
- }
+ }
+ }
+ my $itemnumber = $self->{item}->{itemnumber};
+ foreach (@$shelf) {
+ $debug and warn "shelf has ($_->{itemnumber} for $_->{borrowernumber}). this is ($itemnumber, $self->{patron}->{borrowernumber})";
+ ($_->{itemnumber} eq $itemnumber) or next; # skip it if not this item
+ ($_->{borrowernumber} == $self->{patron}->{borrowernumber}) and last;
+ # if item was waiting for this patron, we're done. AddIssue takes care of the "W" hold.
+ $debug and warn "Item is on hold shelf for another patron.";
+ $self->screen_msg("Item is on hold shelf for another patron.");
+ $noerror = 0;
+ }
unless ($noerror) {
- warn "cannot issue: " . Dumper($issuingimpossible) . "\n" . Dumper($needsconfirmation);
+ $debug and warn "cannot issue: " . Dumper($issuingimpossible) . "\n" . Dumper($needsconfirmation);
$self->ok(0);
return $self;
}
+ # Fill any reserves the patron had on the item.
+ # TODO: this logic should be pulled internal to AddIssue for all Koha.
+ $debug and warn "pending_queue: " . (@$pending) ? Dumper($pending) : '[]';
+ foreach (grep {$_->{borrowernumber} eq $self->{patron}->{borrowernumber}} @$pending) {
+ $debug and warn "Filling reserve (borrowernumber,biblionumber,reservedate): "
+ . sprintf("(%s,%s,%s)\n",$_->{borrowernumber},$_->{biblionumber},$_->{reservedate});
+ ModReserveFill($_);
+ # TODO: adjust representation in $self->item
+ }
# can issue
$debug and warn "do_checkout: calling AddIssue(\$borrower,$barcode, undef, 0)\n"
# . "w/ \$borrower: " . Dumper($borrower)
. "w/ C4::Context->userenv: " . Dumper(C4::Context->userenv);
- my $c4due = AddIssue( $borrower, $barcode, undef, 0 );
+ my $c4due = AddIssue($borrower, $barcode, undef, 0);
my $due = $c4due->output('iso') || undef;
$debug and warn "Item due: $due";
$self->{'due'} = $due;
--
1.5.5.GIT
More information about the Koha-patches
mailing list