Updates Requested PR-5.1-dev Pending

User tests: Successful: Unsuccessful:

avatar Hackwar
Hackwar
25 Mar 2024

Summary of Changes

When running the category model from a CLI script, the workflow triggers an error since the ConsoleApplication does not inherit from the CMSApplication class. Since this was restricted this much when refactoring to the DI container, I think it should be fine to losen the restriction like this.

Testing Instructions

Codereview.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

avatar Hackwar Hackwar - open - 25 Mar 2024
avatar Hackwar Hackwar - change - 25 Mar 2024
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 25 Mar 2024
Category Libraries
avatar Hackwar Hackwar - change - 25 Mar 2024
Labels Added: PR-5.1-dev
avatar wilsonge
wilsonge - comment - 25 Mar 2024

No this isn't OK. The application methods aren't available such as bootComponent and getIdentity in AbstractApplication

avatar Hackwar
Hackwar - comment - 26 Mar 2024

Do you have an alternative? Because right now you can't use category or article model in CLI since this requires CMSApplication.

avatar wilsonge
wilsonge - comment - 26 Mar 2024

Not right now. But you can't typehint something where the methods used aren't in the typehint.

avatar wilsonge
wilsonge - comment - 27 Mar 2024

You need to inject CMSApplicationInterface as the typehint but at the same time inject the Event dispatcher separately as that's a separate interface fpr the getDispatcher method

avatar alikon
alikon - comment - 3 Apr 2024

related to #40653, actually is impossible to use article / category from cli cause of workflow
so despite is not the most perfect fix .... and imho it should go on 4.x too

avatar alikon alikon - test_item - 3 Apr 2024 - Tested successfully
avatar alikon
alikon - comment - 3 Apr 2024

I have tested this item ✅ successfully on 8e61d50


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43155.

avatar laoneo
laoneo - comment - 5 Apr 2024

I suggest to inject the CMSApplicationInterface and when you use some specific functions from CMSApplication, then check if $this->app is instanceof CMSApplication or has the require boot function.

avatar wilsonge
wilsonge - comment - 5 Apr 2024

CMSApplicationInterface and DispatcherAwareInterface are just fine

avatar Hackwar Hackwar - change - 9 Apr 2024
Labels Added: Updates Requested
avatar Hackwar
Hackwar - comment - 9 Apr 2024

I changed the typehint. would be nice if you could have a look at this.

avatar wilsonge
wilsonge - comment - 9 Apr 2024

No. This is still not an acceptable typehint. Inject dispatcher separately.

avatar HLeithner
HLeithner - comment - 24 Apr 2024

This pull request has been automatically rebased to 5.2-dev.

avatar HLeithner HLeithner - change - 24 Apr 2024
Title
[5.1] Workflow: use generic AbstractApplication in type hint
[5.2] Workflow: use generic AbstractApplication in type hint
avatar HLeithner HLeithner - edited - 24 Apr 2024
avatar HLeithner
HLeithner - comment - 2 Sep 2024

This pull request has been automatically rebased to 5.3-dev.

avatar HLeithner HLeithner - change - 2 Sep 2024
Title
[5.2] Workflow: use generic AbstractApplication in type hint
[5.3] Workflow: use generic AbstractApplication in type hint
avatar HLeithner HLeithner - edited - 2 Sep 2024

Add a Comment

Login with GitHub to post a comment