X-Git-Url: http://koha-dev.rot13.org:8081/gitweb/?a=blobdiff_plain;f=C4%2FSIP%2FILS.pm;h=9e2084594edc7fd0a0144e4b508ede7ac8f065c0;hb=700249eb3cbf765a5a731effd21ba543b74eb334;hp=559c1ed6bc2201539fbea5a5578d201cbf0e2698;hpb=ca1d6576c5b603eec0836036a3d18923cf966e4f;p=koha_fer

diff --git a/C4/SIP/ILS.pm b/C4/SIP/ILS.pm
index 559c1ed6bc..9e2084594e 100644
--- a/C4/SIP/ILS.pm
+++ b/C4/SIP/ILS.pm
@@ -18,108 +18,103 @@ use ILS::Transaction::Hold;
 use ILS::Transaction::Renew;
 use ILS::Transaction::RenewAll;
 
+my $debug = 0;
+
 my %supports = (
-		'magnetic media'	=> 1,
-		'security inhibit'	=> 0,
-		'offline operation'	=> 0,
-		"patron status request" => 1,
-		"checkout"		=> 1,
-		"checkin"		=> 1,
-		"block patron"		=> 1,
-		"acs status"		=> 1,
-		"login"			=> 1,
-		"patron information"	=> 1,
-		"end patron session"	=> 1,
-		"fee paid"		=> 0,
-		"item information"	=> 1,
-		"item status update"	=> 0,
-		"patron enable"		=> 1,
-		"hold"			=> 1,
-		"renew"			=> 1,
-		"renew all"		=> 0,
-	       );
+    'magnetic media'        => 1,
+    'security inhibit'      => 0,
+    'offline operation'     => 0,
+    "patron status request" => 1,
+    "checkout"              => 1,
+    "checkin"               => 1,
+    "block patron"          => 1,
+    "acs status"            => 1,
+    "login"                 => 1,
+    "patron information"    => 1,
+    "end patron session"    => 1,
+    "fee paid"              => 0,
+    "item information"      => 1,
+    "item status update"    => 0,
+    "patron enable"         => 1,
+    "hold"                  => 1,
+    "renew"                 => 1,
+    "renew all"             => 1,
+);
 
 sub new {
     my ($class, $institution) = @_;
     my $type = ref($class) || $class;
     my $self = {};
-#use Data::Dumper;
-#warn " INSTITUTION:";
-#warn Dumper($institution);
+	use Data::Dumper;
+	$debug and warn "new ILS: INSTITUTION: " . Dumper($institution);
     syslog("LOG_DEBUG", "new ILS '%s'", $institution->{id});
     $self->{institution} = $institution;
-
     return bless $self, $type;
 }
 
 sub find_patron {
     my $self = shift;
-warn "finding patron";
+ 	$debug and warn "ILS: finding patron";
     return ILS::Patron->new(@_);
 }
 
 sub find_item {
     my $self = shift;
-warn "find item";
+	$debug and warn "ILS: finding item";
     return ILS::Item->new(@_);
 }
 
 sub institution {
     my $self = shift;
+    return $self->{institution}->{id};  # consider making this return the whole institution
+}
 
+sub institution_id {
+    my $self = shift;
     return $self->{institution}->{id};
 }
 
 sub supports {
     my ($self, $op) = @_;
-
     return (exists($supports{$op}) && $supports{$op});
 }
 
 sub check_inst_id {
     my ($self, $id, $whence) = @_;
-
     if ($id ne $self->{institution}->{id}) {
-	syslog("LOG_WARNING", "%s: received institution '%s', expected '%s'",
-	       $whence, $id, $self->{institution}->{id});
+        syslog("LOG_WARNING", "%s: received institution '%s', expected '%s'", $whence, $id, $self->{institution}->{id});
+        # Just an FYI check, we don't expect the user to change location from that in SIPconfig.xml
     }
 }
 
 sub to_bool {
     my $bool = shift;
-
     # If it's defined, and matches a true sort of string, or is
     # a non-zero number, then it's true.
-    return defined($bool) && (($bool =~ /true|y|yes/i) || $bool != 0);
+    defined($bool) or return;                   # false
+    ($bool =~ /true|y|yes/i) and return 1;      # true
+    return ($bool =~ /^\d+$/ and $bool != 0);   # true for non-zero numbers, false otherwise
 }
 
 sub checkout_ok {
     my $self = shift;
-
-    return (exists($self->{policy}->{checkout})
-	    && to_bool($self->{policy}->{checkout}));
+    return (exists($self->{institution}->{policy}->{checkout})
+	    && to_bool($self->{institution}->{policy}->{checkout}));
 }
