Attachment 80207 Details for Bug 21336 – Bug 21336: Search, lock and anonymize methods

[patch] Bug 21336: Search, lock and anonymize methods

Bug-21336-Search-lock-and-anonymize-methods.patch (text/plain), 15.76 KB, created by Marcel de Rooy on 2018-10-08 08:36:09 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2018-10-08 08:36:09 UTC

Size: 15.76 KB

patch

obsolete

>From 030ec58a3c214c4b7398d4ecbc22dba9ae5bff54 Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Wed, 12 Sep 2018 14:25:26 +0200
>Subject: [PATCH] Bug 21336: Search, lock and anonymize methods
>Content-Type: text/plain; charset=utf-8
>
>Add Koha::Patron->lock and anonymize.
>Add Koha::Patrons methods search_unsubscribed, search_anonymize_candidates
>and search_anonymized. And wrappers for lock and anonymize.
>Add unit tests.
>
>Test plan:
>Run t/db_dependent/Koha/Patrons.t
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>---
> Koha/Patron.pm                |  73 +++++++++++++++++++
> Koha/Patrons.pm               | 122 ++++++++++++++++++++++++++++++++
> t/db_dependent/Koha/Patrons.t | 161 +++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 355 insertions(+), 1 deletion(-)
>
>diff --git a/Koha/Patron.pm b/Koha/Patron.pm
>index 41c3ba8..a360cb9 100644
>--- a/Koha/Patron.pm
>+++ b/Koha/Patron.pm
>@@ -43,9 +43,12 @@ use Koha::Virtualshelves;
> use Koha::Club::Enrollments;
> use Koha::Account;
> use Koha::Subscription::Routinglists;
>+use Koha::Token;
> 
> use base qw(Koha::Object);
> 
>+use constant ADMINISTRATIVE_LOCKOUT => -1;
>+
> our $RESULTSET_PATRON_ID_MAPPING = {
>     Accountline          => 'borrowernumber',
>     Aqbasketuser         => 'borrowernumber',
>@@ -1067,6 +1070,76 @@ sub account_locked {
>     return 0;
> }
> 
>+=head3 lock
>+
>+    Koha::Patrons->find($id)->lock({ expire => 1, remove => 1 });
>+
>+    Lock and optionally expire a patron account.
>+    Remove holds and article requests if remove flag set.
>+    In order to distinguish from locking by entering a wrong password, let's
>+    call this an administrative lockout.
>+
>+=cut
>+
>+sub lock {
>+    my ( $self, $params ) = @_;
>+    $self->login_attempts( ADMINISTRATIVE_LOCKOUT );
>+    if( $params->{expire} ) {
>+        $self->dateexpiry( dt_from_string->subtract(days => 1) );
>+    }
>+    $self->store;
>+    if( $params->{remove} ) {
>+        $self->holds->delete;
>+        $self->article_requests->delete;
>+    }
>+    return $self;
>+}
>+
>+=head3 anonymize
>+
>+    Koha::Patrons->find($id)->anonymize;
>+
>+    Anonymize or clear borrower fields. Fields in BorrowerMandatoryField
>+    are randomized, other personal data is cleared too.
>+    Patrons with issues are skipped.
>+
>+=cut
>+
>+sub anonymize {
>+    my ( $self ) = @_;
>+    if( $self->_result->issues->count ) {
>+        warn "Exiting anonymize: patron ".$self->borrowernumber." still has issues";
>+        return;
>+    }
>+    my $mandatory = { map { (lc $_, 1); }
>+        split /\s*\|\s*/, C4::Context->preference('BorrowerMandatoryField') };
>+    $mandatory->{userid} = 1; # needed since sub store does not clear field
>+    my @columns = $self->_result->result_source->columns;
>+    @columns = grep { !/borrowernumber|branchcode|categorycode|^date|password|flags|updated_on|lastseen|lang|login_attempts|flgAnonymized/ } @columns;
>+    push @columns, 'dateofbirth'; # add this date back in
>+    foreach my $col (@columns) {
>+        if( $mandatory->{lc $col} ) {
>+            my $str = $self->_anonymize_column($col);
>+            $self->$col($str);
>+        } else {
>+            $self->$col(undef);
>+        }
>+    }
>+    $self->flgAnonymized(1)->store;
>+}
>+
>+sub _anonymize_column {
>+    my ( $self, $col ) = @_;
>+    my $type = $self->_result->result_source->column_info($col)->{data_type};
>+    if( $type =~ /char|text/ ) {
>+        return Koha::Token->new->generate({ pattern => '\w{10}' });
>+    } elsif( $type =~ /integer|int$|float|dec|double/ ) {
>+        return 0;
>+    } elsif( $type =~ /date|time/ ) {
>+        return dt_from_string;
>+    }
>+}
>+
> =head3 can_see_patron_infos
> 
> my $can_see = $patron->can_see_patron_infos( $patron );
>diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm
>index 0068f34..6ab55f9 100644
>--- a/Koha/Patrons.pm
>+++ b/Koha/Patrons.pm
>@@ -234,6 +234,128 @@ sub delete {
>     return 1;
> }
> 
>+=head3 search_unsubscribed
>+
>+    Koha::Patrons->search_unsubscribed;
>+
>+    Returns a set of Koha patron objects for patrons that recently
>+    unsubscribed and are not locked (candidates for locking).
>+    Depends on UnsubscribeReflectionDelay.
>+
>+=cut
>+
>+sub search_unsubscribed {
>+    my ( $class ) = @_;
>+
>+    my $delay = C4::Context->preference('UnsubscribeReflectionDelay');
>+    if( !defined($delay) || $delay eq q{} ) {
>+        # return empty set
>+        return $class->search({ borrowernumber => undef });
>+    }
>+    my $parser = Koha::Database->new->schema->storage->datetime_parser;
>+    my $dt = dt_from_string()->subtract( days => $delay );
>+    my $str = $parser->format_datetime($dt);
>+    my $fails = C4::Context->preference('FailedLoginAttempts') || 0;
>+    my $cond = [ undef, 0, 1..$fails-1 ]; # NULL, 0, 1..fails-1 (if fails>0)
>+    return $class->search(
>+        {
>+            'patron_consents.refused_on' => { '<=' => $str },
>+            'login_attempts' => $cond,
>+        },
>+        { join => 'patron_consents' },
>+    );
>+}
>+
>+=head3 search_anonymize_candidates
>+
>+    Koha::Patrons->search_anonymize_candidates({ locked => 1 });
>+
>+    Returns a set of Koha patron objects for patrons whose account is expired
>+    and locked (if parameter set). These are candidates for anonymizing.
>+    Depends on PatronAnonymizeDelay.
>+
>+=cut
>+
>+sub search_anonymize_candidates {
>+    my ( $class, $params ) = @_;
>+
>+    my $delay = C4::Context->preference('PatronAnonymizeDelay');
>+    if( !defined($delay) || $delay eq q{} ) {
>+        # return empty set
>+        return $class->search({ borrowernumber => undef });
>+    }
>+    my $cond = {};
>+    my $parser = Koha::Database->new->schema->storage->datetime_parser;
>+    my $dt = dt_from_string()->subtract( days => $delay );
>+    my $str = $parser->format_datetime($dt);
>+    $cond->{dateexpiry} = { '<=' => $str };
>+    $cond->{flgAnonymized} = [ undef, 0 ]; # not yet done
>+    if( $params->{locked} ) {
>+        my $fails = C4::Context->preference('FailedLoginAttempts');
>+        $cond->{login_attempts} = [ -and => { '!=' => undef }, { -not_in => [0, 1..$fails-1 ] } ]; # -not_in does not like undef
>+    }
>+    return $class->search( $cond );
>+}
>+
>+=head3 search_anonymized
>+
>+    Koha::Patrons->search_anonymized;
>+
>+    Returns a set of Koha patron objects for patron accounts that have been
>+    anonymized before and could be removed.
>+    Depends on PatronRemovalDelay.
>+
>+=cut
>+
>+sub search_anonymized {
>+    my ( $class ) = @_;
>+
>+    my $delay = C4::Context->preference('PatronRemovalDelay');
>+    if( !defined($delay) || $delay eq q{} ) {
>+        # return empty set
>+        return $class->search({ borrowernumber => undef });
>+    }
>+    my $cond = {};
>+    my $parser = Koha::Database->new->schema->storage->datetime_parser;
>+    my $dt = dt_from_string()->subtract( days => $delay );
>+    my $str = $parser->format_datetime($dt);
>+    $cond->{dateexpiry} = { '<=' => $str };
>+    $cond->{flgAnonymized} = 1;
>+    return $class->search( $cond );
>+}
>+
>+=head3 lock
>+
>+    Koha::Patrons->search({ some filters })->lock({ expire => 1, remove => 1 })
>+
>+    Lock the passed set of patron objects. Optionally expire and remove holds.
>+    Wrapper around Koha::Patron->lock.
>+
>+=cut
>+
>+sub lock {
>+    my ( $self, $params ) = @_;
>+    while( my $patron = $self->next ) {
>+        $patron->lock($params);
>+    }
>+}
>+
>+=head3 anonymize
>+
>+    Koha::Patrons->search({ some filters })->anonymize;
>+
>+    Anonymize passed set of patron objects.
>+    Wrapper around Koha::Patron->anonymize.
>+
>+=cut
>+
>+sub anonymize {
>+    my ( $self ) = @_;
>+    while( my $patron = $self->next ) {
>+        $patron->anonymize;
>+    }
>+}
>+
> =head3 _type
> 
> =cut
>diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t
>index 57c690f..e232a8e 100644
>--- a/t/db_dependent/Koha/Patrons.t
>+++ b/t/db_dependent/Koha/Patrons.t
>@@ -19,7 +19,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 33;
>+use Test::More tests => 38;
> use Test::Warn;
> use Test::Exception;
> use Test::MockModule;
>@@ -1452,6 +1452,165 @@ subtest 'Log cardnumber change' => sub {
>     is( scalar @logs, 2, 'With BorrowerLogs, Change in cardnumber should be logged, as well as general alert of patron mod.' );
> };
> 
>+subtest 'search_unsubscribed' => sub {
>+    plan tests => 4;
>+
>+    t::lib::Mocks::mock_preference( 'FailedLoginAttempts', 3 );
>+    t::lib::Mocks::mock_preference( 'UnsubscribeReflectionDelay', '' );
>+    is( Koha::Patrons->search_unsubscribed->count, 0, 'Empty delay should return empty set' );
>+
>+    my $patron1 = $builder->build_object({ class => 'Koha::Patrons' });
>+    my $patron2 = $builder->build_object({ class => 'Koha::Patrons' });
>+
>+    t::lib::Mocks::mock_preference( 'UnsubscribeReflectionDelay', 0 );
>+    Koha::Patron::Consents->delete; # for correct counts
>+    Koha::Patron::Consent->new({ borrowernumber => $patron1->borrowernumber, type => 'GDPR_PROCESSING',  refused_on => dt_from_string })->store;
>+    is( Koha::Patrons->search_unsubscribed->count, 1, 'Find patron1' );
>+
>+    # Add another refusal but shift the period
>+    t::lib::Mocks::mock_preference( 'UnsubscribeReflectionDelay', 2 );
>+    Koha::Patron::Consent->new({ borrowernumber => $patron2->borrowernumber, type => 'GDPR_PROCESSING',  refused_on => dt_from_string->subtract(days=>2) })->store;
>+    is( Koha::Patrons->search_unsubscribed->count, 1, 'Find patron2 only' );
>+
>+    # Try another (special) attempts setting
>+    t::lib::Mocks::mock_preference( 'FailedLoginAttempts', 0 );
>+    # Lockout is now disabled
>+    # Patron2 still matches: refused earlier, not locked
>+    is( Koha::Patrons->search_unsubscribed->count, 1, 'Lockout disabled' );
>+};
>+
>+subtest 'search_anonymize_candidates' => sub {
>+    plan tests => 5;
>+    my $patron1 = $builder->build_object({ class => 'Koha::Patrons' });
>+    my $patron2 = $builder->build_object({ class => 'Koha::Patrons' });
>+    $patron1->flgAnonymized(0);
>+    $patron1->dateexpiry( dt_from_string->add(days => 1) )->store;
>+    $patron2->flgAnonymized(undef);
>+    $patron2->dateexpiry( dt_from_string->add(days => 1) )->store;
>+
>+    t::lib::Mocks::mock_preference( 'PatronAnonymizeDelay', q{} );
>+    is( Koha::Patrons->search_anonymize_candidates->count, 0, 'Empty set' );
>+
>+    t::lib::Mocks::mock_preference( 'PatronAnonymizeDelay', 0 );
>+    my $cnt = Koha::Patrons->search_anonymize_candidates->count;
>+    $patron1->dateexpiry( dt_from_string->subtract(days => 1) )->store;
>+    $patron2->dateexpiry( dt_from_string->subtract(days => 3) )->store;
>+    is( Koha::Patrons->search_anonymize_candidates->count, $cnt+2, 'Delay 0' );
>+
>+    t::lib::Mocks::mock_preference( 'PatronAnonymizeDelay', 2 );
>+    $patron1->dateexpiry( dt_from_string->add(days => 1) )->store;
>+    $patron2->dateexpiry( dt_from_string->add(days => 1) )->store;
>+    $cnt = Koha::Patrons->search_anonymize_candidates->count;
>+    $patron1->dateexpiry( dt_from_string->subtract(days => 1) )->store;
>+    $patron2->dateexpiry( dt_from_string->subtract(days => 3) )->store;
>+    is( Koha::Patrons->search_anonymize_candidates->count, $cnt+1, 'Delay 2' );
>+
>+    t::lib::Mocks::mock_preference( 'PatronAnonymizeDelay', 4 );
>+    $patron1->dateexpiry( dt_from_string->add(days => 1) )->store;
>+    $patron2->dateexpiry( dt_from_string->add(days => 1) )->store;
>+    $cnt = Koha::Patrons->search_anonymize_candidates->count;
>+    $patron1->dateexpiry( dt_from_string->subtract(days => 1) )->store;
>+    $patron2->dateexpiry( dt_from_string->subtract(days => 3) )->store;
>+    is( Koha::Patrons->search_anonymize_candidates->count, $cnt, 'Delay 4' );
>+
>+    t::lib::Mocks::mock_preference( 'FailedLoginAttempts', 3 );
>+    $patron1->dateexpiry( dt_from_string->subtract(days => 5) )->store;
>+    $patron1->login_attempts(0)->store;
>+    $patron2->dateexpiry( dt_from_string->subtract(days => 5) )->store;
>+    $patron2->login_attempts(0)->store;
>+    $cnt = Koha::Patrons->search_anonymize_candidates({locked => 1})->count;
>+    $patron1->login_attempts(3)->store;
>+    is( Koha::Patrons->search_anonymize_candidates({locked => 1})->count,
>+        $cnt+1, 'Locked flag' );
>+};
>+
>+subtest 'search_anonymized' => sub {
>+    plan tests => 3;
>+    my $patron1 = $builder->build_object( { class => 'Koha::Patrons' } );
>+
>+    t::lib::Mocks::mock_preference( 'PatronRemovalDelay', q{} );
>+    is( Koha::Patrons->search_anonymized->count, 0, 'Empty set' );
>+
>+    t::lib::Mocks::mock_preference( 'PatronRemovalDelay', 1 );
>+    $patron1->dateexpiry( dt_from_string );
>+    $patron1->flgAnonymized(1)->store;
>+    my $cnt = Koha::Patrons->search_anonymized->count;
>+    is( Koha::Patrons->search_anonymized->count, $cnt, 'Number unchanged' );
>+    $patron1->dateexpiry( dt_from_string->subtract(days => 1) )->store;
>+    is( Koha::Patrons->search_anonymized->count, $cnt+1, 'Found patron1' );
>+};
>+
>+subtest 'lock' => sub {
>+    plan tests => 8;
>+
>+    my $patron1 = $builder->build_object( { class => 'Koha::Patrons' } );
>+    my $patron2 = $builder->build_object( { class => 'Koha::Patrons' } );
>+    my $hold = $builder->build_object({
>+        class => 'Koha::Holds',
>+        value => { borrowernumber => $patron1->borrowernumber },
>+    });
>+
>+    t::lib::Mocks::mock_preference( 'FailedLoginAttempts', 3 );
>+    my $expiry = dt_from_string->add(days => 1);
>+    $patron1->dateexpiry( $expiry );
>+    $patron1->lock;
>+    is( $patron1->login_attempts, Koha::Patron::ADMINISTRATIVE_LOCKOUT, 'Check login_attempts' );
>+    is( $patron1->dateexpiry, $expiry, 'Not expired yet' );
>+    is( $patron1->holds->count, 1, 'No holds removed' );
>+
>+    $patron1->lock({ expire => 1, remove => 1});
>+    isnt( $patron1->dateexpiry, $expiry, 'Expiry date adjusted' );
>+    is( $patron1->holds->count, 0, 'Holds removed' );
>+
>+    # Disable lockout feature
>+    t::lib::Mocks::mock_preference( 'FailedLoginAttempts', q{} );
>+    $patron1->login_attempts(0);
>+    $patron1->dateexpiry( $expiry );
>+    $patron1->store;
>+    $patron1->lock;
>+    is( $patron1->login_attempts, Koha::Patron::ADMINISTRATIVE_LOCKOUT, 'Check login_attempts' );
>+
>+    # Trivial wrapper test (Koha::Patrons->lock)
>+    $patron1->login_attempts(0)->store;
>+    Koha::Patrons->search({ borrowernumber => [ $patron1->borrowernumber, $patron2->borrowernumber ] })->lock;
>+    $patron1->discard_changes; # refresh
>+    $patron2->discard_changes;
>+    is( $patron1->login_attempts, Koha::Patron::ADMINISTRATIVE_LOCKOUT, 'Check login_attempts patron 1' );
>+    is( $patron2->login_attempts, Koha::Patron::ADMINISTRATIVE_LOCKOUT, 'Check login_attempts patron 2' );
>+};
>+
>+subtest 'anonymize' => sub {
>+    plan tests => 9;
>+
>+    my $patron1 = $builder->build_object( { class => 'Koha::Patrons' } );
>+    my $patron2 = $builder->build_object( { class => 'Koha::Patrons' } );
>+
>+    # First try patron with issues
>+    my $issue = $builder->build_object({ class => 'Koha::Checkouts', value => { borrowernumber => $patron2->borrowernumber } });
>+    warning_like { $patron2->anonymize } qr/still has issues/, 'Skip patron with issues';
>+    $issue->delete;
>+
>+    t::lib::Mocks::mock_preference( 'BorrowerMandatoryField', 'surname|email|cardnumber' );
>+    my $surname = $patron1->surname; # expect change, no clear
>+    my $branchcode = $patron1->branchcode; # expect skip
>+    $patron1->anonymize;
>+    is($patron1->flgAnonymized, 1, 'Check flag' );
>+
>+    is( $patron1->dateofbirth, undef, 'Birth date cleared' );
>+    is( $patron1->firstname, undef, 'First name cleared' );
>+    isnt( $patron1->surname, $surname, 'Surname changed' );
>+    ok( $patron1->surname =~ /^\w{10}$/, 'Mandatory surname randomized' );
>+    is( $patron1->branchcode, $branchcode, 'Branch code skipped' );
>+
>+    # Test wrapper in Koha::Patrons
>+    $patron1->surname($surname)->store; # restore
>+    my $rs = Koha::Patrons->search({ borrowernumber => [ $patron1->borrowernumber, $patron2->borrowernumber ] })->anonymize;
>+    $patron1->discard_changes; # refresh
>+    isnt( $patron1->surname, $surname, 'Surname patron1 changed again' );
>+    $patron2->discard_changes; # refresh
>+    is( $patron2->firstname, undef, 'First name patron2 cleared' );
>+};
>+
> $schema->storage->txn_rollback;
> 
> subtest 'Test Koha::Patrons::merge' => sub {
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 21336: 79165 | 79166 | 79167 | 79168 | 79169 | 79170 | 80201 | 80202 | 80203 | 80204 | 80205 | 80206 | 80207 | 80208 | 80213 | 80214 | 80386 | 80387 | 80403 | 81112 | 81113 | 81114 | 81115 | 81116 | 81117 | 81118 | 81119 | 81270 | 81474 | 81475 | 81476 | 81477 | 81478 | 81479 | 81480 | 81481 | 81482 | 81483 | 82051 | 82052 | 82053 | 82054 | 82055 | 82056 | 82057 | 82058 | 82059 | 83202 | 83203 | 83204 | 83205 | 83206 | 83207 | 83208 | 83209 | 83256 | 87158 | 87159 | 87160 | 87161 | 87162 | 87163 | 87164 | 87165 | 87166 | 87167 | 87168 | 87176 | 87177 | 87178 | 87179 | 87180 | 87181 | 87182 | 87183 | 87184 | 87185 | 87387 | 87388 | 87389 | 87390 | 87391 | 87392 | 87393 | 87394 | 87395 | 87396 | 87397 | 87400 | 87401 | 88198 | 88199 | 88217 | 88218