Add exception for invalid limit before grouping #19881
Conversation
add exception for invalid limit before grouping
update int to numeric
update getTransitionsForAction limitBeforeGrouping default to int
remove translation
| * @param string $parts | ||
| * @return array | ||
| * @throws Exception | ||
| */ | ||
| public function getTransitionsForAction($actionName, $actionType, $idSite, $period, $date, | ||
| $segment = false, $limitBeforeGrouping = false, $parts = 'all') | ||
| $segment = false, $limitBeforeGrouping = 0, $parts = 'all') |
There was a problem hiding this comment.
I guess it would make sense to also change that for all other API methods, otherwise the same error could occur there as well.
And it might also make sense to check and change all methods were that parameter is passed through, as some of them also use false as default...
There was a problem hiding this comment.
@sgiehl that makes sense, update a couple of places, and I notice when $limitBeforeGrouping is given as a string '1' or '20' is will state as an error as well, added intval will fix it without through errors, would that works.
update all other place from false to 0
update ranking Query from false to 0
add more tests
update tests
remove throw errors
remove exception for other places
add throw and tests back
update intval to int and false condition
revert ranking query from false to 0
|
There could be a chance of |
There was a problem hiding this comment.
Tests pass, I can't see any outstanding issues with the code.
- Functional review done
- Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
- Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
- Security review done
- n/a Wording review done
- Code review done
- Existing test extended Tests were added if useful/possible
- Reviewed for breaking changes
- Developer changelog updated if needed
- n/a Documentation added if needed
- n/a Existing documentation updated if needed
Description:
Fixes: #19867
Add an exception for invalid limit before grouping. When URL passing
limitBeforeGrouping=allwill throw an exception.Review