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

Having Controller and ControllerAdmin class is confusing #6151

Open
tsteur opened this issue Sep 7, 2014 · 0 comments
Open

Having Controller and ControllerAdmin class is confusing #6151

tsteur opened this issue Sep 7, 2014 · 0 comments
Labels
Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.

Comments

@tsteur
Copy link
Member

tsteur commented Sep 7, 2014

Currently, if a user generates a Controller, the generated template is extending Piwik\Plugin\Controller. If he wants to display the page in the admin he has to change the extended class to Piwik\Plugin\ControllerAdmin and also extend a different template. Having to extend a different template (admin.twig instead of dashboard.twig) is ok for me.

Is it possible that we merge those two controllers? For having backwards compatibility either a new universal Controller class or everything in Controller? I don't think it would be too complex to merge everything from ControllerAdmin into Controller but didn't have a deep look. This would be best.

Alternatively we should have two files in each plugin Controller.php and ControllerAdmin.php along with two different commands to generate them and so on. We'd then have to adjust the router to also have a look in ControllerAdmin and Controller. If an action is defined in both classes which one would be executed? I'd not recommend this solution and would prefer a unified controller.

How to define / create / extend a controller should not depend on where it is displayed. A controller should not care about such things. The view is there for this.

Seeing this definitely mid term and rather sooner than later as part of making our API's better/really good before adding new features.

Part of this issue should also be to document which default view variables are available when extending which template etc. Eg siteName etc

@tsteur tsteur added the Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. label Sep 7, 2014
@tsteur tsteur added this to the Mid term milestone Sep 7, 2014
@mattab mattab modified the milestones: Long term, Mid term Dec 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Projects
None yet
Development

No branches or pull requests

2 participants