[Koha-patches] [PATCH 1/3] bug 3435: SIP Checkin extension for 3M SmartChute - partial
Galen Charlton
galen.charlton at liblime.com
Wed Jul 22 17:38:22 CEST 2009
From: Joe Atzberger <joe.atzberger at liblime.com>
Signed-off-by: Galen Charlton <galen.charlton at liblime.com>
---
C4/Reserves.pm | 81 +++++++++-------------
C4/SIP/ILS.pm | 2 +-
C4/SIP/ILS/Item.pm | 137 ++++++++++++++++++++++++++----------
C4/SIP/ILS/Item.pod | 52 +++++++-------
C4/SIP/ILS/Transaction/Checkin.pm | 10 ++--
C4/SIP/Sip/MsgType.pm | 25 +++----
6 files changed, 176 insertions(+), 131 deletions(-)
diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index fca00f6..11846e7 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -559,6 +559,7 @@ sub GetReservesForBranch {
=item CheckReserves
($status, $reserve) = &CheckReserves($itemnumber);
+ ($status, $reserve) = &CheckReserves(undef, $barcode);
Find a book in the reserves.
@@ -586,65 +587,50 @@ sub CheckReserves {
my ( $item, $barcode ) = @_;
my $dbh = C4::Context->dbh;
my $sth;
+ my $select = "
+ SELECT items.biblionumber,
+ items.biblioitemnumber,
+ itemtypes.notforloan,
+ items.notforloan AS itemnotforloan,
+ items.itemnumber
+ FROM items
+ LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber
+ LEFT JOIN itemtypes ON biblioitems.itemtype = itemtypes.itemtype
+ ";
+
if ($item) {
- my $qitem = $dbh->quote($item);
- # Look up the item by itemnumber
- my $query = "
- SELECT items.biblionumber, items.biblioitemnumber, itemtypes.notforloan, items.notforloan AS itemnotforloan
- FROM items
- LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber
- LEFT JOIN itemtypes ON biblioitems.itemtype = itemtypes.itemtype
- WHERE itemnumber=$qitem
- ";
- $sth = $dbh->prepare($query);
+ $sth = $dbh->prepare("$select WHERE itemnumber = ?");
+ $sth->execute($item);
}
else {
- my $qbc = $dbh->quote($barcode);
- # Look up the item by barcode
- my $query = "
- SELECT items.biblionumber, items.biblioitemnumber, itemtypes.notforloan, items.notforloan AS itemnotforloan
- FROM items
- LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber
- LEFT JOIN itemtypes ON biblioitems.itemtype = itemtypes.itemtype
- WHERE items.biblioitemnumber = biblioitems.biblioitemnumber
- AND biblioitems.itemtype = itemtypes.itemtype
- AND barcode=$qbc
- ";
- $sth = $dbh->prepare($query);
-
- # FIXME - This function uses $item later on. Ought to set it here.
+ $sth = $dbh->prepare("$select WHERE barcode = ?");
+ $sth->execute($barcode);
}
- $sth->execute;
- my ( $biblio, $bibitem, $notforloan_per_itemtype, $notforloan_per_item ) = $sth->fetchrow_array;
- $sth->finish;
+ # 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;
+
+ return ( 0, 0 ) unless $itemnumber; # bail if we got nothing.
+
# if item is not for loan it cannot be reserved either.....
- # execption to notforloan is where items.notforloan < 0 : This indicates the item is holdable.
+ # execpt where items.notforloan < 0 : This indicates the item is holdable.
return ( 0, 0 ) if ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype;
- # get the reserves...
# Find this item in the reserves
- my @reserves = _Findgroupreserve( $bibitem, $biblio, $item );
- my $count = scalar @reserves;
+ my @reserves = _Findgroupreserve( $bibitem, $biblio, $itemnumber );
# $priority and $highest are used to find the most important item
# in the list returned by &_Findgroupreserve. (The lower $priority,
# the more important the item.)
# $highest is the most important item we've seen so far.
- my $priority = 10000000;
my $highest;
- if ($count) {
+ if (scalar @reserves) {
+ my $priority = 10000000;
foreach my $res (@reserves) {
- # FIXME - $item might be undefined or empty: the caller
- # might be searching by barcode.
- if ( $res->{'itemnumber'} == $item && $res->{'priority'} == 0) {
- # Found it
- return ( "Waiting", $res );
- }
- else {
- # See if this item is more important than what we've got
- # so far.
- if ( $res->{'priority'} != 0 && $res->{'priority'} < $priority )
- {
+ if ( $res->{'itemnumber'} == $itemnumber && $res->{'priority'} == 0) {
+ return ( "Waiting", $res ); # Found it
+ } else {
+ # See if this item is more important than what we've got so far
+ if ( $res->{'priority'} && $res->{'priority'} < $priority ) {
$priority = $res->{'priority'};
$highest = $res;
}
@@ -652,10 +638,9 @@ sub CheckReserves {
}
}
- # If we get this far, then no exact match was found. Print the
- # most important item on the list. I think this tells us who's
- # next in line to get this book.
- if ($highest) { # FIXME - $highest might be undefined
+ # If we get this far, then no exact match was found.
+ # We return the most important (i.e. next) reservation.
+ if ($highest) {
$highest->{'itemnumber'} = $item;
return ( "Reserved", $highest );
}
diff --git a/C4/SIP/ILS.pm b/C4/SIP/ILS.pm
index 09c454f..b1e516a 100644
--- a/C4/SIP/ILS.pm
+++ b/C4/SIP/ILS.pm
@@ -157,7 +157,7 @@ sub checkout {
$item->{patron} = $patron_id;
$item->{due_date} = $circ->{due};
push(@{$patron->{items}}, $item_id);
- $circ->desensitize(!$item->magnetic);
+ $circ->desensitize(!$item->magnetic_media);
syslog("LOG_DEBUG", "ILS::Checkout: patron %s has checked out %s",
$patron_id, join(', ', @{$patron->{items}}));
diff --git a/C4/SIP/ILS/Item.pm b/C4/SIP/ILS/Item.pm
index b32ea92..28cd4bb 100644
--- a/C4/SIP/ILS/Item.pm
+++ b/C4/SIP/ILS/Item.pm
@@ -1,8 +1,7 @@
#
# ILS::Item.pm
#
-# A Class for hiding the ILS's concept of the item from the OpenSIP
-# system
+# A Class for hiding the ILS's concept of the item from OpenSIP
#
package ILS::Item;
@@ -11,6 +10,7 @@ use strict;
use warnings;
use Sys::Syslog qw(syslog);
+use Carp;
use ILS::Transaction;
@@ -25,7 +25,7 @@ use C4::Reserves;
use vars qw($VERSION @ISA @EXPORT @EXPORT_OK);
BEGIN {
- $VERSION = 2.10;
+ $VERSION = 2.11;
require Exporter;
@ISA = qw(Exporter);
@EXPORT_OK = qw();
@@ -82,22 +82,27 @@ sub new {
my $type = ref($class) || $class;
my $self;
my $itemnumber = GetItemnumberFromBarcode($item_id);
- my $item = GetBiblioFromItemNumber($itemnumber);
+ my $item = GetBiblioFromItemNumber($itemnumber); # actually biblio.*, biblioitems.* AND items.* (overkill)
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'};
+ $item->{ 'itemnumber' } = $itemnumber;
+ $item->{ 'id' } = $item->{barcode}; # to SIP, the barcode IS the id.
+ $item->{permanent_location}= $item->{homebranch};
+ $item->{'collection_code'} = $item->{ccode};
+ $item->{ 'call_number' } = $item->{itemcallnumber};
+ # $item->{'destination_loc'} = ?
+
# check if its on issue and if so get the borrower
my $issue = GetItemIssue($item->{'itemnumber'});
my $borrower = GetMember($issue->{'borrowernumber'},'borrowernumber');
$item->{patron} = $borrower->{'cardnumber'};
- my @reserves = (@{ GetReservesFromBiblionumber($item->{biblionumber}) });
- $item->{hold_queue} = [ sort priority_sort @reserves ];
+ my ($whatever, $arrayref) = GetReservesFromBiblionumber($item->{biblionumber});
+ $item->{hold_queue} = [ sort priority_sort @$arrayref ];
$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}} )];
+ $item->{pending_queue} = [( grep {(! defined $_->{found}) or $_->{found} ne 'W' } @{$item->{hold_queue}} )];
$self = $item;
bless $self, $type;
@@ -107,17 +112,93 @@ sub new {
return $self;
}
-sub magnetic {
- my $self = shift;
- return $self->{magnetic_media};
+# 0 means read-only
+# 1 means read/write
+
+my %fields = (
+ id => 0,
+ sip_media_type => 0,
+ sip_item_properties => 0,
+ magnetic_media => 0,
+ permanent_location => 0,
+ current_location => 0,
+ print_line => 1,
+ screen_msg => 1,
+ itemnumber => 0,
+ biblionumber => 0,
+ barcode => 0,
+ onloan => 0,
+ collection_code => 0,
+ call_number => 0,
+ enumchron => 0,
+ location => 0,
+ author => 0,
+ title => 0,
+);
+
+sub next_hold {
+ my $self = shift or return;
+ foreach (@$self->{hold_shelf}) { # If this item was taken from the hold shelf, then that reserve still governs
+ next unless ($_->{itemnumber} and $_->{itemnumber} == $self->{itemnumber});
+ return $_;
+ }
+ if (scalar @$self->{pending_queue}) { # Otherwise, if there is at least one hold, the first (best priority) gets it
+ return $self->{pending_queue}->[0];
+ }
+ return;
}
-sub sip_media_type {
- my $self = shift;
- return $self->{sip_media_type};
+
+sub hold_patron_id {
+ my $self = shift or return;
+ my $hold = $self->next_hold() or return;
+ return $hold->{borrowernumber};
+}
+sub hold_patron_name {
+ my $self = shift or return;
+ # return $self->{hold_patron_name} if $self->{hold_patron_name}; TODO: consider caching
+ my $borrowernumber = (@_ ? shift: $self->hold_patron_id()) or return;
+ my $holder = GetMember($borrowernumber, 'borrowernumber');
+ unless ($holder) {
+ syslog("LOG_ERR", "While checking hold, GetMember failed for borrowernumber '$borrowernumber'");
+ return;
+ }
+ my $email = $holder{email} || '';
+ my $phone = $holder{phone} || '';
+ my $extra = ($email and $phone) ? " ($email, $phone)" : # both populated, employ comma
+ ($email or $phone) ? " ($email$phone)" : # only 1 populated, we don't care which: no comma
+ "" ; # niether populated, empty string
+ my $name = $holder->{firstname} ? $holder->{firstname} . ' ' : '';
+ $name .= $holder->{surname} . $extra;
+ # $self->{hold_patron_name} = $name; # TODO: consider caching
+ return $name;
+}
+sub destination_loc {
+ my $self = shift or return;
+ my $hold = $self->next_hold();
+ return ($hold ? $hold->{branchcode}
}
-sub sip_item_properties {
+
+our $AUTOLOAD;
+
+sub DESTROY { } # keeps AUTOLOAD from catching inherent DESTROY calls
+
+sub AUTOLOAD {
my $self = shift;
- return $self->{sip_item_properties};
+ my $class = ref($self) or croak "$self is not an object";
+ my $name = $AUTOLOAD;
+
+ $name =~ s/.*://;
+
+ unless (exists $fields{$name}) {
+ croak "Cannot access '$name' field of class '$class'";
+ }
+
+ if (@_) {
+ $fields{$name} or croak "Field '$name' of class '$class' is READ ONLY.";
+ return $self->{$name} = shift;
+ } else {
+ return $self->{$name};
+ }
}
sub status_update { # FIXME: this looks unimplemented
@@ -128,22 +209,10 @@ sub status_update { # FIXME: this looks unimplemented
return $status;
}
-sub id {
- my $self = shift;
- return $self->{id};
-}
sub title_id {
my $self = shift;
return $self->{title};
}
-sub permanent_location {
- my $self = shift;
- return $self->{permanent_location} || '';
-}
-sub current_location {
- my $self = shift;
- return $self->{current_location} || '';
-}
sub sip_circulation_status {
my $self = shift;
@@ -173,7 +242,7 @@ sub fee_currency {
}
sub owner {
my $self = shift;
- return 'CPL'; # FIXME: UWOLS was hardcoded
+ return $self->{homebranch};
}
sub hold_queue {
my $self = shift;
@@ -217,14 +286,6 @@ sub hold_pickup_date {
my $self = shift;
return $self->{hold_pickup_date} || 0;
}
-sub screen_msg {
- my $self = shift;
- return $self->{screen_msg} || '';
-}
-sub print_line {
- my $self = shift;
- return $self->{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:
diff --git a/C4/SIP/ILS/Item.pod b/C4/SIP/ILS/Item.pod
index e4ba8c1..29f6761 100644
--- a/C4/SIP/ILS/Item.pod
+++ b/C4/SIP/ILS/Item.pod
@@ -11,30 +11,30 @@ ILS::Item - Portable Item status object class for SIP
my $item = new ILS::Item $item_id;
# Basic object access methods
- $item_id = $item->id;
- $title = $item->title_id;
- $media_type = $item->sip_media_type;
- $bool = $item->magnetic;
- $locn = $item->permanent_location;
- $locn = $item->current_location;
- $props = $item->sip_item_props;
- $owner = $item->owner;
- $str = $item->sip_circulation_status;
- $bool = $item->available;
- @hold_queue = $item->hold_queue;
- $pos = $item->hold_queue_position($patron_id);
- $due = $item->due_date;
- $pickup = $item->hold_pickup_date;
- $recall = $item->recall_date;
- $fee = $item->fee;
- $currency = $item->fee_currency;
- $type = $item->sip_fee_type;
- $mark = $item->sip_security_marker;
- $msg = $item->screen_msg;
- $msg = $item->print_line;
-
- # Operations on items
- $status = $item->status_update($item_props);
+ $item_id = $item->id;
+ $title = $item->title_id;
+ $media_type = $item->sip_media_type;
+ $bool = $item->magnetic_media;
+ $locn = $item->permanent_location;
+ $locn = $item->current_location;
+ $props = $item->sip_item_props;
+ $owner = $item->owner;
+ $str = $item->sip_circulation_status;
+ $bool = $item->available;
+ @hold_queue = $item->hold_queue;
+ $pos = $item->hold_queue_position($patron_id);
+ $due = $item->due_date;
+ $pickup = $item->hold_pickup_date;
+ $recall = $item->recall_date;
+ $fee = $item->fee;
+ $currency = $item->fee_currency;
+ $type = $item->sip_fee_type;
+ $mark = $item->sip_security_marker;
+ $msg = $item->screen_msg;
+ $msg = $item->print_line;
+
+ # Operations on items
+ $status = $item->status_update($item_props);
=head1 DESCRIPTION
@@ -93,9 +93,9 @@ behavior at all; it merely passes it through to the self-service
terminal. In particular, it does not set indicators related to
whether an item is magnetic, or whether it should be
desensitized, based on this return type. The
-C<$item-E<gt>magnetic> method will be used for that purpose.
+C<$item-E<gt>magnetic_media> method will be used for that purpose.
-=item C<magnetic>
+=item C<magnetic_media>
Is the item some form of magnetic media (eg, a video or a book
with an accompanying floppy)? This method will not be called
diff --git a/C4/SIP/ILS/Transaction/Checkin.pm b/C4/SIP/ILS/Transaction/Checkin.pm
index 6c4f360..fd5f861 100644
--- a/C4/SIP/ILS/Transaction/Checkin.pm
+++ b/C4/SIP/ILS/Transaction/Checkin.pm
@@ -7,7 +7,7 @@ package ILS::Transaction::Checkin;
use warnings;
use strict;
-use POSIX qw(strftime);
+# use POSIX qw(strftime);
use ILS;
use ILS::Transaction;
@@ -31,7 +31,7 @@ my %fields = (
sub new {
my $class = shift;
- my $self = $class->SUPER::new();
+ my $self = $class->SUPER::new(); # start with an ILS::Transaction object
foreach (keys %fields) {
$self->{_permitted}->{$_} = $fields{$_}; # overlaying _permitted
@@ -59,7 +59,7 @@ sub do_checkin {
$self->alert_type('99');
}
}
- defined $self->alert_type and $self->alert(1); # alert_type could be "00"
+ $self->alert(1) if defined $self->alert_type; # alert_type could be "00"
$self->ok($return);
}
@@ -69,7 +69,7 @@ sub resensitize {
warn "no item found in object to resensitize";
return;
}
- return !$self->{item}->magnetic;
+ return !$self->{item}->magnetic_media;
}
sub patron_id {
@@ -78,7 +78,7 @@ sub patron_id {
warn "no patron found in object";
return;
}
- return !$self->{patron}->id;
+ return $self->{patron}->id;
}
1;
diff --git a/C4/SIP/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm
index 16b1506..01e094f 100644
--- a/C4/SIP/Sip/MsgType.pm
+++ b/C4/SIP/Sip/MsgType.pm
@@ -531,7 +531,7 @@ sub handle_checkout {
$resp = CHECKOUT_RESP . '1';
$resp .= sipbool($status->renew_ok);
if ($ils->supports('magnetic media')) {
- $resp .= sipbool($item->magnetic);
+ $resp .= sipbool($item->magnetic_media);
} else {
$resp .= 'U';
}
@@ -634,10 +634,9 @@ sub handle_checkin {
$resp .= $status->ok ? '1' : '0';
$resp .= $status->resensitize ? 'Y' : 'N';
if ($item && $ils->supports('magnetic media')) {
- $resp .= sipbool($item->magnetic);
+ $resp .= sipbool($item->magnetic_media);
} else {
- # The item barcode was invalid or the system doesn't support
- # the 'magnetic media' indicator
+ # item barcode is invalid or system doesn't support 'magnetic media' indicator
$resp .= 'U';
}
$resp .= $status->alert ? 'Y' : 'N';
@@ -656,17 +655,17 @@ sub handle_checkin {
$resp .= add_field(FID_PATRON_ID, $patron->id);
}
if ($item) {
- $resp .= maybe_add(FID_MEDIA_TYPE, $item->sip_media_type );
- $resp .= maybe_add(FID_ITEM_PROPS, $item->sip_item_properties);
- # $resp .= maybe_add(FID_COLLECTION_CODE, $item->collection_code );
- # $resp .= maybe_add(FID_CALL_NUMBER, $item->call_number );
- # $resp .= maybe_add(FID_DESTINATION, $item->destination_loc );
- # $resp .= maybe_add(FID_ALERT_TYPE, $item->alert_type );
- # $resp .= maybe_add(FID_PATRON_ID, $item->hold_patron_id );
- # $resp .= maybe_add(FID_PATRON_NAME, $item->hold_patron_name );
+ $resp .= maybe_add(FID_MEDIA_TYPE, $item->sip_media_type );
+ $resp .= maybe_add(FID_ITEM_PROPS, $item->sip_item_properties);
+ $resp .= maybe_add(FID_COLLECTION_CODE, $item->collection_code );
+ $resp .= maybe_add(FID_CALL_NUMBER, $item->call_number );
+ $resp .= maybe_add(FID_DESTINATION_LOCATION, $item->destination_loc );
+ $resp .= maybe_add(FID_HOLD_PATRON_ID, $item->hold_patron_id );
+ $resp .= maybe_add(FID_HOLD_PATRON_NAME, $item->hold_patron_name );
}
}
+ $resp .= maybe_add(FID_ALERT_TYPE, $status->alert_type) if $status->alert;
$resp .= maybe_add(FID_SCREEN_MSG, $status->screen_msg);
$resp .= maybe_add(FID_PRINT_LINE, $status->print_line);
@@ -1328,7 +1327,7 @@ sub handle_renew {
$resp .= '1';
$resp .= $status->renewal_ok ? 'Y' : 'N';
if ($ils->supports('magnetic media')) {
- $resp .= sipbool($item->magnetic);
+ $resp .= sipbool($item->magnetic_media);
} else {
$resp .= 'U';
}
--
1.5.6.5
More information about the Koha-patches
mailing list