fix (update center): remove stale config keys and fix misleading badge when switching plugin source#3337
Open
TiTidom-RC wants to merge 2 commits into
Open
fix (update center): remove stale config keys and fix misleading badge when switching plugin source#3337TiTidom-RC wants to merge 2 commits into
TiTidom-RC wants to merge 2 commits into
Conversation
5f06bae to
4054fbc
Compare
Contributor
|
Hello, |
Contributor
Author
|
Hello, Thanks for the review. To make sure I'm going in the right direction: would a dedicated The $allSourceKeys map and the purge logic would live only in update.class.php, and each callsite (update.ajax.php, repo.ajax.php, jeecli.php, market.repo.php) would simply call $update->purgeOrphanedConfigKeys() — no logic duplication, and it keeps the explicit call at the right moment for each flow. Or did you have a different placement in mind ? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Problem
When a plugin is switched from one source to another (e.g. GitHub → Market), the
configuration keys specific to the previous source (
user,repository,tokenfor GitHub;marketfor Market, etc.) are never removed from the database. They become orphaned data.Example of configuration persisted in DB after switching from GitHub to Market:
{"version":"beta","market":1,"doNotUpdate":"0","user":"TiTidom-RC","repository":"Discordlink","token":""}while
source = 'market'.Visible consequence: the Update Center displays
beta - TiTidom-RCfor a plugin now managedby the Market, because the JS renders the
version - userbadge whenever theuserkeyexists in the configuration, without checking
source.Fix 1 —
core/ajax/update.ajax.phpIn the
saveaction, purge all configuration keys not belonging to the new source afterutils::a2o()merges the submitted data.array_diffensures that keys shared betweensources (e.g.
pathfor bothsambaandfile) are never purged on a transition betweenthose two sources. Shared keys (
version,doNotUpdate) are never affected. This alsocleans up any pre-existing orphaned data from multi-hop transitions (e.g. GitHub → URL →
Market, where only
urlwould have been purged with a source-comparison approach).Fix 1b —
core/ajax/repo.ajax.php(install button from Market / repo store)The
installaction sets the new source but never purged the old source's exclusive keyseither. Same purge logic applied here, before
setSource(), covering the case where a pluginpreviously added via GitHub is reinstalled directly from the Market store UI.
Fix 1c —
core/repo/market.repo.php(pullInstall())pullInstall()installs plugins pushed automatically from the Market but only nullifieduser,leaving
repository,token,url, andpathas potential orphaned data. Full purge ofall non-market keys applied before
setSource('market').Fix 1d —
core/php/jeecli.php(CLIplugin installwithout--user/--repository)The
plugin install <id>branch without GitHub arguments setssource = 'market'but neverpurged any prior source keys. Same full purge applied.
Fix 2 —
desktop/js/update.jsGuard the
version - userbadge display with_update.source !== 'market'. This fixes thevisual for all existing installations that already have orphaned data in DB, without requiring
any reconfiguration.
Tested transitions
githubmarketuser,repository,tokenmarketgithubmarketgithuburluser,repository,tokensambafilepathis shared)urlgithuburlgithub→url→market(multi-hop)marketuser,repository,token,urlSuggested changelog entry
Amélioration de la gestion des clés liées à une source spécifique pour un plugin, ainsi que l'affichage dans le centre de mise à jour.
Related issues/external references
Fixes #
Types of changes
PR checklist