Bug 14310 [QA Followup] - Adapt existing code to use new methods

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 7bb5d69..8794ea8 100644 (file)
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -1034,13 +1034,11 @@ Unsuspends all suspended reserves with a suspend_until date from before today.
 =cut
 
 sub AutoUnsuspendReserves {
+    my $today = dt_from_string();
 
-    my $dbh = C4::Context->dbh;
-
-    my $query = "UPDATE reserves SET suspend = 0, suspend_until = NULL WHERE DATE( suspend_until ) < DATE( CURDATE() )";
-    my $sth = $dbh->prepare( $query );
-    $sth->execute();
+    my @holds = Koha::Holds->search( { suspend_until => { '<' => $today->ymd() } } );
 
+    map { $_->suspend(0)->suspend_until(undef)->store() } @holds;
 }
 
 =head2 CancelReserve
@@ -1161,19 +1159,26 @@ sub ModReserve {
         CancelReserve({ reserve_id => $reserve_id });
     }
     elsif ($rank =~ /^\d+/ and $rank > 0) {
-        my $query = "
-            UPDATE reserves SET priority = ? ,branchcode = ?, itemnumber = ?, found = NULL, waitingdate = NULL
-            WHERE reserve_id = ?
-        ";
-        my $sth = $dbh->prepare($query);
-        $sth->execute( $rank, $branchcode, $itemnumber, $reserve_id );
+        my $hold = Koha::Holds->find($reserve_id);
+
+        $hold->set(
+            {
+                priority    => $rank,
+                branchcode  => $branchcode,
+                itemnumber  => $itemnumber,
+                found       => undef,
+                waitingdate => undef
+            }
+        )->store();
 
         if ( defined( $suspend_until ) ) {
             if ( $suspend_until ) {
-                $suspend_until = eval { output_pref( { dt => dt_from_string( $suspend_until ), dateonly => 1, dateformat => 'iso' }); };
-                $dbh->do("UPDATE reserves SET suspend = 1, suspend_until = ? WHERE reserve_id = ?", undef, ( $suspend_until, $reserve_id ) );
+                $suspend_until = eval { dt_from_string( $suspend_until ) };
+                $hold->suspend_hold( $suspend_until );
             } else {
-                $dbh->do("UPDATE reserves SET suspend_until = NULL WHERE reserve_id = ?", undef, ( $reserve_id ) );
+                # FIXME: Why are we doing this? Should this be resuming the hold,
+                # or maybe suspending it indefinitely?
+                $hold->set( { suspend_until => undef } )->store();
             }
         }
 
@@ -1614,29 +1619,15 @@ be cleared when it is unsuspended.
 sub ToggleSuspend {
     my ( $reserve_id, $suspend_until ) = @_;
 
-    $suspend_until = output_pref(
-        {
-            dt         => dt_from_string($suspend_until),
-            dateformat => 'iso',
-            dateonly   => 1
-        }
-    ) if ($suspend_until);
-
-    my $do_until = ( $suspend_until ) ? '?' : 'NULL';
-
-    my $dbh = C4::Context->dbh;
+    $suspend_until = dt_from_string($suspend_until) if ($suspend_until);
 
-    my $sth = $dbh->prepare(
-        "UPDATE reserves SET suspend = NOT suspend,
-        suspend_until = CASE WHEN suspend = 0 THEN NULL ELSE $do_until END
-        WHERE reserve_id = ?
-    ");
+    my $hold = Koha::Holds->find( $reserve_id );
 
-    my @params;
-    push( @params, $suspend_until ) if ( $suspend_until );
-    push( @params, $reserve_id );
-
-    $sth->execute( @params );
+    if ( $hold->is_suspended ) {
+        $hold->resume()
+    } else {
+        $hold->suspend_hold( $suspend_until );
+    }
 }
 
 =head2 SuspendAll
@@ -1661,38 +1652,26 @@ sub SuspendAll {
     my $borrowernumber = $params{'borrowernumber'} || undef;
     my $biblionumber   = $params{'biblionumber'}   || undef;
     my $suspend_until  = $params{'suspend_until'}  || undef;
-    my $suspend        = defined( $params{'suspend'} ) ? $params{'suspend'} :  1;
+    my $suspend = defined( $params{'suspend'} ) ? $params{'suspend'} : 1;
 
-    $suspend_until = eval { output_pref( { dt => dt_from_string( $suspend_until), dateonly => 1, dateformat => 'iso' } ); }
-                     if ( defined( $suspend_until ) );
+    $suspend_until = eval { dt_from_string($suspend_until) }
+      if ( defined($suspend_until) );
 
     return unless ( $borrowernumber || $biblionumber );
 
-    my ( $query, $sth, $dbh, @query_params );
+    my $params;
+    $params->{found}          = undef;
+    $params->{borrowernumber} = $borrowernumber if $borrowernumber;
+    $params->{biblionumber}   = $biblionumber if $biblionumber;
 
-    $query = "UPDATE reserves SET suspend = ? ";
-    push( @query_params, $suspend );
-    if ( !$suspend ) {
-        $query .= ", suspend_until = NULL ";
-    } elsif ( $suspend_until ) {
-        $query .= ", suspend_until = ? ";
-        push( @query_params, $suspend_until );
-    }
-    $query .= " WHERE ";
-    if ( $borrowernumber ) {
-        $query .= " borrowernumber = ? ";
-        push( @query_params, $borrowernumber );
+    my @holds = Koha::Holds->search($params);
+
+    if ($suspend) {
+        map { $_->suspend_hold($suspend_until) } @holds;
     }
-    $query .= " AND " if ( $borrowernumber && $biblionumber );
-    if ( $biblionumber ) {
-        $query .= " biblionumber = ? ";
-        push( @query_params, $biblionumber );
+    else {
+        map { $_->resume() } @holds;
     }
-    $query .= " AND found IS NULL ";
-
-    $dbh = C4::Context->dbh;
-    $sth = $dbh->prepare( $query );
-    $sth->execute( @query_params );
 }
 
 

diff --git a/Koha/Hold.pm b/Koha/Hold.pm
index 707622d..0ecded9 100644 (file)
--- a/Koha/Hold.pm
+++ b/Koha/Hold.pm
@@ -50,13 +50,13 @@ my $hold = $hold->suspend_hold( $suspend_until_dt );
 sub suspend_hold {
     my ( $self, $dt ) = @_;
 
+    $dt = $dt ? $dt->clone()->truncate( to => 'day' ) : undef;
+
     if ( $self->is_waiting ) {    # We can't suspend waiting holds
         carp "Unable to suspend waiting hold!";
         return $self;
     }
 
-    $dt ||= undef;
-
     $self->suspend(1);
     $self->suspend_until( $dt );
 
@@ -235,6 +235,18 @@ sub borrower {
     return $self->{_borrower};
 }
 
+=head3 is_suspended
+
+my $bool = $hold->is_suspended();
+
+=cut
+
+sub is_suspended {
+    my ( $self ) = @_;
+
+    return $self->suspend();
+}
+
 =head3 type
 
 =cut

diff --git a/t/db_dependent/Hold.t b/t/db_dependent/Hold.t
index 48f3831..6307e5d 100755 (executable)
--- a/t/db_dependent/Hold.t
+++ b/t/db_dependent/Hold.t
@@ -75,8 +75,9 @@ $hold->resume();
 is( $hold->suspend, 0, "Hold is not suspended" );
 my $dt = dt_from_string();
 $hold->suspend_hold( $dt );
+$dt->truncate( to => 'day' );
 is( $hold->suspend, 1, "Hold is suspended" );
-is( $hold->suspend_until, "$dt", "Hold is suspended with a date" );
+is( $hold->suspend_until, "$dt", "Hold is suspended with a date, truncation takes place automatically" );
 $hold->resume();
 is( $hold->suspend, 0, "Hold is not suspended" );
 is( $hold->suspend_until, undef, "Hold no longer has suspend_until date" );

diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t
index 8767223..29095d0 100755 (executable)
--- a/t/db_dependent/Holds.t
+++ b/t/db_dependent/Holds.t
@@ -8,7 +8,7 @@ use t::lib::TestBuilder;
 use C4::Context;
 use C4::Branch;
 
-use Test::More tests => 56;
+use Test::More tests => 60;
 use MARC::Record;
 use C4::Biblio;
 use C4::Items;
@@ -145,12 +145,31 @@ ok( !$reserve->{'suspend'}, "Test ToggleSuspend(), no date" );
 
 ToggleSuspend( $reserve_id, '2012-01-01' );
 $reserve = GetReserve( $reserve_id );
-ok( $reserve->{'suspend_until'} eq '2012-01-01 00:00:00', "Test ToggleSuspend(), with date" );
+is( $reserve->{'suspend_until'}, '2012-01-01 00:00:00', "Test ToggleSuspend(), with date" );
 
 AutoUnsuspendReserves();
 $reserve = GetReserve( $reserve_id );
 ok( !$reserve->{'suspend'}, "Test AutoUnsuspendReserves()" );
 
+SuspendAll(
+    borrowernumber => $borrowernumber,
+    biblionumber   => $biblionumber,
+    suspend => 1,
+    suspend_until => '2012-01-01',
+);
+$reserve = GetReserve( $reserve_id );
+is( $reserve->{'suspend'}, 1, "Test SuspendAll()" );
+is( $reserve->{'suspend_until'}, '2012-01-01 00:00:00', "Test SuspendAll(), with date" );
+
+SuspendAll(
+    borrowernumber => $borrowernumber,
+    biblionumber   => $biblionumber,
+    suspend => 0,
+);
+$reserve = GetReserve( $reserve_id );
+is( $reserve->{'suspend'}, 0, "Test resuming with SuspendAll()" );
+is( $reserve->{'suspend_until'}, undef, "Test resuming with SuspendAll(), should have no suspend until date" );
+
 # Add a new hold for the borrower whose hold we canceled earlier, this time at the bib level
 AddReserve(
     $branch_1,