From 590da489810fc5418e8838189e74ccf0406a8291 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 21 Feb 2018 17:19:05 -0300 Subject: [PATCH] Bug 20287: Move fixup_cardnumber Signed-off-by: Josef Moravec Signed-off-by: Tomas Cohen Arazi Signed-off-by: Martin Renvoize --- C4/Members.pm | 40 +++--------------------------------- Koha/Patron.pm | 43 +++++++++++++++++++++++++++++++++++++++ Koha/Patrons/Import.pm | 5 ----- opac/svc/auth/googleopenidconnect | 2 -- t/db_dependent/Members.t | 4 +--- 5 files changed, 47 insertions(+), 47 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index a901a961bc..bd339d5dbb 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -86,7 +86,6 @@ BEGIN { #Check data push @EXPORT, qw( &checkuserpassword - &fixup_cardnumber &checkcardnumber ); } @@ -426,12 +425,6 @@ sub AddMember { $data{'dateenrolled'} = output_pref( { dt => dt_from_string, dateonly => 1, dateformat => 'iso' } ); } - if ( C4::Context->preference("autoMemberNum") ) { - if ( not exists $data{cardnumber} or not defined $data{cardnumber} or $data{cardnumber} eq '' ) { - $data{cardnumber} = fixup_cardnumber( $data{cardnumber} ); - } - } - $data{'privacy'} = $category->default_privacy() eq 'default' ? 1 : $category->default_privacy() eq 'never' ? 2 @@ -481,32 +474,6 @@ sub AddMember { return $data{borrowernumber}; } -=head2 fixup_cardnumber - -Warning: The caller is responsible for locking the members table in write -mode, to avoid database corruption. - -=cut - -sub fixup_cardnumber { - my ($cardnumber) = @_; - my $autonumber_members = C4::Context->boolean_preference('autoMemberNum') || 0; - - # Find out whether member numbers should be generated - # automatically. Should be either "1" or something else. - # Defaults to "0", which is interpreted as "no". - - ($autonumber_members) or return $cardnumber; - my $dbh = C4::Context->dbh; - - my $sth = $dbh->prepare( - 'SELECT MAX( CAST( cardnumber AS SIGNED ) ) FROM borrowers WHERE cardnumber REGEXP "^-?[0-9]+$"' - ); - $sth->execute; - my ($result) = $sth->fetchrow; - return $result + 1; -} - =head2 GetAllIssues $issues = &GetAllIssues($borrowernumber, $sortkey, $limit); @@ -879,11 +846,10 @@ sub IssueSlip { sub AddMember_Auto { my ( %borrower ) = @_; - $borrower{'cardnumber'} ||= fixup_cardnumber(); - $borrower{'borrowernumber'} = AddMember(%borrower); - - return ( %borrower ); + my $patron = Koha::Patrons->find( $borrower{borrowernumber} )->unblessed; + $patron->{password} = $borrower{password}; + return %$patron; } =head2 AddMember_Opac diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 35cfce4ca1..f8efe5b15a 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -83,6 +83,49 @@ Koha::Patron - Koha Patron Object class =cut +=head3 new + +=cut + +sub new { + my ( $class, $params ) = @_; + + return $class->SUPER::new($params); +} + +sub fixup_cardnumber { + my ( $self ) = @_; + my $max = Koha::Patrons->search({ + cardnumber => {-regexp => '^-?[0-9]+$'} + }, { + select => \'CAST(cardnumber AS SIGNED)', + as => ['cast_cardnumber'] + })->_resultset->get_column('cast_cardnumber')->max; + $self->cardnumber($max+1); +} + +sub store { + my( $self ) = @_; + + $self->_result->result_source->schema->txn_do( + sub { + if ( + C4::Context->preference("autoMemberNum") + and ( not defined $self->cardnumber + or $self->cardnumber eq '' ) + ) + { + # Warning: The caller is responsible for locking the members table in write + # mode, to avoid database corruption. + # We are in a transaction but the table is not locked + $self->fixup_cardnumber; + } + + $self->SUPER::store; + } + ); +} + =head3 delete $patron->delete diff --git a/Koha/Patrons/Import.pm b/Koha/Patrons/Import.pm index fbf9701e92..dbf945c632 100644 --- a/Koha/Patrons/Import.pm +++ b/Koha/Patrons/Import.pm @@ -295,11 +295,6 @@ sub import_patrons { ); } else { - # FIXME: fixup_cardnumber says to lock table, but the web interface doesn't so this doesn't either. - # At least this is closer to AddMember than in members/memberentry.pl - if ( !$borrower{'cardnumber'} ) { - $borrower{'cardnumber'} = fixup_cardnumber(undef); - } if ( $borrowernumber = AddMember(%borrower) ) { if ( $borrower{debarred} ) { diff --git a/opac/svc/auth/googleopenidconnect b/opac/svc/auth/googleopenidconnect index e3d7df434a..64d703ea7b 100755 --- a/opac/svc/auth/googleopenidconnect +++ b/opac/svc/auth/googleopenidconnect @@ -186,7 +186,6 @@ elsif ( defined $query->param('code') ) { my $auto_registration = C4::Context->preference('GoogleOpenIDConnectAutoRegister') // q{0}; my $borrower = Koha::Patrons->find( { email => $email } ); if (! $borrower && $auto_registration==1) { - my $cardnumber = fixup_cardnumber(); my $firstname = $claims_json->{'given_name'} // q{}; my $surname = $claims_json->{'family_name'} // q{}; my $delimiter = $firstname ? q{.} : q{}; @@ -198,7 +197,6 @@ elsif ( defined $query->param('code') ) { if (defined $patron_category && defined $library) { my $password = undef; my $borrowernumber = C4::Members::AddMember( - cardnumber => $cardnumber, firstname => $firstname, surname => $surname, email => $email, diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t index 1f9de78f2f..f405a46a69 100755 --- a/t/db_dependent/Members.t +++ b/t/db_dependent/Members.t @@ -388,13 +388,11 @@ is( $borrower->{password} eq $hashed_up, 1, 'Check password hash equals hash of subtest 'Trivial test for AddMember_Auto' => sub { plan tests => 3; - my $members_mock = Test::MockModule->new( 'C4::Members' ); - $members_mock->mock( 'fixup_cardnumber', sub { 12345; } ); my $library = $builder->build({ source => 'Branch' }); my $category = $builder->build({ source => 'Category' }); my %borr = AddMember_Auto( surname=> 'Dick3', firstname => 'Philip', branchcode => $library->{branchcode}, categorycode => $category->{categorycode}, password => '34567890' ); ok( $borr{borrowernumber}, 'Borrower hash contains borrowernumber' ); - is( $borr{cardnumber}, 12345, 'Borrower hash contains cardnumber' ); + like( $borr{cardnumber}, qr/^\d+$/, 'Borrower hash contains cardnumber' ); my $patron = Koha::Patrons->find( $borr{borrowernumber} ); isnt( $patron, undef, 'Patron found' ); }; -- 2.11.0