From efee30f394bd89632e262d6605d4623a8ee2d9f9 Mon Sep 17 00:00:00 2001
From: Yohann Dufour <dufour.yohann@gmail.com>
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 <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
---
 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