Attachment 165953 Details for Bug 27753 – Bug 27753: (QA follow-up) Improve unit tests

[patch] Bug 27753: (QA follow-up) Improve unit tests

Bug-27753-QA-follow-up-Improve-unit-tests.patch (text/plain), 5.69 KB, created by Martin Renvoize (ashimema) on 2024-05-01 09:59:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2024-05-01 09:59:25 UTC

Size: 5.69 KB

patch

obsolete

>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

Actions: View | Diff | Splinter Review

Attachments on bug 27753: 162517 | 163058 | 163059 | 163060 | 163061 | 163084 | 163085 | 163090 | 163469 | 163470 | 163474 | 163475 | 163476 | 163477 | 163479 | 163480 | 165911 | 165912 | 165914 | 165915 | 165916 | 165917 | 165918 | 165919 | 165920 | 165952 | 165953 | 165958 | 165959 | 166128 | 166129 | 166130 | 166131 | 166132 | 166133 | 166134 | 166135 | 166136 | 166137