? ? Pending

User tests: Successful: Unsuccessful:

avatar joomdonation
joomdonation
20 Mar 2017

Pull Request for Issue # .

Summary of Changes

This is a redo of my PR #14511 because there was some changes to component dispatcher implement which was merged today.

This PR contains improvement + clean up to controller class to support both namespace and none namespace components in Joomla 4. See https://github.com/joomdonation/joomla-cms/tree/namespace-com-content/administrator/components/com_content for sample namespace com_content done by @laoneo

  1. Change to base Controller class
  • Support creating namespace model/view classes
  • Simplify getName() method (that method was implemented wrong from beginning, instead of returning name of controller, it return component name ,without com_ prefix), just use substr method instead of regex.
  • Implement getControllerName() method to allow getting name of controller properly (as mentioned above, that should be the role of getName() method)
  1. Change to Form Controller class
  • Call getControllerName method (from base controller class) to get context instead of using regex
  • Use toPlural method of \Joomla\String\Inflector class to detect view_list property instead of using a script found elsewhere in the internet.
  • Remove some redundant else if
  • Supporting column alias for checked_out field.
  1. Change to Admin Controller class

Testing Instructions

This PR is for code review and discussion only. No human testing needed at this stage.

avatar joomdonation joomdonation - open - 20 Mar 2017
avatar joomdonation joomdonation - change - 20 Mar 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 20 Mar 2017
Category Libraries
avatar joomdonation joomdonation - change - 20 Mar 2017
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 20 Mar 2017
Category Libraries Libraries Unit Tests
avatar joomdonation joomdonation - change - 20 Mar 2017
Labels Added: ?
avatar joomdonation joomdonation - change - 21 Mar 2017
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2017-03-21 14:40:30
Closed_By joomdonation
avatar joomdonation joomdonation - close - 21 Mar 2017
avatar joomdonation joomdonation - change - 21 Mar 2017
Status Closed New
Closed_Date 2017-03-21 14:40:30
Closed_By joomdonation
avatar joomdonation joomdonation - change - 21 Mar 2017
Status New Pending
avatar joomdonation joomdonation - reopen - 21 Mar 2017
avatar laoneo
laoneo - comment - 22 Mar 2017

Should this pr not be against the 3.8 branch as we need that in the 3 series as well?

avatar joomdonation
joomdonation - comment - 22 Mar 2017

This actually, depends on the new Component Dispatcher implement (the code which is getting namespace from #__extensions table).

Also, at the moment, we are still unsure if this is accepted or not. If it is accepted, we can easy move it to 3.8 branch

avatar joomdonation
joomdonation - comment - 22 Mar 2017

Just a thought about the discussion getting namespace from database. Is there any problem if when dispatcher create controller, it call controller setNamespace() method to pass component namespace to controller.

Then when controller creates model or view, it will call setNamespace() method to pass namespace to model and view...

I that is not wrong way, we won't have to store namespace (of component) in database.

avatar yvesh
yvesh - comment - 22 Mar 2017

The dispatcher should handover the namespace, the app and the input to the controller (which can than hand it over to the rest.. There are different ways to do this.. i am looking into a container based one, but something simple like (or a registry) would do to:

class Extension 
{
   pub $namespace;
   pub $app;
   pub $input 
   // .. Additional $options
}

And you can than hand over this to the controller from the dispatcher (where you have all information to initialise it)

avatar joomdonation
joomdonation - comment - 22 Mar 2017

So basically, there is no problem if Dispatcher pass namespace Controller, and then Controller pass it to Model and View?

I remember last time (on the first dispatcher PR), I wanted to pass namespace to controller via $config['namespace'] but we disagreed with that option.

avatar yvesh
yvesh - comment - 22 Mar 2017

We need to have a way to pass things like the namespace to the extension. Personally i don't think having $config variables as an simple array is the best way.

As it's going to be confusing which are existing, which are mandatory etc. so i would prefer a different approach. For example let it be an old php object which we load into a registry or a full stack container...

avatar laoneo
laoneo - comment - 22 Mar 2017

What I, and I guess also @mbabker, tried to explain is, that it would make sense to have a factory. I made a quick set up in this branch https://github.com/joomla/joomla-cms/compare/3.8-dev...Digital-Peak:j3/mvc/factory?expand=1 which should illustrate the factory approach. Like that we don't need to care about namespaces at all in controller, model, view or table. If no factory is passed, then the LegacyFactory is used and when we are in the namespaced extension we can pass an MvcFactory instance in the constructors.

avatar joomdonation
joomdonation - comment - 22 Mar 2017

Got a free training about using Factory in real use-case, thanks :). This at least solving the concern with getting / passing namespace around better, and it is even give a chance to implement more advanced factory classes like how FOF3 does.

avatar laoneo
laoneo - comment - 22 Mar 2017

Like that we also do proper unit testing, by providing a mock factory. So we are not related to a database anymore.

avatar laoneo
laoneo - comment - 22 Mar 2017

I made a pr #14858. When we do something like that, then we don't have to deal anymore with namespace names in the controller and model. Additionally when @yvesh wants to do a DI container based dispatcher, then he can create a DIMvcFactory and create the models, views and tables out of the container.

avatar joomdonation
joomdonation - comment - 8 Apr 2017

Close because this PR conflict with the works by other dev. I will open smaller PRs contains improvements and not related to namespace MVC part

avatar joomdonation joomdonation - change - 8 Apr 2017
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2017-04-08 10:26:51
Closed_By joomdonation
avatar joomdonation joomdonation - close - 8 Apr 2017

Add a Comment

Login with GitHub to post a comment