Attachment 107253 Details for Bug 24446 – Bug 24446: Improve StockRotationItem->advance tests

[patch] Bug 24446: Improve StockRotationItem->advance tests

Bug-24446-Improve-StockRotationItem-advance-tests.patch (text/plain), 7.98 KB, created by Martin Renvoize (ashimema) on 2020-07-23 14:11:22 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2020-07-23 14:11:22 UTC

Size: 7.98 KB

patch

obsolete

>From 4a14e9b203f0665dd3d08b3d839cb3b0ba5bc2c4 Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>Date: Tue, 16 Jun 2020 20:38:58 +0100
>Subject: [PATCH] Bug 24446: Improve StockRotationItem->advance tests
>
>This patch adds additional checks in the tests for item homebranch
>updates and cyclical transfers caused by the StockRotationItem->advance
>method.
>
>It highlights a bug in the original logic whereby a cyclical rotation
>would always result in a completed transfer but the physical item would
>not have actually been moved between branches.
>---
> t/db_dependent/StockRotationItems.t | 67 +++++++++++++++++++++--------
> 1 file changed, 50 insertions(+), 17 deletions(-)
>
>diff --git a/t/db_dependent/StockRotationItems.t b/t/db_dependent/StockRotationItems.t
>index 707842e4a5..1829a9b61f 100644
>--- a/t/db_dependent/StockRotationItems.t
>+++ b/t/db_dependent/StockRotationItems.t
>@@ -22,6 +22,7 @@ use Modern::Perl;
> use DateTime;
> use DateTime::Duration;
> use Koha::Database;
>+use Koha::DateUtils;
> use Koha::Item::Transfer;
> use t::lib::TestBuilder;
> 
>@@ -173,7 +174,7 @@ subtest "Tests for needs_advancing." => sub {
>         'itemnumber'  => $dbitem->itemnumber_id,
>         'frombranch'  => $dbitem->stage->branchcode_id,
>         'tobranch'    => $dbitem->stage->branchcode_id,
>-        'datesent'    => DateTime->now,
>+        'datesent'    => dt_from_string,
>         'datearrived' => undef,
>         'reason'      => "StockrotationAdvance",
>     })->store;
>@@ -186,7 +187,7 @@ subtest "Tests for needs_advancing." => sub {
>     $dbitem->fresh(0)->store;
> 
>     # Test item will not be advanced if it has not spent enough time.
>-    $dbtransfer->datearrived(DateTime->now)->store;
>+    $dbtransfer->datearrived(dt_from_string)->store;
>     is($dbitem->needs_advancing, 0, "Not ready to advance: Not spent enough time.");
>     # Test item will be advanced if it has not spent enough time, but is fresh.
>     $dbitem->fresh(1)->store;
>@@ -195,10 +196,10 @@ subtest "Tests for needs_advancing." => sub {
> 
>     # Test item will be advanced if it has spent enough time.
>     $dbtransfer->datesent(      # Item was sent 100 days ago...
>-        DateTime->now - DateTime::Duration->new( days => 100 )
>+        dt_from_string - DateTime::Duration->new( days => 100 )
>     )->store;
>     $dbtransfer->datearrived(   # And arrived 75 days ago.
>-        DateTime->now - DateTime::Duration->new( days => 75 )
>+        dt_from_string - DateTime::Duration->new( days => 75 )
>     )->store;
>     is($dbitem->needs_advancing, 1, "Ready to be advanced.");
> 
>@@ -206,7 +207,7 @@ subtest "Tests for needs_advancing." => sub {
> };
> 
> subtest "Tests for advance." => sub {
>-    plan tests => 15;
>+    plan tests => 23;
>     $schema->storage->txn_begin;
> 
>     my $sritem = $builder->build({
>@@ -259,22 +260,54 @@ subtest "Tests for advance." => sub {
>     $dbitem = Koha::StockRotationItems->find($sritem->{itemnumber_id});
>     ## Test results
>     is($dbitem->stage->stage_id, $dbstage2->stage_id, "Stage updated.");
>+    is(
>+        $dbitem->itemnumber->homebranch,
>+        $dbstage2->branchcode_id,
>+        "Item homebranch updated"
>+    );
>     my $intransfer = $dbitem->itemnumber->get_transfer;
>     is($intransfer->frombranch, $dbstage->branchcode_id, "Origin correct.");
>     is($intransfer->tobranch, $dbstage2->branchcode_id, "Target Correct.");
> 
>+    # Arrive at new branch
>+    $intransfer->datearrived(dt_from_string)->store;
>+    $dbitem->itemnumber->holdingbranch($srstage->{branchcode_id})->store;
>+
>+    # Test a cyclical advance
>+    ok($dbitem->advance, "Cyclical advancement done.");
>+    ## Refetch dbitem
>+    $dbitem = Koha::StockRotationItems->find($sritem->{itemnumber_id});
>+    ## Test results
>+    is($dbitem->stage->stage_id, $dbstage->stage_id, "Stage updated.");
>+    is(
>+        $dbitem->itemnumber->homebranch,
>+        $dbstage->branchcode_id,
>+        "Item homebranch updated"
>+    );
>+    $intransfer = $dbitem->itemnumber->get_transfer;
>+    is($intransfer->frombranch, $dbstage2->branchcode_id, "Origin correct.");
>+    is($intransfer->tobranch, $dbstage->branchcode_id, "Target correct.");
>+
>+    # Arrive at new branch
>+    $intransfer->datearrived(dt_from_string)->store;
>+    $dbitem->itemnumber->holdingbranch($srstage->{branchcode_id})->store;
>+
>     $dbstage->rota->cyclical(0)->store;         # Set Rota to non-cyclical.
> 
>+    # Advance again, to end of rota.
>+    ok($dbitem->advance, "Non-cyclical advance to last stage.");
>+
>     # Arrive at new branch
>-    $intransfer->datearrived(DateTime->now)->store;
>+    $intransfer->datearrived(dt_from_string)->store;
>     $dbitem->itemnumber->holdingbranch($srstage->{branchcode_id})->store;
>-    $dbitem->itemnumber->homebranch($srstage->{branchcode_id})->store;
> 
>     # Advance again, Remove from rota.
>     ok($dbitem->advance, "Non-cyclical advance.");
>     ## Refetch dbitem
>     $dbitem = Koha::StockRotationItems->find($sritem->{itemnumber_id});
>     is($dbitem, undef, "StockRotationItem has been removed.");
>+    my $item = Koha::Items->find($sritem->{itemnumber_id});
>+    is($item->homebranch, $srstage->{branchcode_id}, "Item homebranch remains");
> 
>     $schema->storage->txn_rollback;
> };
>@@ -305,8 +338,8 @@ subtest "Tests for investigate (singular)." => sub {
>         'itemnumber'  => $dbitem->itemnumber_id,
>         'frombranch'  => $dbitem->itemnumber->homebranch,
>         'tobranch'    => $dbitem->itemnumber->homebranch,
>-        'datesent'    => DateTime->now,
>-        'datearrived' => DateTime->now,
>+        'datesent'    => dt_from_string,
>+        'datearrived' => dt_from_string,
>         'reason'      => "StockrotationAdvance",
>     })->store;
>     is($dbitem->investigate->{reason}, 'repatriation', "older item repatriates.");
>@@ -321,8 +354,8 @@ subtest "Tests for investigate (singular)." => sub {
>         'itemnumber'  => $dbitem->itemnumber_id,
>         'frombranch'  => $dbitem->itemnumber->homebranch,
>         'tobranch'    => $dbitem->stage->branchcode_id,
>-        'datesent'    => DateTime->now,
>-        'datearrived' => DateTime->now,
>+        'datesent'    => dt_from_string,
>+        'datearrived' => dt_from_string,
>         'reason'      => "StockrotationAdvance",
>     })->store;
>     $dbitem->itemnumber->homebranch($dbitem->stage->branchcode_id)->store;
>@@ -340,8 +373,8 @@ subtest "Tests for investigate (singular)." => sub {
>         'itemnumber'  => $dbitem->itemnumber_id,
>         'frombranch'  => $dbitem->itemnumber->homebranch,
>         'tobranch'    => $dbitem->stage->branchcode_id,
>-        'datesent'    => DateTime->now - $sent_duration,
>-        'datearrived' => DateTime->now - $arrived_duration,
>+        'datesent'    => dt_from_string - $sent_duration,
>+        'datearrived' => dt_from_string - $arrived_duration,
>         'reason'      => "StockrotationAdvance",
>     })->store;
>     $dbitem->itemnumber->homebranch($dbitem->stage->branchcode_id)->store;
>@@ -360,8 +393,8 @@ subtest "Tests for investigate (singular)." => sub {
>         'itemnumber'  => $dbitem->itemnumber_id,
>         'frombranch'  => $dbitem->itemnumber->homebranch,
>         'tobranch'    => $dbitem->stage->branchcode_id,
>-        'datesent'    => DateTime->now - $sent_duration,
>-        'datearrived' => DateTime->now - $arrived_duration,
>+        'datesent'    => dt_from_string - $sent_duration,
>+        'datearrived' => dt_from_string - $arrived_duration,
>         'reason'      => "StockrotationAdvance",
>     })->store;
>     $dbitem->itemnumber->homebranch($dbitem->stage->branchcode_id)->store;
>@@ -380,8 +413,8 @@ subtest "Tests for investigate (singular)." => sub {
>         'itemnumber'  => $dbitem->itemnumber_id,
>         'frombranch'  => $dbitem->itemnumber->homebranch,
>         'tobranch'    => $dbitem->stage->branchcode_id,
>-        'datesent'    => DateTime->now - $sent_duration,
>-        'datearrived' => DateTime->now - $arrived_duration,
>+        'datesent'    => dt_from_string - $sent_duration,
>+        'datearrived' => dt_from_string - $arrived_duration,
>         'reason'      => "StockrotationAdvance",
>     })->store;
>     is($dbitem->investigate->{reason}, 'repatriation',
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 24446: 105943 | 105944 | 105945 | 107246 | 107247 | 107248 | 107249 | 107250 | 107252 | 107253 | 107254 | 107255 | 107256 | 107301 | 107302 | 107303 | 107304 | 107305 | 110280 | 110281 | 110282 | 110283 | 110533 | 110534 | 110535 | 110536 | 110537 | 110538 | 110539 | 110540 | 110541 | 110542 | 111322 | 116557 | 116558 | 116559 | 116560 | 116561 | 116562 | 116563 | 116564 | 116565 | 116566 | 116567 | 116568 | 116569 | 116570 | 117589 | 117590 | 117591 | 117592 | 117593 | 117594 | 117595 | 117596 | 117597 | 117598 | 117599 | 117600 | 117601 | 117602 | 117603 | 117604 | 117605 | 117606 | 117607 | 120116