[4.0] Add getLanguage to application interface and cleanup console/cli apps#28506
[4.0] Add getLanguage to application interface and cleanup console/cli apps#28506wilsonge merged 2 commits intojoomla:4.0-devfrom
Conversation
|
I have tested this item ✅ successfully on 6b6000b This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28506. |
|
Worth noting before this patch the com_finder cli was broken :) so that one is fixed with this patch |
Any issue exists which we can close? |
|
P.S.: Can confirm it fixes broken cli/finder_indexer.php. |
|
That's the one - didn't know there was an issue :) but added bonus! |
|
@skurvish Please test if this PR here solves your issue #27007 , and if so, mark your test result at the issue tracker https://issues.joomla.org/tracker/joomla-cms/28506 by using the "Test this" button, selecting the appropriate result (Success I hope) and the submitting. |
|
Will do, busy on a few other things right now but given the isolation I should find time over the next few days. |
|
I am not able to test this as the commits for the PR do not seem to be in the 4.0-dev branch. |
|
@skurvish It is a pull request (PR) and so not merged yet into the 4.0-dev branch. It needs test before. You can find here a full install download package which includes the patch of this PR: https://ci.joomla.org:444/artifacts/joomla/joomla-cms/4.0-dev/28506/downloads/30756/Joomla_4.0.0-beta1-dev+pr.28506-Development-Full_Package.zip. Or you have a 4.0-dev installation and apply the patch to that, e.g. using the Patchtester component. |
|
I can confirm that both the joomla and finder cli apps work. I would like to test my own cli if I can before I agree all is well. Quick question, will this be ported to J3.9 or will this be a J4 only implementation? This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28506. |
|
@skurvish Could you mark your test result at the issue tracker https://issues.joomla.org/tracker/joomla-cms/28506 by using the "Test this" button, selecting the appropriate result (Success I hope) and then submitting? Thanks in advance. As I can see this is a fix for J4. No idea if it can and will be backported to J3. |
|
This doesn’t need to get backported to 3.9 - these changes are only required due to work in the 4.0 branch. I will be documenting what changes you need to make to have things work on 3.9 and 4.0 for cli scripts though :) |
|
That would be awesome @wilsonge. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28506. |
|
There are 4 main things:
|
|
Great, thanks. I am out tomorrow but can see if I can integrate and test these changes Tuesday/Wednesday and report back. If I run into any issues where should I report them, here somewhere else? This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28506. |
|
Yeah just post here. I'm going to merge this now as I need this interface change to unblock several other things. If it becomes complex I may get you to open a new issue if needs arise. But here will do as a starting point :) Thank you in advance for any feedback will be really valuable for me to help document migration paths! |
|
I was not able to get the cli app to work without including the joomla configuration in the container. In order to accomplish this I needed to include_once( configuration.php), then add as the second param in the return statement: new Registry(new \JConfig). My app does rely on the the config file and to use Factory::getConfig() in my app required this. I suppose I could have done the same thing in my app but this made more sense to inject it into the ApplicationCli container. |
|
@wilsonge I am also trying to get the ApplicationWeb interface to work. When I create the container then run the getContainer, the app object returned does not have a dispatcher attached to it (i.e. it is null in the object). Did you look at the web interface and might it require similar changes you made to the cli interface? |
|
@wilsonge Hey George, I solved my problems. Turns out in J4 I need to use the CMSApplication class instead of WebApplication. I must also set the dispatcher and session in order to load plugins. |
If you boot your app with the container like here https://github.com/joomla/joomla-cms/blob/4.0-dev/cli/finder_indexer.php#L490-L514 that shouldn't be an issue.
You'll need to set the dispatcher into the app yourself. See how we boot up the web application classes ourselves in the container https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Service/Provider/Application.php#L58-L69 There's nothing missing as far as I know. However don't have any custom web apps to hand to test for you either to give a better idea. Can you give some sample code if the above doesn't help |
|
I attempted to use the container setup exactly like the finder app but the configuration was not available in my app and numerous errors occurred. Inserting the config as stated solved the issue. Your examples for the web app show it using the SiteApplication and AdministratorApplication classes. I attempted to use the WebApplication class which is what I used in J3.x but that didn't work at all. I changed to using the CMSApplication class and that at least instantiated OK but doing the $container->get(DispatcherInterface::class) returned null so I created a dispatcher using new \Joomla\Event\Dispatcher and set it using the setDispatcher method. I am not sure any of this is right but it is working. I would be happy to keep working on it to try new things to sort it out properly if you would like. I guess I would rather get it correct and working than just working and perhaps not in the preffered/correct manner. |
|
@wilsonge did you want me to try anything else? |
Partial Pull Request for Issue #24587 .
Fixes #27007
This ones a bit long - i started by adding the interface and then realised how broken CliApplication was when actually implementing/testing it
Summary of Changes
getLanguageto the CMSApplicationInterfacegetInputmethod to the CliApplication which was in the interface but missingtriggerEventfunction inCliApplicationmessagesused forenqueueMessagein the CliApplication but wasn't definedTesting Instructions
Check CLI application - see commands below If you want you can access any web page too to ensure the interface in the web applications doesn't error too.
Commands to run:
php cli/joomla.php(you can also run as many of the commands from here as you'd like to prove this)php cli/finder_indexer.phpExpected result
Working console apps when new method added to interface
Actual result
Finder Indexer Broken
Documentation Changes Required
Yup interface documentation