[6.0] Fulfill InstallerScriptInterface with a trait #44381
Conversation
| ]; | ||
|
|
||
| if (!$model->save($module)) { | ||
| Factory::getApplication()->enqueueMessage(Text::sprintf('JLIB_INSTALLER_ERROR_COMP_INSTALL_FAILED_TO_CREATE_DASHBOARD', $model->getError())); |
There was a problem hiding this comment.
Would be nice to have a set/getApplication function instead using the one from the factory.
There was a problem hiding this comment.
Hi @laoneo any suggestion how to do this in the trait without causing a lot of hassle for the developer? I guess we have no applicationaware trait yet... If you have a good idea, feel free to create a PR against the branch to improve.
There was a problem hiding this comment.
you already use $this->getApplication() in line 331 above.
In my installer script I get the application from the container and set it in the constructor, but I didn't find out where it is set by a quick review.
|
In general I'm not sure if the trait is the right approach. I would prefer a base class. |
|
What's the value of deprecating and forcing people to move. I mean I'm not fussed whether we use a base class or a trait to be honest - but I don't see the value of just deprecating and moving to the other one forcing code changes for next to no benefit. |
|
I miss an interface for this trait and I would replace the string type with an enum. I think the goal is to get type safety with out b/c break. |
|
The interface exists already, but I couldn't change the InstallerClass without braking bc at the time I made it. |
|
This pull request has been automatically rebased to 6.0-dev. |
HLeithner
changed the title
bembelimen
changed the title
bembelimen
changed the title
| ]; | ||
|
|
||
| if (!$model->save($module)) { | ||
| Factory::getApplication()->enqueueMessage(Text::sprintf('JLIB_INSTALLER_ERROR_COMP_INSTALL_FAILED_TO_CREATE_DASHBOARD', $model->getError())); |
There was a problem hiding this comment.
you already use $this->getApplication() in line 331 above.
In my installer script I get the application from the container and set it in the constructor, but I didn't find out where it is set by a quick review.
| { | ||
| $client = ApplicationHelper::getClientInfo(1); | ||
|
|
||
| $pathname = 'extension_' . ($client ? $client->name : 'root'); |
There was a problem hiding this comment.
can ist be possible that client returns a null object? I mean we use a constant above.
Co-authored-by: Harald Leithner <[email protected]>
Summary of Changes
This implements a trait to be used in combination with the InstallerScriptInterface
Testing Instructions
I have no real idea, how to test it. You need an Extension using a install script with the InstallerScriptInterface. Using this trait should resolve the interface and give you some additional options like:
you can set
There are also two "custom" methods to extend preflight/postflight and still benefit from their functionality.
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: Add new InstallerScriptTrait (PR #44381) Manual#448
No documentation changes for manual.joomla.org needed