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

Extend CMS controller base from framework class #5005

Closed
wants to merge 1 commit into from

Conversation

wilsonge
Copy link
Contributor

@wilsonge wilsonge commented Nov 6, 2014

Executive Summary

Extends JControllerBase from it's framework counterpart class

Detailed Summary

This extends JControllerBase from the Frameworks /Joomla/Controller/AbstractController class. This means it's implements the frameworks interface giving forward compatibility to the future when we will only use that class. In order to do this I have also had to introduce setApplication() and setInput() methods to reflect those in the frameworks class, the protected $app and $input variables are deprecated in favour of the private variables contained in the frameworks class (these cannot be used for b/c reasons)

As in 3.4 JApplicationBase now extends /Joomla/Application/AbstractApplication I have changed the typehinting in our constructor to allow anything extending this class to be injected (note the setApplication() method must still typehint JApplicationBase to be consistent with the interface unfortunately.

I've also changed the loadApplication() doc block to return a JApplicationCms as JFactory::getApplication() always returns said object

Testing

To test this PR you can see the unit tests pass and try using com_config (frontend and backend) to ensure everything there continues to work as expected - as this is the only component currently using the new MVC classes.

@wilsonge wilsonge force-pushed the mvc-framework branch 2 times, most recently from a9b1682 to c7b8856 Compare November 7, 2014 16:37
@wilsonge wilsonge closed this Dec 28, 2014
@wilsonge wilsonge deleted the mvc-framework branch December 28, 2014 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants