Bug 23403: Remove cardnumber from SIP

== Test plan ==
1 - Have two patrons with userids and no cardnumber
2 - Note which of these has the higher borrower number
3 - Use the SIP cli emulator to connect and checkout a book to the patron with higher borrowernumber
      See example after
4 - Note the book may checkout to the wrong patron!
5 - Apply patch
6 - Checkout to both patrons via sip
7 - The patrons get the correct checkouts

=== SIP CLI emulator ===
./misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su term1 -sp term1 \
-l CPL --patron 23529001000463 -m checkout --item 39999000001259

translation: via the koha user term1, checkout item 39999000001259 to
patron 23529001000463

Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Joy Nelson <joy@bywatersolutions.com>

(cherry picked from commit 7e4296c4194afaa722a20b114cf928e2f17d7ad0)
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

diff --git a/C4/SIP/ILS/Item.pm b/C4/SIP/ILS/Item.pm
index 37c9c69..49f1f7b 100644 (file)
--- a/C4/SIP/ILS/Item.pm
+++ b/C4/SIP/ILS/Item.pm
@@ -96,7 +96,7 @@ sub new {
     if ($issue) {
         $self->{due_date} = dt_from_string( $issue->date_due, 'sql' )->truncate( to => 'minute' );
         my $patron = Koha::Patrons->find( $issue->borrowernumber );
-        $self->{patron} = $patron->cardnumber;
+        $self->{borrowernumber} = $patron->borrowernumber;
     }
     my $biblio = Koha::Biblios->find( $self->{biblionumber} );
     my $holds = $biblio->current_holds->unblessed;
@@ -254,7 +254,7 @@ sub title_id {
 
 sub sip_circulation_status {
     my $self = shift;
-    if ( $self->{patron} ) {
+    if ( $self->{borrowernumber} ) {
         return '04';    # charged
     }
     elsif ( grep { $_->{itemnumber} == $self->{itemnumber}  } @{ $self->{hold_shelf} } ) {
@@ -355,8 +355,8 @@ sub available {
        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;
+    if (defined($self->{borrowernumber})) {
+        ($self->{borrowernumber} eq $for_patron) or return 0;
                return ($count ? 0 : 1);
        } else {        # not checked out
         ($count2) and return $self->barcode_is_borrowernumber($for_patron, $self->{hold_shelf}[0]->{borrowernumber});

diff --git a/C4/SIP/ILS/Transaction/Checkout.pm b/C4/SIP/ILS/Transaction/Checkout.pm
index e743974..867dd48 100644 (file)
--- a/C4/SIP/ILS/Transaction/Checkout.pm
+++ b/C4/SIP/ILS/Transaction/Checkout.pm
@@ -50,12 +50,9 @@ sub do_checkout {
        syslog('LOG_DEBUG', "ILS::Transaction::Checkout performing checkout...");
        my $shelf          = $self->{item}->hold_shelf;
        my $barcode        = $self->{item}->id;
-       my $patron_barcode = $self->{patron}->id;
-        my $overridden_duedate; # usually passed as undef to AddIssue
-       $debug and warn "do_checkout: patron (" . $patron_barcode . ")";
-    my $patron = Koha::Patrons->find( { cardnumber => $patron_barcode } );
-    my $borrower = $patron->unblessed;
-       $debug and warn "do_checkout borrower: . " . Dumper $borrower;
+    my $patron         = Koha::Patrons->find($self->{patron}->{borrowernumber});
+    my $overridden_duedate; # usually passed as undef to AddIssue
+    $debug and warn "do_checkout borrower: . " . $patron->borrowernumber;
     my ($issuingimpossible, $needsconfirmation) = _can_we_issue($patron, $barcode,
         C4::Context->preference("AllowItemsOnHoldCheckoutSIP")
     );
@@ -73,7 +70,7 @@ sub do_checkout {
             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);
+                my $x = $self->{item}->available($patron->borrowernumber);
                 if ($x) {
                     $self->screen_msg("Item was reserved for you.");
                 } else {
@@ -110,15 +107,15 @@ sub do_checkout {
     }
     my $itemnumber = $self->{item}->{itemnumber};
     foreach (@$shelf) {
-        $debug and warn "shelf has ($_->{itemnumber} for $_->{borrowernumber}). this is ($itemnumber, $self->{patron}->{borrowernumber})";
+        $debug and warn sprintf( "shelf has (%s for %s). this is (%is, %s)", $_->{itemnumber}, $_->{borrowernumber}, $itemnumber, $patron->borrowernumber );
         ($_->{itemnumber} eq $itemnumber) or next;    # skip it if not this item
-        ($_->{borrowernumber} == $self->{patron}->{borrowernumber}) and last;
+        ($_->{borrowernumber} == $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;
     }
-    my ($fee, undef) = GetIssuingCharges($itemnumber, $self->{patron}->{borrowernumber});
+    my ($fee, undef) = GetIssuingCharges($itemnumber, $patron->borrowernumber);
     if ( $fee > 0 ) {
         $self->{sip_fee_type} = '06';
         $self->{fee_amount} = sprintf '%.2f', $fee;
@@ -132,10 +129,9 @@ sub do_checkout {
                return $self;
        }
        # can issue
-       $debug and warn "do_checkout: calling AddIssue(\$borrower,$barcode, $overridden_duedate, 0)\n"
-               # . "w/ \$borrower: " . Dumper($borrower)
+    $debug and warn sprintf("do_checkout: calling AddIssue(%s, %s, %s, 0)\n", $patron->borrowernumber, $barcode, $overridden_duedate)
                . "w/ C4::Context->userenv: " . Dumper(C4::Context->userenv);
-    my $issue = AddIssue( $borrower, $barcode, $overridden_duedate, 0 );
+    my $issue = AddIssue( $patron->unblessed, $barcode, $overridden_duedate, 0 );
     $self->{due} = $self->duedatefromissue($issue, $itemnumber);
 
        $self->ok(1);

diff --git a/C4/SIP/ILS/Transaction/Hold.pm b/C4/SIP/ILS/Transaction/Hold.pm
index 63e1a06..d7b0705 100644 (file)
--- a/C4/SIP/ILS/Transaction/Hold.pm
+++ b/C4/SIP/ILS/Transaction/Hold.pm
@@ -37,17 +37,12 @@ sub queue_position {
 
 sub do_hold {
     my $self = shift;
-    unless ( $self->{patron} ) {
+    my $patron = Koha::Patrons->find( $self->{patron}->borrowernumber );
+    unless ( $patron ) {
         $self->screen_msg('do_hold called with undefined patron');
         $self->ok(0);
         return $self;
     }
-    my $patron = Koha::Patrons->find( { cardnumber => $self->{patron}->id } );
-    unless ($patron) {
-        $self->screen_msg( 'No borrower matches cardnumber "' . $self->{patron}->id . '".' );
-        $self->ok(0);
-        return $self;
-    }
     my $item = Koha::Items->find({ barcode => $self->{item}->id });
     unless ($item) {
         $self->screen_msg( 'No biblio record matches barcode "' . $self->{item}->id . '".' );
@@ -75,14 +70,9 @@ sub do_hold {
 
 sub drop_hold {
        my $self = shift;
-       unless ($self->{patron}) {
-               $self->screen_msg('drop_hold called with undefined patron');
-               $self->ok(0);
-               return $self;
-       }
-    my $patron = Koha::Patrons->find( { cardnumber => $self->{patron}->id } );
+    my $patron = Koha::Patrons->find( $self->{patron}->borrowernumber );
     unless ($patron) {
-               $self->screen_msg('No borrower matches cardnumber "' . $self->{patron}->id . '".');
+               $self->screen_msg('drop_hold called with undefined patron');
                $self->ok(0);
                return $self;
        }
@@ -105,14 +95,9 @@ sub drop_hold {
 
 sub change_hold {
        my $self = shift;
-       unless ($self->{patron}) {
-               $self->screen_msg('change_hold called with undefined patron');
-               $self->ok(0);
-               return $self;
-       }
-    my $patron = Koha::Patrons->find( { cardnumber => $self->{patron}->id } );
+    my $patron = Koha::Patrons->find( $self->{patron}->borrowernumber );
     unless ($patron) {
-               $self->screen_msg('No borrower matches cardnumber "' . $self->{patron}->id . '".');
+               $self->screen_msg('change_hold called with undefined patron');
                $self->ok(0);
                return $self;
        }

diff --git a/C4/SIP/ILS/Transaction/Renew.pm b/C4/SIP/ILS/Transaction/Renew.pm
index af96f93..7a21c89 100644 (file)
--- a/C4/SIP/ILS/Transaction/Renew.pm
+++ b/C4/SIP/ILS/Transaction/Renew.pm
@@ -61,7 +61,8 @@ sub do_renew_for  {
 
 sub do_renew {
     my $self = shift;
-    my $patron = Koha::Patrons->find( { cardnumber => $self->{patron}->id } );
+    my $patron = Koha::Patrons->find( $self->{patron}->borrowernumber );
+    $patron or return; # FIXME we should log that
     return $self->do_renew_for($patron->unblessed);
 }