From b742d00b330d2b7488adc9e33e996a27b73eb2c9 Mon Sep 17 00:00:00 2001
From: charles <charles.farmer@inlibro.com>
Date: Wed, 27 Jan 2016 15:17:32 -0500
Subject: [PATCH] Bug 15585 - Move C4::Passwordrecovery to the new namespace
 Koha::Patron::Password::Reset

As promised, here is the long-awaited sequel to #8753.

What has changed :

    - The Koha::Patron::Password::Reset is now used in place of C4::Passwordrecovery
    - That ugly shift-grep contraption is no more (goodbye old friend)
    - The generated unique key won't end in a dot anymore
---
 .../Patron/Password/Recovery.pm                    | 15 ++++---
 opac/opac-password-recovery.pl                     | 15 ++++---
 t/db_dependent/Passwordrecovery.t                  | 50 +++++++++++-----------
 3 files changed, 43 insertions(+), 37 deletions(-)
 rename C4/Passwordrecovery.pm => Koha/Patron/Password/Recovery.pm (91%)

diff --git a/C4/Passwordrecovery.pm b/Koha/Patron/Password/Recovery.pm
similarity index 91%
rename from C4/Passwordrecovery.pm
rename to Koha/Patron/Password/Recovery.pm
index ea01e15..c8214d1 100644
--- a/C4/Passwordrecovery.pm
+++ b/Koha/Patron/Password/Recovery.pm
@@ -1,4 +1,4 @@
-package C4::Passwordrecovery;
+package Koha::Patron::Password::Recovery;
 
 # Copyright 2014 Solutions InLibro inc.
 #
