From dc6b6e030c0e219f84350cea706910a7433ca0a4 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 22 Feb 2018 16:55:00 -0300 Subject: [PATCH] Bug 20287: Remove AddMember_Auto I am not sure I understood the point of this subroutine. Did I miss something here? Signed-off-by: Josef Moravec Signed-off-by: Tomas Cohen Arazi Signed-off-by: Martin Renvoize Signed-off-by: Nick Clemens --- C4/Auth_with_shibboleth.pm | 8 ++++---- C4/Members.pm | 17 ++--------------- opac/opac-memberentry.pl | 23 +++++++++++++++++------ t/db_dependent/Members.t | 13 +------------ 4 files changed, 24 insertions(+), 37 deletions(-) diff --git a/C4/Auth_with_shibboleth.pm b/C4/Auth_with_shibboleth.pm index 6b20511df5..f855ae1bdf 100644 --- a/C4/Auth_with_shibboleth.pm +++ b/C4/Auth_with_shibboleth.pm @@ -23,7 +23,7 @@ use C4::Debug; use C4::Context; use Koha::AuthUtils qw(get_script_name); use Koha::Database; -use C4::Members qw( AddMember_Auto ); +use Koha::Patrons; use C4::Members::Messaging; use Carp; use CGI; @@ -122,10 +122,10 @@ sub _autocreate { $borrower{$key} = ( $entry->{'is'} && $ENV{ $entry->{'is'} } ) || $entry->{'content'} || ''; } - %borrower = AddMember_Auto( %borrower ); - C4::Members::Messaging::SetMessagingPreferencesFromDefaults( { borrowernumber => $borrower{'borrowernumber'}, categorycode => $borrower{'categorycode'} } ); + my $patron = Koha::Patron->new( \%borrower )->store; + C4::Members::Messaging::SetMessagingPreferencesFromDefaults( { borrowernumber => $patron->borrowernumber, categorycode => $patron->categorycode } ); - return ( 1, $borrower{'cardnumber'}, $borrower{'userid'} ); + return ( 1, $patron->cardnumber, $patron->userid ); } sub _get_uri { diff --git a/C4/Members.pm b/C4/Members.pm index 20e4f47168..7fc67fba17 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -78,7 +78,6 @@ BEGIN { #Insert data push @EXPORT, qw( - &AddMember_Auto &AddMember_Opac ); @@ -739,18 +738,6 @@ sub IssueSlip { ); } -=head2 AddMember_Auto - -=cut - -sub AddMember_Auto { - my ( %borrower ) = @_; - - my $patron = Koha::Patron->new(\%borrower)->store; - - return %{ $patron->unblessed }; -} - =head2 AddMember_Opac =cut @@ -767,9 +754,9 @@ sub AddMember_Opac { $borrower{'password'} = $password; } - %borrower = AddMember_Auto(%borrower); + my $patron = Koha::Patron->new(\%borrower)->store; - return ( $borrower{'borrowernumber'}, $password ); + return ( $patron->borrowernumber, $password ); } =head2 DeleteExpiredOpacRegistrations diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index 9d3bed837e..9cfcb4df61 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -207,13 +207,24 @@ if ( $action eq 'create' ) { $template->param( OpacPasswordChange => C4::Context->preference('OpacPasswordChange') ); - my ( $borrowernumber, $password ) = AddMember_Opac(%borrower); - C4::Members::Attributes::SetBorrowerAttributes( $borrowernumber, $attributes ); - C4::Form::MessagingPreferences::handle_form_action($cgi, { borrowernumber => $borrowernumber }, $template, 1, C4::Context->preference('PatronSelfRegistrationDefaultCategory') ) if $borrowernumber && C4::Context->preference('EnhancedMessagingPreferences'); + my $patron = Koha::Patron->new( \%borrower )->store; + if ( $patron ) { + C4::Members::Attributes::SetBorrowerAttributes( $patron->borrowernumber, $attributes ); + if ( C4::Context->preference('EnhancedMessagingPreferences') ) { + C4::Form::MessagingPreferences::handle_form_action( + $cgi, + { borrowernumber => $patron->borrowernumber }, + $template, + 1, + C4::Context->preference('PatronSelfRegistrationDefaultCategory') + ); + } - $template->param( password_cleartext => $password ); - my $patron = Koha::Patrons->find( $borrowernumber ); - $template->param( borrower => $patron->unblessed ); + $template->param( password_cleartext => $password ); + $template->param( borrower => $patron->unblessed ); + } else { + # FIXME Handle possible errors here + } $template->param( PatronSelfRegistrationAdditionalInstructions => C4::Context->preference( diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t index 650eed6510..da5d70bbec 100755 --- a/t/db_dependent/Members.t +++ b/t/db_dependent/Members.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 51; +use Test::More tests => 50; use Test::MockModule; use Test::Exception; @@ -386,17 +386,6 @@ $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; my $hashed_up = Koha::AuthUtils::hash_password("Nexus-6", $borrower->{password}); is( $borrower->{password} eq $hashed_up, 1, 'Check password hash equals hash of submitted password' ); -subtest 'Trivial test for AddMember_Auto' => sub { - plan tests => 3; - 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' ); - like( $borr{cardnumber}, qr/^\d+$/, 'Borrower hash contains cardnumber' ); - my $patron = Koha::Patrons->find( $borr{borrowernumber} ); - isnt( $patron, undef, 'Patron found' ); -}; - $schema->storage->txn_rollback; subtest 'Koha::Patron->store (invalid categorycode) tests' => sub { -- 2.39.5