From b3b3781ea3745a382cd38e808a2846a6fd933f3c Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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 Signed-off-by: Julian Maurice Signed-off-by: Jonathan Druart --- 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