fix: omit Connect actions in UPC when plugin is not installed #1417
Conversation
WalkthroughThe detection logic for the connect plugin was updated to check for its presence via a JSON configuration file rather than a binary file. The notifications sidebar component in the user profile view is now always rendered, regardless of the connect plugin's installation status. The "Settings" link in the user profile dropdown was made unconditional with updated text. Lint-staged was configured to include Changes
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
web/components/UserProfile.ce.vue (1)
26-27: 🛠️ Refactor suggestionRemove unused
connectPluginInstalledref
connectPluginInstalledwas destructured but, after the template update, it is no longer referenced.
Vue/TS strict builds will fail with an “unused variable” lint / ts-error.-const { name, description, guid, keyfile, lanIp, connectPluginInstalled } = storeToRefs(serverStore); +const { name, description, guid, keyfile, lanIp } = storeToRefs(serverStore);
🧹 Nitpick comments (1)
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/include/state.php (1)
147-157: Consider adding a fallback / defensive branch for malformedapi.jsonIf
/boot/config/plugins/dynamix.my.servers/configs/api.jsonexists but contains invalid JSON (e.g. is truncated or user-edited) thenjson_decode()returnsnulland we silently skip setting$this->connectPluginInstalled.
A tiny sanity-check with logging would help future debugging.@@ - if (file_exists($apiConfigPath)) { - $apiConfig = @json_decode(file_get_contents($apiConfigPath), true); - if ($apiConfig && isset($apiConfig['plugins']) && is_array($apiConfig['plugins'])) { + if (file_exists($apiConfigPath)) { + $raw = file_get_contents($apiConfigPath); + $apiConfig = @json_decode($raw, true); + if (json_last_error() !== JSON_ERROR_NONE) { + syslog(LOG_WARNING, "UPC: malformed api.json – " . json_last_error_msg()); + } elseif (isset($apiConfig['plugins']) && is_array($apiConfig['plugins'])) { if (in_array('unraid-api-plugin-connect', $apiConfig['plugins'])) { $this->connectPluginInstalled = 'dynamix.unraid.net.plg'; } } }This keeps the new logic intact while making failure modes visible.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/include/state.php(1 hunks)web/components/UserProfile.ce.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Build Unraid UI Library (Webcomponent Version)
- GitHub Check: Build Web App
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
web/components/UserProfile.ce.vue (1)
137-138: ConfirmNotificationsSidebaris safe without the Connect pluginRendering
<NotificationsSidebar />unconditionally relies on that component gracefully handling a missing Connect backend.
Please double-check that the sidebar:
• doesn’t assumeconnectPluginInstalledis truthy in its internals,
• avoids API calls that only exist when the plugin is present, and
• fails softly (no uncaught promise rejections).If any of these assumptions break, wrap the risky parts with the same install check or guard inside the component itself.
pujitm
force-pushed
the
fix/hide-branding
branch
from
cbdb303 to
8931440
Compare
pujitm
force-pushed
the
fix/hide-branding
branch
from
8931440 to
7f8d8bf
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/__test__/components/UserProfile.test.ts (1)
297-304: Simplify the new “always renders notifications sidebar” testThe intention is solid, but the current implementation – mutating
serverStore.connectPluginInstalledafter the component is mounted – doesn’t truly prove that the sidebar is rendered because the state changed. The component no longer referencesconnectPluginInstalled, so the second assertion will always pass, even if the mutation never triggers a re-render.Refactor the test to mount the component with and without the plugin flag instead. This makes the proof explicit and removes the redundant first expectation:
- expect(wrapper.find('[data-testid="notifications-sidebar"]').exists()).toBe(true); - - serverStore.connectPluginInstalled = ''; - await wrapper.vm.$nextTick(); - - expect(wrapper.find('[data-testid="notifications-sidebar"]').exists()).toBe(true); + // When the plugin is installed + expect(wrapper.find('[data-testid="notifications-sidebar"]').exists()).toBe(true); + + // Re-mount with the plugin *not* installed to verify unconditional rendering + const wrapperNoPlugin = mount(UserProfile, { + props: { + server: { ...initialServerData, connectPluginInstalled: '' as ServerconnectPluginInstalled }, + }, + global: { plugins: [pinia], stubs }, + }); + await wrapperNoPlugin.vm.$nextTick(); + + expect(wrapperNoPlugin.find('[data-testid="notifications-sidebar"]').exists()).toBe(true); + wrapperNoPlugin.unmount();This keeps each test focused, avoids state mutation side-effects, and remains type-safe with the explicit cast.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/__test__/components/UserProfile.test.ts(1 hunks)web/components/UserProfile.ce.vue(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/components/UserProfile.ce.vue
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Build Unraid UI Library (Webcomponent Version)
- GitHub Check: Build API
- GitHub Check: Build Web App
- GitHub Check: Test API
- GitHub Check: Analyze (javascript-typescript)
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
web/components/UserProfile/DropdownContent.vue (1)
150-166:⚠️ Potential issuePotential 404:
Settingslink visible even when Connect plugin is absent.When
connectPluginInstalledisfalsethe fallback branch still injectssettingsLink, which points atWEBGUI_CONNECT_SETTINGS. That route is served by the Connect plugin itself, so the link will break if the plugin is not installed – contradicting the PR’s goal to “omit Connect actions”.- : [settingsLink.value, manageUnraidNetAccount.value]), + : connectPluginInstalled.value + ? [settingsLink.value, manageUnraidNetAccount.value] + : [manageUnraidNetAccount.value]),Also update the preceding comment (
// connect plugin links) to reflect the conditional inclusion.
🧹 Nitpick comments (1)
web/components/UserProfile/DropdownContent.vue (1)
125-132:settingsLinkdoes not need to be acomputed– prefer a plain constant.
settingsLinkhas no reactive dependencies (the translation function reference is stable), so wrapping it incomputed()creates needless overhead and changes its identity on every locale switch. A constant object keeps re-renders minimal and makes intent clearer.-const settingsLink = computed((): UserProfileLink => { - return { - href: WEBGUI_CONNECT_SETTINGS.toString(), - icon: CogIcon, - text: props.t('Settings'), - title: props.t('Go to Connect plugin settings'), - }; -}); +const settingsLink: UserProfileLink = { + href: WEBGUI_CONNECT_SETTINGS.toString(), + icon: CogIcon, + text: props.t('Settings'), + title: props.t('Go to Connect plugin settings'), +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.json(1 hunks)web/components/UserProfile/DropdownContent.vue(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Build Web App
- GitHub Check: Test API
- GitHub Check: Build Unraid UI Library (Webcomponent Version)
- GitHub Check: Build API
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
- GitHub Check: Cloudflare Pages
pujitm
force-pushed
the
fix/hide-branding
branch
from
02a81a5 to
df25a89
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/components/UserProfile/DropdownContent.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Build Web App
- GitHub Check: Build Unraid UI Library (Webcomponent Version)
- GitHub Check: Test API
- GitHub Check: Build API
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/components/ConnectSettings/ConnectSettings.ce.vue(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Build Web App
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Build Unraid UI Library (Webcomponent Version)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
web/components/ConnectSettings/ConnectSettings.ce.vue (1)
105-108: Conditional block looks goodWrapping the “Account Status” row in
v-if="connectPluginInstalled"cleanly removes the pair from the grid when the plugin is absent. The CSS nth-child rules still pair labels/values correctly. No action needed.
pujitm
force-pushed
the
fix/hide-branding
branch
from
0f3698d to
c5adc2c
Compare
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/include/state.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Is it ready for this? Since most of these settings are still Connect related
There was a problem hiding this comment.
it'd be weird to see connect settings in the base os (if a user hasn't installed connect), right?
There was a problem hiding this comment.
i was thinking the Connect label (and all labels for plugins) could be dynamically generated via jsonforms
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
🤖 I have created a release *beep* *boop* --- ## [4.9.0](v4.8.0...v4.9.0) (2025-07-08) ### Features * add graphql resource for API plugins ([#1420](#1420)) ([642a220](642a220)) * add management page for API keys ([#1408](#1408)) ([0788756](0788756)) * add rclone ([#1362](#1362)) ([5517e75](5517e75)) * API key management ([#1407](#1407)) ([d37dc3b](d37dc3b)) * api plugin management via CLI ([#1416](#1416)) ([3dcbfbe](3dcbfbe)) * build out docker components ([#1427](#1427)) ([711cc9a](711cc9a)) * docker and info resolver issues ([#1423](#1423)) ([9901039](9901039)) * fix shading in UPC to be less severe ([#1438](#1438)) ([b7c2407](b7c2407)) * info resolver cleanup ([#1425](#1425)) ([1b279bb](1b279bb)) * initial codeql setup ([#1390](#1390)) ([2ade7eb](2ade7eb)) * initialize claude code in codebse ([#1418](#1418)) ([b6c4ee6](b6c4ee6)) * move api key fetching to use api key service ([#1439](#1439)) ([86bea56](86bea56)) * move to cron v4 ([#1428](#1428)) ([b8035c2](b8035c2)) * move to iframe for changelog ([#1388](#1388)) ([fcd6fbc](fcd6fbc)) * native slackware package ([#1381](#1381)) ([4f63b4c](4f63b4c)) * send active unraid theme to docs ([#1400](#1400)) ([f71943b](f71943b)) * slightly better watch mode ([#1398](#1398)) ([881f1e0](881f1e0)) * upgrade nuxt-custom-elements ([#1461](#1461)) ([345e83b](345e83b)) * use bigint instead of long ([#1403](#1403)) ([574d572](574d572)) ### Bug Fixes * activation indicator removed ([5edfd82](5edfd82)) * alignment of settings on ManagementAccess settings page ([#1421](#1421)) ([70c790f](70c790f)) * allow rclone to fail to initialize ([#1453](#1453)) ([7c6f02a](7c6f02a)) * always download 7.1 versioned files for patching ([edc0d15](edc0d15)) * api `pnpm type-check` ([#1442](#1442)) ([3122bdb](3122bdb)) * **api:** connect config `email` validation ([#1454](#1454)) ([b9a1b9b](b9a1b9b)) * backport unraid/webgui[#2269](https://github.com/unraid/api/issues/2269) rc.nginx update ([#1436](#1436)) ([a7ef06e](a7ef06e)) * bigint ([e54d27a](e54d27a)) * config migration from `myservers.cfg` ([#1440](#1440)) ([c4c9984](c4c9984)) * **connect:** fatal race-condition in websocket disposal ([#1462](#1462)) ([0ec0de9](0ec0de9)) * **connect:** mothership connection ([#1464](#1464)) ([7be8bc8](7be8bc8)) * console hidden ([9b85e00](9b85e00)) * debounce is too long ([#1426](#1426)) ([f12d231](f12d231)) * delete legacy connect keys and ensure description ([22fe91c](22fe91c)) * **deps:** pin dependencies ([#1465](#1465)) ([ba75a40](ba75a40)) * **deps:** pin dependencies ([#1470](#1470)) ([412b329](412b329)) * **deps:** storybook v9 ([#1476](#1476)) ([45bb49b](45bb49b)) * **deps:** update all non-major dependencies ([#1366](#1366)) ([291ee47](291ee47)) * **deps:** update all non-major dependencies ([#1379](#1379)) ([8f70326](8f70326)) * **deps:** update all non-major dependencies ([#1389](#1389)) ([cb43f95](cb43f95)) * **deps:** update all non-major dependencies ([#1399](#1399)) ([68df344](68df344)) * **deps:** update dependency @types/diff to v8 ([#1393](#1393)) ([00da27d](00da27d)) * **deps:** update dependency cache-manager to v7 ([#1413](#1413)) ([9492c2a](9492c2a)) * **deps:** update dependency commander to v14 ([#1394](#1394)) ([106ea09](106ea09)) * **deps:** update dependency diff to v8 ([#1386](#1386)) ([e580f64](e580f64)) * **deps:** update dependency dotenv to v17 ([#1474](#1474)) ([d613bfa](d613bfa)) * **deps:** update dependency lucide-vue-next to ^0.509.0 ([#1383](#1383)) ([469333a](469333a)) * **deps:** update dependency marked to v16 ([#1444](#1444)) ([453a5b2](453a5b2)) * **deps:** update dependency shadcn-vue to v2 ([#1302](#1302)) ([26ecf77](26ecf77)) * **deps:** update dependency vue-sonner to v2 ([#1401](#1401)) ([53ca414](53ca414)) * disable file changes on Unraid 7.2 ([#1382](#1382)) ([02de89d](02de89d)) * do not start API with doinst.sh ([7d88b33](7d88b33)) * do not uninstall fully on 7.2 ([#1484](#1484)) ([2263881](2263881)) * drop console with terser ([a87d455](a87d455)) * error logs from `cloud` query when connect is not installed ([#1450](#1450)) ([719f460](719f460)) * flash backup integration with Unraid Connect config ([#1448](#1448)) ([038c582](038c582)) * header padding regression ([#1477](#1477)) ([e791cc6](e791cc6)) * incorrect state merging in redux store ([#1437](#1437)) ([17b7428](17b7428)) * lanip copy button not present ([#1459](#1459)) ([a280786](a280786)) * move to bigint scalar ([b625227](b625227)) * node_modules dir removed on plugin update ([#1406](#1406)) ([7b005cb](7b005cb)) * omit Connect actions in UPC when plugin is not installed ([#1417](#1417)) ([8c8a527](8c8a527)) * parsing of `ssoEnabled` in state.php ([#1455](#1455)) ([f542c8e](f542c8e)) * pin ranges ([#1460](#1460)) ([f88400e](f88400e)) * pr plugin promotion workflow ([#1456](#1456)) ([13bd9bb](13bd9bb)) * proper fallback if missing paths config modules ([7067e9e](7067e9e)) * rc.unraid-api now cleans up older dependencies ([#1404](#1404)) ([83076bb](83076bb)) * remote access lifecycle during boot & shutdown ([#1422](#1422)) ([7bc583b](7bc583b)) * sign out correctly on error ([#1452](#1452)) ([d08fc94](d08fc94)) * simplify usb listing ([#1402](#1402)) ([5355115](5355115)) * theme issues when sent from graph ([#1424](#1424)) ([75ad838](75ad838)) * **ui:** notifications positioning regression ([#1445](#1445)) ([f73e5e0](f73e5e0)) * use some instead of every for connect detection ([9ce2fee](9ce2fee)) ### Reverts * revert package.json dependency updates from commit 711cc9a for api and packages/* ([94420e4](94420e4)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2 participants
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor
Chores