[#40768] - [5.0] Add extensions uninstallation with optional parameter migration on update from 4.4
- Fixed in Code Base
- 15 Jun 2023
- Medium
- Build: 5.0-dev
- # 40768
- Diff
- richard67:5.0-dev-extension-uninstallation-on-update
User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Summary of Changes
This pull request (PR) adds the capability to uninstall core extensions and optionally migrate their parameters to other core extensions when updating from the previous major version, i.e. here from 4.4.
Therefore a new function "uninstallExtensions" is added to script.php and called in the "update" function of the same file.
The call happens at the same place where the "uninstallEosPlugin" was called before it has been removed in 5.0-dev with my PR #40711 , and the new function basically works in the same way as the removed "uninstallEosPlugin" function, except that it is not limited to one plugin but goes through a list of extensions of any type and that it optionally calls a function e.g. to migrate parameters or save data before the extension is uninstalled.
The new function can be used to properly uninstall the demo task plugin as proposed with PR #40147 instead of doing that in an update SQL script, and it can be used for migrating the "System - Log Rotation" plugin to a scheduler task plugin as proposed with PR #40519 .
What this PR here does not do it to implement any parameter migration for extensions which are not uninstalled on update. This shall be done in the "postflight" method like it has been implemented for the migration of the TinyMCE plugin's paremeter with PR #40626 with function "migrateTinymceConfiguration".
Testing Instructions
For testing I have prepared code and packages which use the scheduler task demo plugin as example.
This will work as long as PR #40147 has not been merged, or when it has been merged without the update SQL scripts to delete the obsolete plugin.
Test 1: Uninstall extension without calling a migration function
- Modify script.php of this PR for uninstalling an extension as shown here https://github.com/richard67/joomla-cms/pull/37/files and build a package from that.
Or download the following package which already contains these modifications: https://test5.richard-fath.de/Joomla_5.0.0-alpha2-dev-Development-Update_Package_pr40768-test-1.zip - On a current 4.4-dev nightly build installation, switch on "Debug System" and set "Error Reporting" to "Maximum".
- Update the 4.4-dev to the modified package.
- Check that the extension which shall be uninstalled has properly been uninstalled and that no errors appeared during the update which could be related to the changes from this PR here.
Test 2: Uninstall extension with calling a migration function
- Modify script.php of this PR for uninstalling an extension with calling a migration function as shown here https://github.com/richard67/joomla-cms/pull/38/files and build a package from that.
Or download the following package which already contains these modifications: https://test5.richard-fath.de/Joomla_5.0.0-alpha2-dev-Development-Update_Package_pr40768-test-2.zip - On a current 4.4-dev nightly build installation, switch on "Debug System" and set "Error Reporting" to "Maximum".
- Make sure that PHP errors are logged into a log file, e.g. by using "log_errors = On" and "error_log = ..." in your php.ini file.
- Update the 4.4-dev to the modified package.
- Check that the extension which shall be uninstalled has properly been uninstalled and that no errors appeared during the update which could be related to the changes from this PR here.
- Check the PHP error log file if it shows the log from the testing migration function added in step 1.
Actual result BEFORE applying this Pull Request
Test 1: Uninstall extension without calling a migration function
Currently this is possible to delete the extension with an update SQL script, but this would not remove any assets or other things which would be removed on a regular uninstallation.
Test 2: Uninstall extension with calling a migration function
Currently this is not possible within update SQL scripts.
Expected result AFTER applying this Pull Request
Test 1: Uninstall extension without calling a migration function
The extension has been uninstalled after the update from 4.4-dev.
Test 2: Uninstall extension with calling a migration function
The extension has been uninstalled after the update from 4.4-dev, and the migration function has been called before uninstalling.
Link to documentations
Please select:
-
No documentation changes for docs.joomla.org needed
-
No documentation changes for manual.joomla.org needed
joomla-cms-bot
- | Category | ⇒ | Administration com_admin |
| Status | New | ⇒ | Pending |
| Labels |
Added:
PR-5.0-dev
|
||
thanks, looks good to me, will test later when drone is done
@HLeithner A test with the package built by drone will only show that nothing's broken, but it will not do anything because it doesn't contain any extension to be uninstalled. For this I have prepared 2 other branches and built update packages which are linked in the (to be completed) testing instructions.
thanks, looks good to me, will test later when drone is done
@HLeithner A test with the package built by drone will only show that nothing's broken, but it will not do anything because it doesn't contain any extension to be uninstalled. For this I have prepared 2 other branches and built update packages which are linked in the (to be completed) testing instructions.
Yes I know, I have read the source, you don't need complex test instructions, just that the update should work without error and that the extensions should no longer exist.
Yes I know, I have read the source, you don't need complex test instructions, just that the update should work without error and that the extensions should no longer exist.
@HLeithner As said, the "extensions should no longer exist" can not be tested with the package of this PR, it needs a modified one with an example. I have prepared 2 tests:
And created packages with that:
- https://test5.richard-fath.de/Joomla_5.0.0-alpha2-dev-Development-Update_Package_pr40768-test-1.zip
- https://test5.richard-fath.de/Joomla_5.0.0-alpha2-dev-Development-Update_Package_pr40768-test-2.zip
For the 2nd test I might have to modify that testing dummy function because the "echo" result is not really visible for long enough time. Maybe it has to do an "error_log" or something like that.
| Title |
|
||||||
Ready for testing.
@HLeithner One more question, hopefully the last one: Shall I remove the condition here so it runs also when updating from 5.0.0-alpha1 where we also have the obsolete demo task plugin? https://github.com/joomla/joomla-cms/pull/40768/files#diff-db7eb77540ff419bd7e6557d2779f4cf1e31158c20cb4b63dbbe8d6c426385adR217-R220
If we remove the condition, the function will work on every update, also e.g. when updating a 5.0.1 to a 5.0.2.
If we leave it as it is, all if fine except if people started with a 5.0.0-alpha1 or updated to that version and plan to continue to use that site later, because then it will contain the obsolete plugin forever.
I think we can leave it as it is and maybe create an FAQ doc for 5.0.0-alpha1 (or alpha2).
What do you think?
I think the check is not needed or set it to 5.0.1 this would allow us to use it for all alpha, beta and rc versions too, without the need to do a real semver check.
I think the check is not needed or set it to 5.0.1 this would allow us to use it for all alpha, beta and rc versions too, without the need to do a real semver check.
@HLeithner I've decided to do the 5.0.1 suggestion. Other idea could be to run always but to have a version number in the array with the extensions so it could be decided individually for every extension until which version we do the migration and uninstallation. But I don't think that is necessary because our policy says we are feature complete with beta 1.
And when it turns out that this will be necessary, then it still can be added later in future.
@HLeithner I think it should even work when using version_compare($this->fromVersion, '5.0.0-alpha2', 'ge'), or maybe better with 5.0.0-beta1 as that's the point where we shall be feature complete. Shall I do that? If yes, which one?
Use rc1 and please test it for our usual version stages up to 5.4.0-rc1
Use rc1 and please test it for our usual version stages up to 5.4.0-rc1
@HLeithner 5.4.0-rc1? Or did you mean 4.4.0-rc1? We are checking the version from which we update. I'm using https://onlinephp.io/ for testing with PHP 7.2, 7.4, 8.0, 8.1 and 8.2. Is that sufficient?
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-06-15 09:52:08 |
| Closed_By | ⇒ | HLeithner |
I merge this now so we can use it in real extension uninstall tests. Thanks for your work.
I merge this now so we can use it in real extension uninstall tests. Thanks for your work.
@HLeithner Thanks for advice and suggestions. I will soon make a PR for Allon's branch for PR #40147 for uninstalling the demo task plugin.
@HLeithner I just see that we will need the enabled status of the extension in the $row parameter for any migration function because the migration might depend on if the extension is enabled or not. E.g. for the logrotation this will be the case. The old system plugin could be disabled, so the migration to a task should disable the new task in that case. I will make a follow up PR soon.
thanks, looks good to me, will test later when drone is done