[4.0] Get the MVCFactory trough the ComponentInterface #19835

laoneo · 2018-03-05T09:22:09Z

Pull Request for Issue #17705.

Summary of Changes

A common use case is that extensions need models from a component. There is no standard way to do that. With this pr, the ComponentInterface gets a new function to get it's MVCFactory. Internally this is provided trough a new factory in the service provider.

Testing Instructions

Create a latest articles module on the front end
Open the front end

Expected result

Latest module shows the latest articles.

Actual result

Latest module shows the latest articles.

…ctory/service

…vcfactory/service # Conflicts: # administrator/components/com_content/services/provider.php # libraries/src/Extension/Component.php # libraries/src/Extension/ComponentInterface.php # libraries/src/Extension/LegacyComponent.php # libraries/src/Extension/Service/Provider/Component.php

wilsonge · 2018-03-05T20:49:30Z

libraries/src/Dispatcher/Dispatcher.php

-		$this->input = $input ?: $app->input;
+		$this->app        = $app;
+		$this->input      = $input ?: $app->input;
+		$this->mvcFactory = $mvcFactory;


If the plan is to make this null - there needs to be a default. Else this needs to be first parameter and compulsory

Made it required in the last commit, together with the input.

wilsonge · 2018-03-05T20:50:50Z

libraries/src/Dispatcher/Dispatcher.php

@@ -205,8 +216,22 @@ public function getController(string $name, string $client = '', array $config =
 			throw new \InvalidArgumentException(\JText::sprintf('JLIB_APPLICATION_ERROR_INVALID_CONTROLLER_CLASS', $controllerClass));
 		}

-		$controller = new $controllerClass($config, new MVCFactory($namespace, $this->app), $this->app, $this->input);
+		$factory = null;


This isn't required with your if/else. Also just to be clear - which one are you planning on keeping and which to be removed?

Removed it as the injected factory is required now.

…ctory/service # Conflicts: # libraries/src/Dispatcher/DispatcherFactory.php

wilsonge · 2018-03-08T18:44:23Z

tests/unit/suites/libraries/cms/model/JModelFormTest.php

 		// Create mock of abstract class JModelForm to test concrete methods in there
 		$this->object = $this->getMockBuilder('JModelForm')
 			->getMockForAbstractClass();
-		$mockApp = $this->getMockCmsApp();


What's the reason for this change?

I was too lazy to mock the factory 🙊 . Changed that.

…vcfactory/service

wilsonge · 2018-03-08T22:26:06Z

libraries/src/Dispatcher/Dispatcher.php

+	 *
+	 * @since   __DEPLOY_VERSION__
+	 */
+	private $mvcFactory;


$mvcFactoryFactory please - I got really confused on us calling createFactory on this thing further down until i carefully re-read the doc blocks here

wilsonge · 2018-03-08T22:28:25Z

libraries/src/MVC/Model/BaseDatabaseModel.php

-
-				$factory = new MVCFactory($ns, \JFactory::getApplication());
-			}
+			$factory = Factory::getApplication()->bootComponent($this->option)->createMVCFactory(Factory::getApplication());


I don't think $this->option is namespace aware from a quick look at the code. This is probably going to give rubbish names (and also probably won't be prefixed with com_)

It doesn't have to https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Extension/ExtensionManagerTrait.php#L43

wilsonge · 2018-03-08T22:31:31Z

tests/unit/suites/libraries/cms/model/JModelItemTest.php

