Fix a bug for monthly calculation periodicity

This change reveals a bug in the Monthly calculation date where the
difference between more than one month was discarded. The new calendar
calculator was prepared to avoid overflow at the end of the month.

app/Support/Navigation.php

@@ -26,6 +26,9 @@ namespace FireflyIII\Support;
 use Carbon\Carbon;
 use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Helpers\Fiscal\FiscalHelperInterface;
+use FireflyIII\Support\Calendar\Calculator;
+use FireflyIII\Support\Calendar\Exceptions\IntervalException;
+use FireflyIII\Support\Calendar\Periodicity;
 use Illuminate\Support\Facades\Log;
 use Psr\Container\ContainerExceptionInterface;
 use Psr\Container\NotFoundExceptionInterface;
@@ -35,81 +38,88 @@ use Psr\Container\NotFoundExceptionInterface;
  */
 class Navigation
 {
+    private Calculator $calculator;
+
+    public function __construct(Calculator $calculator = null)
+    {
+        $this->calculator = ($calculator instanceof Calculator) ?: new Calculator();
+    }
+
+    /**
+     * @param Carbon      $epoch
+     * @param Periodicity $periodicity
+     * @param int         $skipInterval
+     * @return Carbon
+     */
+    public function nextDateByInterval(Carbon $epoch, Periodicity $periodicity, int $skipInterval = 0): Carbon
+    {
+        try {
+            return $this->calculator->nextDateByInterval($epoch, $periodicity, $skipInterval);
+        } catch (IntervalException $exception) {
+            Log::warning($exception->getMessage(), ['exception' => $exception]);
+        } catch (\Throwable $exception) {
+            Log::error($exception->getMessage(), ['exception' => $exception]);
+        }
+
+        Log::debug(
+            "Any error occurred to calculate the next date.",
+            ['date' => $epoch, 'periodicity' => $periodicity->name, 'skipInterval' => $skipInterval]
+        );
+
+        return $epoch;
+    }
+
     /**
      * @param Carbon $theDate
      * @param string $repeatFreq
      * @param int    $skip
      *
      * @return Carbon
+     * @deprecated This method will be substituted by nextDateByInterval()
      */
-    public function addPeriod(Carbon $theDate, string $repeatFreq, int $skip): Carbon
+    public function addPeriod(Carbon $theDate, string $repeatFreq, int $skip = 0): Carbon
     {
         $date = clone $theDate;
         $add  = ($skip + 1);
 
         $functionMap = [
-            '1D'        => 'addDays',
+            '1D'        => Periodicity::Daily,
-            'daily'     => 'addDays',
+            'daily'     => Periodicity::Daily,
-            '1W'        => 'addWeeks',
+            '1W'        => Periodicity::Weekly,
-            'weekly'    => 'addWeeks',
+            'weekly'    => Periodicity::Weekly,
-            'week'      => 'addWeeks',
+            'week'      => Periodicity::Weekly,
-            '1M'        => 'addMonths',
+            '1M'        => Periodicity::Monthly,
-            'month'     => 'addMonths',
+            'month'     => Periodicity::Monthly,
-            'monthly'   => 'addMonths',
+            'monthly'   => Periodicity::Monthly,
-            '3M'        => 'addMonths',
+            '3M'        => Periodicity::Quarterly,
-            'quarter'   => 'addMonths',
+            'quarter'   => Periodicity::Quarterly,
-            'quarterly' => 'addMonths',
+            'quarterly' => Periodicity::Quarterly,
-            '6M'        => 'addMonths',
+            '6M'        => Periodicity::HalfYearly,
-            'half-year' => 'addMonths',
+            'half-year' => Periodicity::HalfYearly,
-            'year'      => 'addYears',
+            'year'      => Periodicity::Yearly,
-            'yearly'    => 'addYears',
+            'yearly'    => Periodicity::Yearly,
-            '1Y'        => 'addYears',
+            '1Y'        => Periodicity::Yearly,
-            'custom'    => 'addMonths', // custom? just add one month.
+            'custom'    => Periodicity::Monthly, // custom? just add one month.
             // last X periods? Jump the relevant month / quarter / year
-            'last7'     => 'addDays',
+            'last7'     => Periodicity::Weekly,
-            'last30'    => 'addMonths',
+            'last30'    => Periodicity::Monthly,
-            'last90'    => 'addMonths',
+            'last90'    => Periodicity::Quarterly,
-            'last365'   => 'addYears',
+            'last365'   => Periodicity::Yearly,
-            'MTD'       => 'addMonths',
+            'MTD'       => Periodicity::Monthly,
-            'QTD'       => 'addMonths',
+            'QTD'       => Periodicity::Quarterly,
-            'YTD'       => 'addYears',
+            'YTD'       => Periodicity::Yearly,
-        ];
-        $modifierMap = [
-            'quarter'   => 3,
-            '3M'        => 3,
-            'quarterly' => 3,
-            '6M'        => 6,
-            'half-year' => 6,
-            'last7'     => 7,
-            'last90'    => 3,
-            'QTD'       => 3,
         ];
 
         if (!array_key_exists($repeatFreq, $functionMap)) {
-            Log::error(sprintf('Cannot do addPeriod for $repeat_freq "%s"', $repeatFreq));
+            Log::error(sprintf(
-
+                'The periodicity %s is unknown. Choose one of available periodicity: %s',
+                $repeatFreq,
+                join(', ', array_keys($functionMap))
+            ));
             return $theDate;
         }
-        if (array_key_exists($repeatFreq, $modifierMap)) {
-            $add *= $modifierMap[$repeatFreq];
-        }
-        $function = $functionMap[$repeatFreq];
-        $date->$function($add);
 
-        // if period is 1M and diff in month is 2 and new DOM > 1, sub a number of days:
+        return $this->nextDateByInterval($theDate, $functionMap[$repeatFreq], $skip);
-        // AND skip is 1
-        // result is:
-        // '2019-01-29', '2019-02-28'
-        // '2019-01-30', '2019-02-28'
-        // '2019-01-31', '2019-02-28'
-
-        $months     = ['1M', 'month', 'monthly'];
-        $difference = $date->month - $theDate->month;
-        if (1 === $add && 2 === $difference && $date->day > 0 && in_array($repeatFreq, $months, true)) {
-            $date->subDays($date->day);
-        }
-
-        return $date;
     }
 
     /**

tests/Support/NavigationTest.php

@@ -3,6 +3,7 @@
 namespace Tests\Support;
 
 use Carbon\Carbon;
+use FireflyIII\Support\Calendar\Periodicity;
 use FireflyIII\Support\Navigation;
 use Tests\TestCase;
 
@@ -76,9 +77,10 @@ class NavigationTest extends TestCase
         $this->assertEquals($expected->toDateString(), $period->toDateString());
     }
 
-    public static function providePeriodsWithSkippingParam(): array
+    public static function providePeriodsWithSkippingParam(): \Generator
     {
-        return [
+        $intervals = [
+            '2019-01-31 to 2019-02-11' => ['skip' => 10, 'frequency' => 'daily', 'from' => Carbon::parse('2019-01-31'), 'expected' => Carbon::parse('2019-02-11')],
             '1D'                       => ['skip' => 1, 'frequency' => '1D', 'from' => Carbon::now(), 'expected' => Carbon::now()->addDays(2)],
             'daily'                    => ['skip' => 1, 'frequency' => 'daily', 'from' => Carbon::now(), 'expected' => Carbon::now()->addDays(2)],
             '1W'                       => ['skip' => 1, 'frequency' => '1W', 'from' => Carbon::now(), 'expected' => Carbon::now()->addWeeks(2)],
@@ -94,7 +96,7 @@ class NavigationTest extends TestCase
             '2023-05-31 to 2023-07-31' => ['skip' => 1, 'frequency' => 'monthly', 'from' => Carbon::parse('2023-05-31'), 'expected' => Carbon::parse('2023-07-31')],
             '2023-08-31 to 2023-10-31' => ['skip' => 1, 'frequency' => 'monthly', 'from' => Carbon::parse('2023-08-31'), 'expected' => Carbon::parse('2023-10-31')],
             '2023-10-31 to 2023-12-31' => ['skip' => 1, 'frequency' => 'monthly', 'from' => Carbon::parse('2023-10-31'), 'expected' => Carbon::parse('2023-12-31')],
-            '2023-01-31 to 2023-03-30' => ['skip' => 2, 'frequency' => 'monthly', 'from' => Carbon::parse('2023-01-31'), 'expected' => Carbon::parse('2023-05-1')],
+            '2023-01-31 to 2023-03-30' => ['skip' => 2, 'frequency' => 'monthly', 'from' => Carbon::parse('2023-01-31'), 'expected' => Carbon::parse('2023-04-30')],
             '3M'                       => ['skip' => 1, 'frequency' => '3M', 'from' => Carbon::now(), 'expected' => Carbon::now()->addMonths(6)],
             'quarter'                  => ['skip' => 1, 'frequency' => 'quarter', 'from' => Carbon::now(), 'expected' => Carbon::now()->addMonths(6)],
             'quarterly'                => ['skip' => 1, 'frequency' => 'quarterly', 'from' => Carbon::now(), 'expected' => Carbon::now()->addMonths(6)],
@@ -103,6 +105,7 @@ class NavigationTest extends TestCase
             'year'                     => ['skip' => 1, 'frequency' => 'year', 'from' => Carbon::now(), 'expected' => Carbon::now()->addYears(2)],
             'yearly'                   => ['skip' => 1, 'frequency' => 'yearly', 'from' => Carbon::now(), 'expected' => Carbon::now()->addYears(2)],
             '1Y'                       => ['skip' => 1, 'frequency' => '1Y', 'from' => Carbon::now(), 'expected' => Carbon::now()->addYears(2)],
+            '2023-02-01 to 2023-02-15' => ['skip' => 1, 'frequency' => 'last7', 'from' => Carbon::parse('2023-02-01'), 'expected' => Carbon::parse('2023-02-15')],
             'last7'                    => ['skip' => 1, 'frequency' => 'last7', 'from' => Carbon::now(), 'expected' => Carbon::now()->addDays(14)],
             'last30'                   => ['skip' => 1, 'frequency' => 'last30', 'from' => Carbon::now(), 'expected' => Carbon::now()->addMonths(2)],
             'last90'                   => ['skip' => 1, 'frequency' => 'last90', 'from' => Carbon::now(), 'expected' => Carbon::now()->addMonths(6)],
@@ -111,14 +114,54 @@ class NavigationTest extends TestCase
             'QTD'                      => ['skip' => 1, 'frequency' => 'QTD', 'from' => Carbon::now(), 'expected' => Carbon::now()->addMonths(6)],
             'YTD'                      => ['skip' => 1, 'frequency' => 'YTD', 'from' => Carbon::now(), 'expected' => Carbon::now()->addYears(2)],
         ];
+        foreach ($intervals as $interval) {
+            yield "{$interval["frequency"]} {$interval["from"]->toDateString()} to {$interval["expected"]->toDateString()}" => $interval;
+        }
     }
 
     /**
      * @dataProvider providePeriodsWithSkippingParam
      */
-    public function testGivenAFrequencyWhenCalculateTheDateThenReturnsTheSkippedDateSuccessful(int $skip, string $frequency, Carbon $from, Carbon $expected)
+    public function testGivenAFrequencyAndSkipIntervalWhenCalculateTheDateThenReturnsTheSkippedDateSuccessful(int $skip, string $frequency, Carbon $from, Carbon $expected)
     {
         $period = $this->navigation->addPeriod($from, $frequency, $skip);
         $this->assertEquals($expected->toDateString(), $period->toDateString());
     }
+
+    public static function provideFrequencies(): array
+    {
+        return [
+            Periodicity::Daily->name       => ['periodicity' => Periodicity::Daily, 'from' => Carbon::now(), 'expected' => Carbon::tomorrow()],
+            Periodicity::Weekly->name      => ['periodicity' => Periodicity::Weekly, 'from' => Carbon::now(), 'expected' => Carbon::now()->addWeeks(1)],
+            Periodicity::Fortnightly->name => ['periodicity' => Periodicity::Fortnightly, 'from' => Carbon::now(), 'expected' => Carbon::now()->addWeeks(2)],
+            Periodicity::Monthly->name     => ['periodicity' => Periodicity::Monthly, 'from' => Carbon::now(), 'expected' => Carbon::now()->addMonths(1)],
+            '2019-01-01 to 2019-02-01'     => ['periodicity' => Periodicity::Monthly, 'from' => Carbon::parse('2019-01-01'), 'expected' => Carbon::parse('2019-02-01')],
+            '2019-01-29 to 2019-02-28'     => ['periodicity' => Periodicity::Monthly, 'from' => Carbon::parse('2019-01-29'), 'expected' => Carbon::parse('2019-02-28')],
+            '2019-01-30 to 2019-02-28'     => ['periodicity' => Periodicity::Monthly, 'from' => Carbon::parse('2019-01-30'), 'expected' => Carbon::parse('2019-02-28')],
+            '2019-01-31 to 2019-02-28'     => ['periodicity' => Periodicity::Monthly, 'from' => Carbon::parse('2019-01-31'), 'expected' => Carbon::parse('2019-02-28')],
+            '2023-03-31 to 2023-04-30'     => ['periodicity' => Periodicity::Monthly, 'from' => Carbon::parse('2023-03-31'), 'expected' => Carbon::parse('2023-04-30')],
+            '2023-05-31 to 2023-06-30'     => ['periodicity' => Periodicity::Monthly, 'from' => Carbon::parse('2023-05-31'), 'expected' => Carbon::parse('2023-06-30')],
+            '2023-08-31 to 2023-09-30'     => ['periodicity' => Periodicity::Monthly, 'from' => Carbon::parse('2023-08-31'), 'expected' => Carbon::parse('2023-09-30')],
+            '2023-10-31 to 2023-11-30'     => ['periodicity' => Periodicity::Monthly, 'from' => Carbon::parse('2023-10-31'), 'expected' => Carbon::parse('2023-11-30')],
+            Periodicity::Quarterly->name   => ['periodicity' => Periodicity::Quarterly, 'from' => Carbon::now(), 'expected' => Carbon::now()->addMonths(3)],
+            '2019-01-29 to 2020-04-29'     => ['periodicity' => Periodicity::Quarterly, 'from' => Carbon::parse('2019-01-29'), 'expected' => Carbon::parse('2019-04-29')],
+            '2019-01-30 to 2020-04-30'     => ['periodicity' => Periodicity::Quarterly, 'from' => Carbon::parse('2019-01-30'), 'expected' => Carbon::parse('2019-04-30')],
+            '2019-01-31 to 2020-04-30'     => ['periodicity' => Periodicity::Quarterly, 'from' => Carbon::parse('2019-01-31'), 'expected' => Carbon::parse('2019-04-30')],
+            Periodicity::HalfYearly->name  => ['periodicity' => Periodicity::HalfYearly, 'from' => Carbon::now(), 'expected' => Carbon::now()->addMonths(6)],
+            '2019-01-31 to 2020-07-29'     => ['periodicity' => Periodicity::HalfYearly, 'from' => Carbon::parse('2019-01-29'), 'expected' => Carbon::parse('2019-07-29')],
+            '2019-01-31 to 2020-07-30'     => ['periodicity' => Periodicity::HalfYearly, 'from' => Carbon::parse('2019-01-30'), 'expected' => Carbon::parse('2019-07-30')],
+            '2019-01-31 to 2020-07-31'     => ['periodicity' => Periodicity::HalfYearly, 'from' => Carbon::parse('2019-01-31'), 'expected' => Carbon::parse('2019-07-31')],
+            Periodicity::Yearly->name      => ['periodicity' => Periodicity::Yearly, 'from' => Carbon::now(), 'expected' => Carbon::now()->addYears(1)],
+            '2020-02-29 to 2021-02-28'     => ['periodicity' => Periodicity::Yearly, 'from' => Carbon::parse('2020-02-29'), 'expected' => Carbon::parse('2021-02-28')],
+        ];
+    }
+
+    /**
+     * @dataProvider provideFrequencies
+     */
+    public function testGivenAIntervalWhenCallTheNextDateByIntervalMethodThenReturnsTheExpectedDateSuccessful(Periodicity $periodicity, Carbon $from, Carbon $expected)
+    {
+        $period = $this->navigation->nextDateByInterval($from, $periodicity);
+        $this->assertEquals($expected->toDateString(), $period->toDateString());
+    }
 }