Attachment 70366 Details for Bug 19936 – Bug 19936: Add the Koha::Patron->has_valid_userid method

[patch] Bug 19936: Add the Koha::Patron->has_valid_userid method

Bug-19936-Add-the-KohaPatron-hasvaliduserid-method.patch (text/plain), 5.30 KB, created by Jonathan Druart on 2018-01-09 14:53:30 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2018-01-09 14:53:30 UTC

Size: 5.30 KB

patch

obsolete

>From c69edcbbaf02f3f3804aa8be99e6e54c6fce4441 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Mon, 8 Jan 2018 17:50:48 -0300
>Subject: [PATCH] Bug 19936: Add the Koha::Patron->has_valid_userid method
>
>Reuse how C4::Members::Check_Userid works and adapt it to write
>Koha::Patron->check_userid
>Adapt the tests to use this new method.
>The tests still pass, we can adapt the different callers
>---
> Koha/Patron.pm                | 35 ++++++++++++++++++++++++++++++++++
> t/db_dependent/Koha/Patrons.t | 44 ++++++++++++++++++++++++++++---------------
> 2 files changed, 64 insertions(+), 15 deletions(-)
>
>diff --git a/Koha/Patron.pm b/Koha/Patron.pm
>index e83eed6b55..fd7e06194e 100644
>--- a/Koha/Patron.pm
>+++ b/Koha/Patron.pm
>@@ -709,6 +709,41 @@ sub account_locked {
>           and $self->login_attempts >= $FailedLoginAttempts )? 1 : 0;
> }
> 
>+=head3 has_valid_userid
>+
>+my $patron = Koha::Patrons->find(42);
>+$patron->userid( $new_userid );
>+my $has_a_valid_userid = $patron->has_valid_userid
>+
>+my $patron = Koha::Patron->new( $params );
>+my $has_a_valid_userid = $patron->has_valid_userid
>+
>+Return true if the current userid of this patron is valid/unique, otherwise false.
>+
>+Note that this should be done in $self->store instead and raise an exception if needed.
>+
>+=cut
>+
>+sub has_valid_userid {
>+    my ($self) = @_;
>+
>+    return 0 unless $self->userid;
>+
>+    return 0 if ( $self->userid eq C4::Context->config('user') );    # DB user
>+
>+    my $already_exists = Koha::Patrons->search(
>+        {
>+            userid => $self->userid,
>+            (
>+                $self->in_storage
>+                ? ( borrowernumber => { '!=' => $self->borrowernumber } )
>+                : ()
>+            ),
>+        }
>+    )->count;
>+    return $already_exists ? 0 : 1;
>+}
>+
> =head3 type
> 
> =cut
>diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t
>index 33a3792b64..0c7331add7 100644
>--- a/t/db_dependent/Koha/Patrons.t
>+++ b/t/db_dependent/Koha/Patrons.t
>@@ -956,7 +956,7 @@ subtest 'account_locked' => sub {
> };
> 
> subtest 'userid_is_valid' => sub {
>-    plan tests => 9;
>+    plan tests => 10;
> 
>     my $library = $builder->build_object( { class => 'Koha::Libraries' } );
>     my $patron_category = $builder->build_object(
>@@ -973,32 +973,46 @@ subtest 'userid_is_valid' => sub {
>         branchcode   => $library->branchcode,
>     );
> 
>+    my $expected_userid_patron_1 = 'tomasito.none';
>     my $borrowernumber = AddMember(%data);
>     my $patron_1       = Koha::Patrons->find($borrowernumber);
>+    is ( $patron_1->userid, $expected_userid_patron_1, 'The userid generated should be the one we expect' );
> 
>-    is( Check_Userid( 'tomasito.non', $patron_1->borrowernumber ),
>+    $patron_1->userid( 'tomasito.non' );
>+    is( $patron_1->has_valid_userid, # FIXME Joubu: What is the difference with the next test?
>         1, 'recently created userid -> unique (borrowernumber passed)' );
>-    is( Check_Userid( 'tomasitoxxx', $patron_1->borrowernumber ),
>+
>+    $patron_1->userid( 'tomasitoxxx' );
>+    is( $patron_1->has_valid_userid,
>         1, 'non-existent userid -> unique (borrowernumber passed)' );
>-    is( Check_Userid( 'tomasito.none', '' ),
>-        0, 'userid exists (blank borrowernumber)' );
>-    is( Check_Userid( 'tomasitoxxx', '' ),
>-        1, 'non-existent userid -> unique (blank borrowernumber)' );
>+    $patron_1->discard_changes; # We compare with the original userid later
>+
>+    my $patron_not_in_storage = Koha::Patron->new( { userid => '' } );
>+    is( $patron_not_in_storage->has_valid_userid,
>+        0, 'userid exists for another patron, patron is not in storage yet' );
>+
>+    $patron_not_in_storage = Koha::Patron->new( { userid => 'tomasitoxxx' } );
>+    is( $patron_not_in_storage->has_valid_userid,
>+        1, 'non-existent userid, patron is not in storage yet' );
> 
>     # Regression tests for BZ12226
>-    is( Check_Userid( C4::Context->config('user'), '' ),
>-        0, 'Check_Userid should return 0 for the DB user (Bug 12226)' );
>+    my $db_patron = Koha::Patron->new( { userid => C4::Context->config('user') } );
>+    is( $db_patron->has_valid_userid,
>+        0, 'Koha::Patron->has_valid_userid should return 0 for the DB user (Bug 12226)' );
> 
>     # Add a new borrower with the same userid but different cardnumber
>     $data{cardnumber} = "987654321";
>     my $new_borrowernumber = AddMember(%data);
>-    is( Check_Userid( 'tomasito.none', '' ),
>-        0, 'userid not unique (blank borrowernumber)' );
>-    is( Check_Userid( 'tomasito.none', $new_borrowernumber ),
>-        0, 'userid not unique (second borrowernumber passed)' );
>     my $patron_2 = Koha::Patrons->find($new_borrowernumber);
>-    ok( $patron_2->userid ne 'tomasito',
>-        "Borrower with duplicate userid has new userid generated" );
>+    $patron_2->userid($patron_1->userid);
>+    is( $patron_2->has_valid_userid,
>+        0, 'The userid is already in used, it cannot be used for another patron' );
>+
>+    $patron_2 = Koha::Patrons->find($new_borrowernumber);
>+    isnt( $patron_2->userid, 'tomasito',
>+        "Patron with duplicate userid has new userid generated" );
>+    is( $patron_2->userid, $expected_userid_patron_1 . '1', # TODO we could make that configurable
>+        "Patron with duplicate userid has new userid generated (1 is appened" );
> 
>     my $new_userid = 'a_user_id';
>     $data{cardnumber} = "234567890";
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 19936: 70365 | 70366 | 70367 | 70368 | 70369 | 70370 | 70371 | 70372 | 70373 | 70374 | 71787 | 71788 | 71789 | 71790 | 71791 | 71792 | 71793 | 71794 | 71795 | 71796 | 72129 | 72130 | 72131 | 72132 | 72133 | 72134 | 72135 | 72136 | 72137 | 72138 | 72139 | 72140 | 72141 | 72142 | 72143 | 72144 | 72145 | 72146 | 72147 | 72148 | 72149 | 72150 | 72151 | 72152 | 73630 | 73631 | 73632 | 73633 | 73634 | 73635 | 73636 | 73637 | 73638 | 73639 | 73640 | 73641 | 73963 | 73964 | 73965 | 73966 | 73967 | 73968 | 73969 | 73970 | 73971 | 73972 | 73973 | 73974 | 74071 | 74072 | 74073 | 74074 | 74075 | 74076 | 74077 | 74078 | 74079 | 74080 | 74081 | 74082