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
avatar SimonvanDoorne
SimonvanDoorne - comment - 12 Feb 2025

What is the best way to not only fix this for 5.x but also 4.x? I recently learned the hard way that there are issues with the workflow models and CLI scripts, and i'd like to help get this fixed a.s.a.p. To my eyes the changes suggested by the pull request seem good but this is only for 5.x, and considering many sites of my clients are J4 i'd like to see this fixed there too.

Add a Comment

Login with GitHub to post a comment