X-Git-Url: http://koha-dev.rot13.org:8081/gitweb/?a=blobdiff_plain;f=C4%2FSIP%2FILS.pm;h=e2089e838d78fda6a57adcda3b38711b99cc89e0;hb=735381b371b128fed13b8b34c97746af21a17282;hp=8749d66b48a59d3214ebf01e4bc696902c10ce08;hpb=96c66ecf5e1de04e8a6ea7da0a8c81f99e6c188a;p=koha_fer

diff --git a/C4/SIP/ILS.pm b/C4/SIP/ILS.pm
index 8749d66b48..e2089e838d 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'	=> 0,
-		'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"              => 1,
+    "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;
-
+ 	$debug and warn "ILS: finding patron";
     return ILS::Patron->new(@_);
 }
 
 sub find_item {
     my $self = shift;
-
+	$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}));
 }
 
 #
@@ -135,36 +130,42 @@ sub checkout {
     my ($patron, $item, $circ);
 
     $circ = new ILS::Transaction::Checkout;
-
     # BEGIN TRANSACTION
     $circ->patron($patron = new ILS::Patron $patron_id);
     $circ->item($item = new ILS::Item $item_id);
 
     if (!$patron) {
-	$circ->screen_msg("Invalid Patron");
+		$circ->screen_msg("Invalid Patron");
     } elsif (!$patron->charge_ok) {
-	$circ->screen_msg("Patron Blocked");
+		$circ->screen_msg("Patron Blocked");
     } elsif (!$item) {
-	$circ->screen_msg("Invalid Item");
-    } elsif (@{$item->hold_queue} && ($patron_id ne $item->hold_queue->[0])) {
-	$circ->screen_msg("Item on Hold for Another User");
+		$circ->screen_msg("Invalid Item");
+    # 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");
+		$circ->screen_msg("Item checked out to another patron");
     } else {
-	$circ->ok(1);
-	# If the item is already associated with this patron, then
-	# we're renewing it.
-	$circ->renew_ok($item->{patron} && ($item->{patron} eq $patron_id));
-	$item->{patron} = $patron_id;
-	$item->{due_date} = time + (14*24*60*60); # two weeks
-	push(@{$patron->{items}}, $item_id);
-	$circ->desensitize(!$item->magnetic);
-
-	syslog("LOG_DEBUG", "ILS::Checkout: patron %s has checked out %s",
-	       $patron_id, join(', ', @{$patron->{items}}));
+		$circ->do_checkout();
+		if ($circ->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));
+		
+			$item->{patron} = $patron_id;
+			$item->{due_date} = $circ->{due};
+			push(@{$patron->{items}}, $item_id);
+			$circ->desensitize(!$item->magnetic_media);
+
+			syslog("LOG_DEBUG", "ILS::Checkout: patron %s has checked out %s",
+				$patron_id, join(', ', @{$patron->{items}}));
+		}
+		else {
+			syslog("LOG_ERR", "ILS::Checkout Issue failed");
+		}
     }
-
     # END TRANSACTION
 
     return $circ;
@@ -179,15 +180,26 @@ sub checkin {
     # BEGIN TRANSACTION
     $circ->item($item = new ILS::Item $item_id);
 
-    # It's ok to check it in if it exists, and if it was checked out
-    $circ->ok($item && $item->{patron});
-
-    if ($circ->ok) {
-	$circ->patron($patron = new ILS::Patron $item->{patron});
-	delete $item->{patron};
-	delete $item->{due_date};
-	$patron->{items} = [ grep {$_ ne $item_id} @{$patron->{items}} ];
+    if ($item) {
+        $circ->do_checkin($current_loc);
+    } 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});
+
+	if (!defined($item->{patron})) {
+		$circ->screen_msg("Item not checked out");
+	} else {
+		if ($circ->ok) {
+			$circ->patron($patron = new ILS::Patron $item->{patron});
+			delete $item->{patron};
+			delete $item->{due_date};
+			$patron->{items} = [ grep {$_ ne $item_id} @{$patron->{items}} ];
+		}
+	}
     # END TRANSACTION
 
     return $circ;
@@ -206,14 +218,18 @@ sub pay_fee {
     my ($self, $patron_id, $patron_pwd, $fee_amt, $fee_type,
 	$pay_type, $fee_id, $trans_id, $currency) = @_;
     my $trans;
-    my $patron;
 
-    $trans = new ILS::Transaction::FeePayment;
+    $trans = ILS::Transaction::FeePayment->new();
 
-    $patron = new ILS::Patron $patron_id;
 
     $trans->transaction_id($trans_id);
-    $trans->patron($patron);
+    my $patron;
+    $trans->patron($patron = ILS::Patron->new($patron_id));
+    if (!$patron) {
+        $trans->screen_msg('Invalid patron barcode.');
+        return $trans;
+    }
+    $trans->pay($patron->{borrowernumber},$fee_amt, $pay_type);
     $trans->ok(1);
 
     return $trans;
@@ -223,35 +239,31 @@ 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.");
+	unless ($item = new ILS::Item ($item_id || $title_id)) {
+		$trans->screen_msg("No such item.");
+		return $trans;
+	}
 
-	# END TRANSACTION (conditionally)
-	return $trans;
-    } elsif ($item->fee && ($fee_ack ne 'Y')) {
-	$trans->screen_msg = "Fee required to place hold.";
+    if ( $patron->holds_blocked_by_excessive_fees() ) {
+        $trans->screen_msg("Excessive fees blocking placement of hold.");
+    }
 
-	# 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,
@@ -263,68 +275,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;
 }
@@ -343,22 +349,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});
@@ -372,7 +377,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;
@@ -386,61 +391,53 @@ sub renew {
     my $trans;
 
     $trans = new ILS::Transaction::Renew;
-
     $trans->patron($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 (!$patron->renew_ok) {
-	$trans->screen_msg("Renewals not allowed.");
-
-	return $trans;
+		$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
-	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) {
-		# We have it checked out
-		$item = new ILS::Item $item_id;
-		last;
-	    }
-	}
-    }
+
+    # 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}}) {
+            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);
-    } elsif (!$item->available($patron_id)) {
-	 $trans->screen_msg("Item has outstanding holds");
+		$trans->screen_msg("Item not checked out to " . $patron->name);     # not checked out to $patron_id
+        $trans->ok(0);
     } else {
-	$trans->renewal_ok(1);
-
-	$trans->desensitize(0);	# It's already checked out
-
-	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);
-	$trans->renewal_ok(1);
-
-	return $trans;
+        $trans->do_renew();
+        if ($trans->renewal_ok()) {
+            $item->{due_date} = $trans->{due};
+            $trans->desensitize(0);
+        }
     }
 
     return $trans;
@@ -455,46 +452,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__
+