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

(-)a/Koha/Patron.pm (-27 / +54 lines)
Lines 22-28 use Modern::Perl; Link Here
22
22
23
use List::MoreUtils    qw( any none uniq notall zip6);
23
use List::MoreUtils    qw( any none uniq notall zip6);
24
use JSON               qw( to_json );
24
use JSON               qw( to_json );
25
use Scalar::Util       qw(looks_like_number);
25
use Scalar::Util       qw( blessed looks_like_number );
26
use Unicode::Normalize qw( NFKD );
26
use Unicode::Normalize qw( NFKD );
27
use Try::Tiny;
27
use Try::Tiny;
28
use DateTime ();
28
use DateTime ();
Lines 335-342 sub store { Link Here
335
                $self->discard_changes;
335
                $self->discard_changes;
336
                $self->update_lastseen('creation');
336
                $self->update_lastseen('creation');
337
337
338
                logaction( "MEMBERS", "CREATE", $self->borrowernumber, "" )
338
                if ( C4::Context->preference("BorrowersLog") ) {
339
                    if C4::Context->preference("BorrowersLog");
339
                    my $patron_data = $self->_unblessed_for_log;
340
                    logaction( "MEMBERS", "CREATE", $self->borrowernumber, $patron_data, undef, $patron_data );
341
                }
340
            } else {    #ModMember
342
            } else {    #ModMember
341
343
342
                my $self_from_storage = $self->get_from_storage;
344
                my $self_from_storage = $self->get_from_storage;
Lines 373-390 sub store { Link Here
373
375
374
                # Actionlogs
376
                # Actionlogs
375
                if ( C4::Context->preference("BorrowersLog") || C4::Context->preference("CardnumberLog") ) {
377
                if ( C4::Context->preference("BorrowersLog") || C4::Context->preference("CardnumberLog") ) {
376
                    my $info;
377
                    my $from_storage = $self_from_storage->unblessed;
378
                    my $from_storage = $self_from_storage->unblessed;
378
                    my $from_object  = $self->unblessed;
379
                    my $from_object  = $self->unblessed;
379
380
380
                    # Object's dateexpiry is a DateTime object which stringifies to iso8601 datetime,
381
                    # Object's dateexpiry is a DateTime object which stringifies to iso8601 datetime,
381
                    # but the column in only a date so we need to convert the datetime to just a date
382
                    # but the column is only a date so we need to convert the datetime to just a date
382
                    # to know if it has actually changed.
383
                    # to know if it has actually changed.
383
                    $from_object->{dateexpiry} = dt_from_string( $from_object->{dateexpiry} )->ymd
384
                    $from_object->{dateexpiry} = dt_from_string( $from_object->{dateexpiry} )->ymd
384
                        if $from_object->{dateexpiry};
385
                        if $from_object->{dateexpiry};
385
386
386
                    my @skip_fields = (qw/lastseen updated_on/);
387
                    my @skip_fields = (qw/lastseen updated_on password/);
387
                    my @keys        = C4::Context->preference("BorrowersLog") ? keys %{$from_storage} : ('cardnumber');
388
                    my @keys        = C4::Context->preference("BorrowersLog") ? keys %{$from_storage} : ('cardnumber');
389
                    my $changed;
388
                    for my $key (@keys) {
390
                    for my $key (@keys) {
389
                        next if any { /$key/ } @skip_fields;
391
                        next if any { /$key/ } @skip_fields;
390
                        my $storage_value = $from_storage->{$key} // q{};
392
                        my $storage_value = $from_storage->{$key} // q{};
Lines 392-424 sub store { Link Here
392
                        if (   ( $storage_value || $object_value )
394
                        if (   ( $storage_value || $object_value )
393
                            && ( $storage_value ne $object_value ) )
395
                            && ( $storage_value ne $object_value ) )
394
                        {
396
                        {
395
                            $info->{$key} = {
397
                            $changed->{$key} = 1;
396
                                before => $from_storage->{$key},
397
                                after  => $from_object->{$key}
398
                            };
399
                        }
398
                        }
400
                    }
399
                    }
401
400
402
                    if ( defined($info) ) {
401
                    if ( defined($changed) ) {
403
                        logaction(
402
                        my %log_from = map {
404
                            "MEMBERS",
403
                            my $v = $from_storage->{$_};
405
                            "MODIFY",
404
                            $_ => ( blessed($v) ? "$v" : $v )
406
                            $self->borrowernumber,
405
                        } grep {
407
                            to_json(
406
                            my $k = $_;
408
                                $info,
407
                            !any { /$k/ } @skip_fields
409
                                { utf8 => 1, pretty => 1, canonical => 1 }
408
                        } keys %{$from_storage};
410
                            )
409
                        my %log_to = map {
411
                        ) if C4::Context->preference("BorrowersLog");
410
                            my $v = $from_object->{$_};
411
                            $_ => ( blessed($v) ? "$v" : $v )
412
                        } grep {
413
                            my $k = $_;
414
                            !any { /$k/ } @skip_fields
415
                        } keys %{$from_object};
416
417
                        logaction( "MEMBERS", "MODIFY", $self->borrowernumber, \%log_to, undef, \%log_from )
418
                            if C4::Context->preference("BorrowersLog");
419
412
                        logaction(
420
                        logaction(
413
                            "MEMBERS",
421
                            "MEMBERS",
414
                            "MODIFY_CARDNUMBER",
422
                            "MODIFY_CARDNUMBER",
415
                            $self->borrowernumber,
423
                            $self->borrowernumber,
416
                            to_json(
424
                            { cardnumber => $from_object->{cardnumber} },
417
                                $info->{cardnumber},
425
                            undef,
418
                                { utf8 => 1, pretty => 1, canonical => 1 }
426
                            { cardnumber => $from_storage->{cardnumber} }
419
                            )
427
                        ) if $changed->{cardnumber} && C4::Context->preference("CardnumberLog");
420
                        ) if defined $info->{cardnumber} && C4::Context->preference("CardnumberLog");
421
422
                    }
428
                    }
423
                }
429
                }
424
430
Lines 465-473 sub delete { Link Here
465
            # for patron selfreg
471
            # for patron selfreg
466
            $_->delete for Koha::Patron::Modifications->search( { borrowernumber => $self->borrowernumber } )->as_list;
472
            $_->delete for Koha::Patron::Modifications->search( { borrowernumber => $self->borrowernumber } )->as_list;
467
473
474
            my $patron_data = C4::Context->preference("BorrowersLog") ? $self->_unblessed_for_log : undef;
475
468
            $self->SUPER::delete;
476
            $self->SUPER::delete;
469
477
470
            logaction( "MEMBERS", "DELETE", $self->borrowernumber, "" ) if C4::Context->preference("BorrowersLog");
478
            logaction( "MEMBERS", "DELETE", $self->borrowernumber, $patron_data, undef, $patron_data )
479
                if C4::Context->preference("BorrowersLog");
471
        }
480
        }
472
    );
481
    );
