Attachment 89573 Details for Bug 22812 – Bug 22812: Use Koha::Subscription in NewSubscription

[patch] Bug 22812: Use Koha::Subscription in NewSubscription

Bug-22812-Use-KohaSubscription-in-NewSubscription.patch (text/plain), 6.20 KB, created by Michal Denar on 2019-05-10 20:22:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Michal Denar

Created: 2019-05-10 20:22:33 UTC

Size: 6.20 KB

patch

obsolete

>From 4427c8ccf9dc5b6cbdc4503e3aed8ba988359da6 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Fri, 10 May 2019 11:11:14 -0500
>Subject: [PATCH] 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>
>---
> C4/Serials.pm            | 77 ++++++++++++++++++++++++++++--------------------
> t/db_dependent/Serials.t | 15 +++++++++-
> 2 files changed, 59 insertions(+), 33 deletions(-)
>
>diff --git a/C4/Serials.pm b/C4/Serials.pm
>index 9f86ab8126..8069620be8 100644
>--- a/C4/Serials.pm
>+++ b/C4/Serials.pm
>@@ -1373,37 +1373,50 @@ 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, mana_id)
>-        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, $mana_id
>-    );
>-
>-    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,
>+            mana_id           => $mana_id,
>+        }
>+    )->store;
>+    $subscription->discard_changes;
>+    my $subscriptionid = $subscription->subscriptionid;
>+    my ( $query, $sth );
>     unless ($enddate) {
>         $enddate = GetExpirationDate( $subscriptionid, $startdate );
>         $query = qq|
>@@ -1425,7 +1438,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 b096894ff8..d30a32a48f 100755
>--- 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');
>@@ -349,3 +349,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");
>+};
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 22812: 89391 | 89392 | 89393 | 89454 | 89455 | 89456 | 89535 | 89536 | 89537 | 89561 | 89570 | 89571 | 89572 | 89573 | 89590 | 89591 | 89592 | 89593