From bc3ecf72b375ace1dc9376d2d8e0a8158894ddf6 Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Wed, 31 May 2017 16:19:35 +0200
Subject: [PATCH] Bug 18697: Final polishing

GetFictiveIssueNumber:
Returns undef instead of 0 for irregular frequencies. Also added to POD.
Removed unused variable $wkno.
Adding a return makes the if(unit) unneeded.
Replaced (a+b)/b by 1+a/b.

_delta_units:
Added a comment about its parameters.

GetFictiveIssueNumber.t:
Adjusted the tests for irregular frequencies accordingly.

Test plan:
[1] Run t/db_dependent/Serials/GetFictiveIssueNumber.t
[2] Run t/db_dependent/Serials/GetNextDate.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
---
 C4/Serials.pm                                 | 24 +++++++++----------
 .../Serials/GetFictiveIssueNumber.t           |  4 ++--
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/C4/Serials.pm b/C4/Serials.pm
index 2d6c7e1358..96f67a5740 100644
--- a/C4/Serials.pm
+++ b/C4/Serials.pm
@@ -2232,6 +2232,8 @@ depending on how many rows are in serial table.
 The issue number calculation is based on subscription frequency, first acquisition
 date, and $publisheddate.
 
+Returns undef when called for irregular frequencies.
+
 The routine is used to skip irregularities when calculating the next issue
 date (in GetNextDate) or the next issue number (in GetNextSeq).
 
@@ -2242,26 +2244,24 @@ sub GetFictiveIssueNumber {
 
     my $frequency = GetSubscriptionFrequency($subscription->{'periodicity'});
     my $unit = $frequency->{unit} ? lc $frequency->{'unit'} : undef;
-    my $issueno = 0;
+    return if !$unit;
+    my $issueno;
 
-    if($unit) {
-        my ($year, $month, $day) = split /-/, $publisheddate;
-        my ($fa_year, $fa_month, $fa_day) = split /-/, $subscription->{'firstacquidate'};
-        my $wkno;
-        my $delta = _delta_units( [$fa_year, $fa_month, $fa_day], [$year, $month, $day], $unit );
+    my ( $year, $month, $day ) = split /-/, $publisheddate;
+    my ( $fa_year, $fa_month, $fa_day ) = split /-/, $subscription->{'firstacquidate'};
+    my $delta = _delta_units( [$fa_year, $fa_month, $fa_day], [$year, $month, $day], $unit );
 
-        if($frequency->{'unitsperissue'} == 1) {
-            $issueno = $delta * $frequency->{'issuesperunit'} + $subscription->{'countissuesperunit'};
-        } else {
-            # Assuming issuesperunit == 1
-            $issueno = int( ($delta + $frequency->{'unitsperissue'}) / $frequency->{'unitsperissue'} );
-        }
+    if( $frequency->{'unitsperissue'} == 1 ) {
+        $issueno = $delta * $frequency->{'issuesperunit'} + $subscription->{'countissuesperunit'};
+    } else { # issuesperunit == 1
+        $issueno = 1 + int( $delta / $frequency->{'unitsperissue'} );
     }
     return $issueno;
 }
 
 sub _delta_units {
     my ( $date1, $date2, $unit ) = @_;
+    # date1 and date2 are array refs in the form [ yy, mm, dd ]
 
     if( $unit eq 'day' ) {
         return Delta_Days( @$date1, @$date2 );
diff --git a/t/db_dependent/Serials/GetFictiveIssueNumber.t b/t/db_dependent/Serials/GetFictiveIssueNumber.t
index 0f3613a739..993baf4cfb 100644
--- a/t/db_dependent/Serials/GetFictiveIssueNumber.t
+++ b/t/db_dependent/Serials/GetFictiveIssueNumber.t
@@ -27,8 +27,8 @@ subtest 'Tests for irregular frequency' => sub {
         periodicity => $freq_irr,
         firstacquidate => '1972-02-07',
     };
-    is( C4::Serials::GetFictiveIssueNumber($subscription, '1972-12-31'), 0, 'Irregular: should be zero' );
-    is( C4::Serials::GetFictiveIssueNumber($subscription, '1973-12-31'), 0, 'Irregular: still zero' );
+    is( C4::Serials::GetFictiveIssueNumber($subscription, '1972-12-31'), undef, 'Irregular: should be undef' );
+    is( C4::Serials::GetFictiveIssueNumber($subscription, '1973-12-31'), undef, 'Irregular: still undef' );
 };
 
 subtest 'Tests for yearly frequencies' => sub {
-- 
2.39.5