Allow plugins to define custom action types #12556
Conversation
| * | ||
| * @param array $types | ||
| */ | ||
| Piwik::postEvent('Actions.registerActionType', [&$types]); |
There was a problem hiding this comment.
I wonder if it would make sense to define it in an array like this:
array(
array('id' => 76, 'name' => 'media_play')
)The problem is that those action types are based on numbers and are totally "made up" by the plugin. So it could happen that there are two plugins defining the same action ID. By having it in an array like above we could iterate over it, and throw an exception as soon as two plugins have the same ID (maybe only during dev mode?). The good thing be also that the structure is more extensible in the future and could add possibly add more attributes but might not be needed.
Alternatively, because there is already a method getAllActions() maybe we could think about adding a method getType() and maybe getTypeName() to the Piwik\Tracker\Action and each action could return the type they define so we wouldn't need to add a new event (by default nothing would be returned)? I reckon it be good to have all such related code in one class and not needing an event.
In case we keep the event, should we rather use Actions.addActionTypes for consistent naming with other events?
I have no clear preference here but be good if we could catch it in case two plugins define the same type number.
There was a problem hiding this comment.
I thought about adding methods to the action classes, but currently everything is dependent to the request. So theoretically it is possible to have one action class that represents multiple action types. It might for example create different action types based on the request given to the constructor.
I don't think we are using this anywhere, but I'm not sure if we should remove that possibility, as some custom plugins might use it...
There was a problem hiding this comment.
Couldn't find a class that defines multiple types. Is it in core?
There was a problem hiding this comment.
I don't think we are using this anywhere
Its more a theoretical use case, not anything that is in use in any of our plugins afaik
There was a problem hiding this comment.
👍 As it is mainly used by us anyway it is not needed to use action classes
There was a problem hiding this comment.
If updated the PR. It still uses event but now check if a action with same id is defined multiple times
sgiehl
force-pushed
the
customactiontypes
branch
from
8bd8e05
to
6bcd145
Compare
| @@ -40,12 +32,45 @@ class ActionType extends ActionDimension | |||
|
|
|||
| public function __construct() | |||
| { | |||
| $this->acceptValues = sprintf('A type of action, such as: %s', implode(', ', $this->types)); | |||
| $this->acceptValues = sprintf('A type of action, such as: %s', implode(', ', $this->getEnumColumnValues())); | |||
There was a problem hiding this comment.
Can we maybe hard code some of the values here instead of listing them all? The text says "such as" anyway. The thing is that we otherwise on each request request all actions etc which makes page load time a wee bit slower than it has to be. We can still execute it, just saying ideally we would only list a few to not make things even slower.
Action types available for segmentation are currently fixed to those defined in core.
Plugins can now use the new event
Actions.addActionTypesto register their custom action types.