Bugzilla – Attachment 14089 Details for
Bug 7067
allow patron self registration via the opac
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 7067 QA Followup
Bug-7067-QA-Followup.patch (text/plain), 12.43 KB, created by
Jared Camins-Esakov
on 2012-12-13 17:20:17 UTC
(
hide
)
Description:
Bug 7067 QA Followup
Filename:
MIME Type:
Creator:
Jared Camins-Esakov
Created:
2012-12-13 17:20:17 UTC
Size:
12.43 KB
patch
obsolete
>From 74837e922809d083de7d54f987b18db45e29bc48 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Mon, 10 Dec 2012 15:45:28 -0500 >Subject: [PATCH] Bug 7067 QA Followup > >Adjusts calling conventions to use hashrefs and eliminate redundant >procedural/OO mixed code. >--- > Koha/Borrower/Modifications.pm | 121 ++++++++------------------ > opac/opac-memberentry.pl | 4 +- > opac/opac-registration-verify.pl | 2 +- > t/db_dependent/Koha_borrower_modifications.t | 94 ++++++++++++++++++++ > 4 files changed, 134 insertions(+), 87 deletions(-) > create mode 100755 t/db_dependent/Koha_borrower_modifications.t > >diff --git a/Koha/Borrower/Modifications.pm b/Koha/Borrower/Modifications.pm >index 443febb..1d32d16 100644 >--- a/Koha/Borrower/Modifications.pm >+++ b/Koha/Borrower/Modifications.pm >@@ -28,34 +28,15 @@ use C4::Context; > use C4::Debug; > use C4::SQLHelper qw(InsertInTable UpdateInTable); > >-use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS); >- >-BEGIN { >- >- # set the version for version checking >- $VERSION = 0.01; >- require Exporter; >- @ISA = qw(Exporter); >- @EXPORT = qw( >- >- ); >- >- my $debug = C4::Context->preference("DebugLevel"); >-} >- > sub new { > my ( $class, %args ) = @_; >- my $self = bless( {}, $class ); >- >- $self->{'verification_token'} = $args{'verification_token'}; >- $self->{'borrowernumber'} = $args{'borrowernumber'}; > >- return $self; >+ return bless( \%args, $class ); > } > > =head2 AddModifications > >-Koha::Borrower::Modifications->AddModifications( %data ); >+Koha::Borrower::Modifications->AddModifications( $data ); > > Adds or updates modifications for a borrower. > >@@ -65,13 +46,13 @@ to be part of the passed in hash. > =cut > > sub AddModifications { >- my ( $self, %data ) = @_; >+ my ( $self, $data ) = @_; > > if ( $self->{'borrowernumber'} ) { >- delete $data{'borrowernumber'}; >+ delete $data->{'borrowernumber'}; > >- if ( keys %data ) { >- $data{'borrowernumber'} = $self->{'borrowernumber'}; >+ if ( keys %$data ) { >+ $data->{'borrowernumber'} = $self->{'borrowernumber'}; > my $dbh = C4::Context->dbh; > > my $query = " >@@ -85,20 +66,20 @@ sub AddModifications { > my $result = $sth->fetchrow_hashref(); > > if ( $result->{'count'} ) { >- $data{'verification_token'} = q{}; >- return UpdateInTable( "borrower_modifications", \%data ); >+ $data->{'verification_token'} = q{}; >+ return UpdateInTable( "borrower_modifications", $data ); > } > else { >- return InsertInTable( "borrower_modifications", \%data ); >+ return InsertInTable( "borrower_modifications", $data ); > } > } > > } > elsif ( $self->{'verification_token'} ) { >- delete $data{'borrowernumber'}; >- $data{'verification_token'} = $self->{'verification_token'}; >+ delete $data->{'borrowernumber'}; >+ $data->{'verification_token'} = $self->{'verification_token'}; > >- return InsertInTable( "borrower_modifications", \%data ); >+ return InsertInTable( "borrower_modifications", $data ); > } > else { > return; >@@ -116,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 = " >@@ -219,17 +198,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 }); > } > } > >@@ -245,50 +222,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 }); > } > > =head2 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; >@@ -309,38 +273,27 @@ sub DelModifications { > > =head2 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 039d499..7f8230d 100755 >--- a/opac/opac-memberentry.pl >+++ b/opac/opac-memberentry.pl >@@ -107,7 +107,7 @@ if ( $action eq 'create' ) { > > Koha::Borrower::Modifications->new( > verification_token => $verification_token ) >- ->AddModifications(%borrower); >+ ->AddModifications(\%borrower); > > #Send verification email > my $letter = C4::Letters::GetPreparedLetter( >@@ -188,7 +188,7 @@ elsif ( $action eq 'update' ) { > borrowernumber => $borrowernumber ); > > $m->DelModifications; >- $m->AddModifications(%borrower_changes); >+ $m->AddModifications(\%borrower_changes); > $template->param( > borrower => GetMember( borrowernumber => $borrowernumber ), > ); >diff --git a/opac/opac-registration-verify.pl b/opac/opac-registration-verify.pl >index 682dacb..daf1e12 100755 >--- a/opac/opac-registration-verify.pl >+++ b/opac/opac-registration-verify.pl >@@ -55,7 +55,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 >new file mode 100755 >index 0000000..cc30313 >--- /dev/null >+++ b/t/db_dependent/Koha_borrower_modifications.t >@@ -0,0 +1,94 @@ >+#!/usr/bin/perl >+ >+use Modern::Perl; >+use Test::More tests => 14; >+ >+use C4::Context; >+use C4::Members; >+ >+use Koha::Borrower::Modifications; >+ >+C4::Context->dbh->do("TRUNCATE TABLE borrower_modifications"); >+ >+## Create new pending modification >+Koha::Borrower::Modifications->new( verification_token => '1234567890' )->AddModifications({ surname => 'Hall', firstname => 'Kyle' }); >+ >+## Get the new pending modification >+my $borrower = Koha::Borrower::Modifications->GetModifications({ >+ verification_token => '1234567890' >+}); >+ >+## Verify we get the same data >+ok( $borrower->{'surname'} = 'Hall' ); >+ >+## Check the Verify method >+ok( Koha::Borrower::Modifications->Verify( '1234567890' ) ); >+ >+## Delete the pending modification >+$borrower = Koha::Borrower::Modifications->DelModifications({ >+ verification_token => '1234567890' >+}); >+ >+## Verify it's no longer in the database >+$borrower = Koha::Borrower::Modifications->GetModifications({ >+ verification_token => '1234567890' >+}); >+ok( !defined( $borrower->{'surname'} ) ); >+ >+## Check the Verify method >+ok( !Koha::Borrower::Modifications->Verify( '1234567890' ) ); >+ >+## Create new pending modification, but for an existing borrower >+Koha::Borrower::Modifications->new( borrowernumber => '2' )->AddModifications({ surname => 'Hall', firstname => 'Kyle' }); >+ >+## Test the counter >+ok( Koha::Borrower::Modifications->GetPendingModificationsCount() == 1 ); >+ >+## Create new pending modification for another existing borrower >+Koha::Borrower::Modifications->new( borrowernumber => '3' )->AddModifications({ surname => 'Smith', firstname => 'Sandy' }); >+ >+## Test the counter >+ok( Koha::Borrower::Modifications->GetPendingModificationsCount() == 2 ); >+ >+## Check GetPendingModifications >+my $pending = Koha::Borrower::Modifications->GetPendingModifications(); >+ok( $pending->[0]->{'firstname'} eq 'Kyle' ); >+ok( $pending->[1]->{'firstname'} eq 'Sandy' ); >+ >+## This should delete the row from the table >+Koha::Borrower::Modifications->DenyModifications( '3' ); >+ >+## Test the counter >+ok( Koha::Borrower::Modifications->GetPendingModificationsCount() == 1 ); >+ >+## Save a copy of the borrowers original data >+my $old_borrower = GetMember(borrowernumber => '2' ); >+ >+## Apply the modifications >+Koha::Borrower::Modifications->ApproveModifications( '2' ); >+ >+## Test the counter >+ok( Koha::Borrower::Modifications->GetPendingModificationsCount() == 0 ); >+ >+## Get a copy of the borrowers current data >+my $new_borrower = GetMember(borrowernumber => '2' ); >+ >+## Check to see that the approved modifications were saved >+ok( $new_borrower->{'surname'} eq 'Hall' ); >+ >+## Now let's put it back the way it was >+Koha::Borrower::Modifications->new( borrowernumber => '2' )->AddModifications({ surname => $old_borrower->{'surname'}, firstname => $old_borrower->{'firstname'} }); >+ >+## Test the counter >+ok( Koha::Borrower::Modifications->GetPendingModificationsCount() == 1 ); >+ >+## Apply the modifications >+Koha::Borrower::Modifications->ApproveModifications( '2' ); >+ >+## Test the counter >+ok( Koha::Borrower::Modifications->GetPendingModificationsCount() == 0 ); >+ >+$new_borrower = GetMember(borrowernumber => '2' ); >+ >+## Test to verify the borrower has been updated with the original values >+ok( $new_borrower->{'surname'} eq $old_borrower->{'surname'} ); >-- >1.7.9.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 7067
:
11960
|
11961
|
11962
|
11965
|
11966
|
11967
|
11968
|
11970
|
11973
|
11978
|
12006
|
12007
|
12013
|
12104
|
12109
|
12114
|
12116
|
12129
|
12135
|
12136
|
12138
|
12153
|
12154
|
12171
|
12190
|
12191
|
12196
|
12197
|
12242
|
12243
|
12244
|
12245
|
12277
|
12278
|
13038
|
13039
|
13040
|
13048
|
13053
|
13259
|
13260
|
13744
|
13745
|
13746
|
13747
|
13748
|
13765
|
13766
|
13767
|
13768
|
13769
|
13984
|
14001
|
14085
|
14086
|
14087
|
14088
|
14089
|
14090
|
14092
|
14093
|
14119
|
14190
|
14191
|
14193
|
14194
|
14282
|
14283