-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[4.0] Add Controller suffix to extension controllers #17624
Conversation
Accidentally deleted branch. Thanks reflog!! |
libraries/src/Controller/Form.php
Outdated
@@ -93,12 +93,13 @@ public function __construct($config = array(), MvcFactoryInterface $factory = nu | |||
{ | |||
$r = null; | |||
|
|||
if (!preg_match('/(.*)Controller(.*)/i', get_class($this), $r)) | |||
if (!preg_match('/(.*)Controller\\\\(.*)/i', get_class($this), $r)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to break non-namespaced controllers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, will fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it, tested it with web links and it worked
$controllerClass = $namespace . $client . '\\Controller\\' . ucfirst($name); | ||
} | ||
|
||
// @todo Remove me when core extensions are converted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just do this now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to make it reviewable. But I will change them after the merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And first, #17682 needs to get it, otherwise I git tons of merge conflicts
…olandd-4.0-dev * '4.0-dev' of https://github.com/joomla/joomla-cms: (35 commits) Delete redis handler in favor of fw handler (joomla#17798) Remove deprecated JArrayHelper (joomla#17795) [4.0] codestyle (joomla#17779) Update error renderers for PHP 7 code structure, update Exception/Throwable references to only reference Throwable (joomla#17750) Improve article association links Fix parsing routes with language filter enabled Fix JString use [4.0] Fix content margin if no "top" modules are assigned (joomla#17699) Removed required state for Secret Key field (joomla#17713) [4.0] [installation] set proper default for lastResetTime (joomla#16847) [4.0] Remove FOF From Joomla Core (joomla#17687) [4.0] Add Controller suffix to extension controllers (joomla#17624) Fix menu association form field not loading remove html imports (joomla#17691) [4.0] Update Bootstrap to beta-1 (joomla#17496) Move files [4.0] Cleanup classmap and include it properly for stubs generation (joomla#17667) Add back class that got deleted somewhere Fix Sql field class name (joomla#17666) [4.0] Fix namespaced form fields Part 2 (joomla#17664) ...
Summary of Changes
This are the controller changes to give the component MVC classes more meaningful names. Controllers will have now
Controller
as suffix to their class name. SoJoomla\Component\Banners\Administrator\Controller\Banners
becomesJoomla\Component\Banners\Administrator\Controller\BannersController
.Additionally the default controller which is called just
Controller
gets the nameDisplayController
, to better reflect whet the class does.Testing Instructions
Expected result
All is working as expected.
Actual result
All is working as expected.
Documentation Changes Required
If there is already documentation written for the namespaced controllers, then it needs to be updated.