From ddbb32d6985aaf6a2e9935fff2d81c4db5db0edd Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Wed, 20 Nov 2019 12:22:58 +0100
Subject: [PATCH] Bug 23064: Use Koha::Subscription in ModSubscription

We must use Koha::Subscription instead of raw SQL.
It will fix issue with default and integer values.

Test plan:
Edit a subscription and set number of issues = "f"
Save
=> Without this patch there is a SQL error in the log:
  Incorrect integer value: 'f' for column 'numberlength'
=> With this patch the other changes are effective.

Note: We also could change the type attribute of the input to "number",
to have a client-side check

Also, the return value of ModSuggestion is never used, so we are safe
with that.

Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
The <strict_sql_modes> flag must be present and set to 1 in the <config> section of koha-conf.xml
---
 C4/Serials.pm | 74 ++++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 45 insertions(+), 29 deletions(-)

diff --git a/C4/Serials.pm b/C4/Serials.pm
index 2b9e1a62a5a..9d34b14bae3 100644
--- a/C4/Serials.pm
+++ b/C4/Serials.pm
@@ -1305,37 +1305,53 @@ sub ModSubscription {
     $itemtype, $previousitemtype, $mana_id
     ) = @_;
 
-    my $dbh   = C4::Context->dbh;
-    my $query = "UPDATE subscription
-        SET librarian=?, branchcode=?, aqbooksellerid=?, cost=?, aqbudgetid=?,
-            startdate=?, periodicity=?, firstacquidate=?, irregularity=?,
-            numberpattern=?, locale=?, numberlength=?, weeklength=?, monthlength=?,
-            lastvalue1=?, innerloop1=?, lastvalue2=?, innerloop2=?,
-            lastvalue3=?, innerloop3=?, status=?, biblionumber=?,
-            callnumber=?, notes=?, letter=?, manualhistory=?,
-            internalnotes=?, serialsadditems=?, staffdisplaycount=?,
-            opacdisplaycount=?, graceperiod=?, location = ?, enddate=?,
-            skip_serialseq=?, itemtype=?, previousitemtype=?, mana_id=?
-        WHERE subscriptionid = ?";
-
-    my $sth = $dbh->prepare($query);
-    $sth->execute(
-        $auser,           $branchcode,     $aqbooksellerid, $cost,
-        $aqbudgetid,      $startdate,      $periodicity,    $firstacquidate,
-        $irregularity,    $numberpattern,  $locale,         $numberlength,
-        $weeklength,      $monthlength,    $lastvalue1,     $innerloop1,
-        $lastvalue2,      $innerloop2,     $lastvalue3,     $innerloop3,
-        $status,          $biblionumber,   $callnumber,     $notes,
-        $letter,          ($manualhistory ? $manualhistory : 0),
-        $internalnotes, $serialsadditems, $staffdisplaycount, $opacdisplaycount,
-        $graceperiod,     $location,       $enddate,        $skip_serialseq,
-        $itemtype,        $previousitemtype, $mana_id,
-        $subscriptionid
-    );
-    my $rows = $sth->rows;
+    my $subscription = Koha::Subscriptions->find($subscriptionid);
+    $subscription->set(
+        {
+            librarian         => $auser,
+            branchcode        => $branchcode,
+            aqbooksellerid    => $aqbooksellerid,
+            cost              => $cost,
+            aqbudgetid        => $aqbudgetid,
+            biblionumber      => $biblionumber,
+            startdate         => $startdate,
+            periodicity       => $periodicity,
+            numberlength      => $numberlength,
+            weeklength        => $weeklength,
+            monthlength       => $monthlength,
+            lastvalue1        => $lastvalue1,
+            innerloop1        => $innerloop1,
+            lastvalue2        => $lastvalue2,
+            innerloop2        => $innerloop2,
+            lastvalue3        => $lastvalue3,
+            innerloop3        => $innerloop3,
+            status            => $status,
+            notes             => $notes,
+            letter            => $letter,
+            firstacquidate    => $firstacquidate,
+            irregularity      => $irregularity,
+            numberpattern     => $numberpattern,
+            locale            => $locale,
+            callnumber        => $callnumber,
+            manualhistory     => $manualhistory,
+            internalnotes     => $internalnotes,
+            serialsadditems   => $serialsadditems,
+            staffdisplaycount => $staffdisplaycount,
+            opacdisplaycount  => $opacdisplaycount,
+            graceperiod       => $graceperiod,
+            location          => $location,
+            enddate           => $enddate,
+            skip_serialseq    => $skip_serialseq,
+            itemtype          => $itemtype,
+            previousitemtype  => $previousitemtype,
+            mana_id           => $mana_id,
+        }
+    )->store;
 
     logaction( "SERIAL", "MODIFY", $subscriptionid, "" ) if C4::Context->preference("SubscriptionLog");
-    return $rows;
+
+    $subscription->discard_changes;
+    return $subscription;
 }
 
 =head2 NewSubscription
-- 
2.11.0