-
 sub checkin_ok {
     my $self = shift;
-
-    return (exists($self->{policy}->{checkin})
-	    && to_bool($self->{policy}->{checkin}));
+    return (exists($self->{institution}->{policy}->{checkin})
+	    && to_bool($self->{institution}->{policy}->{checkin}));
 }
-
 sub status_update_ok {
     my $self = shift;
-
-    return (exists($self->{policy}->{status_update})
-	    && to_bool($self->{policy}->{status_update}));
-
+    return (exists($self->{institution}->{policy}->{status_update})
+	    && to_bool($self->{institution}->{policy}->{status_update}));
 }
-
 sub offline_ok {
     my $self = shift;
-
-    return (exists($self->{policy}->{offline})
-	    && to_bool($self->{policy}->{offline}));
+    return (exists($self->{institution}->{policy}->{offline})
+	    && to_bool($self->{institution}->{policy}->{offline}));
 }
 
 #
@@ -145,15 +140,16 @@ sub checkout {
 		$circ->screen_msg("Patron Blocked");
     } elsif (!$item) {
 		$circ->screen_msg("Invalid Item");
-    } elsif ($item->hold_queue && @{$item->hold_queue} && ($patron_id ne $item->hold_queue->[0])) {
-		$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");
     } else {
 		$circ->do_checkout();
 		if ($circ->ok){
-			warn "circ is ok";
+			$debug and warn "circ is ok";
 			# If the item is already associated with this patron, then
 			# we're renewing it.
 			$circ->renew_ok($item->{patron} && ($item->{patron} eq $patron_id));
@@ -161,17 +157,15 @@ 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}}));
 		}
 		else {
-			syslog("LOG_DEBUG", "ILS::Checkout Issue failed");
-			
+			syslog("LOG_ERR", "ILS::Checkout Issue failed");
 		}
     }
-
     # END TRANSACTION
 
     return $circ;
