-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[6.1] Improvements to extension updater #46054
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 6.1-dev
Are you sure you want to change the base?
Conversation
Avoid PHP warning on missing target platform information. For instance: https://raw.githubusercontent.com/trananhmanh89/ff-update-server/master/com_ffexplorer.xml
This reverts commit d7e5e1e.
administrator/components/com_installer/src/Model/UpdateModel.php
Outdated
Show resolved
Hide resolved
administrator/components/com_installer/src/Model/UpdateModel.php
Outdated
Show resolved
Hide resolved
libraries/src/Event/Installer/AfterPackageDownloadFailedEvent.php
Outdated
Show resolved
Hide resolved
Co-authored-by: Richard Fath <[email protected]>
Co-authored-by: Richard Fath <[email protected]>
Co-authored-by: Richard Fath <[email protected]>
Co-authored-by: Richard Fath <[email protected]>
@BrainforgeUK Three more suggestions, then I think PHPCS should be ok. |
in case you missed it: #46045 (comment) |
Co-authored-by: Richard Fath <[email protected]>
Co-authored-by: Richard Fath <[email protected]>
Co-authored-by: Richard Fath <[email protected]>
@BrainforgeUK Code style is ok now. But PHPStan reports errors. You can see them when checking the changed files on GitHub https://github.com/joomla/joomla-cms/pull/46054/files . |
Co-authored-by: Richard Fath <[email protected]>
Co-authored-by: Richard Fath <[email protected]>
@BrainforgeUK It is best practice to create a pr in a branch of your fork. This way it is easier for you to create multiple independent PR to the same branch of Joomla. Just a note for the future |
I agree requiring an extension developer to write installer plugin(s) in order to handle special events returned by their update server is limiting. Hence I have now provided the capability for the update server to return the package in JSON data together with messages and any other useful information. The Joomla! code changes I propose provide basic handling for this JSON data. If they wish an extension developer can still capture after events and process them in any way they wish. Handling of existing update servers is unchanged. Above I have provided plg_installer_generic.zip which illustrates how these events can be handled. I called it generic with the view that it may form the basis of something which could be shipped with Joomla! core and enabled or replaced with something developer specific as need arises. Although if, in the unlikely event that, all update servers respond with JSON data such a generic event handler would be redundant! If the JSON handling part of this proposal is rejected then please leave the events in as I have a use case for them. |
Re: https://github.com/joomla/joomla-cms/pull/46054#issuecomment-3271070728 What should I be doing about the PHPstan error - eventawareinterface ? |
@BrainforgeUK I don’t know the right solution. @Fedik Could you advise? |
Can ignore that, Aside that, the PR have many other code issues. But before I start complaining we need to agree if this PR is needed. Maybe in some other form as @Ruud68 commented. |
I have responded to @Ruud68 comments above. |
Thanks for your detailed response. My question was based on your first PR but I see now, walking through the code, that you have altered it, I think this is indeed an improvement and adds value not only to extension developers, but also to 'customers' giving them more detailed information on why a download didn't succeed.
One of my extensions is for selling subscriptions and providing downloads using download keys acting as a update server. |
Thanks Ruud68 for your comments. I am obviously 'small fry'! This pull request came out of an overdue upgrade to my update server and thoughts on where a project in the pipeline might go ... |
libraries/src/Event/Installer/AfterPackageDownloadFailedEvent.php
Outdated
Show resolved
Hide resolved
Co-authored-by: Brian Teeman <[email protected]>
Co-authored-by: Brian Teeman <[email protected]>
Pull Request for Issue # .
Summary of Changes
Added event handler response checks for:
Added events:
Testing Instructions
Install these extensions - no need to enable them.
plg_test_test1v1.zip
plg_test_test2v1.zip
Go to check for updates.
v2.0.0 of these will be available.
plg_test_test1
plg_test_test2
Update plg_test_test1
Works fine, updated to v2.0.0
Update plg_test_test2
Actual result BEFORE applying this Pull Request
Expected result AFTER applying this Pull Request
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
https://manual.joomla.org/docs/building-extensions/install-update/update-server/ will need to be updated along the following lines:
Here is an example of a generic installer plugin which provides similar functionality for update servers which don't provide a JSON response. It also attempts to detect an update site which returns a HTML document and not a ZIP!
plg_installer_generic.zip