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
Allow rearchive_reports_in_past_last_n_months setting value to be an int. #17368
Conversation
Do we want to change the default settings in the What about the other files where we use this setting? We still check for if (isset($general['rearchive_reports_in_past_last_n_months'])) {
$reArchiveLastN = (int) substr($general['rearchive_reports_in_past_last_n_months'], 4);
} |
this is up to @tsteur
A PR exists for those now. |
Either |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
thanks for the quick review @flamisz 👍 |
if (empty($lastNMonthsToInvalidate)) { | ||
return null; | ||
if (!is_numeric($lastNMonthsToInvalidate)) { | ||
$lastNMonthsToInvalidate = (int)str_replace('last', '', $lastNMonthsToInvalidate); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory this would also allow values like last2last1
and would result in 21
. Not sure if we wouldn't want to discard such incorrect values instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @tsteur
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's not needed and could keep things simple. People will mostly just configure a number anyway.
@diosmosis thanks for the fix! This should help to fix this issue: https://forum.matomo.org/t/new-funnel-triggers-past-report-generation/41152/4 That being said, there is still a problem. If I set |
good catch @nelhefni, i'll fix this today |
Description:
Fixes #17366
Review