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

Remove dependency from Actions plugin to Contents (and maybe Events) #6204

Closed
tsteur opened this issue Sep 13, 2014 · 5 comments
Closed

Remove dependency from Actions plugin to Contents (and maybe Events) #6204

tsteur opened this issue Sep 13, 2014 · 5 comments
Assignees
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Milestone

Comments

@tsteur
Copy link
Member

tsteur commented Sep 13, 2014

Currently, only dimensions of activated plugins will be installed but we should probably always install all dimensions defined by Piwik core plugins.

Problem: If Contents or Events plugin is disabled by default and Actions plugin enabled the tracking would not work because the Actions plugin requires the dimensions of both plugins to be installed. See https://github.com/piwik/piwik/blob/master/plugins/Actions/Archiver.php#L124

Ideally we would not always install all core dimensions as those plugins should not dependent on each other. So solution could be also to fix this dependency maybe.

@tsteur tsteur added the Bug For errors / faults / flaws / inconsistencies etc. label Sep 13, 2014
@tsteur tsteur added this to the Short term milestone Sep 13, 2014
@mattab
Copy link
Member

mattab commented Sep 14, 2014

So solution could be also to fix this dependency maybe.

+1 for this

in short term maybe we could do: #5480 and document the dependencies in plugin.json file?

@tsteur
Copy link
Member Author

tsteur commented Sep 14, 2014

This does not really help in this case and is probably more work. What I meant with this was adding events to let other plugins extend the actions plugin there for instance. Or to develop it in a way this dependency to other plugins is not needed. That'd be ideal.

tsteur added a commit that referenced this issue Sep 26, 2014
…ndency from Actions to Contents plugin. The dependency to the event plugin would be still there. It might work now as I changed Contents plugin to use its own action type recently
tsteur added a commit that referenced this issue Sep 26, 2014
@tsteur
Copy link
Member Author

tsteur commented Sep 26, 2014

Installing always all core dimensions makes actually no sense. I noticed after implementing it this way. It would of course install all dimensions, even the ones from the example plugins etc. We could workaround for this case by not installing the dimensions from core plugins that are disabled by default but then everything is kinda pointless again and unpredictable.

The better solution is indeed to not have such dependencies to other plugins and if so, remove those by using events or refactoring the plugin. For the case of Actions plugin dependency to Events and Contents I can probably kinda refactor the archiver. At least to remove the dependency to Contents

@tsteur tsteur modified the milestones: Piwik 2.8.0, Short term Sep 26, 2014
@tsteur tsteur self-assigned this Sep 26, 2014
@tsteur tsteur changed the title Core dimensions should always be installed Remove dependency from Actions plugin to Contents (and maybe Events) Sep 26, 2014
@tsteur
Copy link
Member Author

tsteur commented Sep 26, 2014

I could probably remove the dependency from Actions plugin to Events plugin as well by doing the same for events as I did for contents, see: https://github.com/piwik/piwik/pull/6310/files

It would make Actions archiving a bit slower as we would always iterate over all actions and later filter out the event actions in PHP. So far there seems to be no problem with this dependency so I would probably not do it. @mattab ?

@mattab
Copy link
Member

mattab commented Sep 26, 2014

Let's not do it for now as it decreases speed (there can be so many events). we'll need another solution later for it. looks good!

@tsteur tsteur closed this as completed Sep 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Projects
None yet
Development

No branches or pull requests

2 participants