-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[PHP 8.1 compatibility] Improve type checking for Transitions API method getTransitionsForAction #20599
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
Conversation
@bx80 We maybe should stop trying to support incorrectly provided parameters. The action name is meant to be passed as a string. We do not expect it to be an array. With Matomo 5 we introduced the possibility to typehint API parameters. So we could simply add a typehint to all of the API methods that expect action name or type to be a string. If a user provides it as an array the API should then give a proper error message. |
Since we do have parameters elsewhere that do accept multiple types it's hard to know exactly what should be allowed where. Moving to type hints wherever possible will be a good way to solve this and add some certainty, though it might expose a few bugs on the way! I'll adjust this PR to simply add type hints and use that approach for any similar issues. 👍 |
@bx80 we added a lot code in the past to handle incorrectly provided parameters as there was no easier way to do it and we wanted to avoid php errors to be thrown. But using the new typehints possibility makes it a lot simpler, as the API will directly return a proper error and we might actually be able to get rid of a lot error handling code in the APIs over time. |
Description:
The transitions plugin
getTransitionsForAction()
API method doesn't handle theactionName
parameter being passed as an array and throws a fatal error.This PR allows a single element array or simple string to be correctly parsed, multi-element or empty arrays will return a NoDataForAction error.
Includes tests.
Fixes #20590
Review