From da4e437453acb55df7e0d67044b71ee4cf0eae7d Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Tue, 11 Dec 2012 10:32:23 -0500 Subject: [PATCH] Bug 7067 [7] - QA Followup 2 --- Koha/Borrower/Modifications.pm | 82 ++++++++------------------ opac/opac-memberentry.pl | 7 +-- opac/opac-registration-verify.pl | 2 +- t/db_dependent/Koha_borrower_modifications.t | 12 ++-- 4 files changed, 34 insertions(+), 69 deletions(-) diff --git a/Koha/Borrower/Modifications.pm b/Koha/Borrower/Modifications.pm index 2233b31..13e007c 100644 --- a/Koha/Borrower/Modifications.pm +++ b/Koha/Borrower/Modifications.pm @@ -30,10 +30,6 @@ use C4::SQLHelper qw(InsertInTable UpdateInTable); sub new { my ( $class, %args ) = @_; - my $self = bless( {}, $class ); - - $self->{'verification_token'} = $args{'verification_token'}; - $self->{'borrowernumber'} = $args{'borrowernumber'}; return bless( \%args, $class ); } @@ -101,12 +97,10 @@ Returns true if the passed in token is valid. sub Verify { my ( $self, $verification_token ) = @_; - if ( ref($self) ) { - $verification_token = - ($verification_token) - ? $verification_token - : $self->{'verification_token'}; - } + $verification_token = + ($verification_token) + ? $verification_token + : $self->{'verification_token'}; my $dbh = C4::Context->dbh; my $query = " @@ -190,17 +184,15 @@ them from the modifications table. sub ApproveModifications { my ( $self, $borrowernumber ) = @_; - if ( ref($self) ) { - $borrowernumber = - ($borrowernumber) ? $borrowernumber : $self->{'borrowernumber'}; - } + $borrowernumber = + ($borrowernumber) ? $borrowernumber : $self->{'borrowernumber'}; return unless $borrowernumber; - my $data = $self->GetModifications( borrowernumber => $borrowernumber ); + my $data = $self->GetModifications({ borrowernumber => $borrowernumber }); if ( UpdateInTable( "borrowers", $data ) ) { - $self->DelModifications( borrowernumber => $borrowernumber ); + $self->DelModifications({ borrowernumber => $borrowernumber }); } } @@ -216,50 +208,37 @@ without commiting the changes to the borrower record. sub DenyModifications { my ( $self, $borrowernumber ) = @_; - if ( ref($self) ) { - $borrowernumber = - ($borrowernumber) ? $borrowernumber : $self->{'borrowernumber'}; - } + $borrowernumber = + ($borrowernumber) ? $borrowernumber : $self->{'borrowernumber'}; return unless $borrowernumber; - return $self->DelModifications( borrowernumber => $borrowernumber ); + return $self->DelModifications({ borrowernumber => $borrowernumber }); } =head DelModifications -Koha::Borrower::Modifications->DelModifications( +Koha::Borrower::Modifications->DelModifications({ [ borrowernumber => $borrowernumber ], [ verification_token => $verification_token ] -); +}); Deletes the modifications for the given borrowernumber or verification token. =cut sub DelModifications { - my ( $self, %params ) = @_; + my ( $self, $params ) = @_; my ( $field, $value ); - if ( $params{'borrowernumber'} ) { + if ( $params->{'borrowernumber'} ) { $field = 'borrowernumber'; - $value = $params{'borrowernumber'}; + $value = $params->{'borrowernumber'}; } - elsif ( $params{'verification_token'} ) { + elsif ( $params->{'verification_token'} ) { $field = 'verification_token'; - $value = $params{'verification_token'}; - } - - if ( ref($self) && !$value ) { - if ( $self->{'borrowernumber'} ) { - $field = 'borrowernumber'; - $value = $self->{'borrowernumber'}; - } - elsif ( $self->{'verification_token'} ) { - $field = 'verification_token'; - $value = $self->{'verification_token'}; - } + $value = $params->{'verification_token'}; } return unless $value; @@ -280,38 +259,27 @@ sub DelModifications { =head GetModifications -$hashref = Koha::Borrower::Modifications->GetModifications( +$hashref = Koha::Borrower::Modifications->GetModifications({ [ borrowernumber => $borrowernumber ], [ verification_token => $verification_token ] -); +}); Gets the modifications for the given borrowernumber or verification token. =cut sub GetModifications { - my ( $self, %params ) = @_; + my ( $self, $params ) = @_; my ( $field, $value ); - if ( $params{'borrowernumber'} ) { + if ( defined( $params->{'borrowernumber'} ) ) { $field = 'borrowernumber'; - $value = $params{'borrowernumber'}; + $value = $params->{'borrowernumber'}; } - elsif ( $params{'verification_token'} ) { + elsif ( defined( $params->{'verification_token'} ) ) { $field = 'verification_token'; - $value = $params{'verification_token'}; - } - - if ( ref($self) && !$value ) { - if ( $self->{'borrowernumber'} ) { - $field = 'borrowernumber'; - $value = $self->{'borrowernumber'}; - } - elsif ( $self->{'verification_token'} ) { - $field = 'verification_token'; - $value = $self->{'verification_token'}; - } + $value = $params->{'verification_token'}; } return unless $value; diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index 7b368f0..b0b4224 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -157,11 +157,8 @@ elsif ( $action eq 'update' ) { %borrower = DelUnchangedFields( $borrowernumber, %borrower ); - my $m = - Koha::Borrower::Modifications->new( borrowernumber => $borrowernumber ); - - $m->DelModifications; - $m->AddModifications(\%borrower); + Koha::Borrower::Modifications->DelModifications({ borrowernumber => $borrowernumber }); + Koha::Borrower::Modifications->AddModifications(\%borrower); } elsif ($borrowernumber) { #Display logged in borrower's data $action = 'edit'; diff --git a/opac/opac-registration-verify.pl b/opac/opac-registration-verify.pl index bf0eae4..abd2355 100755 --- a/opac/opac-registration-verify.pl +++ b/opac/opac-registration-verify.pl @@ -54,7 +54,7 @@ if ( $m->Verify() ) { ( $borrowernumber, $password ) = AddMember_Opac(%$borrower); if ($borrowernumber) { - $m->DelModifications(); + Koha::Borrower::Modifications->DelModifications({ verification_token => $token }); $template->param( password_cleartext => $password ); $template->param( diff --git a/t/db_dependent/Koha_borrower_modifications.t b/t/db_dependent/Koha_borrower_modifications.t index 7fdedd8..cc30313 100755 --- a/t/db_dependent/Koha_borrower_modifications.t +++ b/t/db_dependent/Koha_borrower_modifications.t @@ -14,9 +14,9 @@ C4::Context->dbh->do("TRUNCATE TABLE borrower_modifications"); Koha::Borrower::Modifications->new( verification_token => '1234567890' )->AddModifications({ surname => 'Hall', firstname => 'Kyle' }); ## Get the new pending modification -my $borrower = Koha::Borrower::Modifications->GetModifications( +my $borrower = Koha::Borrower::Modifications->GetModifications({ verification_token => '1234567890' -); +}); ## Verify we get the same data ok( $borrower->{'surname'} = 'Hall' ); @@ -25,14 +25,14 @@ ok( $borrower->{'surname'} = 'Hall' ); ok( Koha::Borrower::Modifications->Verify( '1234567890' ) ); ## Delete the pending modification -$borrower = Koha::Borrower::Modifications->DelModifications( +$borrower = Koha::Borrower::Modifications->DelModifications({ verification_token => '1234567890' -); +}); ## Verify it's no longer in the database -$borrower = Koha::Borrower::Modifications->GetModifications( +$borrower = Koha::Borrower::Modifications->GetModifications({ verification_token => '1234567890' -); +}); ok( !defined( $borrower->{'surname'} ) ); ## Check the Verify method -- 1.7.2.5