From da5f8bc1c3fb0695f4be36a264ed01a9dbecb475 Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Wed, 1 May 2024 10:52:52 +0100
Subject: [PATCH] Bug 27753: (QA follow-up) Improve unit tests

This patch adds unit tests that test the functionality more specifically
and adds a few notes/fixme for things we need to consider here
---
 t/db_dependent/Circulation.t | 75 +++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 36 deletions(-)

diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
index c232a46d0a1..a6fab4a6947 100755
--- a/t/db_dependent/Circulation.t
+++ b/t/db_dependent/Circulation.t
@@ -2633,59 +2633,62 @@ subtest 'CanBookBeIssued + Statistic patrons "X"' => sub {
 };
 
 subtest "Bug 27753 - Add AutoClaimReturnStatusOnCheckin" => sub {
-    plan tests => 8;
+    plan tests => 1;
+
     t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' );
     t::lib::Mocks::mock_userenv( { branchcode => $library2->{branchcode} } );
     t::lib::Mocks::mock_preference( 'ClaimReturnedLostValue',         1 );
-    t::lib::Mocks::mock_preference( 'AutoClaimReturnStatusOnCheckin', 1 );
+    t::lib::Mocks::mock_preference( 'AutoClaimReturnStatusOnCheckin', '' );
     my $item     = $builder->build_sample_item( { library => $library2->{branchcode} } );
     my $patron   = $builder->build_object( { class => 'Koha::Patrons' } );
-    my $checkout = AddIssue( $patron, $item->barcode );
-
-    my $claim = $checkout->claim_returned(
+    my $future   = dt_from_string()->add( days => 7 );
+    my $checkout = $builder->build_object(
         {
-            created_by => $patron->id,
-            notes      => "Test note",
+            class => 'Koha::Checkouts',
+            value => {
+                returndate      => undef,
+                renewals_count  => 0,
+                auto_renew      => 0,
+                borrowernumber  => $patron->borrowernumber,
+                itemnumber      => $item->itemnumber,
+                onsite_checkout => 0,
+                date_due        => $future,
+            }
         }
     );
-    is( $claim->issue_id,       $checkout->id, "Claim issue id matches" );
-    is( $claim->itemnumber,     $item->id,     "Claim itemnumber matches" );
-    is( $claim->borrowernumber, $patron->id,   "Claim borrowernumber matches" );
-    is( $claim->notes,          "Test note",   "Claim notes match" );
-    is( $claim->created_by,     $patron->id,   "Claim created_by matches" );
-    ok( $claim->created_on, "Claim created_on is set" );
-
-    my ( $doreturn, $messages ) = AddReturn( $item->barcode, $library->{branchcode} );
-    is( ref $messages->{ClaimAutoResolved}, 'Koha::Checkouts::ReturnClaim', "Claim auto resolved upon checkin" );
-    $claim->discard_changes;
-    ok( $claim->resolved_by, "Claim is resolved" );
-};
-
-subtest "Bug 27753 - Add AutoTestClaimReturnStatusOnCheckin" => sub {
-    plan tests => 7;
-    t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' );
-    t::lib::Mocks::mock_userenv( { branchcode => $library2->{branchcode} } );
-    t::lib::Mocks::mock_preference( 'ClaimReturnedLostValue',         1 );
-    t::lib::Mocks::mock_preference( 'AutoClaimReturnStatusOnCheckin', 0 );
-    my $item     = $builder->build_sample_item( { library => $library2->{branchcode} } );
-    my $patron   = $builder->build_object( { class => 'Koha::Patrons' } );
-    my $checkout = AddIssue( $patron, $item->barcode );
 
+    # Claim return
     my $claim = $checkout->claim_returned(
         {
             created_by => $patron->id,
             notes      => "Test note",
         }
     );
-    is( $claim->issue_id,       $checkout->id, "Claim issue id matches" );
-    is( $claim->itemnumber,     $item->id,     "Claim itemnumber matches" );
-    is( $claim->borrowernumber, $patron->id,   "Claim borrowernumber matches" );
-    is( $claim->notes,          "Test note",   "Claim notes match" );
-    is( $claim->created_by,     $patron->id,   "Claim created_by matches" );
-    ok( $claim->created_on, "Claim created_on is set" );
+    is( $claim->issue_id, $checkout->id,    "Return claim created for issue" );
+    $item->discard_changes;
+    is( $item->itemlost,  1, "Item set to lost as 1" );
 
+    # Return tests with feature disabled
     my ( $doreturn, $messages ) = AddReturn( $item->barcode, $library->{branchcode} );
-    is( ref $messages->{ReturnClaims}, 'Koha::Checkouts::ReturnClaim', "Claim was returned" );
+    is(
+        ref $messages->{ReturnClaims}, 'Koha::Checkouts::ReturnClaim',
+        "AddReturn returns message 'ReturnClaims' containing the ReturnClaim object"
+    );
+    # FIXME: We should reset the checkout here else we hit a 'No patron' defined issue in the AutoClaim code.
+    #        This might highligt a bug in the code however.. should we allow trigger this code when the return
+    #        spots that there isn't a current issue
+
+    # Now test with the feature enabled
+    t::lib::Mocks::mock_preference( 'AutoClaimReturnStatusOnCheckin', 'RETURNED_ON_CLAIM' );
+    ( $doreturn, $messages ) = AddReturn( $item->barcode, $library->{branchcode} );
+    is(
+        ref $messages->{ClaimAutoResolved}, 'Koha::Checkouts::ReturnClaim',
+        "AddReturn returns message 'ClaimAutoResolved' containing the ReturnClaim object"
+    );
+    $claim->discard_changes;
+    is( $claim->resolved_by, '', "Claim marked as resolved by '' when AutoClaimReturnStatusOnCheckin set" );
+    # FIXME: The code sets the resolved_by to the patron who was issued the item.. should this be the librarian performing the return instead?
+    is( $claim->resolution, 'RETURNED_ON_CLAIM', "Claim resolution set to match AutoClaimReturnStatusOnCheckin value" );
 };
 
 subtest 'MultipleReserves' => sub {
-- 
2.44.0