Bug 21395: Make perlcritic happy
[srvgit] / C4 / Serials.pm
index e929e21..35983d6 100644 (file)
@@ -30,12 +30,13 @@ use C4::Log;    # logaction
 use C4::Debug;
 use C4::Serials::Frequency;
 use C4::Serials::Numberpattern;
-use Koha::AdditionalField;
+use Koha::AdditionalFieldValues;
 use Koha::DateUtils;
 use Koha::Serial;
 use Koha::Subscriptions;
 use Koha::Subscription::Histories;
 use Koha::SharedContent;
+use Scalar::Util qw( looks_like_number );
 
 use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
 
@@ -74,7 +75,7 @@ BEGIN {
 
       &GetNextSeq &GetSeq &NewIssue           &GetSerials
       &GetLatestSerials   &ModSerialStatus    &GetNextDate       &GetSerials2
-      &ReNewSubscription  &GetLateOrMissingIssues
+      &GetSubscriptionLength &ReNewSubscription  &GetLateOrMissingIssues
       &GetSerialInformation                   &AddItem2Serial
       &PrepareSerialsData &GetNextExpected    &ModNextExpected
       &GetPreviousSerialid
@@ -274,13 +275,6 @@ sub GetSubscription {
 
     $subscription->{cannotedit} = not can_edit_subscription( $subscription );
 
-    # Add additional fields to the subscription into a new key "additional_fields"
-    my $additional_field_values = Koha::AdditionalField->fetch_all_values({
-            tablename => 'subscription',
-            record_id => $subscriptionid,
-    });
-    $subscription->{additional_fields} = $additional_field_values->{$subscriptionid};
-
     if ( my $mana_id = $subscription->{mana_id} ) {
         my $mana_subscription = Koha::SharedContent::get_entity_by_id(
             'subscription', $mana_id, {usecomments => 1});
@@ -311,7 +305,7 @@ sub GetFullSubscription {
             serial.publisheddatetext,
             serial.status, 
             serial.notes as notes,
-            year(IF(serial.publisheddate="00-00-0000",serial.planneddate,serial.publisheddate)) as year,
+            year(IF(serial.publisheddate IS NULL,serial.planneddate,serial.publisheddate)) as year,
             aqbooksellers.name as aqbooksellername,
             biblio.title as bibliotitle,
             subscription.branchcode AS branchcode,
@@ -323,17 +317,20 @@ sub GetFullSubscription {
   LEFT JOIN biblio on biblio.biblionumber=subscription.biblionumber 
   WHERE     serial.subscriptionid = ? 
   ORDER BY year DESC,
-          IF(serial.publisheddate="00-00-0000",serial.planneddate,serial.publisheddate) DESC,
+          IF(serial.publisheddate IS NULL,serial.planneddate,serial.publisheddate) DESC,
           serial.subscriptionid
           |;
     $debug and warn "GetFullSubscription query: $query";
     my $sth = $dbh->prepare($query);
     $sth->execute($subscriptionid);
     my $subscriptions = $sth->fetchall_arrayref( {} );
-    my $cannotedit = not can_edit_subscription( $subscriptions->[0] ) if scalar @$subscriptions;
-    for my $subscription ( @$subscriptions ) {
-        $subscription->{cannotedit} = $cannotedit;
+    if (scalar @$subscriptions) {
+        my $cannotedit = not can_edit_subscription( $subscriptions->[0] );
+        for my $subscription ( @$subscriptions ) {
+            $subscription->{cannotedit} = $cannotedit;
+        }
     }
+
     return $subscriptions;
 }
 
@@ -353,9 +350,6 @@ sub PrepareSerialsData {
     my $year;
     my @res;
     my $startdate;
-    my $aqbooksellername;
-    my $bibliotitle;
-    my @loopissues;
     my $first;
     my $previousnote = "";
 
@@ -435,9 +429,7 @@ sub GetSubscriptionsFromBiblionumber {
         } else {
             $subs->{histenddate} = "";
         }
-        $subs->{opacnote}     =~ s/\n/\<br\/\>/g;
-        $subs->{missinglist}  =~ s/\n/\<br\/\>/g;
-        $subs->{recievedlist} =~ s/\n/\<br\/\>/g;
+        $subs->{opacnote}     //= "";
         $subs->{ "periodicity" . $subs->{periodicity} }     = 1;
         $subs->{ "numberpattern" . $subs->{numberpattern} } = 1;
         $subs->{ "status" . $subs->{'status'} }             = 1;
@@ -473,7 +465,7 @@ sub GetFullSubscriptionsFromBiblionumber {
             serial.publisheddatetext,
             serial.status, 
             serial.notes as notes,
-            year(IF(serial.publisheddate="00-00-0000",serial.planneddate,serial.publisheddate)) as year,
+            year(IF(serial.publisheddate IS NULL,serial.planneddate,serial.publisheddate)) as year,
             biblio.title as bibliotitle,
             subscription.branchcode AS branchcode,
             subscription.subscriptionid AS subscriptionid
@@ -484,16 +476,19 @@ sub GetFullSubscriptionsFromBiblionumber {
   LEFT JOIN biblio on biblio.biblionumber=subscription.biblionumber 
   WHERE     subscription.biblionumber = ? 
   ORDER BY year DESC,
-          IF(serial.publisheddate="00-00-0000",serial.planneddate,serial.publisheddate) DESC,
+          IF(serial.publisheddate IS NULL,serial.planneddate,serial.publisheddate) DESC,
           serial.subscriptionid
           |;
     my $sth = $dbh->prepare($query);
     $sth->execute($biblionumber);
     my $subscriptions = $sth->fetchall_arrayref( {} );
-    my $cannotedit = not can_edit_subscription( $subscriptions->[0] ) if scalar @$subscriptions;
-    for my $subscription ( @$subscriptions ) {
-        $subscription->{cannotedit} = $cannotedit;
+    if (scalar @$subscriptions) {
+        my $cannotedit = not can_edit_subscription( $subscriptions->[0] );
+        for my $subscription ( @$subscriptions ) {
+            $subscription->{cannotedit} = $cannotedit;
+        }
     }
+
     return $subscriptions;
 }
 
@@ -529,12 +524,13 @@ sub SearchSubscriptions {
     my $additional_fields = $args->{additional_fields} // [];
     my $matching_record_ids_for_additional_fields = [];
     if ( @$additional_fields ) {
-        $matching_record_ids_for_additional_fields = Koha::AdditionalField->get_matching_record_ids({
-                fields => $additional_fields,
-                tablename => 'subscription',
-                exact_match => 0,
-        });
-        return () unless @$matching_record_ids_for_additional_fields;
+        my @subscriptions = Koha::Subscriptions->filter_by_additional_fields($additional_fields);
+
+        return () unless @subscriptions;
+
+        $matching_record_ids_for_additional_fields = [ map {
+            $_->subscriptionid
+        } @subscriptions ];
     }
 
     my $query = q|
@@ -631,11 +627,10 @@ sub SearchSubscriptions {
         $subscription->{cannotedit} = not can_edit_subscription( $subscription );
         $subscription->{cannotdisplay} = not can_show_subscription( $subscription );
 
-        my $additional_field_values = Koha::AdditionalField->fetch_all_values({
-            record_id => $subscription->{subscriptionid},
-            tablename => 'subscription'
-        });
-        $subscription->{additional_fields} = $additional_field_values->{$subscription->{subscriptionid}};
+        my $subscription_object = Koha::Subscriptions->find($subscription->{subscriptionid});
+        $subscription->{additional_fields} = { map { $_->field->name => $_->value }
+            $subscription_object->additional_field_values->as_list };
+
     }
 
     return @$results;
@@ -669,7 +664,7 @@ sub GetSerials {
         publisheddatetext, planneddate,notes, routingnotes
                         FROM   serial
                         WHERE  subscriptionid = ? AND status NOT IN ( $statuses )
-                        ORDER BY IF(publisheddate<>'0000-00-00',publisheddate,planneddate) DESC";
+                        ORDER BY IF(publisheddate IS NULL,planneddate,publisheddate) DESC";
     my $sth = $dbh->prepare($query);
     $sth->execute($subscriptionid);
 
@@ -691,7 +686,7 @@ sub GetSerials {
        FROM     serial
        WHERE    subscriptionid = ?
        AND      status IN ( $statuses )
-       ORDER BY IF(publisheddate<>'0000-00-00',publisheddate,planneddate) DESC
+       ORDER BY IF(publisheddate IS NULL,planneddate,publisheddate) DESC
       ";
     $sth = $dbh->prepare($query);
     $sth->execute($subscriptionid);
@@ -792,8 +787,6 @@ sub GetLatestSerials {
     my @serials;
     while ( my $line = $sth->fetchrow_hashref ) {
         $line->{ "status" . $line->{status} } = 1;                        # fills a "statusX" value, used for template status select list
-        $line->{planneddate}   = output_pref( { dt => dt_from_string( $line->{planneddate} ),   dateonly => 1 } );
-        $line->{publisheddate} = output_pref( { dt => dt_from_string( $line->{publisheddate} ), dateonly => 1 } );
         push @serials, $line;
     }
 
@@ -1095,13 +1088,13 @@ sub ModSerialStatus {
     #It is a usual serial
     # 1st, get previous status :
     my $dbh   = C4::Context->dbh;
-    my $query = "SELECT serial.subscriptionid,serial.status,subscription.periodicity
+    my $query = "SELECT serial.subscriptionid,serial.status,subscription.periodicity,serial.routingnotes
         FROM serial, subscription
         WHERE serial.subscriptionid=subscription.subscriptionid
             AND serialid=?";
     my $sth   = $dbh->prepare($query);
     $sth->execute($serialid);
-    my ( $subscriptionid, $oldstatus, $periodicity ) = $sth->fetchrow;
+    my ( $subscriptionid, $oldstatus, $periodicity, $routingnotes ) = $sth->fetchrow;
     my $frequency = GetSubscriptionFrequency($periodicity);
 
     # change status & update subscriptionhistory
@@ -1109,16 +1102,15 @@ sub ModSerialStatus {
     if ( $status == DELETED ) {
         DelIssue( { 'serialid' => $serialid, 'subscriptionid' => $subscriptionid, 'serialseq' => $serialseq } );
     } else {
-
         my $query = '
             UPDATE serial
             SET serialseq = ?, publisheddate = ?, publisheddatetext = ?,
-                planneddate = ?, status = ?, notes = ?
+                planneddate = ?, status = ?, notes = ?, routingnotes = ?
             WHERE  serialid = ?
         ';
         $sth = $dbh->prepare($query);
         $sth->execute( $serialseq, $publisheddate, $publisheddatetext,
-            $planneddate, $status, $notes, $serialid );
+            $planneddate, $status, $notes, $routingnotes, $serialid );
         $query = "SELECT * FROM   subscription WHERE  subscriptionid = ?";
         $sth   = $dbh->prepare($query);
         $sth->execute($subscriptionid);
@@ -1130,19 +1122,16 @@ sub ModSerialStatus {
             my ( $missinglist, $recievedlist ) = $sth->fetchrow;
 
             if ( $status == ARRIVED || ($oldstatus == ARRIVED && $status != ARRIVED) ) {
-                $recievedlist .= "; $serialseq"
-                    if ($recievedlist !~ /(^|;)\s*$serialseq(?=;|$)/);
+                $recievedlist = _handle_seqno($serialseq, $recievedlist);
             }
 
             # in case serial has been previously marked as missing
             if (grep /$status/, (EXPECTED, ARRIVED, LATE, CLAIMED)) {
-                $missinglist=~ s/(^|;)\s*$serialseq(?=;|$)//g;
+                $missinglist = _handle_seqno($serialseq, $missinglist, 'REMOVE');
             }
 
-            $missinglist .= "; $serialseq"
-                if ( ( grep { $_ == $status } ( MISSING_STATUSES ) ) && ( $missinglist !~/(^|;)\s*$serialseq(?=;|$)/ ) );
-            $missinglist .= "; not issued $serialseq"
-                if ( $status == NOT_ISSUED && $missinglist !~ /(^|;)\s*$serialseq(?=;|$)/ );
+            $missinglist = _handle_seqno($serialseq, $missinglist) if grep { $_ == $status } MISSING_STATUSES;
+            $missinglist .= "; not issued $serialseq" if $status == NOT_ISSUED and not _handle_seqno($serialseq, $missinglist, 'CHECK');
 
             $query = "UPDATE subscriptionhistory SET recievedlist=?, missinglist=? WHERE  subscriptionid=?";
             $sth   = $dbh->prepare($query);
@@ -1173,9 +1162,8 @@ sub ModSerialStatus {
                     WHERE  subscriptionid = ?";
         $sth = $dbh->prepare($query);
         $sth->execute( $newlastvalue1, $newlastvalue2, $newlastvalue3, $newinnerloop1, $newinnerloop2, $newinnerloop3, $subscriptionid );
-
-        NewIssue( $newserialseq, $subscriptionid, $subscription->{'biblionumber'}, 1, $nextpubdate, $nextpubdate );
-
+        my $newnote = C4::Context->preference('PreserveSerialNotes') ? $notes : "";
+        NewIssue( $newserialseq, $subscriptionid, $subscription->{'biblionumber'}, 1, $nextpubdate, $nextpubdate, undef, $newnote, $routingnotes );
         # check if an alert must be sent... (= a letter is defined & status became "arrived"
         if ( $subscription->{letter} && $status == ARRIVED && $oldstatus != ARRIVED ) {
             require C4::Letters;
@@ -1186,6 +1174,24 @@ sub ModSerialStatus {
     return;
 }
 
+sub _handle_seqno {
+# Adds or removes seqno from list when needed; returns list
+# Or checks and returns true when present
+
+    my ( $seq, $list, $op ) = @_; # op = ADD | REMOVE | CHECK (default: ADD)
+    my $seq_r = $seq;
+    $seq_r =~ s/([()])/\\$1/g; # Adjust disturbing parentheses for regex, maybe extend in future
+
+    if( !$op or $op eq 'ADD' ) {
+        $list .= "; $seq" if $list !~ /(^|;)\s*$seq_r(?=;|$)/;
+    } elsif( $op eq 'REMOVE' ) {
+        $list=~ s/(^|;)\s*(not issued )?$seq_r(?=;|$)//g;
+    } else { # CHECK
+        return $list =~ /(^|;)\s*$seq_r(?=;|$)/ ? 1 : q{};
+    }
+    return $list;
+}
+
 =head2 GetNextExpected
 
 $nextexpected = GetNextExpected($subscriptionid)
@@ -1315,40 +1321,56 @@ sub ModSubscription {
     $biblionumber, $callnumber, $notes, $letter, $manualhistory,
     $internalnotes, $serialsadditems, $staffdisplaycount, $opacdisplaycount,
     $graceperiod, $location, $enddate, $subscriptionid, $skip_serialseq,
-    $itemtype, $previousitemtype
+    $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=?
-        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,
-        $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
@@ -1380,32 +1402,50 @@ sub NewSubscription {
     ) = @_;
     my $dbh = C4::Context->dbh;
 
-    #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|
@@ -1427,7 +1467,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
@@ -1453,7 +1493,7 @@ sub NewSubscription {
     my $biblio = Koha::Biblios->find( $biblionumber );
     if ( $biblio and !$biblio->serial ) {
         my $record = GetMarcBiblio({ biblionumber => $biblionumber });
-        my ( $tag, $subf ) = GetMarcFromKohaField( 'biblio.serial', $biblio->frameworkcode );
+        my ( $tag, $subf ) = GetMarcFromKohaField( 'biblio.serial' );
         if ($tag) {
             eval { $record->field($tag)->update( $subf => 1 ); };
         }
@@ -1462,16 +1502,49 @@ sub NewSubscription {
     return $subscriptionid;
 }
 
+=head2 GetSubscriptionLength
+
+my ($numberlength, $weeklength, $monthlength) = GetSubscriptionLength( $subtype, $sublength );
+
+This function calculates the subscription length.
+
+=cut
+
+sub GetSubscriptionLength {
+    my ($subtype, $length) = @_;
+
+    return unless looks_like_number($length);
+
+    return
+    (
+          $subtype eq 'issues' ? $length : 0,
+          $subtype eq 'weeks'  ? $length : 0,
+          $subtype eq 'months' ? $length : 0,
+    );
+}
+
+
 =head2 ReNewSubscription
 
-ReNewSubscription($subscriptionid,$user,$startdate,$numberlength,$weeklength,$monthlength,$note)
+ReNewSubscription($params);
+
+$params is a hashref with the following keys: subscriptionid, user, startdate, numberlength, weeklength, monthlength, note, branchcode
 
 this function renew a subscription with values given on input args.
 
 =cut
 
 sub ReNewSubscription {
-    my ( $subscriptionid, $user, $startdate, $numberlength, $weeklength, $monthlength, $note ) = @_;
+    my ( $params ) = @_;
+    my $subscriptionid = $params->{subscriptionid};
+    my $user           = $params->{user};
+    my $startdate      = $params->{startdate};
+    my $numberlength   = $params->{numberlength};
+    my $weeklength     = $params->{weeklength};
+    my $monthlength    = $params->{monthlength};
+    my $note           = $params->{note};
+    my $branchcode     = $params->{branchcode};
+
     my $dbh          = C4::Context->dbh;
     my $subscription = GetSubscription($subscriptionid);
     my $query        = qq|
@@ -1491,8 +1564,9 @@ sub ReNewSubscription {
                 'title'         => $subscription->{bibliotitle},
                 'author'        => $biblio->{author},
                 'publishercode' => $biblio->{publishercode},
-                'note'          => $biblio->{note},
-                'biblionumber'  => $subscription->{biblionumber}
+                'note'          => $note,
+                'biblionumber'  => $subscription->{biblionumber},
+                'branchcode'    => $branchcode,
             }
         );
     }
@@ -1524,7 +1598,7 @@ sub ReNewSubscription {
 
 =head2 NewIssue
 
-NewIssue($serialseq,$subscriptionid,$biblionumber,$status, $planneddate, $publisheddate,  $notes)
+NewIssue($serialseq,$subscriptionid,$biblionumber,$status, $planneddate, $publisheddate, $notes, $routingnotes)
 
 Create a new issue stored on the database.
 Note : we have to update the recievedlist and missinglist on subscriptionhistory for this subscription.
@@ -1534,7 +1608,7 @@ returns the serial id
 
 sub NewIssue {
     my ( $serialseq, $subscriptionid, $biblionumber, $status, $planneddate,
-        $publisheddate, $publisheddatetext, $notes ) = @_;
+        $publisheddate, $publisheddatetext, $notes, $routingnotes ) = @_;
     ### FIXME biblionumber CAN be provided by subscriptionid. So Do we STILL NEED IT ?
 
     return unless ($subscriptionid);
@@ -1556,6 +1630,7 @@ sub NewIssue {
             publisheddate     => $publisheddate,
             publisheddatetext => $publisheddatetext,
             notes             => $notes,
+            routingnotes      => $routingnotes
         }
     )->store();
 
@@ -1571,7 +1646,7 @@ sub NewIssue {
         ### Would use substr and index But be careful to previous presence of ()
         $recievedlist .= "; $serialseq" unless ( index( $recievedlist, $serialseq ) > 0 );
     }
-    if ( grep { /^$status$/ } (MISSING_STATUSES) ) {
+    if ( grep { $_ eq $status } (MISSING_STATUSES) ) {
         $missinglist .= "; $serialseq" unless ( index( $missinglist, $serialseq ) > 0 );
     }
 
@@ -1697,13 +1772,11 @@ sub DelSubscription {
     my ($subscriptionid) = @_;
     my $dbh = C4::Context->dbh;
     $dbh->do("DELETE FROM subscription WHERE subscriptionid=?", undef, $subscriptionid);
-    $dbh->do("DELETE FROM subscriptionhistory WHERE subscriptionid=?", undef, $subscriptionid);
-    $dbh->do("DELETE FROM serial WHERE subscriptionid=?", undef, $subscriptionid);
 
-    my $afs = Koha::AdditionalField->all({tablename => 'subscription'});
-    foreach my $af (@$afs) {
-        $af->delete_values({record_id => $subscriptionid});
-    }
+    Koha::AdditionalFieldValues->search({
+        'field.tablename' => 'subscription',
+        'me.record_id' => $subscriptionid,
+    }, { join => 'field' })->delete;
 
     logaction( "SERIAL", "DELETE", $subscriptionid, "" ) if C4::Context->preference("SubscriptionLog");
 }
@@ -1833,11 +1906,9 @@ sub GetLateOrMissingIssues {
         }
         $line->{"status".$line->{status}}   = 1;
 
-        my $additional_field_values = Koha::AdditionalField->fetch_all_values({
-            record_id => $line->{subscriptionid},
-            tablename => 'subscription'
-        });
-        %$line = ( %$line, additional_fields => $additional_field_values->{$line->{subscriptionid}} );
+        my $subscription_object = Koha::Subscriptions->find($line->{subscriptionid});
+        $line->{additional_fields} = { map { $_->field->name => $_->value }
+            $subscription_object->additional_field_values->as_list };
 
         push @issuelist, $line;
     }
@@ -1888,7 +1959,7 @@ sub check_routing {
     my $sth              = $dbh->prepare(
         "SELECT count(routingid) routingids FROM subscription LEFT JOIN subscriptionroutinglist 
                               ON subscription.subscriptionid = subscriptionroutinglist.subscriptionid
-                              WHERE subscription.subscriptionid = ? ORDER BY ranking ASC
+                              WHERE subscription.subscriptionid = ? GROUP BY routingid ORDER BY ranking ASC
                               "
     );
     $sth->execute($subscriptionid);
@@ -2128,7 +2199,7 @@ sub abouttoexpire {
             }
         }
 
-    } elsif ($subscription->{numberlength}>0) {
+    } elsif ( $subscription->{numberlength} && $subscription->{numberlength}>0) {
         return (countissuesfrom($subscriptionid,$subscription->{'startdate'}) >=$subscription->{numberlength}-1);
     }