Bugzilla – Attachment 33144 Details for
Bug 1861
Unique patrons logins not (totally) enforced
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 1861 [QA Followup] - Fix Check_Userid and unit tests
Bug-1861-QA-Followup---Fix-CheckUserid-and-unit-te.patch (text/plain), 3.99 KB, created by
Kyle M Hall (khall)
on 2014-11-03 13:31:26 UTC
(
hide
)
Description:
Bug 1861 [QA Followup] - Fix Check_Userid and unit tests
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2014-11-03 13:31:26 UTC
Size:
3.99 KB
patch
obsolete
>From 2b862dbaa36efafda6f9cff5f40392e89bdffcb4 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Mon, 3 Nov 2014 07:38:48 -0500 >Subject: [PATCH] Bug 1861 [QA Followup] - Fix Check_Userid and unit tests > >Check_Userid assumes that a borrowernumber will always be passed in >and thus fails to to return 0 for an already used userid when creating >a new patron. > >Unit tests must now also me modified to no longer assume it is possible >to create multiple patrons with the same userid. >--- > C4/Members.pm | 31 +++++++++++++++++-------------- > t/db_dependent/Members.t | 13 ++++++------- > 2 files changed, 23 insertions(+), 21 deletions(-) > >diff --git a/C4/Members.pm b/C4/Members.pm >index 0978078..d3a36b1 100644 >--- a/C4/Members.pm >+++ b/C4/Members.pm >@@ -826,7 +826,8 @@ sub AddMember { > my $dbh = C4::Context->dbh; > > # generate a proper login if none provided >- $data{'userid'} = Generate_Userid($data{'borrowernumber'}, $data{'firstname'}, $data{'surname'}) if $data{'userid'} eq ''; >+ $data{'userid'} = Generate_Userid( $data{'borrowernumber'}, $data{'firstname'}, $data{'surname'} ) >+ if ( $data{'userid'} eq '' || Check_Userid( $data{'userid'} ) ); > > # add expiration date if it isn't already there > unless ( $data{'dateexpiry'} ) { >@@ -874,19 +875,21 @@ sub AddMember { > =cut > > sub Check_Userid { >- my ($uid,$member) = @_; >- my $dbh = C4::Context->dbh; >- my $sth = >- $dbh->prepare( >- "SELECT * FROM borrowers WHERE userid=? AND borrowernumber != ?"); >- $sth->execute( $uid, $member ); >- if ( (( $uid ne '' ) && ( my $row = $sth->fetchrow_hashref )) or >- (( $uid ne '' ) && ( $uid eq C4::Context->config('user') )) ) { >- return 0; >- } >- else { >- return 1; >- } >+ my ( $uid, $borrowernumber ) = @_; >+ >+ return 1 unless ($uid); >+ >+ return 0 if ( $uid eq C4::Context->config('user') ); >+ >+ my $rs = Koha::Database->new()->schema()->resultset('Borrower'); >+ >+ my $params; >+ $params->{userid} = $uid; >+ $params->{borrowernumber} = { '!=' => $borrowernumber } if ($borrowernumber); >+ >+ my $count = $rs->count( $params ); >+ >+ return $count ? 0 : 1; > } > > =head2 Generate_Userid >diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t >index 998320b..83e44b1 100755 >--- a/t/db_dependent/Members.t >+++ b/t/db_dependent/Members.t >@@ -278,15 +278,14 @@ ok (!_find_member($results), "Delete member") > branchcode => "MPL", > dateofbirth => '', > dateexpiry => '9999-12-31', >- userid => 'tomasito' > ); > # Add a new borrower > my $borrowernumber = AddMember( %data ); >-is( Check_Userid( 'tomasito', $borrowernumber ), 1, >+is( Check_Userid( 'tomasito.non', $borrowernumber ), 1, > 'recently created userid -> unique (borrowernumber passed)' ); > is( Check_Userid( 'tomasitoxxx', $borrowernumber ), 1, > 'non-existent userid -> unique (borrowernumber passed)' ); >-is( Check_Userid( 'tomasito', '' ), 0, >+is( Check_Userid( 'tomasito.none', '' ), 0, > 'userid exists (blank borrowernumber)' ); > is( Check_Userid( 'tomasitoxxx', '' ), 1, > 'non-existent userid -> unique (blank borrowernumber)' ); >@@ -294,12 +293,12 @@ is( Check_Userid( 'tomasitoxxx', '' ), 1, > # Add a new borrower with the same userid but different cardnumber > $data{ cardnumber } = "987654321"; > my $new_borrowernumber = AddMember( %data ); >-is( Check_Userid( 'tomasito', '' ), 0, >+is( Check_Userid( 'tomasito.none', '' ), 0, > 'userid not unique (blank borrowernumber)' ); >-is( Check_Userid( 'tomasito', $borrowernumber ), 0, >- 'userid not unique (first borrowernumber passed)' ); >-is( Check_Userid( 'tomasito', $new_borrowernumber ), 0, >+is( Check_Userid( 'tomasito.none', $new_borrowernumber ), 0, > 'userid not unique (second borrowernumber passed)' ); >+my $borrower = GetMember( borrowernumber => $new_borrowernumber ); >+ok( $borrower->{userid} ne 'tomasito', "Borrower with duplicate userid has new userid generated" ); > > # Regression tests for BZ12226 > is( Check_Userid( C4::Context->config('user'), '' ), 0, >-- >1.7.2.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 1861
:
29573
|
29616
|
30561
|
30828
|
33143
|
33144
|
33749
|
33750
|
33751
|
33752
|
33983
|
33984
|
33985
|
34820
|
34821
|
34822
|
34823