Skip to content
Permalink
Browse files

Added missing App::uses() statement. Closes #3331

  • Loading branch information...
ADmad committed Nov 1, 2012
1 parent f0e13af commit cc6b699db4fe729542ae06447b9c2f8d9a738e98
@@ -19,6 +19,8 @@
* @license MIT License (http://www.opensource.org/licenses/mit-license.php)
*/
App::uses('AppController', 'Controller');
/**
* Error Handling Controller
*
@@ -142,15 +142,14 @@ public function __construct(Exception $exception) {
* @return Controller
*/
protected function _getController($exception) {
App::uses('AppController', 'Controller');
App::uses('CakeErrorController', 'Controller');
if (!$request = Router::getRequest(true)) {
$request = new CakeRequest();
}
$response = new CakeResponse(array('charset' => Configure::read('App.encoding')));
try {
if (class_exists('AppController')) {
$controller = new CakeErrorController($request, $response);
}
$controller = new CakeErrorController($request, $response);
} catch (Exception $e) {
}
if (empty($controller)) {
@@ -19,7 +19,6 @@
App::uses('ExceptionRenderer', 'Error');
App::uses('Controller', 'Controller');
App::uses('AppController', 'Controller');
App::uses('Component', 'Controller');
App::uses('Router', 'Routing');

13 comments on commit cc6b699

@bar

This comment has been minimized.

Copy link
Contributor

bar replied Nov 3, 2012

I think this is not closed yet, I still have an error in ControllerTest. Reverting 0fb4d1d fixes it.

@ADmad

This comment has been minimized.

Copy link
Member Author

ADmad replied Nov 3, 2012

Which ControllerTest?

@bar

This comment has been minimized.

Copy link
Contributor

bar replied Nov 3, 2012

lib/Cake/Test/Case/Controller/ControllerTest.php
@dereuromark

This comment has been minimized.

Copy link
Member

dereuromark replied Nov 3, 2012

The current 2.3 branch now has the App uses twice in CakeErrorController

@ADmad

This comment has been minimized.

Copy link
Member Author

ADmad replied Nov 3, 2012

@bar Exactly what error did you get? I don't understand how adding this App::uses() to the ErrorHandler affects ControllerTest. The builds on travis and ci.cakephp were passing even before you reverted my commit 0fb4d1d

@dereuromark A faulty merge perhaps, I will fix it.

@bar

This comment has been minimized.

Copy link
Contributor

bar replied Nov 3, 2012

$ cake test core Controller/Controller

.PHP Fatal error:  Class 'AppController' not found in /home/bar/www/cakephp/lib/Cake/Test/test_app/Plugin/TestPlugin/Controller/TestPluginAppController.php on line 19
Fatal Error Error: Class 'AppController' not found in [/home/bar/www/cakephp/lib/Cake/Test/test_app/Plugin/TestPlugin/Controller/TestPluginAppController.php, line 19]

Fatal error: Class 'AppController' not found in /home/bar/www/cakephp/lib/Cake/Test/test_app/Plugin/TestPlugin/Controller/TestPluginAppController.php on line 19

I believe lib/Cake/Test/test_app/Plugin/TestPlugin/Controller/TestPluginAppController.phpmisses App::uses('AppController', 'Controller'); and thats the error I get.

@ADmad

This comment has been minimized.

Copy link
Member Author

ADmad replied Nov 3, 2012

Right, so that's where the missing App::uses() should be added, not in the ErrorHandler class.

@bar

This comment has been minimized.

Copy link
Contributor

bar replied Nov 3, 2012

Perfect, when you advised me to find the right place for it, I started looking and made a patch last night, never PR it though. BTW, how come travis never said something?

Edit: Never mind, It is a dummy plugin :P

@ADmad

This comment has been minimized.

Copy link
Member Author

ADmad replied Nov 3, 2012

During a normal request the dispatcher take care of including the AppController before a particular controller due to which we can avoid having App::uses('AppController', 'Controller'); in each controller file. But for test cases files we need it.

@ADmad

This comment has been minimized.

Copy link
Member Author

ADmad replied Nov 3, 2012

No problem, I will revert your revert 😄

@bar

This comment has been minimized.

Copy link
Contributor

bar replied Nov 3, 2012

Hahaha nice! and sorry for the mess, I should have asked you and wait in the first place.

@ADmad

This comment has been minimized.

Copy link
Member Author

ADmad replied Nov 3, 2012

No worries, if we are gonna blame someone let's just blame Mark for merging your PR 😄

@markstory

This comment has been minimized.

Copy link
Member

markstory replied Nov 3, 2012

I'll take the blame there, I didn't really thing through all the implications of the change. Additional App::uses() are generally harmless.

Please sign in to comment.
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.