Attachment #70366 for bug #19936

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




          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




};

subtest 'userid_is_valid' => sub {
    plan tests => 10;

    my $library = $builder->build_object( { class => 'Koha::Libraries' } );
    my $patron_category = $builder->build_object(

        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' );

    $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)' );

    $patron_1->userid( 'tomasitoxxx' );
    is( $patron_1->has_valid_userid,
        1, 'non-existent userid -> unique (borrowernumber passed)' );
    $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
    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);
    $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";
- 

Return to bug 19936

Lines 709-714 sub account_locked { Link Here

(-)a/Koha/Patron.pm (+35 lines)
709	and $self->login_attempts >= $FailedLoginAttempts )? 1 : 0;	709	and $self->login_attempts >= $FailedLoginAttempts )? 1 : 0;
710	}	710	}
711		711
		712	=head3 has_valid_userid
		713
		714	my $patron = Koha::Patrons->find(42);
		715	$patron->userid( $new_userid );
		716	my $has_a_valid_userid = $patron->has_valid_userid
		717
		718	my $patron = Koha::Patron->new( $params );
		719	my $has_a_valid_userid = $patron->has_valid_userid
		720
		721	Return true if the current userid of this patron is valid/unique, otherwise false.
		722
		723	Note that this should be done in $self->store instead and raise an exception if needed.
		724
		725	=cut
		726
		727	sub has_valid_userid {
		728	my ($self) = @_;
		729
		730	return 0 unless $self->userid;
		731
		732	return 0 if ( $self->userid eq C4::Context->config('user') ); # DB user
		733
		734	my $already_exists = Koha::Patrons->search(
		735	{
		736	userid => $self->userid,
		737	(
		738	$self->in_storage
		739	? ( borrowernumber => { '!=' => $self->borrowernumber } )
		740	: ()
		741	),
		742	}
		743	)->count;
		744	return $already_exists ? 0 : 1;
		745	}
		746
712	=head3 type	747	=head3 type
713		748
714	=cut	749	=cut

Lines 956-962 subtest 'account_locked' => sub { Link Here

(-)a/t/db_dependent/Koha/Patrons.t (-16 / +29 lines)
956	};	956	};
957		957
958	subtest 'userid_is_valid' => sub {	958	subtest 'userid_is_valid' => sub {
959	plan tests => 9;	959	plan tests => 10;
960		960
961	my $library = $builder->build_object( { class => 'Koha::Libraries' } );	961	my $library = $builder->build_object( { class => 'Koha::Libraries' } );
962	my $patron_category = $builder->build_object(	962	my $patron_category = $builder->build_object(
Lines 973-1004 subtest 'userid_is_valid' => sub { Link Here
973	branchcode => $library->branchcode,	973	branchcode => $library->branchcode,
974	);	974	);
975		975
		976	my $expected_userid_patron_1 = 'tomasito.none';
976	my $borrowernumber = AddMember(%data);	977	my $borrowernumber = AddMember(%data);
977	my $patron_1 = Koha::Patrons->find($borrowernumber);	978	my $patron_1 = Koha::Patrons->find($borrowernumber);
		979	is ( $patron_1->userid, $expected_userid_patron_1, 'The userid generated should be the one we expect' );
978		980
979	is( Check_Userid( 'tomasito.non', $patron_1->borrowernumber ),	981	$patron_1->userid( 'tomasito.non' );
		982	is( $patron_1->has_valid_userid, # FIXME Joubu: What is the difference with the next test?
980	1, 'recently created userid -> unique (borrowernumber passed)' );	983	1, 'recently created userid -> unique (borrowernumber passed)' );
981	is( Check_Userid( 'tomasitoxxx', $patron_1->borrowernumber ),	984
		985	$patron_1->userid( 'tomasitoxxx' );
		986	is( $patron_1->has_valid_userid,
982	1, 'non-existent userid -> unique (borrowernumber passed)' );	987	1, 'non-existent userid -> unique (borrowernumber passed)' );
983	is( Check_Userid( 'tomasito.none', '' ),	988	$patron_1->discard_changes; # We compare with the original userid later
984	0, 'userid exists (blank borrowernumber)' );	989
985	is( Check_Userid( 'tomasitoxxx', '' ),	990	my $patron_not_in_storage = Koha::Patron->new( { userid => '' } );
986	1, 'non-existent userid -> unique (blank borrowernumber)' );	991	is( $patron_not_in_storage->has_valid_userid,
		992	0, 'userid exists for another patron, patron is not in storage yet' );
		993
		994	$patron_not_in_storage = Koha::Patron->new( { userid => 'tomasitoxxx' } );
		995	is( $patron_not_in_storage->has_valid_userid,
		996	1, 'non-existent userid, patron is not in storage yet' );
987		997
988	# Regression tests for BZ12226	998	# Regression tests for BZ12226
989	is( Check_Userid( C4::Context->config('user'), '' ),	999	my $db_patron = Koha::Patron->new( { userid => C4::Context->config('user') } );
990	0, 'Check_Userid should return 0 for the DB user (Bug 12226)' );	1000	is( $db_patron->has_valid_userid,
		1001	0, 'Koha::Patron->has_valid_userid should return 0 for the DB user (Bug 12226)' );
991		1002
992	# Add a new borrower with the same userid but different cardnumber	1003	# Add a new borrower with the same userid but different cardnumber
993	$data{cardnumber} = "987654321";	1004	$data{cardnumber} = "987654321";
994	my $new_borrowernumber = AddMember(%data);	1005	my $new_borrowernumber = AddMember(%data);
995	is( Check_Userid( 'tomasito.none', '' ),
996	0, 'userid not unique (blank borrowernumber)' );
997	is( Check_Userid( 'tomasito.none', $new_borrowernumber ),
998	0, 'userid not unique (second borrowernumber passed)' );
999	my $patron_2 = Koha::Patrons->find($new_borrowernumber);	1006	my $patron_2 = Koha::Patrons->find($new_borrowernumber);
1000	ok( $patron_2->userid ne 'tomasito',	1007	$patron_2->userid($patron_1->userid);
1001	"Borrower with duplicate userid has new userid generated" );	1008	is( $patron_2->has_valid_userid,
		1009	0, 'The userid is already in used, it cannot be used for another patron' );
		1010
		1011	$patron_2 = Koha::Patrons->find($new_borrowernumber);
		1012	isnt( $patron_2->userid, 'tomasito',
		1013	"Patron with duplicate userid has new userid generated" );
		1014	is( $patron_2->userid, $expected_userid_patron_1 . '1', # TODO we could make that configurable
		1015	"Patron with duplicate userid has new userid generated (1 is appened" );
1002		1016
1003	my $new_userid = 'a_user_id';	1017	my $new_userid = 'a_user_id';
1004	$data{cardnumber} = "234567890";	1018	$data{cardnumber} = "234567890";
1005	-