Attachment 49411 Details for Bug 15585 – Bug 15585 - Move C4::Passwordrecovery to the new namespace Koha::Patron::Password::Reset

[patch] Bug 15585 - Move C4::Passwordrecovery to the new namespace Koha::Patron::Password::Reset

Bug-15585---Move-C4Passwordrecovery-to-the-new-nam.patch (text/plain), 10.12 KB, created by Jonathan Druart on 2016-03-22 11:05:18 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2016-03-22 11:05:18 UTC

Size: 10.12 KB

patch

obsolete

>From 8b0a23094d53b1b0140e68b17609295df41ff891 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
>
>Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
>
>Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>---
> .../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 a67c4d4..6cb1d94 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 aa98040..2f7fbfe 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::Patrons;
>@@ -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 a428033..9a3b8b2 100755
>--- a/t/db_dependent/Passwordrecovery.t
>+++ b/t/db_dependent/Passwordrecovery.t
>@@ -24,7 +24,7 @@ use Koha::Patrons;
> 
> 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::Patrons->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;
>-- 
>2.7.0

Actions: View | Diff | Splinter Review

Attachments on bug 15585: 47359 | 48762 | 49411 | 49412