Skip to content

Commit

Permalink
Fix scheduled report generation for custom periods (4.x) (matomo-org#…
Browse files Browse the repository at this point in the history
…22033)

* Add more tests for scheduled report date/period combinations

* Fix scheduled report generation for range periods
  • Loading branch information
mneudert authored Mar 25, 2024
1 parent 7c3b4bc commit 59115ef
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 8 deletions.
14 changes: 10 additions & 4 deletions plugins/ScheduledReports/API.php
Original file line number Diff line number Diff line change
Expand Up @@ -385,9 +385,7 @@ public function generateReport(
$period = $report['period_param'];
}

$this->checkSinglePeriod($period, $date);

$date = Date::factory($date)->toString('Y-m-d');
$this->checkDateAndPeriodCombination($date, $period);

// override report format
if (!empty($reportFormat)) {
Expand Down Expand Up @@ -1088,10 +1086,18 @@ private function checkUserHasViewPermission($login, $idSite)
}
}

private function checkSinglePeriod($period, $date)
private function checkDateAndPeriodCombination($date, $period): void
{
if ('range' === $period) {
Period::checkDateFormat($date);

return;
}

if (Period::isMultiplePeriod($date, $period)) {
throw new Http\BadRequestException("This API method does not support multiple periods.");
}

Date::factory($date);
}
}
118 changes: 114 additions & 4 deletions plugins/ScheduledReports/tests/Integration/ApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,82 @@ public function test_generateReport_CatchesIndividualReportProcessExceptions_Wit
self::assertStringNotContainsString('id="UserCountry_getCountry"', $result);
}

/**
* @dataProvider getValidDatePeriodCombinationsForGenerateReport
*
* @param string|false $period
*/
public function test_generateReport_generatesAReportForAllValidDatePeriodCombinations(
string $date,
$period
): void {
$idReport = APIScheduledReports::getInstance()->addReport(
1,
'',
Schedule::PERIOD_DAY,
0,
ScheduledReports::EMAIL_TYPE,
ReportRenderer::HTML_FORMAT,
[
'VisitsSummary_get',
],
[
ScheduledReports::DISPLAY_FORMAT_PARAMETER => ScheduledReports::DISPLAY_FORMAT_TABLES_ONLY
]
);

$result = APIScheduledReports::getInstance()->generateReport(
$idReport,
$date,
false,
APIScheduledReports::OUTPUT_RETURN,
$period
);

self::assertStringContainsString('id="VisitsSummary_get"', $result);
}

/**
* @return iterable<string, array{string, string|false}>
*/
public function getValidDatePeriodCombinationsForGenerateReport(): iterable
{
yield 'default period' => [
'2024-01-01',
false,
];

yield 'single day' => [
'2024-01-01',
'day',
];

yield 'single week' => [
'2024-01-01',
'week',
];

yield 'single month' => [
'2024-01-01',
'month',
];

yield 'single year' => [
'2024-01-01',
'year',
];

yield 'custom range' => [
'2024-01-01,2024-01-02',
'range',
];

yield 'named range' => [
'last7',
'range',
];
}

public function test_generateReport_throwsIfMultiplePeriodsRequested()
{
$this->expectException(\Piwik\Http\BadRequestException::class);
Expand All @@ -559,8 +635,15 @@ public function test_generateReport_throwsIfMultiplePeriodsRequested()
$language = false, $outputType = APIScheduledReports::OUTPUT_RETURN);
}

public function test_generateReport_throwsIfInvalidDateRequested(): void
{
/**
* @dataProvider getInvalidDatePeriodCombinationsForGenerateReport
*
* @param string|false $period
*/
public function test_generateReport_throwsIfInvalidDatePeriodCombinationRequested(
string $date,
$period
): void {
$this->expectException(\Exception::class);
$this->expectExceptionMessage('General_ExceptionInvalidDateFormat');

Expand All @@ -581,12 +664,39 @@ public function test_generateReport_throwsIfInvalidDateRequested(): void

APIScheduledReports::getInstance()->generateReport(
$idReport,
'DoesNotParse',
$date,
false,
APIScheduledReports::OUTPUT_RETURN
APIScheduledReports::OUTPUT_RETURN,
$period
);
}

/**
* @return iterable<string, array{string, string|false}>
*/
public function getInvalidDatePeriodCombinationsForGenerateReport(): iterable
{
yield 'invalid default period' => [
'2024-xx-01',
false,
];

yield 'invalid day' => [
'2024.01.01',
'day',
];

yield 'invalid range format' => [
'2024-01-01//2024-01-02',
'range',
];

yield 'invalid named range' => [
'lastTen',
'range',
];
}

public function test_generateReport_throwsIfInvalidReportRequested(): void
{
$this->expectException(\Exception::class);
Expand Down

0 comments on commit 59115ef

Please sign in to comment.