@@ -185,8 +179,14 @@ sub checkin {
     $circ = new ILS::Transaction::Checkin;
     # BEGIN TRANSACTION
     $circ->item($item = new ILS::Item $item_id);
-    
-    $circ->do_checkin();    
+
+    if ($item) {
+        $circ->do_checkin($current_loc, $return_date);
+    } else {
+        $circ->alert(1);
+        $circ->alert_type(99);
+        $circ->screen_msg('Invalid Item');
+    }
 	# It's ok to check it in if it exists, and if it was checked out
 	$circ->ok($item && $item->{patron});
 
@@ -195,7 +195,6 @@ sub checkin {
 		delete $item->{patron};
 		delete $item->{due_date};
 		$patron->{items} = [ grep {$_ ne $item_id} @{$patron->{items}} ];
-		
     }
     # END TRANSACTION
 
@@ -232,35 +231,27 @@ sub add_hold {
     my ($self, $patron_id, $patron_pwd, $item_id, $title_id,
 	$expiry_date, $pickup_location, $hold_type, $fee_ack) = @_;
     my ($patron, $item);
-    my $hold;
-    my $trans;
-
 
-    $trans = new ILS::Transaction::Hold;
+	my $trans = new ILS::Transaction::Hold;
 
-    # BEGIN TRANSACTION
     $patron = new ILS::Patron $patron_id;
     if (!$patron
 	|| (defined($patron_pwd) && !$patron->check_password($patron_pwd))) {
-	$trans->screen_msg("Invalid Patron.");
-
-	return $trans;
+		$trans->screen_msg("Invalid Patron.");
+		return $trans;
     }
 
-    $item = new ILS::Item ($item_id || $title_id);
-    if (!$item) {
-	$trans->screen_msg("No such item.");
-
-	# END TRANSACTION (conditionally)
-	return $trans;
-    } elsif ($item->fee && ($fee_ack ne 'Y')) {
-	$trans->screen_msg = "Fee required to place hold.";
+	unless ($item = new ILS::Item ($item_id || $title_id)) {
+		$trans->screen_msg("No such item.");
+		return $trans;
+	}
 
-	# END TRANSACTION (conditionally)
-	return $trans;
+   if ($item->fee and $fee_ack ne 'Y') {
+		$trans->screen_msg = "Fee required to place hold.";
+		return $trans;
     }
 
-    $hold = {
+    my $hold = {
 	item_id         => $item->id,
 	patron_id       => $patron->id,
 	expiration_date => $expiry_date,
@@ -272,68 +263,62 @@ sub add_hold {
     $trans->patron($patron);
     $trans->item($item);
     $trans->pickup_location($pickup_location);
+	$trans->do_hold;
 
-    push(@{$item->hold_queue}, $hold);
+    push(@{$item->hold_queue},     $hold);
     push(@{$patron->{hold_items}}, $hold);
 
-
-    # END TRANSACTION
     return $trans;
 }
 
 sub cancel_hold {
     my ($self, $patron_id, $patron_pwd, $item_id, $title_id) = @_;
     my ($patron, $item, $hold);
-    my $trans;
 
-    $trans = new ILS::Transaction::Hold;
+	my $trans = new ILS::Transaction::Hold;
 
-    # BEGIN TRANSACTION
     $patron = new ILS::Patron $patron_id;
     if (!$patron) {
-	$trans->screen_msg("Invalid patron barcode.");
-
-	return $trans;
+		$trans->screen_msg("Invalid patron barcode.");
+		return $trans;
     } elsif (defined($patron_pwd) && !$patron->check_password($patron_pwd)) {
-	$trans->screen_msg('Invalid patron password.');
-
-	return $trans;
+		$trans->screen_msg('Invalid patron password.');
+		return $trans;
     }
 
-    $item = new ILS::Item ($item_id || $title_id);
-    if (!$item) {
-	$trans->screen_msg("No such item.");
-
-	# END TRANSACTION (conditionally)
-	return $trans;
+    unless ($item = new ILS::Item ($item_id || $title_id)) {
+		$trans->screen_msg("No such item.");
+		return $trans;
     }
 
+    $trans->patron($patron);
+    $trans->item($item);
+	$trans->drop_hold;
+	unless ($trans->ok) {
+		$trans->screen_msg("Error with transaction drop_hold: " . $trans->screen_msg);
+		return $trans;
+	}
     # Remove the hold from the patron's record first
-    $trans->ok($patron->drop_hold($item_id));
+    $trans->ok($patron->drop_hold($item_id));	# different than the transaction drop!
 
-    if (!$trans->ok) {
-	# We didn't find it on the patron record
-	$trans->screen_msg("No such hold on patron record.");
-
-	# END TRANSACTION (conditionally)
-	return $trans;
+    unless ($trans->ok) {
+		# We didn't find it on the patron record
+		$trans->screen_msg("No such hold on patron record.");
+		return $trans;
     }
 
     # Now, remove it from the item record.  If it was on the patron
     # record but not on the item record, we'll treat that as success.
     foreach my $i (0 .. scalar @{$item->hold_queue}) {
-	$hold = $item->hold_queue->[$i];
-
-	if ($hold->{patron_id} eq $patron->id) {
-	    # found it: delete it.
-	    splice @{$item->hold_queue}, $i, 1;
-	    last;
-	}
+		$hold = $item->hold_queue->[$i];
+		if ($item->barcode_is_borrowernumber($patron->id, $hold->{borrowernumber})) {
+		    # found it: delete it.
+		    splice @{$item->hold_queue}, $i, 1;
+		    last;		# ?? should we keep going, in case there are multiples
+		}
     }
 
     $trans->screen_msg("Hold Cancelled.");
-    $trans->patron($patron);
-    $trans->item($item);
 
     return $trans;
 }
@@ -352,22 +337,21 @@ sub alter_hold {
 
     # BEGIN TRANSACTION
     $patron = new ILS::Patron $patron_id;
-    if (!$patron) {
-	$trans->screen_msg("Invalid patron barcode.");
-
-	return $trans;
+    unless ($patron) {
+		$trans->screen_msg("Invalid patron barcode: '$patron_id'.");
+		return $trans;
     }
 
     foreach my $i (0 .. scalar @{$patron->{hold_items}}) {
-	$hold = $patron->{hold_items}[$i];
+		$hold = $patron->{hold_items}[$i];
 
 	if ($hold->{item_id} eq $item_id) {
 	    # Found it.  So fix it.
-	    $hold->{expiration_date} = $expiry_date if $expiry_date;
+	    $hold->{expiration_date} = $expiry_date     if $expiry_date;
 	    $hold->{pickup_location} = $pickup_location if $pickup_location;
-	    $hold->{hold_type} = $hold_type if $hold_type;
-
-	    $trans->ok(1);
+	    $hold->{hold_type}       = $hold_type       if $hold_type;
+		$trans->change_hold();
+	    # $trans->ok(1);
 	    $trans->screen_msg("Hold updated.");
 	    $trans->patron($patron);
 	    $trans->item(new ILS::Item $hold->{item_id});
@@ -381,7 +365,7 @@ sub alter_hold {
     # the item, since it's already been updated by the patron code.
 
     if (!$trans->ok) {
-	$trans->screen_msg("No such outstanding hold.");
+		$trans->screen_msg("No such outstanding hold.");
     }
 
     return $trans;
@@ -399,56 +383,54 @@ sub renew {
 
     if (!$patron) {
 		$trans->screen_msg("Invalid patron barcode.");
-
 		return $trans;
     } elsif (!$patron->renew_ok) {
 		$trans->screen_msg("Renewals not allowed.");
-
 		return $trans;
     }
 
-    if (defined($title_id)) {
-	# renewing a title, rather than an item (sort of)
+	# Previously: renewing a title, rather than an item (sort of)
 	# This is gross, but in a real ILS it would be better
+
+    # if (defined($title_id)) {
+	#	foreach my $i (@{$patron->{items}}) {
+	#		$item = new ILS::Item $i;
+	#		last if ($title_id eq $item->title_id);
+	#		$item = undef;
+	#	}
+    # } else {
+		my $j = 0;
+		my $count = scalar @{$patron->{items}};
 		foreach my $i (@{$patron->{items}}) {
-			$item = new ILS::Item $i;
-			last if ($title_id eq $item->title_id);
-			$item = undef;
-		}
-    } else {
-		foreach my $i (@{$patron->{items}}) {
-			if ($i == $item_id) {
+            unless (defined $i->{barcode}) {    # FIXME: using data instead of objects may violate the abstraction layer
+                syslog("LOG_ERR", "No barcode for item %s of %s: $item_id", $j+1, $count);
+                next;
+            }
+            syslog("LOG_DEBUG", "checking item %s of %s: $item_id vs. %s", ++$j, $count, $i->{barcode});
+            if ($i->{barcode} eq $item_id) {
 				# We have it checked out
 				$item = new ILS::Item $item_id;
 				last;
 			}
 		}
-    }
+    # }
 
     $trans->item($item);
 
     if (!defined($item)) {
-	# It's not checked out to $patron_id
-		$trans->screen_msg("Item not checked out to " . $patron->name);
+		$trans->screen_msg("Item not checked out to " . $patron->name);     # not checked out to $patron_id
+        $trans->ok(0);
     } elsif (!$item->available($patron_id)) {
-		$trans->screen_msg("Item has outstanding holds");
+		$trans->screen_msg("Item unavailable due to outstanding holds");
+        $trans->ok(0);
     } else {
 		$trans->renewal_ok(1);
-
 		$trans->desensitize(0);	# It's already checked out
 		$trans->do_renew();
-#		warn "done renew $trans->renewal_ok(1);";
-
-#		if ($no_block eq 'Y') {
-#			$item->{due_date} = $nb_due_date;
-#		} else {
-#			$item->{due_date} = time + (14*24*60*60); # two weeks
-#		}
-#		if ($item_props) {
-#			$item->{sip_item_properties} = $item_props;
-#		}
-#		$trans->ok(1);
-#		return $trans;
+		syslog("LOG_DEBUG", "done renew (ok:%s): %s renews %s", $trans->renewal_ok, $patron_id, $item_id);
+
+#		$item->{due_date} = $nb_due_date if $no_block eq 'Y';
+#		$item->{sip_item_properties} = $item_props if $item_props;
     }
 
     return $trans;
@@ -463,46 +445,27 @@ sub renew_all {
 
     $trans->patron($patron = new ILS::Patron $patron_id);
     if (defined $patron) {
-	syslog("LOG_DEBUG", "ILS::renew_all: patron '%s': renew_ok: %s",
-	       $patron->name, $patron->renew_ok);
+        syslog("LOG_DEBUG", "ILS::renew_all: patron '%s': renew_ok: %s", $patron->name, $patron->renew_ok);
     } else {
-	syslog("LOG_DEBUG", "ILS::renew_all: Invalid patron id: '%s'",
-	       $patron_id);
+        syslog("LOG_DEBUG", "ILS::renew_all: Invalid patron id: '%s'", $patron_id);
     }
 
     if (!defined($patron)) {
-	$trans->screen_msg("Invalid patron barcode.");
-	return $trans;
+		$trans->screen_msg("Invalid patron barcode.");
+		return $trans;
     } elsif (!$patron->renew_ok) {
-	$trans->screen_msg("Renewals not allowed.");
-	return $trans;
+		$trans->screen_msg("Renewals not allowed.");
+		return $trans;
     } elsif (defined($patron_pwd) && !$patron->check_password($patron_pwd)) {
-	$trans->screen_msg("Invalid patron password.");
-	return $trans;
-    }
-
-    foreach $item_id (@{$patron->{items}}) {
-	my $item = new ILS::Item $item_id;
-
-	if (!defined($item)) {
-	    syslog("LOG_WARNING",
-		   "renew_all: Invalid item id associated with patron '%s'",
-		   $patron->id);
-	    next;
-	}
-
-	if (@{$item->hold_queue}) {
-	    # Can't renew if there are outstanding holds
-	    push @{$trans->unrenewed}, $item_id;
-	} else {
-	    $item->{due_date} = time + (14*24*60*60); # two weeks hence
-	    push @{$trans->renewed}, $item_id;
-	}
+		$trans->screen_msg("Invalid patron password.");
+		return $trans;
     }
 
+	$trans->do_renew_all;
     $trans->ok(1);
-
     return $trans;
 }
 
 1;
+__END__
+