fix(web-ui): Plugin Manager - enable aliased installs, Update All, on-demand modes, long installs, categories, GitHub-URL install - #746
Merged
Conversation
… grid updateAll() preferred PluginStateManager.installedPlugins over window.installedPlugins. Only updateAll's own end-of-run refresh ever fills PluginStateManager, so from the second run on it sent the first run's plugins: one uninstalled since failed with "plugin not found" and one installed since was never updated. That refresh also only replaced window.installedPlugins, so the installed cards and the Updates badge kept offering "Update to vX" for what had just been updated. Read window.installedPlugins, the list plugins_manager.js republishes after every install, uninstall and refresh, keeping PluginStateManager as the fallback for a page without it, and refresh through pluginManager.loadInstalledPlugins(true), which redraws the grid. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The on-demand modal fills its Display Mode select from plugin.display_modes, but /plugins/installed never sent the field. Every plugin offered one option, its own id, under "This plugin exposes a single display mode"; the display resolved that id to the plugin's first mode, so a multi-mode plugin could only be started, or pinned, there. Add display_modes to each entry, read from the plugin catalog (get_plugin_display_modes), the same declared list /display/modes and on-demand/start use, keeping only strings. Single-mode plugins still get one option and the same hint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…einstall The store's Install button enabled the new plugin by the registry id it installed. Weather, Music, Stocks and Leaderboard install under the id their manifests declare (weather -> ledmatrix-weather); the plugin list, the config section and /plugins/toggle know only that id, so the toggle answered 404 "Plugin not found" and the plugin stayed disabled behind "installed, but enabling it failed". The same button on an installed plugin (Reinstall) enabled it too, switching a plugin the user had turned off back on. POST /plugins/install now names the installed plugin: plugin_id in the direct answer and in the queued operation's result, read from the installed manifest found the way the store's update and uninstall find it (_find_plugin_path: id, aliases, plugin_path name), else the requested id. The client reloads the list, then enables that id; from an answer without it, the installed entry the store entry matches (findInstalledStorePlugin, which isStorePluginInstalled now uses). A reinstall, decided by the same match that labelled the button, reloads the list and leaves the enabled state alone. test/js/plugins_manager_sandbox.js runs the whole of plugins_manager.js in a vm context against a fake DOM and API, for suites that drive its real flows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pollOperationStatus gave a queued install 60 polls, a second apart, then reported "Install operation timed out" as an error and stopped. The server allows the plugin's dependency install 300 s on its own (install_requirements_file in store_install.py), after a download that fetches the plugin a file at a time, so installs that went on to succeed were reported as failed, never enabled, and left out of the installed list until the page was reloaded. Give installs INSTALL_POLL_MAX_ATTEMPTS (600, ten minutes). When even that runs out, reload the installed list and the store badges and warn that the install may still be running; nothing is enabled without the operation's answer. Uninstall keeps the default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The #plugin-category select listed seven fixed categories while the registry uses about twenty (productivity, utility, transit, finance, ...), so roughly a third of the store could not be filtered to, and "Financial" missed the plugin filed under "finance". The template now ships only "All Categories"; syncStoreCategoryOptions, run by applyStoreFiltersAndSort, adds one option per category the cached store plugins have (case folded, as the filter compares), keeps the current choice, and rebuilds only when the set changes or the partial was swapped in afresh -- the way the Starlark section builds its own. The test sandbox gains window.addEventListener (initPluginsPage needs it) and quiets the script's "element not found" warnings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#install-plugin-from-url had an inline onclick calling window.handleGitHubPluginInstall, and attachInstallButtonHandler also gave it a click listener that installs, so both ran on every click (and on Enter, which clicks it). The inline handler threw a ReferenceError -- it called isGithubUrl, which is local to the plugin-manager IIFE, from outside it -- so only the listener's request went out; correcting that scope alone would have sent every install twice. Remove the inline onclick and the window.handleGitHubPluginInstall it called, which nothing else uses. The listener, which already sent the only request, is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 28 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (17)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 6 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
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
1 participant
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.
Summary
Seven Plugin Manager bugs, in six commits.
weather), but the install is named by the manifest's ID (ledmatrix-weather). The user saw "Plugin not found" and then "installed, but enabling it failed".plugin_id, found the way update/uninstall find it. The client enables that ID; it falls back to matching aliases, asisStorePluginInstalleddoes.PluginStateManager.installedPlugins, which only Update All itself ever filled. A second run sent the first run's plugins, and the grid kept showing "Update to vX".window.installedPluginsand finishes withloadInstalledPlugins(true), which redraws the grid and the Updates badge./plugins/installednever returneddisplay_modes, so Pin always pinned the first mode.onclickcalling an IIFE-local function. Installs only worked because a second listener also fired. The inline handler is removed, and one click sends one request.Tests
test_store_install.js(13),test_install_polling.js(8),test_store_categories.js(9),test_github_url_install.js(8), extendedtest_update_all.js. Plus a sharedplugins_manager_sandbox.jsharness.test_api_v3_installed_display_modes.py(4),test_api_v3_install_reports_installed_id.py(8).node test/js/run_all.js: all suites pass. DOM suites, run against the emulator web UI: store 50/50, no-double-fetch 8/8, installed 56/56.restart_requiredfrom the registry ID, so an aliased plugin never asks for a restart.On ledpi:
/plugins/installedlisteddisplay_modesfor 27 of 28 plugins (baseball:mlb_live,mlb_recent,mlb_upcoming, …).Part of a bug sweep
This is one of 10 independent fix PRs from one sweep, all based on
mainef69201.main: the same 62 failures and 6 errors on both. These are the known Windows path and file-locking tests. 191 more tests pass.test_backup_manager.py::test_create_backup_contents(os.replace→WinError 5on a temp zip), was a Windows file-lock flake. It passes on rerun, and nothing here touchescreate_backup.main, with a clean start and no errors or render stalls in the journal. ledpi is back on plainmain.🤖 Generated with Claude Code