Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate report period for scheduled reports #21043

Merged
merged 1 commit into from Jul 24, 2023
Merged

Validate report period for scheduled reports #21043

merged 1 commit into from Jul 24, 2023

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Jul 20, 2023

Description:

fixes #21040

Review

@sgiehl sgiehl added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Jul 20, 2023
@sgiehl sgiehl added this to the 5.0.0 milestone Jul 20, 2023
@@ -122,6 +122,10 @@ public function addReport(

self::validateCommonReportAttributes($period, $hour, $description, $idSegment, $reportType, $reportFormat, $evolutionPeriodFor, $evolutionPeriodN);

if (null !== $periodParam) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we adopting the Yoda style? :)

Strictly technically speaking — it is safer and can be checked by a linter, so why not...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer using yoda style, even though I sometimes miss to use it myself 🙈
But at some point we can consider to force that using a linter maybe.

Copy link
Contributor

@bx80 bx80 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look good, it does. Issues found, there are not. 👍

@sgiehl sgiehl merged commit 7781c27 into 5.x-dev Jul 24, 2023
18 of 20 checks passed
@sgiehl sgiehl deleted the validateperiod branch July 24, 2023 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fatal error: Scheduler: Error The period '/wp-conten' is not supported.
3 participants