473
    return $self;
482
    return $self;
Lines 3667-3672 sub _type { Link Here
3667
    return 'Borrower';
3676
    return 'Borrower';
3668
}
3677
}
3669
3678
3679
=head3 _unblessed_for_log
3680
3681
Returns a plain hashref of patron column values safe for JSON serialisation.
3682
Date/datetime columns inflated to DateTime objects are converted to their
3683
ISO string representation, and sensitive/noisy fields are excluded.
3684
3685
=cut
3686
3687
sub _unblessed_for_log {
3688
    my ($self) = @_;
3689
    my $data = $self->unblessed;
3690
    delete @{$data}{qw(password lastseen updated_on)};
3691
    for my $key ( keys %$data ) {
3692
        $data->{$key} = "$data->{$key}" if blessed( $data->{$key} );
3693
    }
3694
    return $data;
3695
}
3696
3670
=head1 AUTHORS
3697
=head1 AUTHORS
3671
3698
3672
Kyle M Hall <kyle@bywatersolutions.com>
3699
Kyle M Hall <kyle@bywatersolutions.com>
(-)a/t/db_dependent/Koha/Patrons.t (-7 / +12 lines)
Lines 2200-2208 subtest 'BorrowersLog and CardnumberLog tests' => sub { Link Here
2200
2200
2201
    my @logs = $schema->resultset('ActionLog')
2201
    my @logs = $schema->resultset('ActionLog')
2202
        ->search( { module => 'MEMBERS', action => 'MODIFY', object => $patron->borrowernumber } );
2202
        ->search( { module => 'MEMBERS', action => 'MODIFY', object => $patron->borrowernumber } );
2203
    my $log_info = from_json( $logs[0]->info );
2203
    my $log_diff = from_json( $logs[0]->diff );
2204
    is( $log_info->{cardnumber}->{after},  'TESTCARDNUMBER', 'Got correct new cardnumber' );
2204
    is( $log_diff->{D}->{cardnumber}->{N}, 'TESTCARDNUMBER', 'Got correct new cardnumber in diff' );
2205
    is( $log_info->{cardnumber}->{before}, $cardnumber,      'Got correct old cardnumber' );
2205
    is( $log_diff->{D}->{cardnumber}->{O}, $cardnumber,      'Got correct old cardnumber in diff' );
2206
    is( scalar @logs, 1, 'With BorrowersLog, one detailed MODIFY action should be logged for the modification.' );
2206
    is( scalar @logs, 1, 'With BorrowersLog, one detailed MODIFY action should be logged for the modification.' );
2207
2207
2208
    t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'connection' );
2208
    t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'connection' );
Lines 2254-2262 subtest 'BorrowersLog and CardnumberLog tests' => sub { Link Here
2254
        scalar @logs, 2,
2254
        scalar @logs, 2,
2255
        'With CardnumberLogs, one more detailed MODIFY_CARDNUMBER action should be logged for the modification.'
2255
        'With CardnumberLogs, one more detailed MODIFY_CARDNUMBER action should be logged for the modification.'
2256
    );
2256
    );
2257
    $log_info = from_json( $logs[1]->info );
2257
    my $cn_log_diff = from_json( $logs[1]->diff );
2258
    is( $log_info->{after},  'TESTCARDNUMBER',   'Got correct new cardnumber' );
2258
    is(
2259
    is( $log_info->{before}, 'TESTCARDNUMBER_1', 'Got correct old cardnumber' );
2259
        $cn_log_diff->{D}->{cardnumber}->{N}, 'TESTCARDNUMBER',
2260
        'Got correct new cardnumber in MODIFY_CARDNUMBER diff'
2261
    );
2262
    is(
2263
        $cn_log_diff->{D}->{cardnumber}->{O}, 'TESTCARDNUMBER_1',
2264
        'Got correct old cardnumber in MODIFY_CARDNUMBER diff'
2265
    );
2260
2266
2261
    t::lib::Mocks::mock_preference( 'CardnumberLog', 0 );
2267
    t::lib::Mocks::mock_preference( 'CardnumberLog', 0 );
2262
    $patron->set( { cardnumber => 'TESTCARDNUMBER_1' } )->store;
2268
    $patron->set( { cardnumber => 'TESTCARDNUMBER_1' } )->store;
2263
- 

Return to bug 40136