From 72583589cafedbf9571a71a0a9eab009ec53b6a3 Mon Sep 17 00:00:00 2001 From: "Joe Atzberger (siptest" Date: Tue, 4 Nov 2008 14:34:53 -0600 Subject: [PATCH] Allow SIP checkout to pre-empt unfilled holds. 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 --- 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 1a4fc542b9..2dcc616b59 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 2a4f850653..b32ea92430 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 257f9e23ab..2252be58fd 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 f4f7ff8e0a..8a14877f01 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; -- 2.39.5