From dc6b6e030c0e219f84350cea706910a7433ca0a4 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
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 <josef.moravec@gmail.com>

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
---
 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