Bug 42032: (follow-up) Use MARC-in-JSON for authority action log diffs
Mirrors the bug 42188 main commit for biblios. Replace the custom
_marc_record_to_diffable helper with MARC::Record::MiJ's to_mij_structure
so AUTHORITIES action log diffs use the same MARC-in-JSON shape as
CATALOGUING diffs. Add Koha::Authority::_unblessed_for_log to expose
only the auth_header columns that exist independently of the MARC
record (authtypecode, origincode, linkid), avoiding duplication of
MARC-derived data (heading, authtrees), raw marcxml, and system
timestamps. The explicit whitelist also removes the need for the
post-hoc empty-field stripping added in a previous follow-up.
Test plan:
1. prove t/db_dependent/AuthoritiesMarc.t
2. Enable AuthoritiesLog. Create, edit, and delete an authority via
the staff UI, then inspect action_logs.diff in the DB:
- _marc contains MiJ shape { leader: "...", fields: [ { tag: {...} }, ... ] }
- Changes to heading fields produce diffs on
_marc.fields[N].TAG.subfields[0].a
- The diff no longer carries a redundant top-level heading entry
(it's MARC-derived and now only appears in _marc).
3. prove t/db_dependent/Log.t
4. prove t/db_dependent/Koha/Authority.t
5. prove t/db_dependent/Koha/Biblio.t
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Martin Renvoize
Lucas Gass
parent
a489907260
commit
3ca746f4fe
|
|
@ -53,6 +53,7 @@ BEGIN {
|
|||
}
|
||||
|
||||
use MARC::Field;
|
||||
use MARC::Record::MiJ;
|
||||
use Scalar::Util qw(blessed);
|
||||
use Try::Tiny qw( try catch );
|
||||
|
||||
|
|
@ -1537,48 +1538,21 @@ sub _get_authid_subfield {
|
|||
return $field->subfield('9') || $field->subfield('3');
|
||||
}
|
||||
|
||||
=head2 _marc_record_to_diffable
|
||||
|
||||
Return the required fields for the action logs to store a diff
|
||||
|
||||
=cut
|
||||
|
||||
sub _marc_record_to_diffable {
|
||||
my ($record) = @_;
|
||||
return {} unless defined $record;
|
||||
my %marc;
|
||||
for my $field ( $record->fields ) {
|
||||
my $tag = $field->tag;
|
||||
if ( $field->is_control_field ) {
|
||||
$marc{$tag} = $field->data;
|
||||
} else {
|
||||
my $ind1 = $field->indicator(1);
|
||||
my $ind2 = $field->indicator(2);
|
||||
my $formatted = "$ind1$ind2";
|
||||
for my $subfield ( $field->subfields ) {
|
||||
$formatted .= " \$$subfield->[0] $subfield->[1]";
|
||||
}
|
||||
push @{ $marc{$tag} }, $formatted;
|
||||
}
|
||||
}
|
||||
return \%marc;
|
||||
}
|
||||
|
||||
=head2 _authority_log_data
|
||||
|
||||
Return the data needed by _marc_record_to_diffable
|
||||
Returns a hashref combining the authority columns that are independent of
|
||||
the MARC record with a C<_marc> key containing the record in MARC-in-JSON
|
||||
format, suitable for passing to C<logaction> as either the C<$infos> or
|
||||
C<$original> argument.
|
||||
|
||||
=cut
|
||||
|
||||
sub _authority_log_data {
|
||||
my ( $authority, $marc_record ) = @_;
|
||||
my %data = %{ $authority->unblessed };
|
||||
delete $data{marcxml};
|
||||
for my $key ( keys %data ) {
|
||||
delete $data{$key} unless defined $data{$key} && $data{$key} ne '';
|
||||
}
|
||||
$data{_marc} = _marc_record_to_diffable($marc_record);
|
||||
return \%data;
|
||||
return {
|
||||
%{ $authority->_unblessed_for_log },
|
||||
_marc => ( $marc_record ? $marc_record->to_mij_structure : {} ),
|
||||
};
|
||||
}
|
||||
|
||||
=head2 GetHeaderAuthority
|
||||
|
|
|
|||
|
|
@ -19,6 +19,8 @@ package Koha::Authority;
|
|||
|
||||
use Modern::Perl;
|
||||
|
||||
use Scalar::Util qw( blessed );
|
||||
|
||||
use base qw(Koha::Object);
|
||||
|
||||
use Koha::Authority::ControlledIndicators;
|
||||
|
|
@ -380,6 +382,34 @@ sub record_strip_nonxml {
|
|||
|
||||
=head2 Internal methods
|
||||
|
||||
=head3 _unblessed_for_log
|
||||
|
||||
Returns a plain hashref of authority column values that are independent of
|
||||
the attached MARC record, for use in action log diffing.
|
||||
|
||||
Columns derived from MARC (C<heading>, C<authtrees>), the raw C<marcxml>
|
||||
blob, and system-managed timestamps (C<datecreated>, C<modification_time>)
|
||||
are omitted because they either duplicate MARC data already captured in
|
||||
the C<_marc> key of the log entry or add no diff signal. Only columns
|
||||
carrying independent Koha metadata are retained: C<authtypecode>,
|
||||
C<origincode>, and C<linkid>.
|
||||
|
||||
=cut
|
||||
|
||||
# auth_header columns that exist independently of the MARC record.
|
||||
my @_LOG_FIELDS = qw( authtypecode origincode linkid );
|
||||
|
||||
sub _unblessed_for_log {
|
||||
my ($self) = @_;
|
||||
my $full = $self->unblessed;
|
||||
my %data;
|
||||
for my $key (@_LOG_FIELDS) {
|
||||
next unless exists $full->{$key} && defined $full->{$key};
|
||||
$data{$key} = blessed( $full->{$key} ) ? "$full->{$key}" : $full->{$key};
|
||||
}
|
||||
return \%data;
|
||||
}
|
||||
|
||||
=head3 _type
|
||||
|
||||
=cut
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ use Test::MockModule;
|
|||
use Test::Warn;
|
||||
use MARC::Field;
|
||||
use MARC::Record;
|
||||
use JSON qw( from_json );
|
||||
|
||||
use t::lib::Mocks;
|
||||
use t::lib::TestBuilder;
|
||||
|
|
@ -400,8 +401,8 @@ subtest 'DelAuthority() tests' => sub {
|
|||
$schema->storage->txn_rollback;
|
||||
};
|
||||
|
||||
subtest 'Authority action logs include MARC diff' => sub {
|
||||
plan tests => 12;
|
||||
subtest 'Authority action logs include MARC-in-JSON diff' => sub {
|
||||
plan tests => 20;
|
||||
|
||||
$schema->storage->txn_begin;
|
||||
|
||||
|
|
@ -409,39 +410,55 @@ subtest 'Authority action logs include MARC diff' => sub {
|
|||
|
||||
my $auth_type = 'GEOGR_NAME';
|
||||
|
||||
# ADD
|
||||
# --- ADD ---
|
||||
my $add_record = MARC::Record->new;
|
||||
$add_record->add_fields( [ '151', ' ', ' ', a => 'France' ] );
|
||||
my $auth_id = AddAuthority( $add_record, undef, $auth_type );
|
||||
|
||||
my $add_logs = Koha::ActionLogs->search( { object => $auth_id, module => 'AUTHORITIES', action => 'ADD' } );
|
||||
is( $add_logs->count, 1, 'ADD action logged' );
|
||||
my $add_log = $add_logs->next;
|
||||
my $add_log = Koha::ActionLogs->search( { object => $auth_id, module => 'AUTHORITIES', action => 'ADD' } )->next;
|
||||
ok( defined $add_log, 'ADD action logged' );
|
||||
ok( defined $add_log->diff, 'ADD: diff column populated' );
|
||||
like( $add_log->diff, qr/_marc/, 'ADD: diff contains MARC field data' );
|
||||
like( $add_log->diff, qr/France/, 'ADD: diff contains heading content' );
|
||||
|
||||
# MODIFY - change the heading
|
||||
my $add_diff = from_json( $add_log->diff );
|
||||
my $add_added = $add_diff->{D}{_marc}{A};
|
||||
ok( defined $add_added, 'ADD diff contains _marc key' );
|
||||
ok( exists $add_added->{leader}, '_marc has leader' );
|
||||
ok( ref $add_added->{fields} eq 'ARRAY', '_marc.fields is an array (MiJ shape)' );
|
||||
|
||||
my ($f151) = grep { exists $_->{'151'} } @{ $add_added->{fields} };
|
||||
ok( defined $f151, '151 field present in _marc.fields' );
|
||||
is( $f151->{'151'}{subfields}[0]{a}, 'France', '151 $a correct' );
|
||||
|
||||
ok( !exists $add_diff->{D}{heading}, 'MARC-derived heading column absent from diff' );
|
||||
ok( !exists $add_diff->{D}{authtrees}, 'MARC-derived authtrees column absent from diff' );
|
||||
|
||||
# --- MODIFY ---
|
||||
my $mod_record = MARC::Record->new;
|
||||
$mod_record->add_fields( [ '151', ' ', ' ', a => 'France (updated)' ] );
|
||||
ModAuthority( $auth_id, $mod_record, $auth_type, { skip_merge => 1 } );
|
||||
|
||||
my $mod_logs = Koha::ActionLogs->search( { object => $auth_id, module => 'AUTHORITIES', action => 'MODIFY' } );
|
||||
is( $mod_logs->count, 1, 'MODIFY action logged' );
|
||||
my $mod_log = $mod_logs->next;
|
||||
my $mod_log = Koha::ActionLogs->search( { object => $auth_id, module => 'AUTHORITIES', action => 'MODIFY' } )->next;
|
||||
ok( defined $mod_log, 'MODIFY action logged' );
|
||||
ok( defined $mod_log->diff, 'MODIFY: diff column populated' );
|
||||
like( $mod_log->diff, qr/_marc/, 'MODIFY: diff contains MARC field data' );
|
||||
|
||||
my $mod_diff = from_json( $mod_log->diff );
|
||||
ok( exists $mod_diff->{D}{_marc}, 'MODIFY diff contains _marc key' );
|
||||
ok( !exists $mod_diff->{D}{heading}, 'MARC-derived heading column absent from MODIFY diff' );
|
||||
ok( !exists $mod_diff->{D}{authtrees}, 'MARC-derived authtrees column absent from MODIFY diff' );
|
||||
like( $mod_log->diff, qr/France/, 'MODIFY: diff captures before value' );
|
||||
like( $mod_log->diff, qr/France.*updated/s, 'MODIFY: diff captures after value' );
|
||||
|
||||
# DELETE
|
||||
# --- DELETE ---
|
||||
DelAuthority( { authid => $auth_id, skip_merge => 1 } );
|
||||
|
||||
my $del_logs = Koha::ActionLogs->search( { object => $auth_id, module => 'AUTHORITIES', action => 'DELETE' } );
|
||||
is( $del_logs->count, 1, 'DELETE action logged' );
|
||||
my $del_log = $del_logs->next;
|
||||
my $del_log = Koha::ActionLogs->search( { object => $auth_id, module => 'AUTHORITIES', action => 'DELETE' } )->next;
|
||||
ok( defined $del_log, 'DELETE action logged' );
|
||||
ok( defined $del_log->diff, 'DELETE: diff column populated' );
|
||||
like( $del_log->diff, qr/_marc/, 'DELETE: diff contains MARC field data' );
|
||||
|
||||
my $del_diff = from_json( $del_log->diff );
|
||||
my $del_removed = $del_diff->{D}{_marc}{R};
|
||||
ok( defined $del_removed, 'DELETE diff contains _marc key' );
|
||||
ok( ref $del_removed->{fields} eq 'ARRAY', '_marc.fields is an array in DELETE diff' );
|
||||
|
||||
$schema->storage->txn_rollback;
|
||||
};
|
||||
|
|
|
|||
Loading…
Reference in New Issue