From b3b3781ea3745a382cd38e808a2846a6fd933f3c Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Wed, 2 Dec 2020 09:56:04 +0100
Subject: [PATCH] Bug 27003: Item creation log must be done after the item is
 created

Otherwise we don't have its itemnumber yet!

Test plan:
Add and modify an item and confirm that the action logs are created
correctly

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 Koha/Item.pm                | 18 ++++++++--------
 t/db_dependent/Koha/Items.t | 41 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/Koha/Item.pm b/Koha/Item.pm
index 10d4eb2c48..5c9d86e49a 100644
--- a/Koha/Item.pm
+++ b/Koha/Item.pm
@@ -89,8 +89,8 @@ sub store {
         $self->itype($self->biblio->biblioitem->itemtype);
     }
 
-    my $today         = dt_from_string;
-    my $plugin_action = 'create';
+    my $today  = dt_from_string;
+    my $action = 'create';
 
     unless ( $self->in_storage ) { #AddItem
         unless ( $self->permanent_location ) {
@@ -114,12 +114,9 @@ sub store {
             $self->cn_sort($cn_sort);
         }
 
-        logaction( "CATALOGUING", "ADD", $self->itemnumber, "item" )
-          if $log_action && C4::Context->preference("CataloguingLog");
-
     } else { # ModItem
 
-        $plugin_action = 'modify';
+        $action = 'modify';
 
         my %updated_columns = $self->_result->get_dirty_columns;
         return $self->SUPER::store unless %updated_columns;
@@ -186,8 +183,6 @@ sub store {
             $self->_set_found_trigger($pre_mod_item);
         }
 
-        logaction( "CATALOGUING", "MODIFY", $self->itemnumber, "item " . Dumper($self->unblessed) )
-          if $log_action && C4::Context->preference("CataloguingLog");
     }
 
     unless ( $self->dateaccessioned ) {
@@ -195,10 +190,15 @@ sub store {
     }
 
     my $result = $self->SUPER::store;
+    if ( $log_action && C4::Context->preference("CataloguingLog") ) {
+        $action eq 'create'
+          ? logaction( "CATALOGUING", "ADD", $self->itemnumber, "item" )
+          : logaction( "CATALOGUING", "MODIFY", $self->itemnumber, "item " . Dumper( $self->unblessed ) );
+    }
     my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX });
     $indexer->index_records( $self->biblionumber, "specialUpdate", "biblioserver" )
         unless $params->{skip_record_index};
-    $self->get_from_storage->_after_item_action_hooks({ action => $plugin_action });
+    $self->get_from_storage->_after_item_action_hooks({ action => $action });
 
     return $result;
 }
diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t
index fc5d8773b8..0737881609 100755
--- a/t/db_dependent/Koha/Items.t
+++ b/t/db_dependent/Koha/Items.t
@@ -65,7 +65,7 @@ my $retrieved_item_1 = Koha::Items->find( $new_item_1->itemnumber );
 is( $retrieved_item_1->barcode, $new_item_1->barcode, 'Find a item by id should return the correct item' );
 
 subtest 'store' => sub {
-    plan tests => 6;
+    plan tests => 7;
 
     my $biblio = $builder->build_sample_biblio;
     my $today  = dt_from_string->set( hour => 0, minute => 0, second => 0 );
@@ -1140,6 +1140,45 @@ subtest 'store' => sub {
 
         };
     };
+
+    subtest 'log_action' => sub {
+        plan tests => 2;
+        t::lib::Mocks::mock_preference( 'CataloguingLog', 1 );
+
+        my $item = Koha::Item->new(
+            {
+                homebranch    => $library->{branchcode},
+                holdingbranch => $library->{branchcode},
+                biblionumber  => $biblio->biblionumber,
+                location      => 'my_loc',
+            }
+        )->store;
+        is(
+            Koha::ActionLogs->search(
+                {
+                    module => 'CATALOGUING',
+                    action => 'ADD',
+                    object => $item->itemnumber,
+                    info   => 'item'
+                }
+            )->count,
+            1,
+            "Item creation logged"
+        );
+
+        $item->location('another_loc')->store;
+        is(
+            Koha::ActionLogs->search(
+                {
+                    module => 'CATALOGUING',
+                    action => 'MODIFY',
+                    object => $item->itemnumber
+                }
+            )->count,
+            1,
+            "Item modification logged"
+        );
+    };
 };
 
 subtest 'get_transfer' => sub {
-- 
2.39.5