PR-5.0-dev Pending

User tests: Successful: Unsuccessful:

avatar richard67
richard67
14 Jun 2023

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

  1. 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
  2. On a current 4.4-dev nightly build installation, switch on "Debug System" and set "Error Reporting" to "Maximum".
  3. Update the 4.4-dev to the modified package.
  4. 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

  1. 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
  2. On a current 4.4-dev nightly build installation, switch on "Debug System" and set "Error Reporting" to "Maximum".
  3. 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.
  4. Update the 4.4-dev to the modified package.
  5. 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.
  6. 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

avatar joomla-cms-bot joomla-cms-bot - change - 14 Jun 2023
Category Administration com_admin
avatar richard67 richard67 - open - 14 Jun 2023
avatar richard67 richard67 - change - 14 Jun 2023
Status New Pending
avatar richard67 richard67 - change - 14 Jun 2023
The description was changed
avatar richard67 richard67 - edited - 14 Jun 2023
avatar richard67 richard67 - change - 14 Jun 2023
The description was changed
avatar richard67 richard67 - edited - 14 Jun 2023
avatar richard67 richard67 - change - 14 Jun 2023
The description was changed
avatar richard67 richard67 - edited - 14 Jun 2023
avatar richard67 richard67 - change - 14 Jun 2023
The description was changed
avatar richard67 richard67 - edited - 14 Jun 2023
avatar richard67 richard67 - change - 14 Jun 2023
Labels Added: PR-5.0-dev
avatar HLeithner
HLeithner - comment - 14 Jun 2023

thanks, looks good to me, will test later when drone is done

avatar richard67
richard67 - comment - 14 Jun 2023

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.

avatar HLeithner
HLeithner - comment - 14 Jun 2023

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.

avatar richard67
richard67 - comment - 14 Jun 2023

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:

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.

avatar richard67 richard67 - change - 14 Jun 2023
The description was changed
avatar richard67 richard67 - edited - 14 Jun 2023
avatar richard67 richard67 - change - 14 Jun 2023
Title
[5.0] [WiP] Add extensions uninstallation with optional parameter migration on update from 4.4
[5.0] Add extensions uninstallation with optional parameter migration on update from 4.4
avatar richard67 richard67 - edited - 14 Jun 2023
avatar richard67
richard67 - comment - 14 Jun 2023

Ready for testing.

avatar richard67
richard67 - comment - 14 Jun 2023

@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?

avatar HLeithner
HLeithner - comment - 14 Jun 2023

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.

avatar richard67
richard67 - comment - 14 Jun 2023

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.

avatar richard67
richard67 - comment - 15 Jun 2023

@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?

avatar HLeithner
HLeithner - comment - 15 Jun 2023

Use rc1 and please test it for our usual version stages up to 5.4.0-rc1

avatar richard67
richard67 - comment - 15 Jun 2023

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?

avatar HLeithner HLeithner - close - 15 Jun 2023
avatar HLeithner HLeithner - merge - 15 Jun 2023
avatar HLeithner HLeithner - change - 15 Jun 2023
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2023-06-15 09:52:08
Closed_By HLeithner
avatar HLeithner
HLeithner - comment - 15 Jun 2023

I merge this now so we can use it in real extension uninstall tests. Thanks for your work.

avatar richard67
richard67 - comment - 15 Jun 2023

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.

avatar richard67
richard67 - comment - 15 Jun 2023

@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.

avatar richard67
richard67 - comment - 15 Jun 2023

Follow-up PR for the above comment is #40775 .

Add a Comment

Login with GitHub to post a comment