@@ -33,8 +35,10 @@ class JModelItemTest extends TestCase
 	public function setUp()
 	{
 		// Create mock of abstract class JModelForm to test concrete methods in there
-		$this->object = $this->getMockBuilder('JModelItem')
-			->getMockForAbstractClass();
+		$this->object = $this->getMockForAbstractClass(


Can we use the builder still here please? PHPUnit is slowly deprecating all other approaches so I'd rather stay ahead of the curve here

wilsonge · 2018-03-08T22:31:47Z

tests/unit/suites/libraries/cms/model/JModelFormTest.php

@@ -35,8 +37,10 @@ public function setUp()
 		$this->saveFactoryState();

 		// Create mock of abstract class JModelForm to test concrete methods in there
-		$this->object = $this->getMockBuilder('JModelForm')
-			->getMockForAbstractClass();
+		$this->object = $this->getMockForAbstractClass(


Same here - keep the builder just use the right methods on it please

wilsonge · 2018-03-08T22:31:53Z

tests/unit/suites/libraries/cms/model/JModelAdminTest.php

@@ -33,8 +35,10 @@ class JModelAdminTest extends TestCase
 	public function setUp()
 	{
 		// Create mock of abstract class JModelAdmin to test concrete methods in there
-		$this->object = $this->getMockBuilder('JModelAdmin')
-			->getMockForAbstractClass();
+		$this->object = $this->getMockForAbstractClass(


And here :)

wilsonge · 2018-03-08T22:33:56Z

modules/mod_articles_latest/mod_articles_latest.php

@@ -12,6 +12,6 @@
 use Joomla\CMS\Helper\ModuleHelper;
 use Joomla\Module\ArticlesLatest\Site\Helper\ArticlesLatestHelper;

-$list = ArticlesLatestHelper::getList($params);
+$list = ArticlesLatestHelper::getList($params, $app->bootComponent('com_content')->createMVCFactory($app)->createModel('Articles', 'Site'));


Aren't we going to end up with issues here because we're no longer able to insert the array('ignore_request' => true) param here - I think on list views that will mean this module breaks

…ctory/service

laoneo added 7 commits February 13, 2018 09:42

Introduce a factory to create MVCFactory objects

e9eeed8

Merge branch '4.0-dev' into j4/mvcfactory/service

65589d0

mvcfactory

e357d9e

Merge remote-tracking branch 'remotes/upstream/4.0-dev' into j4/mvcfa…

181019a

…ctory/service

CS

351d80f

Revert

7e1ba02

joomla-cms-bot added the PR-4.0-dev label Mar 5, 2018

laoneo mentioned this pull request Mar 5, 2018

[4.0] Normalise getting JTable objects #17705

Closed

Inject the factory

b5cf8f3

wilsonge reviewed Mar 5, 2018

View reviewed changes

Require MVC factory

eaa4c43

laoneo force-pushed the j4/mvcfactory/service branch from e72be05 to eaa4c43 Compare March 6, 2018 07:02

laoneo added 3 commits March 6, 2018 08:04

Merge branch '4.0-dev' into j4/mvcfactory/service

ec7839b

Merge remote-tracking branch 'remotes/upstream/4.0-dev' into j4/mvcfa…

0860219

…ctory/service # Conflicts: # libraries/src/Dispatcher/DispatcherFactory.php

Setup factory in the model tests

1b496b2

joomla-cms-bot added the Unit/System Tests label Mar 8, 2018

No void return

b8fdcfe

wilsonge reviewed Mar 8, 2018

View reviewed changes

laoneo added 3 commits March 8, 2018 19:51

Merge branch '4.0-dev' into j4/mvcfactory/service

23e7064

Mock the factory

4c09f55

Merge remote-tracking branch 'origin/j4/mvcfactory/service' into j4/m…

a5bdbb3

…vcfactory/service

laoneo force-pushed the j4/mvcfactory/service branch from db8f66f to a5bdbb3 Compare March 8, 2018 19:16

wilsonge reviewed Mar 8, 2018

View reviewed changes

laoneo added 4 commits March 9, 2018 07:45

Merge remote-tracking branch 'remotes/upstream/4.0-dev' into j4/mvcfa…

9a90b5e

…ctory/service

Rename

bbad6fd

Use mock builder

6d9f9f9

Ignore request in model

24850bc

laoneo force-pushed the j4/mvcfactory/service branch from 0a81c4f to 6580ab7 Compare March 9, 2018 07:01

Better name here too

873533d

laoneo force-pushed the j4/mvcfactory/service branch from 6580ab7 to 873533d Compare March 9, 2018 07:04

wilsonge merged commit 0a0c86f into joomla:4.0-dev Mar 9, 2018

wilsonge deleted the j4/mvcfactory/service branch March 9, 2018 10:34

wilsonge added this to the Joomla 4.0 milestone Mar 9, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[4.0] Get the MVCFactory trough the ComponentInterface #19835

[4.0] Get the MVCFactory trough the ComponentInterface #19835

laoneo commented Mar 5, 2018

wilsonge Mar 5, 2018

laoneo Mar 6, 2018

wilsonge Mar 5, 2018

laoneo Mar 6, 2018

wilsonge Mar 8, 2018

laoneo Mar 8, 2018

wilsonge Mar 8, 2018

wilsonge Mar 8, 2018

laoneo Mar 9, 2018

wilsonge Mar 8, 2018

wilsonge Mar 8, 2018

wilsonge Mar 8, 2018

wilsonge Mar 8, 2018

[4.0] Get the MVCFactory trough the ComponentInterface #19835

[4.0] Get the MVCFactory trough the ComponentInterface #19835

Conversation

laoneo commented Mar 5, 2018

Summary of Changes

Testing Instructions

Expected result

Actual result

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment