View | Details | Raw Unified | Return to bug 40136
Collapse All | Expand All

(-)a/C4/Log.pm (-1 / +3 lines)
Lines 142-148 sub logaction { Link Here
142
142
143
    $infos = encode_json($infos) if ( ref $infos eq 'HASH' );
143
    $infos = encode_json($infos) if ( ref $infos eq 'HASH' );
144
144
145
    Koha::ActionLog->new(
145
    my $action_log = Koha::ActionLog->new(
146
        {
146
        {
147
            timestamp => \'NOW()',
147
            timestamp => \'NOW()',
148
            user      => $usernumber,
148
            user      => $usernumber,
Lines 176-181 sub logaction { Link Here
176
            );
176
            );
177
        }
177
        }
178
    );
178
    );
179
180
    return $action_log;
179
}
181
}
180
182
181
=item cronlogaction
183
=item cronlogaction
(-)a/Koha/Patron.pm (-5 / +65 lines)
Lines 20-27 package Koha::Patron; Link Here
20
20
21
use Modern::Perl;
21
use Modern::Perl;
22
22
23
use JSON               qw( encode_json decode_json );
23
use List::MoreUtils    qw( any none uniq notall zip6);
24
use List::MoreUtils    qw( any none uniq notall zip6);
24
use Scalar::Util       qw( blessed looks_like_number );
25
use Scalar::Util       qw( blessed looks_like_number );
26
use Struct::Diff       qw( diff );
25
use Unicode::Normalize qw( NFKD );
27
use Unicode::Normalize qw( NFKD );
26
use Try::Tiny;
28
use Try::Tiny;
27
use DateTime ();
29
use DateTime ();
Lines 32-37 use C4::Letters qw( GetPreparedLetter EnqueueLetter SendQueuedMessages ); Link Here
32
use C4::Log     qw( logaction );
34
use C4::Log     qw( logaction );
33
use C4::Scrubber;
35
use C4::Scrubber;
34
use Koha::Account;
36
use Koha::Account;
37
use Koha::ActionLogs;
35
use Koha::ArticleRequests;
38
use Koha::ArticleRequests;
36
use Koha::AuthUtils;
39
use Koha::AuthUtils;
37
use Koha::Caches;
40
use Koha::Caches;
Lines 336-342 sub store { Link Here
336
339
337
                if ( C4::Context->preference("BorrowersLog") ) {
340
                if ( C4::Context->preference("BorrowersLog") ) {
338
                    my $patron_data = $self->_unblessed_for_log;
341
                    my $patron_data = $self->_unblessed_for_log;
339
                    logaction( "MEMBERS", "CREATE", $self->borrowernumber, $patron_data, undef, $patron_data );
342
                    my $create_log =
343
                        logaction( "MEMBERS", "CREATE", $self->borrowernumber, $patron_data, undef, $patron_data );
344
                    $self->{_patron_log_id} = $create_log->action_id;
340
                }
345
                }
341
            } else {    #ModMember
346
            } else {    #ModMember
342
347
Lines 393-400 sub store { Link Here
393
                        my %log_from = map { $_ => $from_storage->{$_} } keys %{$changed};
398
                        my %log_from = map { $_ => $from_storage->{$_} } keys %{$changed};
394
                        my %log_to   = map { $_ => $from_object->{$_} } keys %{$changed};
399
                        my %log_to   = map { $_ => $from_object->{$_} } keys %{$changed};
395
400
396
                        logaction( "MEMBERS", "MODIFY", $self->borrowernumber, \%log_to, undef, \%log_from )
401
                        if ( C4::Context->preference("BorrowersLog") ) {
397
                            if C4::Context->preference("BorrowersLog");
402
                            my $modify_log =
403
                                logaction( "MEMBERS", "MODIFY", $self->borrowernumber, \%log_to, undef, \%log_from );
404
                            $self->{_patron_log_id} = $modify_log->action_id;
405
                        }
398
406
399
                        logaction(
407
                        logaction(
400
                            "MEMBERS",
408
                            "MEMBERS",
Lines 450-456 sub delete { Link Here
450
            # for patron selfreg
458
            # for patron selfreg
451
            $_->delete for Koha::Patron::Modifications->search( { borrowernumber => $self->borrowernumber } )->as_list;
459
            $_->delete for Koha::Patron::Modifications->search( { borrowernumber => $self->borrowernumber } )->as_list;
452
460
453
            my $patron_data = C4::Context->preference("BorrowersLog") ? $self->_unblessed_for_log : undef;
461
            my $patron_data;
462
            if ( C4::Context->preference("BorrowersLog") ) {
463
                $patron_data = $self->_unblessed_for_log;
464
                for my $attr ( $self->extended_attributes->as_list ) {
465
                    my $key = "attribute." . $attr->code;
466
                    if ( $attr->type->repeatable ) {
467
                        $patron_data->{$key} //= [];
468
                        push @{ $patron_data->{$key} }, $attr->attribute;
469
                    } else {
470
                        $patron_data->{$key} = $attr->attribute;
471
                    }
472
                }
473
            }
454
474
455
            $self->SUPER::delete;
475
            $self->SUPER::delete;
456
476
Lines 2681-2687 sub extended_attributes { Link Here
2681
                if ( %{$all_changes} ) {
2701
                if ( %{$all_changes} ) {
2682
                    my %log_from = map { $_ => $all_changes->{$_}->{before} } keys %{$all_changes};
2702
                    my %log_from = map { $_ => $all_changes->{$_}->{before} } keys %{$all_changes};
2683
                    my %log_to   = map { $_ => $all_changes->{$_}->{after} } keys %{$all_changes};
2703
                    my %log_to   = map { $_ => $all_changes->{$_}->{after} } keys %{$all_changes};
2684
                    logaction( "MEMBERS", "MODIFY", $self->borrowernumber, \%log_to, undef, \%log_from );
2704
                    if ( my $log_id = delete $self->{_patron_log_id} ) {
2705
                        $self->_merge_attribute_log( $log_id, \%log_to, \%log_from );
2706
                    } else {
2707
                        logaction( "MEMBERS", "MODIFY", $self->borrowernumber, \%log_to, undef, \%log_from );
2708
                    }
2709
                } else {
2710
                    delete $self->{_patron_log_id};
2685
                }
2711
                }
2686
            }
2712
            }
2687
        );
2713
        );
Lines 3652-3657 sub _type { Link Here
3652
    return 'Borrower';
3678
    return 'Borrower';
3653
}
3679
}
3654
3680
3681
=head3 _merge_attribute_log
3682
3683
Merges extended attribute changes into an existing action log entry (identified
3684
by action_id), combining field and attribute changes into a single log entry.
3685
Used to unify CREATE/MODIFY log entries with subsequent attribute changes.
3686
3687
=cut
3688
3689
sub _merge_attribute_log {
3690
    my ( $self, $log_id, $log_to, $log_from ) = @_;
3691
3692
    my $log = Koha::ActionLogs->find($log_id);
3693
    return unless $log;
3694
3695
    my $info = {};
3696
    eval { $info = decode_json( $log->info ) } if $log->info;
3697
    %{$info} = ( %{$info}, %{$log_to} );
3698
3699
    my $existing_diff = {};
3700
    eval { $existing_diff = decode_json( $log->diff ) } if $log->diff;
3701
    my $attr_diff = diff( $log_from, $log_to, noU => 1 );
3702
    if ( $attr_diff->{D} ) {
3703
        $existing_diff->{D} //= {};
3704
        %{ $existing_diff->{D} } = ( %{ $existing_diff->{D} }, %{ $attr_diff->{D} } );
3705
    }
3706
3707
    $log->set(
3708
        {
3709
            info => encode_json($info),
3710
            diff => encode_json($existing_diff),
3711
        }
3712
    )->store;
3713
}
3714
3655
=head3 _unblessed_for_log
3715
=head3 _unblessed_for_log
3656
3716
3657
Returns a plain hashref of patron column values safe for JSON serialisation.
3717
Returns a plain hashref of patron column values safe for JSON serialisation.
(-)a/t/db_dependent/Koha/Patrons.t (-2 / +74 lines)
Lines 20-26 Link Here
20
use Modern::Perl;
20
use Modern::Perl;
21
21
22
use Test::NoWarnings;
22
use Test::NoWarnings;
23
use Test::More tests => 49;
23
use Test::More tests => 50;
24
use Test::Warn;
24
use Test::Warn;
25
use Test::Exception;
25
use Test::Exception;
26
use Test::MockModule;
26
use Test::MockModule;
Lines 3845-3848 subtest 'check_for_existing_matches' => sub { Link Here
3845
    $schema->storage->txn_rollback;
3845
    $schema->storage->txn_rollback;
3846
};
3846
};
3847
3847
3848
subtest 'BorrowersLog with extended patron attributes' => sub {
3849
    plan tests => 12;
3850
3851
    $schema->storage->txn_begin;
3852
3853
    t::lib::Mocks::mock_preference( 'BorrowersLog',             1 );
3854
    t::lib::Mocks::mock_preference( 'ExtendedPatronAttributes', 1 );
3855
3856
    my $attr_type = $builder->build_object(
3857
        {
3858
            class => 'Koha::Patron::Attribute::Types',
3859
            value => { repeatable => 0, unique_id => 0 },
3860
        }
3861
    );
3862
3863
    # --- CREATE with attributes: one unified log entry ---
3864
    # Must use Koha::Patron->new()->store() (not build_object) to exercise the
3865
    # full Koha::Patron::store() code path that sets _patron_log_id.
3866
    # Build fresh library/category since the outer transaction was rolled back.
3867
    my $test_library  = $builder->build_object( { class => 'Koha::Libraries' } );
3868
    my $test_category = $builder->build_object( { class => 'Koha::Patron::Categories' } );
3869
    my $patron        = Koha::Patron->new(
3870
        {
3871
            cardnumber   => 'test_attrs_log_cn',
3872
            surname      => 'AttrLogTest',
3873
            branchcode   => $test_library->branchcode,
3874
            categorycode => $test_category->categorycode,
3875
        }
3876
    )->store;
3877
    $patron->extended_attributes( [ { code => $attr_type->code, attribute => 'created_value' } ] );
3878
3879
    my @all_logs = Koha::ActionLogs->search( { module => 'MEMBERS', object => $patron->borrowernumber } )->as_list;
3880
    is( scalar @all_logs,     1,        'CREATE + attribute set produces one log entry (not CREATE + MODIFY)' );
3881
    is( $all_logs[0]->action, 'CREATE', 'The single entry is a CREATE action' );
3882
    my $create_diff = from_json( $all_logs[0]->diff );
3883
    my $attr_key    = 'attribute.' . $attr_type->code;
3884
    ok( exists $create_diff->{D}->{$attr_key}, 'CREATE diff includes the set attribute' );
3885
    is( $create_diff->{D}->{$attr_key}->{N}, 'created_value', 'CREATE diff has correct attribute value' );
3886
3887
    # --- MODIFY fields + attributes: one unified log entry ---
3888
    $patron->extended_attributes( [ { code => $attr_type->code, attribute => 'original_value' } ] );
3889
    Koha::ActionLogs->search( { object => $patron->borrowernumber } )->delete;
3890
    $patron = $patron->get_from_storage;
3891
3892
    my $new_surname = $patron->surname . '_changed';
3893
    $patron->set( { surname => $new_surname } )->store;
3894
    $patron->extended_attributes( [ { code => $attr_type->code, attribute => 'changed_value' } ] );
3895
3896
    my @modify_logs =
3897
        Koha::ActionLogs->search( { module => 'MEMBERS', action => 'MODIFY', object => $patron->borrowernumber } )
3898
        ->as_list;
3899
    is( scalar @modify_logs, 1, 'Field change + attribute change produces one MODIFY log entry' );
3900
    my $modify_diff = from_json( $modify_logs[0]->diff );
3901
    ok( exists $modify_diff->{D}->{surname}, 'MODIFY diff includes the field change' );
3902
    is( $modify_diff->{D}->{surname}->{N}, $new_surname, 'MODIFY diff has correct new field value' );
3903
    ok( exists $modify_diff->{D}->{$attr_key}, 'MODIFY diff includes the attribute change' );
3904
    is( $modify_diff->{D}->{$attr_key}->{N}, 'changed_value', 'MODIFY diff has correct new attribute value' );
3905
3906
    # --- DELETE: log includes attribute data ---
3907
    Koha::ActionLogs->search( { object => $patron->borrowernumber } )->delete;
3908
    my $borrowernumber = $patron->borrowernumber;
3909
    $patron->delete;
3910
3911
    my @delete_logs =
3912
        Koha::ActionLogs->search( { module => 'MEMBERS', action => 'DELETE', object => $borrowernumber } )->as_list;
3913
    is( scalar @delete_logs, 1, 'DELETE produces one log entry' );
3914
    my $delete_diff = from_json( $delete_logs[0]->diff );
3915
    ok( exists $delete_diff->{D}->{$attr_key}, 'DELETE diff includes attribute data' );
3916
    is( $delete_diff->{D}->{$attr_key}->{R}, 'changed_value', 'DELETE diff has correct removed attribute value' );
3917
3918
    $schema->storage->txn_rollback;
3919
};
3920
3848
$schema->storage->txn_rollback;
3921
$schema->storage->txn_rollback;
3849
- 

Return to bug 40136