From d53eb86b87989bb8e49ab2b2f39df688b543daa7 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 9 Mar 2026 19:59:08 +0000 Subject: [PATCH] Bug 42030: Populate diff column for suggestion CREATE/MODIFY/DELETE logs Update suggestion action logging to populate the diff column using the correct before/after semantics: - CREATE: empty before-state ({}), new suggestion data as after-state - MODIFY: pre-change unblessed data as before, post-store object as after - DELETE: log before the SQL DELETE so the object can still be fetched; pass the suggestion as both $infos (info column) and $original (diff before-state), logaction sets after-state to {} Also add tests verifying the diff column is populated for all three actions. --- C4/Suggestions.pm | 7 +++-- Koha/Suggestion.pm | 11 +++++-- t/db_dependent/Koha/Suggestions.t | 50 ++++++++++++++++++++++++++++++- 3 files changed, 62 insertions(+), 6 deletions(-) diff --git a/C4/Suggestions.pm b/C4/Suggestions.pm index 9c937709175..368ce7c6937 100644 --- a/C4/Suggestions.pm +++ b/C4/Suggestions.pm @@ -185,15 +185,16 @@ sub DelSuggestion { $borrowernumber //= ''; if ( defined $type && $type eq 'intranet' || $suggestedby eq $borrowernumber ) { + my $suggestion = Koha::Suggestions->find($suggestionid); + if ( C4::Context->preference("SuggestionsLog") ) { + logaction( 'SUGGESTION', 'DELETE', $suggestionid, $suggestion, undef, $suggestion ); + } my $queryDelete = q{ DELETE FROM suggestions WHERE suggestionid=? }; $sth = $dbh->prepare($queryDelete); my $suggestiondeleted = $sth->execute($suggestionid); - if ( C4::Context->preference("SuggestionsLog") ) { - logaction( 'SUGGESTION', 'DELETE', $suggestionid, '' ); - } return $suggestiondeleted; } } diff --git a/Koha/Suggestion.pm b/Koha/Suggestion.pm index 229ee83d06e..9708f8f6ada 100644 --- a/Koha/Suggestion.pm +++ b/Koha/Suggestion.pm @@ -71,12 +71,19 @@ sub store { my $emailpurchasesuggestions = C4::Context->preference("EmailPurchaseSuggestions"); my $new_suggestion = !$self->in_storage; + my $original = $new_suggestion ? undef : $self->get_from_storage->unblessed; my $result = $self->SUPER::store(); if ( C4::Context->preference("SuggestionsLog") ) { - my $action = $new_suggestion ? 'CREATE' : 'MODIFY'; - logaction( 'SUGGESTION', $action, $result->suggestionid, $self ); + if ($new_suggestion) { + logaction( + 'SUGGESTION', 'CREATE', $result->suggestionid, undef, undef, + $result->get_from_storage->unblessed + ); + } else { + logaction( 'SUGGESTION', 'MODIFY', $result->suggestionid, $result, undef, $original ); + } } if ( $emailpurchasesuggestions && $self->STATUS eq 'ASKED' ) { diff --git a/t/db_dependent/Koha/Suggestions.t b/t/db_dependent/Koha/Suggestions.t index e4488fbc378..76a8e122127 100755 --- a/t/db_dependent/Koha/Suggestions.t +++ b/t/db_dependent/Koha/Suggestions.t @@ -20,10 +20,12 @@ use Modern::Perl; use Test::NoWarnings; -use Test::More tests => 12; +use Test::More tests => 13; use Test::Exception; use Test::Warn; +use C4::Suggestions qw( DelSuggestion ); +use Koha::ActionLogs; use Koha::Suggestions; use Koha::Notice::Messages; use Koha::Database; @@ -513,3 +515,49 @@ subtest 'filter_by_suggested_days_range() tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'Suggestion action logs populate the diff column' => sub { + + plan tests => 9; + + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( 'SuggestionsLog', 1 ); + + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + + # CREATE + my $suggestion = Koha::Suggestion->new( + { + suggestedby => $patron->borrowernumber, + title => 'Test title', + } + )->store; + my $suggestionid = $suggestion->suggestionid; + + my $create_log = + Koha::ActionLogs->search( { module => 'SUGGESTION', action => 'CREATE', object => $suggestionid } )->single; + ok( defined $create_log, 'CREATE log entry exists' ); + ok( defined $create_log->diff, 'diff column is populated for CREATE' ); + like( $create_log->diff, qr/title/, 'CREATE diff contains suggestion fields' ); + + # MODIFY + $suggestion->title('Updated title')->store; + + my $modify_log = + Koha::ActionLogs->search( { module => 'SUGGESTION', action => 'MODIFY', object => $suggestionid } )->single; + ok( defined $modify_log, 'MODIFY log entry exists' ); + ok( defined $modify_log->diff, 'diff column is populated for MODIFY' ); + like( $modify_log->diff, qr/title/, 'MODIFY diff contains changed field' ); + + # DELETE via DelSuggestion + C4::Suggestions::DelSuggestion( undef, $suggestionid, 'intranet' ); + + my $delete_log = + Koha::ActionLogs->search( { module => 'SUGGESTION', action => 'DELETE', object => $suggestionid } )->single; + ok( defined $delete_log, 'DELETE log entry exists' ); + ok( defined $delete_log->diff, 'diff column is populated for DELETE' ); + like( $delete_log->diff, qr/title/, 'DELETE diff contains suggestion fields' ); + + $schema->storage->txn_rollback; +}; -- 2.53.0