@@ -38,11 +38,11 @@ BEGIN {
 
 =head1 NAME
 
-C4::Passwordrecovery - Koha password recovery module
+Koha::Patron::Password::Recovery - Koha password recovery module
 
 =head1 SYNOPSIS
 
-use C4::Passwordrecovery;
+use Koha::Patron::Password::Recovery;
 
 =head1 FUNCTIONS
 
@@ -110,8 +110,10 @@ sub SendPasswordRecoveryEmail {
     my $schema = Koha::Database->new->schema;
 
     # generate UUID
-    my @chars = ( "A" .. "Z", "a" .. "z", "0" .. "9" );
-    my $uuid_str = '$2a$08$'.en_base64(Koha::AuthUtils::generate_salt('weak', 16));
+    my $uuid_str;
+    do {
+        $uuid_str = '$2a$08$'.en_base64(Koha::AuthUtils::generate_salt('weak', 16));
+    } while ( substr ( $uuid_str, -1, 1 ) eq '.' );
 
     # insert into database
     my $expirydate =
@@ -134,7 +136,8 @@ sub SendPasswordRecoveryEmail {
     }
 
     # create link
-    my $uuidLink = C4::Context->preference('OPACBaseURL')
+    my $opacbase = C4::Context->preference('OPACBaseURL') || '';
+    my $uuidLink = $opacbase
       . "/cgi-bin/koha/opac-password-recovery.pl?uniqueKey=$uuid_str";
 
     # prepare the email
diff --git a/opac/opac-password-recovery.pl b/opac/opac-password-recovery.pl
index 7e3eae2..306cfdf 100755
--- a/opac/opac-password-recovery.pl
+++ b/opac/opac-password-recovery.pl
@@ -8,7 +8,7 @@ use C4::Koha;
 use C4::Members qw(changepassword);
 use C4::Output;
 use C4::Context;
-use C4::Passwordrecovery
+use Koha::Patron::Password::Recovery
   qw(SendPasswordRecoveryEmail ValidateBorrowernumber GetValidLinkInfo CompletePasswordRecovery);
 use Koha::AuthUtils qw(hash_password);
 use Koha::Borrowers;
@@ -71,6 +71,12 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) {
         $username ||= $borrower->userid;
         my @emails = ( $borrower->email, $borrower->emailpro, $borrower->B_email );
 
+        my $firstNonEmptyEmail = '';
+        foreach my $address ( @emails ) {
+            $firstNonEmptyEmail = $address if length $address;
+            last if $firstNonEmptyEmail;
+        }
+
         # Is the given email one of the borrower's ?
         if ( $email && !( grep { $_ eq $email } @emails ) ) {
             $hasError    = 1;
@@ -78,12 +84,9 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) {
         }
 
 # If we dont have an email yet. Get one of the borrower's email or raise an error.
-# FIXME: That ugly shift-grep contraption.
-# $email = shift [ grep { length() } @emails ]
-# It's supposed to get a non-empty string from the @emails array. There's surely a simpler way
-        elsif ( !$email && !( $email = shift [ grep { length() } @emails ] ) ) {
+        elsif ( !$email && !( $email = $firstNonEmptyEmail ) ) {
             $hasError           = 1;
-            $errNoBorrowerFound = 1;
+            $errNoBorrowerEmail = 1;
         }
 
 # Check if a password reset already issued for this borrower AND we are not asking for a new email
diff --git a/t/db_dependent/Passwordrecovery.t b/t/db_dependent/Passwordrecovery.t
index e0ec8ec..76f3a50 100755
--- a/t/db_dependent/Passwordrecovery.t
+++ b/t/db_dependent/Passwordrecovery.t
@@ -24,7 +24,7 @@ use Koha::Borrowers;
 
 use Test::More tests => 16;
 
-use_ok('C4::Passwordrecovery');
+use_ok('Koha::Patron::Password::Recovery');
 
 my $schema = Koha::Database->new()->schema();
 $schema->storage->txn_begin();
@@ -90,34 +90,34 @@ $schema->resultset('BorrowerPasswordRecovery')->create(
     }
 );
 
-can_ok( "C4::Passwordrecovery", qw(ValidateBorrowernumber GetValidLinkInfo SendPasswordRecoveryEmail CompletePasswordRecovery) );
+can_ok( "Koha::Patron::Password::Recovery", qw(ValidateBorrowernumber GetValidLinkInfo SendPasswordRecoveryEmail CompletePasswordRecovery) );
 
-################################################
-# C4::Passwordrecovery::ValidateBorrowernumber #
-################################################
+############################################################
+# Koha::Patron::Password::Recovery::ValidateBorrowernumber #
+############################################################
 
-ok(   C4::Passwordrecovery::ValidateBorrowernumber($borrowernumber1), "[ValidateBorrowernumber] Borrower has a password recovery entry" );
-ok( ! C4::Passwordrecovery::ValidateBorrowernumber($borrowernumber2), "[ValidateBorrowernumber] Borrower's number is not found; password recovery entry is expired" );
-ok( ! C4::Passwordrecovery::ValidateBorrowernumber(9999), "[ValidateBorrowernumber] Borrower has no password recovery entry" );
+ok(   Koha::Patron::Password::Recovery::ValidateBorrowernumber($borrowernumber1), "[ValidateBorrowernumber] Borrower has a password recovery entry" );
+ok( ! Koha::Patron::Password::Recovery::ValidateBorrowernumber($borrowernumber2), "[ValidateBorrowernumber] Borrower's number is not found; password recovery entry is expired" );
+ok( ! Koha::Patron::Password::Recovery::ValidateBorrowernumber(9999), "[ValidateBorrowernumber] Borrower has no password recovery entry" );
 
-##########################################
-# C4::Passwordrecovery::GetValidLinkInfo #
-##########################################
+######################################################
+# Koha::Patron::Password::Recovery::GetValidLinkInfo #
+######################################################
 
-my ($bnum1, $uname1) = C4::Passwordrecovery::GetValidLinkInfo($uuid1);
-my ($bnum2, $uname2) = C4::Passwordrecovery::GetValidLinkInfo($uuid2);
-my ($bnum3, $uname3) = C4::Passwordrecovery::GetValidLinkInfo("THISISANINVALIDUUID");
+my ($bnum1, $uname1) = Koha::Patron::Password::Recovery::GetValidLinkInfo($uuid1);
+my ($bnum2, $uname2) = Koha::Patron::Password::Recovery::GetValidLinkInfo($uuid2);
+my ($bnum3, $uname3) = Koha::Patron::Password::Recovery::GetValidLinkInfo("THISISANINVALIDUUID");
 
 is( $bnum1, $borrowernumber1, "[GetValidLinkInfo] Borrower has a valid link" );
 is( $uname1, $userid1, "[GetValidLinkInfo] Borrower's username is fetched when a valid link is found" );
 ok( ! defined($bnum2), "[GetValidLinkInfo] Borrower's link is no longer valid; entry is expired" );
 ok( ! defined($bnum3), "[GetValidLinkInfo] Invalid UUID returns no borrowernumber" );
 
-##################################################
-# C4::Passwordrecovery::CompletePasswordRecovery #
-##################################################
+##############################################################
+# Koha::Patron::Password::Recovery::CompletePasswordRecovery #
+##############################################################
 
-ok( C4::Passwordrecovery::CompletePasswordRecovery($uuid1) == 2, "[CompletePasswordRecovery] Completing a password recovery deletes the entry and expired entries" );
+ok( Koha::Patron::Password::Recovery::CompletePasswordRecovery($uuid1) == 2, "[CompletePasswordRecovery] Completing a password recovery deletes the entry and expired entries" );
 
 $schema->resultset('BorrowerPasswordRecovery')->create(
     {
@@ -127,22 +127,22 @@ $schema->resultset('BorrowerPasswordRecovery')->create(
     }
 );
 
-ok( C4::Passwordrecovery::CompletePasswordRecovery($uuid2) == 1, "[CompletePasswordRecovery] An expired or invalid UUID purges expired entries" );
-ok( C4::Passwordrecovery::CompletePasswordRecovery($uuid2) == 0, "[CompletePasswordRecovery] Returns 0 on a clean table" );
+ok( Koha::Patron::Password::Recovery::CompletePasswordRecovery($uuid2) == 1, "[CompletePasswordRecovery] An expired or invalid UUID purges expired entries" );
+ok( Koha::Patron::Password::Recovery::CompletePasswordRecovery($uuid2) == 0, "[CompletePasswordRecovery] Returns 0 on a clean table" );
 
-###################################################
-# C4::Passwordrecovery::SendPasswordRecoveryEmail #
-###################################################
+###############################################################
+# Koha::Patron::Password::Recovery::SendPasswordRecoveryEmail #
+###############################################################
 
 my $borrower = shift [ Koha::Borrowers->search( { userid => $userid1 } ) ];
-ok( C4::Passwordrecovery::SendPasswordRecoveryEmail($borrower, $email1, 0) == 1, "[SendPasswordRecoveryEmail] Returns 1 on success" );
+ok( Koha::Patron::Password::Recovery::SendPasswordRecoveryEmail($borrower, $email1, 0) == 1, "[SendPasswordRecoveryEmail] Returns 1 on success" );
 my $letters = C4::Letters::GetQueuedMessages( { borrowernumber => $borrowernumber1, limit => 99 } );
 ok( scalar @$letters == 1, "[SendPasswordRecoveryEmail] There is a letter in the queue for our borrower");
 
 my $bpr = $schema->resultset('BorrowerPasswordRecovery')->search( { borrowernumber => $borrowernumber1 } );
 my $tempuuid1 = $bpr->next->uuid;
 
-C4::Passwordrecovery::SendPasswordRecoveryEmail($borrower, $email1, 1);
+Koha::Patron::Password::Recovery::SendPasswordRecoveryEmail($borrower, $email1, 1);
 
 $bpr = $schema->resultset('BorrowerPasswordRecovery')->search( { borrowernumber => $borrowernumber1 } );
 my $tempuuid2 = $bpr->next->uuid;
-- 
1.9.1