From 563879c218f73717f485e6c794442f5ab72ecb4d Mon Sep 17 00:00:00 2001 From: Antonio Spinelli Date: Mon, 3 Jul 2023 00:59:01 -0300 Subject: [PATCH] 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 | 122 +++++++++++++++++-------------- tests/Support/NavigationTest.php | 51 ++++++++++++- 2 files changed, 113 insertions(+), 60 deletions(-) diff --git a/app/Support/Navigation.php b/app/Support/Navigation.php index 7d5f90b23b..f8e435c675 100644 --- a/app/Support/Navigation.php +++ b/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', - 'daily' => 'addDays', - '1W' => 'addWeeks', - 'weekly' => 'addWeeks', - 'week' => 'addWeeks', - '1M' => 'addMonths', - 'month' => 'addMonths', - 'monthly' => 'addMonths', - '3M' => 'addMonths', - 'quarter' => 'addMonths', - 'quarterly' => 'addMonths', - '6M' => 'addMonths', - 'half-year' => 'addMonths', - 'year' => 'addYears', - 'yearly' => 'addYears', - '1Y' => 'addYears', - 'custom' => 'addMonths', // custom? just add one month. + '1D' => Periodicity::Daily, + 'daily' => Periodicity::Daily, + '1W' => Periodicity::Weekly, + 'weekly' => Periodicity::Weekly, + 'week' => Periodicity::Weekly, + '1M' => Periodicity::Monthly, + 'month' => Periodicity::Monthly, + 'monthly' => Periodicity::Monthly, + '3M' => Periodicity::Quarterly, + 'quarter' => Periodicity::Quarterly, + 'quarterly' => Periodicity::Quarterly, + '6M' => Periodicity::HalfYearly, + 'half-year' => Periodicity::HalfYearly, + 'year' => Periodicity::Yearly, + 'yearly' => Periodicity::Yearly, + '1Y' => Periodicity::Yearly, + 'custom' => Periodicity::Monthly, // custom? just add one month. // last X periods? Jump the relevant month / quarter / year - 'last7' => 'addDays', - 'last30' => 'addMonths', - 'last90' => 'addMonths', - 'last365' => 'addYears', - 'MTD' => 'addMonths', - 'QTD' => 'addMonths', - 'YTD' => 'addYears', - ]; - $modifierMap = [ - 'quarter' => 3, - '3M' => 3, - 'quarterly' => 3, - '6M' => 6, - 'half-year' => 6, - 'last7' => 7, - 'last90' => 3, - 'QTD' => 3, + 'last7' => Periodicity::Weekly, + 'last30' => Periodicity::Monthly, + 'last90' => Periodicity::Quarterly, + 'last365' => Periodicity::Yearly, + 'MTD' => Periodicity::Monthly, + 'QTD' => Periodicity::Quarterly, + 'YTD' => Periodicity::Yearly, ]; 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: - // 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; + return $this->nextDateByInterval($theDate, $functionMap[$repeatFreq], $skip); } /** diff --git a/tests/Support/NavigationTest.php b/tests/Support/NavigationTest.php index 1f4d36a54f..5ffdd0dd10 100644 --- a/tests/Support/NavigationTest.php +++ b/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()); + } }