From b7709b69cff5828769e0fdddb001e46e7eb32045 Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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 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 --- 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