Bug 23103: Cannot checkin items lost by deleted patrons with fines attached
authorKyle M Hall <kyle@bywatersolutions.com>
Wed, 12 Jun 2019 19:08:08 +0000 (15:08 -0400)
committerMartin Renvoize <martin.renvoize@ptfs-europe.com>
Fri, 21 Jun 2019 11:37:21 +0000 (12:37 +0100)
Test Plan:
1) Checkout an item to a patron
2) Ensure the item has a replacement cost (or itemtype has default)
3) Ensure patrons are charged when items lost
4) Mark the item lost
5) Confirm patron has a fine
6) Write off the fine
7) Delete the patron
8) Check in the item
9) Note the internal server error
10) Apply this patch
11) Repeat steps 1-8
12) Note there is no internal server error!
13) prove t/db_dependent/Circulation.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

C4/Circulation.pm
t/db_dependent/Circulation.t

index 3061a8e..773f570 100644 (file)
@@ -2418,7 +2418,12 @@ sub _FixAccountForLostAndReturned {
     return unless $accountlines->count > 0;
     my $accountline     = $accountlines->next;
     my $total_to_refund = 0;
-    my $account = Koha::Patrons->find( $accountline->borrowernumber )->account;
+
+    return unless $accountline->borrowernumber;
+    my $patron = Koha::Patrons->find( $accountline->borrowernumber );
+    return undef unless $patron; # Patron has been deleted, nobody to credit the return to
+
+    my $account = $patron->account;
 
     # Use cases
     if ( $accountline->amount > $accountline->amountoutstanding ) {
index 3fafee9..c2c69aa 100755 (executable)
@@ -18,7 +18,7 @@
 use Modern::Perl;
 use utf8;
 
-use Test::More tests => 44;
+use Test::More tests => 45;
 use Test::MockModule;
 
 use Data::Dumper;
@@ -2599,6 +2599,48 @@ subtest '_FixOverduesOnReturn' => sub {
     is( $credit->amountoutstanding + 0, 0, "Credit amountoutstanding is correctly set to 0" );
 };
 
+subtest '_FixAccountForLostAndReturned returns undef if patron is deleted' => sub {
+    plan tests => 1;
+
+    my $manager = $builder->build_object({ class => "Koha::Patrons" });
+    t::lib::Mocks::mock_userenv({ patron => $manager, branchcode => $manager->branchcode });
+
+    my $biblio = $builder->build_sample_biblio({ author => 'Hall, Kylie' });
+
+    my $branchcode  = $library2->{branchcode};
+
+    my $item = $builder->build_sample_item(
+        {
+            biblionumber     => $biblio->biblionumber,
+            library          => $branchcode,
+            replacementprice => 99.00,
+            itype            => $itemtype,
+        }
+    );
+
+    my $patron = $builder->build_object( { class => 'Koha::Patrons' } );
+
+    ## Start with basic call, should just close out the open fine
+    my $accountline = Koha::Account::Line->new(
+        {
+            borrowernumber => $patron->id,
+            accounttype    => 'L',
+            status         => undef,
+            itemnumber     => $item->itemnumber,
+            amount => 99.00,
+            amountoutstanding => 99.00,
+            interface => 'test',
+        }
+    )->store();
+
+    $patron->delete();
+
+    my $return_value = C4::Circulation::_FixAccountForLostAndReturned( $patron->id, $item->itemnumber );
+
+    is( $return_value, undef, "_FixAccountForLostAndReturned returns undef if patron is deleted" );
+
+};
+
 subtest 'Set waiting flag' => sub {
     plan tests => 4;