Skip to content

Conversation

@richard67
Copy link
Member

@richard67 richard67 commented Jun 15, 2023

Pull Request for #40768 (comment) .

Summary of Changes

With my just merged PR #40768 , extensions uninstallation with optional parameter migration on update was implemented in 5.0-dev.

But the enabled status of the extension is not passed to the migration function, so that function can not work differently depending on that status. There might also be other columns in the extensions table which might be of interest of a migration function.

This PR here fixes that by selecting all columns with the database query, so the $row object passed to the migration function contains them all.

Testing Instructions

Code review should be enough.

Actual result BEFORE applying this Pull Request

The extensions uninstallation with parameter migration on update passes only the extension_id and the params properties of the extension to the migration function.

Expected result AFTER applying this Pull Request

The extensions uninstallation with parameter migration on update passes all properties of the extension to the migration function.

Link to documentations

Please select:

  • No documentation changes for docs.joomla.org needed

  • No documentation changes for manual.joomla.org needed

@HLeithner
Copy link
Member

is there a reason why we don't load all columns?

@richard67
Copy link
Member Author

is there a reason why we don't load all columns?

@HLeithner You mean with a SELECT * FROM ...? Not really a reason, except maybe that when specifying columns explicitly I can see in the code which columns I have. We won't need all of them, but selecting all is of course better for code maintainability, one secret place less to change. If you want I can do that.

@HLeithner
Copy link
Member

please change it to * thanks

@richard67 richard67 changed the title [5.0] Add enabled property to parameter for migration methods of extensions uninstallation on update [5.0] Select all columns in extensions uninstallation on update Jun 16, 2023
@richard67
Copy link
Member Author

please change it to * thanks

Done, and title and description changed accordingly.

@HLeithner HLeithner merged commit 1cdebd0 into joomla:5.0-dev Jun 16, 2023
@HLeithner
Copy link
Member

thanks

@richard67 richard67 deleted the 5.0-dev-extend-uninstallation-on-update-2023-06-15 branch June 16, 2023 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants