From b7709b69cff5828769e0fdddb001e46e7eb32045 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: [20.05.x] 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: Magnus Enger <magnus@libriotech.no>
Applied the patch on a live server showing missing info in the logs.
Created and deleted an item. Verified the logs now have the same
info as in the olden days.

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
---
 Koha/Item.pm                | 18 +++++++++-------
 t/db_dependent/Koha/Items.t | 43 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/Koha/Item.pm b/Koha/Item.pm
index a599d9834b..6e2aa1e728 100644
--- a/Koha/Item.pm
+++ b/Koha/Item.pm
@@ -87,7 +87,9 @@ sub store {
         $self->itype($self->biblio->biblioitem->itemtype);
     }
 
-    my $today = dt_from_string;
+    my $today  = dt_from_string;
+    my $action = 'create';
+
     unless ( $self->in_storage ) { #AddItem
         unless ( $self->permanent_location ) {
             $self->permanent_location($self->location);
@@ -110,13 +112,10 @@ sub store {
             $self->cn_sort($cn_sort);
         }
 
-        logaction( "CATALOGUING", "ADD", $self->itemnumber, "item" )
-          if $log_action && C4::Context->preference("CataloguingLog");
-
-        $self->_after_item_action_hooks({ action => 'create' });
-
     } else { # ModItem
 
+        $action = 'modify';
+
         { # Update *_on  fields if needed
           # Why not for AddItem as well?
             my @fields = qw( itemlost withdrawn damaged );
@@ -167,8 +166,6 @@ sub store {
 
         $self->_after_item_action_hooks({ action => 'modify' });
 
-        logaction( "CATALOGUING", "MODIFY", $self->itemnumber, "item " . Dumper($self->unblessed) )
-          if $log_action && C4::Context->preference("CataloguingLog");
     }
 
     unless ( $self->dateaccessioned ) {
@@ -176,6 +173,11 @@ 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};
diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t
index d04559a16d..878862991d 100644
--- a/t/db_dependent/Koha/Items.t
+++ b/t/db_dependent/Koha/Items.t
@@ -64,7 +64,8 @@ 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 => 4;
+    plan tests => 5;
+
     my $biblio = $builder->build_sample_biblio;
     my $today = dt_from_string->set( hour => 0, minute => 0, second => 0 );
     my $item = Koha::Item->new(
@@ -82,6 +83,46 @@ subtest 'store' => sub {
     is( $item->itype, $biblio->biblioitem->itemtype, 'items.itype must have been set to biblioitem.itemtype is not given');
     is( $item->permanent_location, $item->location, 'permanent_location must have been set to location if not given' );
     $item->delete;
+
+    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"
+        );
+        $item->delete;
+    };
 };
 
 subtest 'get_transfer' => sub {
-- 
2.39.5