Bug 22812: Use Koha::Subscription in NewSubscription

Starting to write tests I realize that we are going to add too much
specific logic which is already handled in Koha::Object->store.
The easiest and safe way is to use it :)

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(cherry picked from commit b41f215bfedbae0898a9291d2f14e2a3dd560049)
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/C4/Serials.pm b/C4/Serials.pm
index b339dca..a8be428 100644 (file)
--- a/C4/Serials.pm
+++ b/C4/Serials.pm
@@ -1371,37 +1371,49 @@ sub NewSubscription {
     ) = @_;
     my $dbh = C4::Context->dbh;
 
-    $_ ||= undef # Set to undef for integer values, not empty string
-      for (
-        $aqbooksellerid, $lastvalue1, $innerloop1, $lastvalue2,
-        $innerloop2,     $lastvalue3, $innerloop3,
-      );
-    #save subscription (insert into database)
-    my $query = qq|
-        INSERT INTO subscription
-            (librarian, branchcode, aqbooksellerid, cost, aqbudgetid,
-            biblionumber, startdate, periodicity, numberlength, weeklength,
-            monthlength, lastvalue1, innerloop1, lastvalue2, innerloop2,
-            lastvalue3, innerloop3, status, notes, letter, firstacquidate,
-            irregularity, numberpattern, locale, callnumber,
-            manualhistory, internalnotes, serialsadditems, staffdisplaycount,
-            opacdisplaycount, graceperiod, location, enddate, skip_serialseq,
-            itemtype, previousitemtype)
-        VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
-        |;
-    my $sth = $dbh->prepare($query);
-    $sth->execute(
-        $auser, $branchcode, $aqbooksellerid, $cost, $aqbudgetid, $biblionumber,
-        $startdate, $periodicity, $numberlength, $weeklength,
-        $monthlength, $lastvalue1, $innerloop1, $lastvalue2, $innerloop2,
-        $lastvalue3, $innerloop3, $status, $notes, $letter,
-        $firstacquidate, $irregularity, $numberpattern, $locale, $callnumber,
-        $manualhistory, $internalnotes, $serialsadditems, $staffdisplaycount,
-        $opacdisplaycount, $graceperiod, $location, $enddate, $skip_serialseq,
-        $itemtype, $previousitemtype
-    );
-
-    my $subscriptionid = $dbh->{'mysql_insertid'};
+    my $subscription = Koha::Subscription->new(
+        {
+            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,
+        }
+    )->store;
+    $subscription->discard_changes;
+    my $subscriptionid = $subscription->subscriptionid;
+    my ( $query, $sth );
     unless ($enddate) {
         $enddate = GetExpirationDate( $subscriptionid, $startdate );
         $query = qq|
@@ -1423,7 +1435,7 @@ sub NewSubscription {
     $sth->execute( $biblionumber, $subscriptionid, $startdate);
 
     # reread subscription to get a hash (for calculation of the 1st issue number)
-    my $subscription = GetSubscription($subscriptionid);
+    $subscription = GetSubscription($subscriptionid); # We should not do that
     my $pattern = C4::Serials::Numberpattern::GetSubscriptionNumberpattern($subscription->{numberpattern});
 
     # calculate issue number

diff --git a/t/db_dependent/Serials.t b/t/db_dependent/Serials.t
index 1023413..8d26e44 100755 (executable)
--- a/t/db_dependent/Serials.t
+++ b/t/db_dependent/Serials.t
@@ -18,7 +18,7 @@ use Koha::DateUtils;
 use Koha::Acquisition::Booksellers;
 use t::lib::Mocks;
 use t::lib::TestBuilder;
-use Test::More tests => 47;
+use Test::More tests => 48;
 
 BEGIN {
     use_ok('C4::Serials');
@@ -348,3 +348,16 @@ subtest "Do not generate an expected if one already exists" => sub {
     @serialsByStatus = C4::Serials::findSerialsByStatus( 1, $subscriptionid );
     is( @serialsByStatus, 1, "ModSerialStatus delete corectly serial expected and not create another if exists" );
 };
+
+subtest "NewSubscription" => sub {
+    plan tests => 1;
+    my $subscriptionid = NewSubscription(
+        "",      "",     "", "", $budget_id, $biblionumber,
+        '2013-01-01', $frequency_id, "", "",  "",
+        "",      "",  "", "", "", "",
+        1,          $notes,"", '2013-01-01', "", $pattern_id,
+        "",       "",  0,    $internalnotes,  0,
+        "", "", 0,          "",         '2013-12-31', 0
+    );
+    ok($subscriptionid, "Sending empty string instead of undef to reflect use of the interface");
+};