From f90999c977041a895f729e277fd42b42adfd8114 Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Tue, 16 May 2017 14:46:55 +0200
Subject: [PATCH] Bug 18607: Fix date calculations for monthly frequencies in
 Serials
Content-Type: text/plain; charset=utf-8

Similarly to the solution of bug 18356, this patch fixes the date
calculation for monthly frequencies.

The calculation in GetFictiveIssueNumber was that similar that we could
merge the algorithm for months and years. Note that we add a wrapper
around Date::Calc::N_Delta_YMD in order to improve its results.

The calculation in _get_next_date_month is also very similar to the one
in _get_next_date_year. I do not merge them here, but this could still
be considered later on. At least consistency is achieved now between
both routines. The connection with firstacquidate has been cut thru
just like for year units.

Test plan:
[1] Edit an subscription. Check some monthly prediction patterns.
[2] The next patch adjusts related unit tests.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
---
 C4/Serials.pm | 72 +++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 48 insertions(+), 24 deletions(-)

diff --git a/C4/Serials.pm b/C4/Serials.pm
index 04e907d..1e5785b 100644
--- a/C4/Serials.pm
+++ b/C4/Serials.pm
@@ -2294,17 +2294,18 @@ sub GetFictiveIssueNumber {
             ($wkno, $year) = Week_of_Year($year, $month, $day);
             my ($fa_wkno, $fa_yr) = Week_of_Year($fa_year, $fa_month, $fa_day);
             $delta = ($fa_yr == $year) ? ($wkno - $fa_wkno) : ( ($year-$fa_yr-1)*52 + (52-$fa_wkno+$wkno) );
-        } elsif($unit eq 'month') {
-            $delta = ($fa_year == $year)
-                   ? ($month - $fa_month)
-                   : ( ($year-$fa_year-1)*12 + (12-$fa_month+$month) );
-        } elsif( $unit eq 'year' ) {
-            ( $delta ) = N_Delta_YMD( $fa_year, $fa_month, $fa_day, $year, $month, $day );
+
+        } elsif( $unit eq 'month' || $unit eq 'year' ) {
+            $delta = _delta_units( [$fa_year, $fa_month, $fa_day], [$year, $month, $day], $unit );
             if( $frequency->{issuesperunit} > 1 ) {
                 $issueno = 1 + $delta * $frequency->{'issuesperunit'};
-                my @fa_upd = Add_Delta_YM( $fa_year, $fa_month, $fa_day, $delta, 0 );
-                $delta = Delta_Days( @fa_upd, $year, $month, $day );
-                my $incr = int( 365 / $frequency->{issuesperunit} );
+                # Now calculate issues within last (incomplete) unit
+                $delta = Delta_Days(
+                    Add_Delta_YM( $fa_year, $fa_month, $fa_day,
+                        $unit eq 'month' ? ( 0, $delta ) : ( $delta, 0 )),
+                    $year, $month, $day );
+                my $incr = $unit eq 'month' ? 30 : 365;
+                $incr = int( $incr / $frequency->{issuesperunit} );
                 if( $incr > 0 ) {
                     $delta = int( $delta / $incr );
                     $delta = $delta < $frequency->{issuesperunit} ? $delta : $frequency->{issuesperunit} - 1;
@@ -2327,6 +2328,27 @@ sub GetFictiveIssueNumber {
     return $issueno;
 }
 
+sub _delta_units {
+# wrapper around N_Delta_YMD
+# Note that N_Delta_YMD returns 29 days between e.g. 22-2-72 and 22-3-72
+# while we expect 1 month.
+    my ( $date1, $date2, $unit ) = @_;
+    my @delta = N_Delta_YMD( @$date1, @$date2 );
+    if( $delta[2] > 27 ) {
+        # Check if we could add a month
+        my @jump = Add_Delta_YM( @$date1, $delta[0], 1 + $delta[1] );
+        if( Delta_Days( @jump, @$date2 ) >= 0 ) {
+            $delta[1]++;
+        }
+    }
+    if( $delta[1] >= 12 ) {
+        $delta[0]++;
+        $delta[1] -= 12;
+    }
+    # if unit is year, we only return full years
+    return $unit eq 'month' ? $delta[0] * 12 + $delta[1] : $delta[0];
+}
+
 sub _get_next_date_day {
     my ($subscription, $freqdata, $year, $month, $day) = @_;
 
@@ -2368,24 +2390,26 @@ sub _get_next_date_week {
 sub _get_next_date_month {
     my ($subscription, $freqdata, $year, $month, $day) = @_;
 
-    my $fa_day;
-    (undef, undef, $fa_day) = split /-/, $subscription->{firstacquidate};
+    my @newissue; # ( yy, mm, dd )
+    my $delta_days = int( 30 / $freqdata->{issuesperunit} );
 
-    if ($subscription->{countissuesperunit} + 1 > $freqdata->{issuesperunit}){
-        $subscription->{countissuesperunit} = 1;
-        ($year,$month,$day) = Add_Delta_YM($year,$month,$day, 0,
-            $freqdata->{unitsperissue});
-        my $days_in_month = Days_in_Month($year, $month);
-        $day = $fa_day <= $days_in_month ? $fa_day : $days_in_month;
-    } else {
-        # Try to guess the next day in month
-        my $days_in_month = Days_in_Month($year, $month);
-        my $delta_days = int(($days_in_month - ($fa_day - 1)) / $freqdata->{issuesperunit});
-        ($year,$month,$day) = Add_Delta_Days($year, $month, $day, $delta_days);
+    if( $freqdata->{issuesperunit} == 1 ) {
+        # Add full months
+        @newissue = Add_Delta_YM(
+            $year, $month, $day, 0, $freqdata->{"unitsperissue"} );
+    } elsif ( $subscription->{countissuesperunit} < $freqdata->{issuesperunit} ) {
+        # Add rounded number of days based on frequency.
+        @newissue = Add_Delta_Days( $year, $month, $day, $delta_days );
         $subscription->{countissuesperunit}++;
+    } else {
+        # We finished a cycle of issues within a unit.
+        # Subtract delta * (issues - 1), add 1 month
+        @newissue = Add_Delta_Days( $year, $month, $day,
+            -$delta_days * ($freqdata->{issuesperunit} - 1) );
+        @newissue = Add_Delta_YM( @newissue, 0, 1 );
+        $subscription->{countissuesperunit} = 1;
     }
-
-    return ($year, $month, $day);
+    return @newissue;
 }
 
 sub _get_next_date_year {
-- 
2.1.4