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

[4.0] Module dispatcher #19834

Merged
merged 91 commits into from
Oct 18, 2018
Merged

Conversation

laoneo
Copy link
Member

@laoneo laoneo commented Mar 5, 2018

Summary of Changes

Introduces a ModuleDispatcher as replacement for the single entry file for modules. The workflow is then similar to components with a Dispatcher and a service provider.

A module needs then a file services/provider.php which does register the ModuleInterface::class resource in the container:

public function register(Container $container)
{
	$container->registerServiceProvider(new DispatcherFactory('\\Joomla\\Module\\MyModule'));

	$container->registerServiceProvider(new Module);
}

and a dispatcher created by the DispatcherFactory. The dispatcher can override the function getLayoutData() to provide additional data for the layout. The rest stays the same.

More information can be found in #20217 and #19667.

Testing Instructions

Open the front end.

Expected result

The Menu module should work as before.

Actual result

All is working fine.

Documentation Changes Required

Needs to be documented as with the Dispatcher for components.

@shur
Copy link
Contributor

shur commented Mar 5, 2018

administrator/components/com_content/Dispatcher/Dispatcher.php
Is everything correct here?
Must file name start with the capital letter?

@laoneo
Copy link
Member Author

laoneo commented Mar 5, 2018

Yes, that's the new approach we go, introduced by pr #19811. I will convert the rest of the core extensions when all services like Associations, etc. is done.

@laoneo
Copy link
Member Author

laoneo commented Sep 13, 2018

Any more feedback?

@roland-d
Copy link
Contributor

@laoneo Why is it called DefaultModuleDispatcher.php versus the ComponentDispatcher.php? I would expect a ModuleDispatcher.php?

Other than that, the consistency looks good. This PR is also so huge I think we should just start using it and see if we run into any issues :)

@laoneo
Copy link
Member Author

laoneo commented Sep 20, 2018

Was a suggestion from @wilsonge to have it crystal clear that this is the default one. Having a look now, it would probably make sense to rename it back to ModuleDispatcher.php.

@roland-d
Copy link
Contributor

@laoneo Just my first thought was why does that have a different name. Makes more sense to me to stick to one naming scheme.

@laoneo
Copy link
Member Author

laoneo commented Sep 24, 2018

Ok, renamed it. Looks indeed more clear.

@roland-d
Copy link
Contributor

@laoneo the drone failed??

@zero-24
Copy link
Contributor

zero-24 commented Sep 25, 2018

just rebooted the tests.

@laoneo
Copy link
Member Author

laoneo commented Sep 26, 2018

Drone is failing atm all the time with a user system test. Rebooting doesn't help, the test needs to be fixed.

@wilsonge wilsonge merged commit 46799a5 into joomla:4.0-dev Oct 18, 2018
@wilsonge
Copy link
Contributor

Thanks for being so patient on this one!

@wilsonge wilsonge deleted the j4/module/services branch October 18, 2018 10:30
@laoneo
Copy link
Member Author

laoneo commented Oct 18, 2018

Time brought a couple of good ideas.

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

8 participants