From efee30f394bd89632e262d6605d4623a8ee2d9f9 Mon Sep 17 00:00:00 2001 From: Yohann Dufour Date: Fri, 8 Aug 2014 15:50:12 +0200 Subject: [PATCH] Bug 12623: SQLHelper replacement - Borrower::Modifications With this patch, the subroutines AddModification and ApproveModifications uses DBIx::Class instead of C4::SQLHelper. Moreover, the tests has been wrapped in a transaction. Test plan: 1) Apply the patch 2) Execute the unit tests by launching : prove t/db_dependent/Koha_borrower_modifications.t 3) The result has to be a success without error or warning : t/db_dependent/Koha_borrower_modifications.t .. ok All tests successful. Files=1, Tests=14, 2 wallclock secs ( 0.03 usr 0.01 sys + 1.60 cusr 0.08 csys = 1.72 CPU) Result: PASS Signed-off-by: Kyle M Hall Signed-off-by: Marcel de Rooy Signed-off-by: Tomas Cohen Arazi --- Koha/Borrower/Modifications.pm | 55 +++++++------------- t/db_dependent/Koha_borrower_modifications.t | 18 ++++--- 2 files changed, 30 insertions(+), 43 deletions(-) diff --git a/Koha/Borrower/Modifications.pm b/Koha/Borrower/Modifications.pm index 1d32d16308..00083fb7af 100644 --- a/Koha/Borrower/Modifications.pm +++ b/Koha/Borrower/Modifications.pm @@ -26,7 +26,6 @@ use Modern::Perl; use C4::Context; use C4::Debug; -use C4::SQLHelper qw(InsertInTable UpdateInTable); sub new { my ( $class, %args ) = @_; @@ -48,42 +47,20 @@ to be part of the passed in hash. sub AddModifications { my ( $self, $data ) = @_; - if ( $self->{'borrowernumber'} ) { - delete $data->{'borrowernumber'}; - - if ( keys %$data ) { - $data->{'borrowernumber'} = $self->{'borrowernumber'}; - my $dbh = C4::Context->dbh; - - my $query = " - SELECT COUNT(*) AS count - FROM borrower_modifications - WHERE borrowernumber = ? - "; - - my $sth = $dbh->prepare($query); - $sth->execute( $self->{'borrowernumber'} ); - my $result = $sth->fetchrow_hashref(); - - if ( $result->{'count'} ) { - $data->{'verification_token'} = q{}; - return UpdateInTable( "borrower_modifications", $data ); - } - else { - return InsertInTable( "borrower_modifications", $data ); - } - } - + delete $data->{borrowernumber}; + if( $self->{borrowernumber} ) { + return if( not keys %$data ); + $data->{borrowernumber} = $self->{borrowernumber}; } - elsif ( $self->{'verification_token'} ) { - delete $data->{'borrowernumber'}; - $data->{'verification_token'} = $self->{'verification_token'}; - - return InsertInTable( "borrower_modifications", $data ); + elsif( $self->{verification_token} ) { + $data->{verification_token} = $self->{verification_token}; } else { return; } + + my $rs = Koha::Database->new()->schema->resultset('BorrowerModification'); + return $rs->update_or_create($data); } =head2 Verify @@ -120,6 +97,7 @@ sub Verify { $count = Koha::Borrower::Modifications->GetPendingModificationsCount(); Returns the number of pending modifications for existing borrowers. + =cut sub GetPendingModificationsCount { @@ -203,10 +181,15 @@ sub ApproveModifications { return unless $borrowernumber; - my $data = $self->GetModifications({ borrowernumber => $borrowernumber }); + my $data = $self->GetModifications( { borrowernumber => $borrowernumber } ); + delete $data->{timestamp}; + delete $data->{verification_token}; - if ( UpdateInTable( "borrowers", $data ) ) { - $self->DelModifications({ borrowernumber => $borrowernumber }); + my $rs = Koha::Database->new()->schema->resultset('Borrower')->search({ + borrowernumber => $data->{borrowernumber}, + }); + if( $rs->update($data) ) { + $self->DelModifications( { borrowernumber => $borrowernumber } ); } } @@ -227,7 +210,7 @@ sub DenyModifications { return unless $borrowernumber; - return $self->DelModifications({ borrowernumber => $borrowernumber }); + return $self->DelModifications( { borrowernumber => $borrowernumber } ); } =head2 DelModifications diff --git a/t/db_dependent/Koha_borrower_modifications.t b/t/db_dependent/Koha_borrower_modifications.t index 9849502a06..ff5d1945c7 100755 --- a/t/db_dependent/Koha_borrower_modifications.t +++ b/t/db_dependent/Koha_borrower_modifications.t @@ -8,7 +8,11 @@ use C4::Members; use Koha::Borrower::Modifications; -C4::Context->dbh->do("TRUNCATE TABLE borrower_modifications"); +my $dbh = C4::Context->dbh; +$dbh->{RaiseError} = 1; +$dbh->{AutoCommit} = 0; + +$dbh->do("DELETE FROM borrower_modifications"); ## Create new pending modification Koha::Borrower::Modifications->new( verification_token => '1234567890' ) @@ -62,12 +66,10 @@ ok( ); ## Check GetPendingModifications -my $pending = Koha::Borrower::Modifications->GetPendingModifications(); -ok( - $pending->[0]->{'firstname'} eq 'Sandy', - 'Test GetPendingModifications() again' -); -ok( $pending->[1]->{'firstname'} eq 'Kyle', 'Test GetPendingModifications()' ); +my $pendings = Koha::Borrower::Modifications->GetPendingModifications(); +my @firstnames_mod = sort ( $pendings->[0]->{firstname}, $pendings->[1]->{firstname} ); +ok( $firstnames_mod[0] eq 'Kyle', 'Test GetPendingModifications()' ); +ok( $firstnames_mod[1] eq 'Sandy', 'Test GetPendingModifications() again' ); ## This should delete the row from the table Koha::Borrower::Modifications->DenyModifications('3'); @@ -123,3 +125,5 @@ ok( $new_borrower->{'surname'} eq $old_borrower->{'surname'}, 'Test ApproveModifications() applys modification to borrower, again' ); + +$dbh->rollback(); -- 2.39.5