Bug 40136: (follow-up) Refactor MODIFY log to use _unblessed_for_log

Replace the manual unblessed + skip_fields + blessed-stringify logic in
the MODIFY actionlog section with calls to _unblessed_for_log, which
already handles all three concerns. This removes the need for @skip_fields,
the dateexpiry normalisation workaround, and the inline blessed checks.

Also restrict the log_from/log_to hashrefs to only the changed keys, so
the diff column contains only fields that actually changed rather than
the full patron state.

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
This commit is contained in:
Martin Renvoize 2026-03-09 22:05:08 +00:00
parent f83424d3dd
commit 9ae4911704
No known key found for this signature in database
GPG Key ID: 2D86E0EAEC83711B
1 changed files with 5 additions and 25 deletions

View File

@ -375,20 +375,12 @@ sub store {
# Actionlogs
if ( C4::Context->preference("BorrowersLog") || C4::Context->preference("CardnumberLog") ) {
my $from_storage = $self_from_storage->unblessed;
my $from_object = $self->unblessed;
my $from_storage = $self_from_storage->_unblessed_for_log;
my $from_object = $self->_unblessed_for_log;
# Object's dateexpiry is a DateTime object which stringifies to iso8601 datetime,
# but the column is only a date so we need to convert the datetime to just a date
# to know if it has actually changed.
$from_object->{dateexpiry} = dt_from_string( $from_object->{dateexpiry} )->ymd
if $from_object->{dateexpiry};
my @skip_fields = (qw/lastseen updated_on password/);
my @keys = C4::Context->preference("BorrowersLog") ? keys %{$from_storage} : ('cardnumber');
my @keys = C4::Context->preference("BorrowersLog") ? keys %{$from_storage} : ('cardnumber');
my $changed;
for my $key (@keys) {
next if any { /$key/ } @skip_fields;
my $storage_value = $from_storage->{$key} // q{};
my $object_value = $from_object->{$key} // q{};
if ( ( $storage_value || $object_value )
@ -399,20 +391,8 @@ sub store {
}
if ( defined($changed) ) {
my %log_from = map {
my $v = $from_storage->{$_};
$_ => ( blessed($v) ? "$v" : $v )
} grep {
my $k = $_;
!any { /$k/ } @skip_fields
} keys %{$from_storage};
my %log_to = map {
my $v = $from_object->{$_};
$_ => ( blessed($v) ? "$v" : $v )
} grep {
my $k = $_;
!any { /$k/ } @skip_fields
} keys %{$from_object};
my %log_from = map { $_ => $from_storage->{$_} } keys %{$changed};
my %log_to = map { $_ => $from_object->{$_} } keys %{$changed};
logaction( "MEMBERS", "MODIFY", $self->borrowernumber, \%log_to, undef, \%log_from )
if C4::Context->preference("BorrowersLog");