Navigation Menu

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

Jump to bottom

Move methods that verify user permissions from Piwik into another class #6585

Closed
tsteur opened this issue Nov 4, 2014 · 2 comments
Closed

Move methods that verify user permissions from Piwik into another class #6585

tsteur opened this issue Nov 4, 2014 · 2 comments
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. duplicate For issues that already existed in our issue tracker and were reported previously. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Milestone
Priority Backlog ...

Comments

@tsteur
Copy link
Member

tsteur commented Nov 4, 2014

Currently, the Piwik\Piwik class contains many methods to verify user permissions such as (check|has|is)(UserHasSuperUserAccess|UserHasSomeAdminAccess), getCurrentUserLogin and many more. Similar to the Common class it is only a huge collection of random methods that actually do not belong in this class.

It should be rather in a class like User or Access. If the name of the class was something like User the methods could be named like getCurrentLogin, checkHasSomeAdminAcccess, hasSomeAdminAccess etc. Probably we even have to split some methods into multiple classes. Eg doAsSuperUser might belong to another class.

Another problem with the current implementation is that the core accesses methods from plugins although the core should be stupid and not know anything about the plugins. Instead an Event or better DI should be used. This will other plugins allow to completely replace this part and we'd have a cleaner core. We might have to create another issue for this as it adds a lot of complexity to this issue.

At the latest this change should be made for Piwik 3.0 but the sooner the better.
@tsteur tsteur added the c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. label Nov 4, 2014
@tsteur tsteur mentioned this issue Nov 4, 2014
Closed
@mnapoli
Copy link
Contributor

mnapoli commented Nov 4, 2014

👍 and we can keep BC, like for other refactorings, by keeping the old static methods as proxies to the new class. Using an simple object injectable using DI would make totally sense.

@mattab mattab added this to the Short term milestone Nov 4, 2014
@mattab mattab added the Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. label Dec 1, 2014
@mattab
Copy link
Member

mattab commented Dec 18, 2014

We'd need to do this before Piwik 3.0.0 #6074

@mattab mattab modified the milestones: Piwik 3.0.0, Short term Apr 7, 2015
@sgiehl sgiehl mentioned this issue Aug 24, 2023
Open
@mattab mattab closed this as not planned Won't fix, can't repro, duplicate, stale Dec 10, 2023
@sgiehl sgiehl added the duplicate For issues that already existed in our issue tracker and were reported previously. label Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. duplicate For issues that already existed in our issue tracker and were reported previously. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Projects
None yet
Milestone
Priority Backlog (Help wanted)
Development

No branches or pull requests
4 participants
@tsteur @mattab @mnapoli @sgiehl