User tests: Successful: Unsuccessful:
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.
Codereview.
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
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
Labels |
Added:
PR-5.1-dev
|
Do you have an alternative? Because right now you can't use category or article model in CLI since this requires CMSApplication.
Not right now. But you can't typehint something where the methods used aren't in the typehint.
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
I have tested this item ✅ successfully on 8e61d50
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.
CMSApplicationInterface and DispatcherAwareInterface are just fine
Labels |
Added:
Updates Requested
|
I changed the typehint. would be nice if you could have a look at this.
No. This is still not an acceptable typehint. Inject dispatcher separately.
This pull request has been automatically rebased to 5.2-dev.
Title |
|
This pull request has been automatically rebased to 5.3-dev.
Title |
|
No this isn't OK. The application methods aren't available such as bootComponent and getIdentity in AbstractApplication