feat: nuxtui notifiactions #1860
Conversation
fix context menu trigger area and dropdown styles implement 'move to folder' implement 'start / stop' bulk & row actions implement pause / resume implement column visibility customization fix(organizer): over-eager addition of untracked containers to root folder scaffold docker form service docker container management prototype use compacted table as sidebar tree add drag and drop for re-ordering containers & folders right click to reame & delete folders add bottom padding to container to make it easier to drag items to bottom in sidebar click overview row to open details track active container in query param refactor: extract composables refactor: simplify organizer operations refactor!: rm intermediate tree resolution of organizers BREAKING CHANGE: ResolvedOrganizerView -> root is replaced by rootId + flatEntries. `root` resolved a tree representation of an organizer, but this required clients to defined and use their own tree operations, thus increasing client complexity. Instead, `flatEntries` is more suitable for client operations and only requires an initial mapping step on the client for efficiency. fix: rm extra Root layer in table map containers to their template files feat: icon support fix: container state badge chore: fix formatting fix: search filtering fix: context menu feat: filtering & bulk actions in compact mode feat: critical notifications feat: notifyIfUnique service method tmp: critical notifications system fix: nuxt ui portal styling fix: notifications type check fix api tests fix: css Revert "fix: css" This reverts commit 234c2e2. add docker constants flatten css scopes feat: file modification for replacing docker overview table feat: navigate to container update page feat: implement manage settings action fix: column visibility toggle fix: move update to a badge + popover feat: save column visibility preferences across visits fix: add feature flag to containers file mod fix: circular dependency in docker service add a flag to opt out of version check in super.shouldApply in file mods fix: optimistic column toggle update refactor: optimistic column toggle feat: container start & stop order feat: bulk toggle auto-start fix: add background style reset, apply to button:hover as well feat: add ENABLE_NEXT_DOCKER_RELEASE=true to staging environment chore(api): add dev/notifications to gitignore feat: container update actions fix: container update logic feat: bulk container updates feat: container sizes fix: container sizes modla overlay fix: checkbox alignment fix: revert color in main css chore: ignore build output in lint & fix feat: server-side container ip add docker feature flag to .env.production fix: container port duplication feat: multi-value copyable badges feat: make lanIpPorts a list, not a csv feat: visit button feat: include indexed search fields in filter input title feat: sync userprefs.cfg for rollback compat feat: port conflicts refactor: port conflict frontend stuff feat: update all containers bulk action feat: docker logs fix: use container name instead of id in 'view logs' modal make webgui iframable feat: re-add compact mode manage settings > compact view fix styles feat: container stats refactor: docker autostart service refactor: docker log, event, and port services
- update VSCode settings for Tailwind CSS support
…rror messages commit addresses the following two bugs/issues: 1. infinite network requests 2. make error messages more accurate bug details: - when scrolled all the way down in the notification pane (when api is down), unraid infinitely sends network requests. - must be scrolled all the way to the bottom and stay at the bottom of the pane while the api is down technical details: - for infinite loop, added try/catch that sets a canLoadMore flag to false when it encounters an error, preventing infinite loop - errors now look at non-standard locations as well impact: - performance benefits - more graceful ux on failure
… improved consistency - replaced Heroicons components with UIcon for better integration - refactored Sidebar.vue to utilize USlideover and UButton for a cleaner UI - removed unused imports and styles in main.css for better maintainability NOTES: - had to change main.css variables for it to work properly. Need to make sure this doesn't ruin other people's code. - still needs to be further refactored to align with existing ui variables
…mproved UI consistency - modified vite.config.ts to integrate app configuration into UI setup - updated app.config.ts to include new button, tabs, and slideover variants for better theming - cleaned up main.css by removing unused styles and ensuring proper imports - refactored notification components to streamline structure and improve readability
> [!Note] This stubs the unraid-ui/src/components/common/toast. Initially created a shim to convert vue-sonnner toasts to nuxtui. However, since there weren't that many, I just did a clean replacement. - replace router link with window.location.assign The `UButton` component attempts to inject the Vue Router instance when the `:to` prop is used. In the standalone component environment (where the router is not installed), this caused a "TypeError: inject(...) is undefined" crash when rendering notifications with links. This change replaces the `:to` prop with a standard `@click` handler that uses `window.location.assign`, ensuring navigation works correctly without requiring the router context.
…ng logic Problem this solution addresses: Basically, when users filtered by alert, warning, or info, results were being paginated first, then filtered by the requested importance, so filtered notifications were not working properly in some (a lot) of cases. - added a new async generator method to load notifications in batches, enhancing performance and error handling. - refactored the notification loading logic to utilize the generator, improving readability and maintainability. - updated filtering logic to streamline the process of matching notifications based on importance and type.
- updated file watching logic to ignore initial files, improving performance. - added duplicate check for archive notifications to prevent double counting. - implemented retry mechanism for loading notifications, enhancing reliability. - introduced handling for risky notifications to ensure proper file creation and avoid legacy script failures.
This change ensures that Nuxt UI notifications respect the display position configured in the legacy webGUI settings. Backend: - Added `NotificationSettings` to the GraphQL model. - Exposed `settings` field on the `Notifications` resolver. - Implemented `getSettings` in `NotificationsService` to read `notify.position` from the Dynamix store. Frontend: - Added `getNotificationSettings` GraphQL query. - Updated `mount-engine.ts` to fetch settings before mounting. - Mapped legacy position values (e.g., 'center') to Nuxt UI compatible values (e.g., 'top-center').
…with Nuxt UI 4.0.0-alpha.0
…istency with legacy webGUI
…on drift
This commit addresses several critical stability issues in the notification system spanning the legacy PHP script, the Node.js API, and the Vue frontend.
**Backend / API:**
- **Refactor `notify` script (PHP):** Added `-u` flag to accept a custom filename/ID from the caller. Added filename length sanitization (255 chars) to prevent filesystem errors.
- **Fix ID Mismatch:** The Node API now generates a UUID and passes it to the `notify` script via `-u`. This guarantees that the API, Frontend, and Filesystem all reference the same ID, removing the need for "Risky" notification logic.
- **Fix Counter Bugs:**
- `handleNotificationAdd` no longer ignores duplicate files in the archive.
- `archiveNotification` now checks if a file exists in the archive before moving. If it exists, it simply deletes the unread copy without double-counting.
- `archiveAll` now leverages the robust single-archive logic.
**Frontend (Web):**
- **Fix Infinite Scroll "Drift":**
- Switched `List.vue` to use a **Debounced Refetch** (500ms) for subscription updates instead of manual cache manipulation. This handles rapid-fire events (mass adds) without corruption.
- Increased `pageSize` to `50` to minimize race conditions where new items shift pagination offsets.
- Added **Drift Detection**: If `fetchMore` returns a full page of duplicates (indicating the list has shifted), the component now automatically triggers a full refetch to self-heal.
…asts" This reverts commit b147762.
…e files Problem: ESLint was flagging auto-imported composables (like `useToast`) as undefined variables in `.vue` files. While TypeScript correctly identified these globals via `auto-imports.d.ts`, the default `no-undef` rule in our Vue ESLint config did not account for them, causing false positive linting errors. Solution: - Disable the `no-undef` rule specifically for `.vue` files in `eslint.config.mjs`. - Rely on TypeScript (via `vue-tsc`) and `auto-imports.d.ts` to handle global variable validation, ensuring type safety without conflicting with the auto-import build process. - Remove manual imports of `useToast` that were added as a workaround (p sure the path I used didn't even exist in node modules).
…ts to type declarations
…posable in ConnectSettings, DockerContainersTable, and RCloneOverview components
…ssages in RCloneConfig component
…r external URL handling across multiple components
…logging and improve extractGraphQLErrorMessage to handle additional error structures
…olors to streamline notification rendering across components
…d on notification data
Updates the notification sidebar to properly refresh notification counts (badges) when the API connection is restored or when the user manually retries. Changes: - Emit `refetched` event from `NotificationsList` when the query successfully returns data. - Trigger `recalculateOverview` and `refetch` in `Sidebar` when the list is refreshed or when the API status becomes 'online'. - Fixes an issue where notification badges would remain at 0 after an API outage, even though the notification list had populated correctly.
…uxtui toasts"" This reverts commit 2707f69.
- Added optional properties `expand`, `duration`, and `max` to the Notify interface. - Updated NotificationSettings class to include validation for new properties. - Modified the NotificationsService to parse and return the new settings. - Adjusted the toaster configuration in the app to utilize the new settings. - Updated GraphQL query to fetch the new notification settings.
…nd clean up imports - Created a new file for ToastPosition type definitions to improve code organization. - Removed the inline ToastPosition type definition from mount-engine.ts and updated the import accordingly. - Added a comment in app.config.ts for clarity on toaster settings.
…coping - Introduced a new class to modify the default base CSS file by wrapping content after the 'body' selector in a CSS scope. - Implemented methods to read the file, generate a patch, and apply the necessary modifications. - Added error handling for cases where the 'body' block cannot be found.
Azure theme causing problems because it's the only theme where the color of the icon can't be inverted (bruh)
…ations. Supports 7.0-7.2.3
WalkthroughThis pull request introduces enhanced notification settings management (UI and backend), adds a Docker container form service, refactors notification components, upgrades Nuxt UI from alpha to 4.2.1, adds file modification classes for CSS scoping and configuration patching, implements external navigation abstraction, and extends GraphQL operations for notifications and Docker functionality. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
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 |
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| }); | ||
|
|
||
| // Mock the docker client util | ||
| vi.mock('@app/unraid-api/graph/resolvers/docker/utils/docker-client.js', () => ({ |
Check failure
Code scanning / CodeQL
Assignment to constant Error test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 9 hours ago
To fix the problem without changing behavior, we should avoid having two different mockDockerClientInstance constants and rely on the vi.hoisted version that Vitest recommends for values used in vi.mock factories. The best approach is:
- Remove the first
const mockDockerClientInstance = { getEvents: vi.fn(), };at lines 54–56 and its associatedvi.mock('./utils/docker-client.js', ...)that uses it. - Adjust the earlier
vi.mock('./utils/docker-client.js', ...)so that it uses the hoistedmockDockerClientInstancefromvi.hoisted, not a separate local const. Sincevi.hoistedmust be declared before anyvi.mockthat uses it, we should move thevi.hoisteddeclaration above thesevi.mockcalls. - Keep a single
const { mockDockerClientInstance } = vi.hoisted(...);and use that same variable in both mock definitions:vi.mock('./utils/docker-client.js', ...)vi.mock('@app/unraid-api/graph/resolvers/docker/utils/docker-client.js', ...)
Because we are only changing how the mock object is defined and referenced, and not its shape or behavior (getEvents: vi.fn()), the observable behavior of the tests should remain the same while eliminating the conflicting constant declarations.
Concretely, in api/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts:
- Insert the
vi.hoistedblock (lines 69–74) earlier in the file, before anyvi.mockthat needsmockDockerClientInstance. - Delete the original
const mockDockerClientInstance = { ... };(lines 54–56). - Update the
vi.mock('./utils/docker-client.js', ...)block to callgetDockerClient: vi.fn(() => mockDockerClientInstance)using the hoisted variable. - Ensure the later
vi.mock('@app/unraid-api/graph/resolvers/docker/utils/docker-client.js', ...)still uses the samemockDockerClientInstanceand no additional conflicting declarations are introduced.
No new imports or external methods are required.
| @@ -50,10 +50,14 @@ | ||
| }, | ||
| })); | ||
|
|
||
| const { mockDockerClientInstance } = vi.hoisted(() => { | ||
| const mock = { | ||
| getEvents: vi.fn(), | ||
| } as unknown as Docker; | ||
| return { mockDockerClientInstance: mock }; | ||
| }); | ||
|
|
||
| // Mock the docker client utility - this is what the service actually uses | ||
| const mockDockerClientInstance = { | ||
| getEvents: vi.fn(), | ||
| }; | ||
| vi.mock('./utils/docker-client.js', () => ({ | ||
| getDockerClient: vi.fn(() => mockDockerClientInstance), | ||
| })); | ||
| @@ -66,13 +69,6 @@ | ||
| })), | ||
| })); | ||
|
|
||
| const { mockDockerClientInstance } = vi.hoisted(() => { | ||
| const mock = { | ||
| getEvents: vi.fn(), | ||
| } as unknown as Docker; | ||
| return { mockDockerClientInstance: mock }; | ||
| }); | ||
|
|
||
| // Mock the docker client util | ||
| vi.mock('@app/unraid-api/graph/resolvers/docker/utils/docker-client.js', () => ({ | ||
| getDockerClient: vi.fn().mockReturnValue(mockDockerClientInstance), |
| import { PrefixedID } from '@unraid/shared/prefixed-id-scalar.js'; | ||
| import { GraphQLBigInt, GraphQLJSON, GraphQLPort } from 'graphql-scalars'; | ||
|
|
||
| import { DataSlice } from '@app/unraid-api/types/json-forms.js'; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 9 hours ago
To fix an unused import, the appropriate action is to remove the import line (or the unused specifier from a multi‑symbol import) so that the file only imports what it actually uses. This reduces clutter and avoids misleading future maintainers into thinking the imported symbol is relevant.
In this specific file api/src/unraid-api/graph/resolvers/docker/docker.model.ts, the best fix is to delete the import of DataSlice on line 16:
import { DataSlice } from '@app/unraid-api/types/json-forms.js';No other changes are required, since DataSlice is not referenced anywhere in the provided snippet. We do not need to adjust any existing imports, add new imports, or modify class definitions: the functionality of the file remains exactly the same, just without the unused symbol.
| @@ -13,8 +13,6 @@ | ||
| import { PrefixedID } from '@unraid/shared/prefixed-id-scalar.js'; | ||
| import { GraphQLBigInt, GraphQLJSON, GraphQLPort } from 'graphql-scalars'; | ||
|
|
||
| import { DataSlice } from '@app/unraid-api/types/json-forms.js'; | ||
|
|
||
| export enum ContainerPortType { | ||
| TCP = 'TCP', | ||
| UDP = 'UDP', |
| import Docker from 'dockerode'; | ||
| import { execa } from 'execa'; | ||
|
|
||
| import { AppError } from '@app/core/errors/app-error.js'; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 9 hours ago
- In general, unused imports should be removed to improve readability and avoid confusion about unused dependencies.
- The best fix here is to delete the unused
AppErrorimport line fromapi/src/unraid-api/graph/resolvers/docker/docker.service.ts, leaving all other imports untouched. This does not change runtime behavior because the symbol is never referenced. - Specifically, remove line 8:
import { AppError } from '@app/core/errors/app-error.js';from the import section at the top ofdocker.service.ts. - No additional methods, imports, or definitions are needed; we are only removing an unused import.
| @@ -5,7 +5,6 @@ | ||
| import Docker from 'dockerode'; | ||
| import { execa } from 'execa'; | ||
|
|
||
| import { AppError } from '@app/core/errors/app-error.js'; | ||
| import { pubsub, PUBSUB_CHANNEL } from '@app/core/pubsub.js'; | ||
| import { catchHandlers } from '@app/core/utils/misc/catch-handlers.js'; | ||
| import { sleep } from '@app/core/utils/misc/sleep.js'; |
|
|
||
| // Add bottom-center and top-center position options if not present | ||
| const positionSelectStart = '<select name="position">'; | ||
| const positionSelectEnd = '</select>'; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 9 hours ago
In general, unused variables should be removed to improve readability and avoid confusion. Here, positionSelectEnd is defined but not used anywhere, while the other constants in that block are used. The safest, behavior‑preserving fix is to remove the unused declaration.
Concretely, in api/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.ts, within the applyToSource method, delete the line declaring const positionSelectEnd = '</select>'; and leave the remaining declarations (positionSelectStart, bottomCenterOption, topCenterOption) unchanged. No additional imports, methods, or definitions are needed.
| @@ -37,7 +37,6 @@ | ||
|
|
||
| // Add bottom-center and top-center position options if not present | ||
| const positionSelectStart = '<select name="position">'; | ||
| const positionSelectEnd = '</select>'; | ||
| const bottomCenterOption = | ||
| ' <?=mk_option($notify[\'position\'], "bottom-center", _("bottom-center"))?>'; | ||
| const topCenterOption = ' <?=mk_option($notify[\'position\'], "top-center", _("top-center"))?>'; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/src/components/Common/BaseTreeTable.vue (1)
419-456: Remove development logging artifacts before merge.The drag handle code contains multiple
console.logstatements used for debugging (lines 419-427, 430-442, 445-456). These development artifacts should not be shipped to production.🔎 Proposed cleanup
onMousedown: (e: MouseEvent) => { - console.log('[DragHandle] mousedown', { - rowId: enhancedRow.original.id, - rowName: enhancedRow.original.name, - isBusy, - isDraggingThis, - draggable: !isBusy && !isDraggingThis, - target: e.target, - currentTarget: e.currentTarget, - }); }, onDragstart: (e: DragEvent) => { - console.log('[DragHandle] dragstart fired', { - rowId: enhancedRow.original.id, - rowName: enhancedRow.original.name, - isBusy, - dataTransfer: e.dataTransfer, - }); if (isBusy) { - console.log('[DragHandle] dragstart prevented - row is busy'); e.preventDefault(); return; } handleDragStart(e, enhancedRow.original); - console.log('[DragHandle] handleDragStart called, draggingIds:', draggingIds.value); }, onDragend: (e: DragEvent) => { - console.log('[DragHandle] dragend fired', { - rowId: enhancedRow.original.id, - dropEffect: e.dataTransfer?.dropEffect, - }); handleDragEnd(); }, onDrag: (e: DragEvent) => { - // Log occasionally during drag (throttled by checking if clientX changed significantly) - if (e.clientX % 50 < 5) { - console.log('[DragHandle] dragging...', { x: e.clientX, y: e.clientY }); - } },api/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts (1)
54-79: Critical: Duplicate declaration ofmockDockerClientInstanceand duplicate mocks.There are two declarations of
mockDockerClientInstance:
- Lines 54-56:
const mockDockerClientInstance = { getEvents: vi.fn() }- Lines 69-74:
vi.hoisted(() => { ... })Additionally,
docker-client.jsis mocked twice (lines 57-59 and 77-79). This causes static analysis failures and will lead to unpredictable test behavior.Keep only the hoisted version and remove the duplicate.
🔎 Proposed fix
-// Mock the docker client utility - this is what the service actually uses -const mockDockerClientInstance = { - getEvents: vi.fn(), -}; -vi.mock('./utils/docker-client.js', () => ({ - getDockerClient: vi.fn(() => mockDockerClientInstance), -})); - // Mock DockerService vi.mock('./docker.service.js', () => ({ DockerService: vi.fn().mockImplementation(() => ({ clearContainerCache: vi.fn(), getAppInfo: vi.fn().mockResolvedValue({ info: { apps: { installed: 1, running: 1 } } }), })), })); const { mockDockerClientInstance } = vi.hoisted(() => { const mock = { getEvents: vi.fn(), - } as unknown as Docker; + }; return { mockDockerClientInstance: mock }; }); // Mock the docker client util vi.mock('@app/unraid-api/graph/resolvers/docker/utils/docker-client.js', () => ({ getDockerClient: vi.fn().mockReturnValue(mockDockerClientInstance), }));
🧹 Nitpick comments (24)
web/src/components/Common/MultiValueCopyBadges.vue (2)
186-196: Consider multi-line formatting for better readability.The inline formatting creates very long lines (120+ characters) that are difficult to scan and maintain. Multi-line formatting for components with many attributes is standard practice in Vue codebases and makes it easier to:
- Quickly identify which attributes are being passed
- Review individual attribute changes in git diffs
- Maintain consistent code style
💡 Example of multi-line formatting
- <component :is="UBadge" v-for="entry in inlineEntries" :key="entry.key" :color="getBadgeColor(entry.key)" - :variant="getBadgeVariant(entry.key)" :size="size" :title="getBadgeTitle(entry.key)" - :aria-label="getBadgeAriaLabel(entry.key)" data-stop-row-click="true" role="button" tabindex="0" - class="max-w-[20ch] cursor-pointer items-center gap-1 truncate select-none" @click.stop="handleCopy(entry)" - @keydown="handleBadgeKeydown($event, entry)"> + <component + :is="UBadge" + v-for="entry in inlineEntries" + :key="entry.key" + :color="getBadgeColor(entry.key)" + :variant="getBadgeVariant(entry.key)" + :size="size" + :title="getBadgeTitle(entry.key)" + :aria-label="getBadgeAriaLabel(entry.key)" + data-stop-row-click="true" + role="button" + tabindex="0" + class="max-w-[20ch] cursor-pointer items-center gap-1 truncate select-none" + @click.stop="handleCopy(entry)" + @keydown="handleBadgeKeydown($event, entry)" + > <span class="flex min-w-0 items-center gap-1"> - <component :is="UIcon" v-if="isBadgeCopied(entry.key)" name="i-lucide-check" - class="h-4 w-4 flex-shrink-0 text-white/90" /> + <component + :is="UIcon" + v-if="isBadgeCopied(entry.key)" + name="i-lucide-check" + class="h-4 w-4 flex-shrink-0 text-white/90" + /> <span class="truncate">{{ entry.value }}</span> </span> </component>
208-218: Apply the same multi-line formatting for consistency.The overflow badge rendering has the same inline formatting that reduces readability. Applying multi-line formatting here would maintain consistency with the suggestion for inline entries.
web/src/helpers/functions.ts (2)
7-20: Debug utility looks reasonable, minor simplification possible.The
JSON.parse(JSON.stringify(err))on line 14 could be simplified to justJSON.stringify(err, null, 2)for logging purposes, unless you specifically need the parsed object representation in the console.🔎 Optional simplification
try { - console.log('json:', JSON.parse(JSON.stringify(err))); + console.log('json:', JSON.stringify(err, null, 2)); } catch { console.log('json:', 'failed to parse'); - console.log('json:', err); }
46-47: Redundant type assertions and non-null assertion can be simplified.The
networkErroris already typed as{ message?: string }on line 27, making the repeated casts unnecessary. The non-null assertion on line 47 can be avoided.As per coding guidelines, casting should be avoided when possible.
🔎 Proposed simplification
- } else if ((e?.networkError as { message?: string })?.message) { - message = (e.networkError as { message?: string }).message!; + } else if (e?.networkError?.message) { + message = e.networkError.message; } else if (api/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.ts (1)
45-45: Consider more robust duplicate detection.The current approach uses exact string matching to check if content already exists (e.g., checking for
bottomCenterOptionliteral at line 45 and'_(Stack notifications)_:'at line 80). While this works for known Unraid file formats, it's fragile to whitespace or formatting variations. Consider using more flexible patterns (e.g., checking for key identifiers like"bottom-center"orname="expand") to make the duplicate detection more resilient.Also applies to: 80-80
web/scripts/deploy-dev.sh (1)
16-33: Consider adding logging for better debugging.The retry logic is correct, but consider logging retry attempts to help diagnose transient failures during deployment.
🔎 Proposed enhancement with logging
# Simple retry helper: retry <attempts> <delay_seconds> <command...> retry() { local attempts="$1"; shift local delay_seconds="$1"; shift local try=1 while true; do "$@" local exit_code=$? if [ $exit_code -eq 0 ]; then return 0 fi if [ $try -ge $attempts ]; then + echo "Command failed after $attempts attempts: $*" >&2 return $exit_code fi + echo "Attempt $try failed (exit code: $exit_code), retrying in ${delay_seconds}s..." >&2 sleep "$delay_seconds" try=$((try + 1)) done }api/src/unraid-api/unraid-file-modifier/modifications/default-cfg.modification.ts (1)
27-33: Remove excessive comments per coding guidelines.The comments at lines 27-33, 41-42, and 50-51 explain what the code does rather than why, which is evident from the code itself. Per the coding guidelines: "Never add comments unless they are needed for clarity of function" and "Never add comments for obvious things."
🔎 Proposed cleanup
- // Target: [notify] section - // We want to insert: - // expand="true" - // duration="5000" - // max="3" - // - // Inserting after [notify] line seems safest. - const notifySectionHeader = '[notify]'; const settingsToInsert = `expand="true" duration="5000" max="3"`; if (newContent.includes(notifySectionHeader)) { - // Check if already present to avoid duplicates (idempotency) - // Using a simple check for 'expand="true"' might be enough, or rigorous regex if (!newContent.includes('expand="true"')) {} else { - // If [notify] missing, append it? - // Unlikely for default.cfg, but let's append at end if missing newContent += `\n${notifySectionHeader}\n${settingsToInsert}\n`; }As per coding guidelines.
Also applies to: 41-42, 50-51
api/src/unraid-api/organizer/organizer.ts (1)
221-234: ParametericonMapis accepted but unused.The
iconMapparameter is added toresolveOrganizerViewandresolveOrganizerbut is never used in the function body. If this is intentional scaffolding for future work, consider adding a TODO comment. Otherwise, defer adding the parameter until it's actually needed.api/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts (1)
88-93: Remove unusedgetDockerClientfrom mock implementation.The
mockDockerServiceImplincludesgetDockerClientbutDockerServicedoesn't expose this method—it's an internal detail. This mock property is unused and should be removed for clarity.🔎 Proposed fix
const mockDockerServiceImpl = { - getDockerClient: vi.fn(), clearContainerCache: vi.fn(), getAppInfo: vi.fn().mockResolvedValue({ info: { apps: { installed: 1, running: 1 } } }), };api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts (1)
53-57: InjectedDockerTemplateIconServiceis unused.The service is injected but never used in any method. If this is scaffolding for upcoming functionality, consider deferring the injection until it's needed, or add a TODO indicating planned usage.
api/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts (1)
23-196: Consider adding validation for critical string replacements.This modification performs many
string.replace()operations. If the source file doesn't match expected patterns (e.g., upstream changes), replacements silently fail and the patch may be incomplete or break functionality. Consider validating that critical replacements succeeded.🔎 Example validation pattern
function replaceOrThrow(content: string, search: string, replacement: string, description: string): string { if (!content.includes(search)) { throw new Error(`Failed to find pattern for ${description}`); } return content.replace(search, replacement); }api/src/unraid-api/unraid-file-modifier/modifications/default-base-css.modification.spec.ts (1)
41-42: Mock setup is unused in this test.The
vi.mocked(readFile).mockResolvedValue(inputCss)on line 42 is never used becauseapplyToSourceis called directly withinputCssrather than going throughgeneratePatch. This mock can be removed from this specific test.🔎 Suggested cleanup
- // Mock readFile to return our inputCss - vi.mocked(readFile).mockResolvedValue(inputCss); - // @ts-expect-error accessing private method const result = modification.applyToSource(inputCss);api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts (1)
182-197: Good fixture management pattern.The
ensureFixtureExistshelper downloads fixtures on-demand with proper error handling. This avoids committing large fixture files while ensuring tests have the required data.Consider adding a timeout to the fetch call to prevent tests from hanging if GitHub is unreachable.
🔎 Optional: Add fetch timeout
- const response = await fetch(testCase.fileUrl); + const response = await fetch(testCase.fileUrl, { signal: AbortSignal.timeout(30000) });api/src/unraid-api/unraid-file-modifier/modifications/default-base-css.modification.ts (1)
70-78: Comment numbering inconsistency.The numbered comments skip from "2." to "4." (line 75 → 77). This appears to be a minor oversight.
🔎 Fix comment numbering
// 1. .Theme--sidebar definition e.g. .Theme--sidebar { .replace(/(\.Theme--sidebar)(\s*\{)/g, ':scope$1$2') // 2. .Theme--sidebar #displaybox .replace(/(\.Theme--sidebar)(\s+#displaybox)/g, ':scope$1$2') - // 4. .Theme--width-boxed #displaybox + // 3. .Theme--width-boxed #displaybox .replace(/(\.Theme--width-boxed)(\s+#displaybox)/g, ':scope$1$2');api/src/unraid-api/unraid-file-modifier/modifications/default-azure-css.modification.ts (1)
32-54: Consider extracting shared CSS scoping logic to a base class.The
applyToSourcemethod is identical across multiple CSS modifications (DefaultAzure, DefaultBlack, DefaultGray, DefaultWhite, DefaultBase). Extracting this to a shared abstract base class (e.g.,CssBodyScopeModification) would reduce duplication and centralize maintenance.Additionally,
source.indexOf('}', bodyOpenBraceIndex)finds the first closing brace after the body opening, which can fail if the body block contains nested braces (e.g., media queries). For simple body blocks this works, but consider a brace-counting approach if the CSS structure is ever more complex.api/src/unraid-api/unraid-file-modifier/modifications/default-black-css.modification.ts (1)
26-54: Duplicate implementation across CSS modifications.This entire file (generatePatch + applyToSource) is a near-copy of
DefaultAzureCssModification, differing only in theidandfilePath. Consider introducing a shared base class to encapsulate the common logic.🔎 Example shared base class approach
// css-scope-modification.base.ts export abstract class CssScopeModification extends FileModification { protected async generatePatch(overridePath?: string): Promise<string> { const fileContent = await readFile(this.filePath, 'utf-8'); const newContent = this.applyToSource(fileContent); return this.createPatchWithDiff(overridePath ?? this.filePath, fileContent, newContent); } protected applyToSource(source: string): string { // shared body-scoping logic } } // default-black-css.modification.ts export default class DefaultBlackCssModification extends CssScopeModification { id = 'default-black-css-modification'; public readonly filePath = '/usr/local/emhttp/plugins/dynamix/styles/default-black.css'; }web/src/components/RClone/RCloneOverview.vue (1)
39-55: Switch to useToast for delete feedback is solid; consider adding onError handlingUsing
const toast = useToast()andtoast.add(...)for both the success path and the “deleteRCloneRemote returned false” case gives clear user feedback and avoids reliance on a globalwindow.toast. You might optionally also register anonErrorhandler on this mutation to surface network/GraphQL failures (which won’t hitonDeleteDone) so users see a message if the delete request itself fails.web/src/components/ApiKeyAuthorize.standalone.vue (1)
7-8: Centralizing navigation via navigate() is good; align import with ESM.jsconventionReplacing direct
window.location.hrefwrites indenyandreturnToAppwith the sharednavigate(url)helper keeps all external redirects going through one place and makes this logic easier to test and evolve.To stay consistent with the project’s ESM import guideline and with the other imports in this file (e.g.,
useAuthorizationLink.js,useClipboardWithToast.js,useApiKeyStore.js), consider changing the new import to:import { navigate } from '~/helpers/external-navigation.js';so you’re always using
.jsextensions from TypeScript.Also applies to: 93-103, 107-113
web/src/composables/useDockerEditNavigation.ts (1)
2-2: Inconsistent import path alias.This file uses
@/helpers/external-navigationwhile other files in this PR use~/helpers/external-navigation. Consider standardizing on one alias for consistency across the codebase.🔎 Suggested fix for consistency
-import { navigate } from '@/helpers/external-navigation'; +import { navigate } from '~/helpers/external-navigation';web/src/components/HeaderOsVersion.standalone.vue (1)
42-59: Theme-specific styling fix looks reasonable.The implementation correctly handles header overlap issues for Azure/Gray themes in Unraid 7.0. The version check (
version > 0 && version < 7.1) safely handles cases where version parsing might fail.Optional: Consider more robust version parsing
The current
parseFloat(osVersion.value || '0')approach works for simple versions but would fail on pre-release versions like "7.0.0-beta1". If pre-release versions need to be supported in the future, consider using a more robust version comparison library.However, the current guard (
version > 0) ensures the code won't break on invalid versions, making this a low-priority enhancement.web/src/components/Notifications/constants.ts (1)
15-23: Consider the semantic mapping of INFO to 'success'.The
NOTIFICATION_TOAST_COLORSmapping uses'success'forINFOimportance notifications. While this may be intentional for UI consistency (green for informational messages), it could be semantically confusing since "success" and "info" typically represent different message types in most design systems.If the toast component supports an
'info'variant, consider:export const NOTIFICATION_TOAST_COLORS: Record< NotificationImportance, 'error' | 'warning' | 'info' | 'success' > = { [NotificationImportance.ALERT]: 'error', [NotificationImportance.WARNING]: 'warning', - [NotificationImportance.INFO]: 'success', + [NotificationImportance.INFO]: 'info', };However, if this mapping is intentional for your design system (e.g., showing info notifications in green), the current implementation is acceptable.
web/src/components/Wrapper/mount-engine.ts (2)
177-189: Simplify position mapping logic.The position mapping can be simplified using a more concise approach without the explicit map object.
🔎 Proposed refactor
if (fetchedSettings.position) { - const map: Record<string, ToastPosition> = { - 'top-left': 'top-left', - 'top-right': 'top-right', - 'bottom-left': 'bottom-left', - 'bottom-right': 'bottom-right', - 'bottom-center': 'bottom-center', - 'top-center': 'top-center', - }; - const mappedPosition = map[fetchedSettings.position]; - if (mappedPosition) { - toasterSettings.position = mappedPosition; - } + const validPositions: ToastPosition[] = [ + 'top-left', 'top-right', 'bottom-left', + 'bottom-right', 'top-center', 'bottom-center' + ]; + if (validPositions.includes(fetchedSettings.position as ToastPosition)) { + toasterSettings.position = fetchedSettings.position as ToastPosition; + } }This reduces the redundant identity mapping and makes the validation more explicit.
168-171: Consider caching notification settings.The
network-onlyfetch policy means settings are fetched on every mount, which could impact performance. If notification settings don't change frequently, consider usingcache-firstor implementing a short-lived cache.For example:
fetchPolicy: 'cache-first',This would use cached settings when available while still allowing updates when settings change server-side.
web/src/components/Notifications/List.vue (1)
86-90: Remove debug console.log statements before merging.Multiple
console.logandconsole.warnstatements are present for debugging. These should be removed or converted to a debug utility that can be disabled in production.Also applies to: 107-108, 140-141, 163-164, 186-188, 193-193, 200-201, 221-227, 230-230
| state: { | ||
| type: 'string', | ||
| title: 'State', | ||
| enum: ['RUNNING', 'EXITED'], | ||
| }, |
There was a problem hiding this comment.
Missing PAUSED state in enum.
The state enum only includes ['RUNNING', 'EXITED'], but ContainerState (from docker.model.ts) also defines PAUSED. This could cause validation issues or UI inconsistencies when a container is paused.
🔎 Proposed fix
state: {
type: 'string',
title: 'State',
- enum: ['RUNNING', 'EXITED'],
+ enum: ['RUNNING', 'PAUSED', 'EXITED'],
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| state: { | |
| type: 'string', | |
| title: 'State', | |
| enum: ['RUNNING', 'EXITED'], | |
| }, | |
| state: { | |
| type: 'string', | |
| title: 'State', | |
| enum: ['RUNNING', 'PAUSED', 'EXITED'], | |
| }, |
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/docker/docker-form.service.ts around lines
70 to 74, the state schema enum only lists ['RUNNING','EXITED'] while
ContainerState also defines 'PAUSED'; update the enum to include 'PAUSED' so
validation/UI accepts paused containers, and run/adjust any affected type checks
or UI form logic that relies on this schema.
| import { type Layout } from '@jsonforms/core'; | ||
| import { Node } from '@unraid/shared/graphql.model.js'; | ||
| import { PrefixedID } from '@unraid/shared/prefixed-id-scalar.js'; | ||
| import { GraphQLBigInt, GraphQLJSON, GraphQLPort } from 'graphql-scalars'; | ||
|
|
||
| import { DataSlice } from '@app/unraid-api/types/json-forms.js'; | ||
|
|
There was a problem hiding this comment.
Remove unused imports.
Layout and DataSlice are imported but not used in this file. Static analysis confirms DataSlice is unused. These types may be intended for future use but should be added when actually needed.
🔎 Proposed fix
-import { type Layout } from '@jsonforms/core';
import { Node } from '@unraid/shared/graphql.model.js';
import { PrefixedID } from '@unraid/shared/prefixed-id-scalar.js';
import { GraphQLBigInt, GraphQLJSON, GraphQLPort } from 'graphql-scalars';
-
-import { DataSlice } from '@app/unraid-api/types/json-forms.js';
-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { type Layout } from '@jsonforms/core'; | |
| import { Node } from '@unraid/shared/graphql.model.js'; | |
| import { PrefixedID } from '@unraid/shared/prefixed-id-scalar.js'; | |
| import { GraphQLBigInt, GraphQLJSON, GraphQLPort } from 'graphql-scalars'; | |
| import { DataSlice } from '@app/unraid-api/types/json-forms.js'; | |
| import { Node } from '@unraid/shared/graphql.model.js'; | |
| import { PrefixedID } from '@unraid/shared/prefixed-id-scalar.js'; | |
| import { GraphQLBigInt, GraphQLJSON, GraphQLPort } from 'graphql-scalars'; |
🧰 Tools
🪛 GitHub Check: CodeQL
[notice] 16-16: Unused variable, import, function or class
Unused import DataSlice.
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/docker/docker.model.ts around lines 11 to
17, the imports "Layout" and "DataSlice" are unused; remove these two imports
from the top of the file (delete "import { type Layout } from
'@jsonforms/core';" and "import { DataSlice } from
'@app/unraid-api/types/json-forms.js;") so only actually used symbols remain
imported, then run the linter/TS compile to confirm no remaining unused-import
warnings.
| import Docker from 'dockerode'; | ||
| import { execa } from 'execa'; | ||
|
|
||
| import { AppError } from '@app/core/errors/app-error.js'; |
There was a problem hiding this comment.
Remove unused import.
AppError is imported but never used in this file. Static analysis confirms this is dead code.
🔎 Proposed fix
-import { AppError } from '@app/core/errors/app-error.js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { AppError } from '@app/core/errors/app-error.js'; |
🧰 Tools
🪛 GitHub Check: CodeQL
[notice] 8-8: Unused variable, import, function or class
Unused import AppError.
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/docker/docker.service.ts around line 8,
the import "AppError" from '@app/core/errors/app-error.js' is unused; remove
that import statement (or delete only the AppError specifier if multiple imports
exist on the same line) and ensure the file still compiles and lints cleanly by
running the TypeScript compiler and your linter after the change.
| async shouldApply({ | ||
| checkOsVersion = true, | ||
| }: { checkOsVersion?: boolean } = {}): Promise<ShouldApplyWithReason> { | ||
| // Apply ONLY if version < 7.1.0 | ||
| if (await this.isUnraidVersionLessThanOrEqualTo('7.1.0', { includePrerelease: false })) { | ||
| return super.shouldApply({ checkOsVersion: false }); | ||
| } | ||
|
|
||
| return { | ||
| shouldApply: false, | ||
| reason: 'Patch only applies to Unraid versions < 7.1.0', | ||
| }; | ||
| } |
There was a problem hiding this comment.
Inconsistency between comment/message and actual version check.
The comment on line 15 and the reason on line 22 both state < 7.1.0, but isUnraidVersionLessThanOrEqualTo includes version 7.1.0 itself. Clarify which is intended.
🔎 If the intent is to include 7.1.0, update the comments
- // Apply ONLY if version < 7.1.0
+ // Apply ONLY if version <= 7.1.0- reason: 'Patch only applies to Unraid versions < 7.1.0',
+ reason: 'Patch only applies to Unraid versions <= 7.1.0',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async shouldApply({ | |
| checkOsVersion = true, | |
| }: { checkOsVersion?: boolean } = {}): Promise<ShouldApplyWithReason> { | |
| // Apply ONLY if version < 7.1.0 | |
| if (await this.isUnraidVersionLessThanOrEqualTo('7.1.0', { includePrerelease: false })) { | |
| return super.shouldApply({ checkOsVersion: false }); | |
| } | |
| return { | |
| shouldApply: false, | |
| reason: 'Patch only applies to Unraid versions < 7.1.0', | |
| }; | |
| } | |
| async shouldApply({ | |
| checkOsVersion = true, | |
| }: { checkOsVersion?: boolean } = {}): Promise<ShouldApplyWithReason> { | |
| // Apply ONLY if version <= 7.1.0 | |
| if (await this.isUnraidVersionLessThanOrEqualTo('7.1.0', { includePrerelease: false })) { | |
| return super.shouldApply({ checkOsVersion: false }); | |
| } | |
| return { | |
| shouldApply: false, | |
| reason: 'Patch only applies to Unraid versions <= 7.1.0', | |
| }; | |
| } |
🤖 Prompt for AI Agents
In
api/src/unraid-api/unraid-file-modifier/modifications/default-azure-css.modification.ts
around lines 12 to 24, the inline comment and the returned reason say "< 7.1.0"
but the code calls isUnraidVersionLessThanOrEqualTo('7.1.0'), which includes
7.1.0; make them consistent by either changing the function call to exclude
7.1.0 (use a strict less-than check) or—preferably—update the comment and the
reason to state "<= 7.1.0" (or "≤ 7.1.0") and mention includePrerelease: false
if relevant so the text accurately reflects the implemented logic.
| async shouldApply({ | ||
| checkOsVersion = true, | ||
| }: { checkOsVersion?: boolean } = {}): Promise<ShouldApplyWithReason> { | ||
| // Apply ONLY if version < 7.1.0 | ||
| if (await this.isUnraidVersionLessThanOrEqualTo('7.1.0', { includePrerelease: false })) { | ||
| return super.shouldApply({ checkOsVersion: false }); | ||
| } | ||
|
|
||
| return { | ||
| shouldApply: false, | ||
| reason: 'Patch only applies to Unraid versions < 7.1.0', | ||
| }; | ||
| } |
There was a problem hiding this comment.
Same comment/message vs code discrepancy as in DefaultAzureCssModification.
The comment and reason string say < 7.1.0 but isUnraidVersionLessThanOrEqualTo includes 7.1.0. Align these with the intended behavior.
🔎 If the intent is to include 7.1.0, update the comments
- // Apply ONLY if version < 7.1.0
+ // Apply ONLY if version <= 7.1.0- reason: 'Patch only applies to Unraid versions < 7.1.0',
+ reason: 'Patch only applies to Unraid versions <= 7.1.0',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async shouldApply({ | |
| checkOsVersion = true, | |
| }: { checkOsVersion?: boolean } = {}): Promise<ShouldApplyWithReason> { | |
| // Apply ONLY if version < 7.1.0 | |
| if (await this.isUnraidVersionLessThanOrEqualTo('7.1.0', { includePrerelease: false })) { | |
| return super.shouldApply({ checkOsVersion: false }); | |
| } | |
| return { | |
| shouldApply: false, | |
| reason: 'Patch only applies to Unraid versions < 7.1.0', | |
| }; | |
| } | |
| async shouldApply({ | |
| checkOsVersion = true, | |
| }: { checkOsVersion?: boolean } = {}): Promise<ShouldApplyWithReason> { | |
| // Apply ONLY if version <= 7.1.0 | |
| if (await this.isUnraidVersionLessThanOrEqualTo('7.1.0', { includePrerelease: false })) { | |
| return super.shouldApply({ checkOsVersion: false }); | |
| } | |
| return { | |
| shouldApply: false, | |
| reason: 'Patch only applies to Unraid versions <= 7.1.0', | |
| }; | |
| } |
| import { gql } from '@apollo/client'; | ||
|
|
||
| export const GET_DOCKER_CONTAINER_OVERVIEW_FORM = gql` | ||
| query GetDockerContainerOverviewForm($skipCache: Boolean = false) { | ||
| dockerContainerOverviewForm(skipCache: $skipCache) { | ||
| id | ||
| dataSchema | ||
| uiSchema | ||
| data | ||
| } | ||
| } | ||
| `; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use graphql helper for type generation consistency.
Similar to other .query.ts files in the codebase, this should use the graphql helper from ~/composables/gql/gql instead of gql from @apollo/client to ensure proper type generation and consistency with the established pattern.
🔎 Proposed fix
-import { gql } from '@apollo/client';
+import { graphql } from '~/composables/gql/gql';
-export const GET_DOCKER_CONTAINER_OVERVIEW_FORM = gql`
+export const GET_DOCKER_CONTAINER_OVERVIEW_FORM = graphql(/* GraphQL */ `
query GetDockerContainerOverviewForm($skipCache: Boolean = false) {
dockerContainerOverviewForm(skipCache: $skipCache) {
id
dataSchema
uiSchema
data
}
}
-`;
+`);After making this change, run pnpm codegen in the web directory to regenerate types.
As per coding guidelines, GraphQL queries must use the graphql helper for proper code generation support.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { gql } from '@apollo/client'; | |
| export const GET_DOCKER_CONTAINER_OVERVIEW_FORM = gql` | |
| query GetDockerContainerOverviewForm($skipCache: Boolean = false) { | |
| dockerContainerOverviewForm(skipCache: $skipCache) { | |
| id | |
| dataSchema | |
| uiSchema | |
| data | |
| } | |
| } | |
| `; | |
| import { graphql } from '~/composables/gql/gql.js'; | |
| export const GET_DOCKER_CONTAINER_OVERVIEW_FORM = graphql(/* GraphQL */ ` | |
| query GetDockerContainerOverviewForm($skipCache: Boolean = false) { | |
| dockerContainerOverviewForm(skipCache: $skipCache) { | |
| id | |
| dataSchema | |
| uiSchema | |
| data | |
| } | |
| } | |
| `); |
🤖 Prompt for AI Agents
In web/src/components/Docker/docker-overview.query.ts lines 1-12, the file
imports and uses gql from @apollo/client which breaks the repository's type
generation pattern; replace the import with the graphql helper from
~/composables/gql/gql and wrap the query with graphql(...) instead of gql`...`,
then run pnpm codegen in the web directory to regenerate types and verify the
generated types match usage.
| export const getNotificationSettings = gql` | ||
| query GetNotificationSettings { | ||
| notifications { | ||
| id | ||
| settings { | ||
| position | ||
| expand | ||
| duration | ||
| max | ||
| } | ||
| } | ||
| } | ||
| `; |
There was a problem hiding this comment.
Use graphql helper for type generation consistency.
This query uses gql from graphql-tag, while all other queries in this file use the graphql helper from ~/composables/gql/gql. This inconsistency prevents proper type generation and breaks the established pattern.
🔎 Proposed fix
-export const getNotificationSettings = gql`
+export const getNotificationSettings = graphql(/* GraphQL */ `
query GetNotificationSettings {
notifications {
id
settings {
position
expand
duration
max
}
}
}
-`;
+`);Also remove the unused import:
-import gql from 'graphql-tag';
-
import { graphql } from '~/composables/gql/gql';After making this change, run pnpm codegen in the web directory to regenerate types.
As per coding guidelines, GraphQL queries must use the graphql helper for proper code generation support.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/src/components/Notifications/graphql/notification.query.ts around lines
39 to 51, replace the usage of gql from graphql-tag with the project's graphql
helper (imported from ~/composables/gql/gql) to match other queries and enable
proper type generation; remove the now-unused graphql-tag import; then run pnpm
codegen in the web directory to regenerate types.
| <div v-if="loading" class="grid place-content-center py-3"> | ||
| <LoadingSpinner /> | ||
| <!-- 3 skeletons to replace shadcn's LoadingSpinner --> | ||
| <div v-if="loading" class="space-y-4 py-3"> | ||
| <div v-for="n in 3" :key="n" class="py-3"> | ||
| <div class="flex items-center gap-2"> | ||
| <USkeleton class="size-5 rounded-full" /> | ||
| <USkeleton class="h-4 w-40" /> | ||
| <div class="ml-auto"> | ||
| <USkeleton class="h-3 w-24" /> | ||
| </div> | ||
| </div> | ||
| <div class="mt-2"> | ||
| <USkeleton class="h-3 w-3/4" /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Redundant nested loading check.
The outer <div v-if="loading"> at line 288 already guards this block, but there's another <div v-if="loading"> at line 290 inside it. Remove the redundant inner check.
🔎 Proposed fix
<div v-if="loading" class="grid place-content-center py-3">
- <!-- 3 skeletons to replace shadcn's LoadingSpinner -->
- <div v-if="loading" class="space-y-4 py-3">
+ <div class="space-y-4 py-3">
<div v-for="n in 3" :key="n" class="py-3">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div v-if="loading" class="grid place-content-center py-3"> | |
| <LoadingSpinner /> | |
| <!-- 3 skeletons to replace shadcn's LoadingSpinner --> | |
| <div v-if="loading" class="space-y-4 py-3"> | |
| <div v-for="n in 3" :key="n" class="py-3"> | |
| <div class="flex items-center gap-2"> | |
| <USkeleton class="size-5 rounded-full" /> | |
| <USkeleton class="h-4 w-40" /> | |
| <div class="ml-auto"> | |
| <USkeleton class="h-3 w-24" /> | |
| </div> | |
| </div> | |
| <div class="mt-2"> | |
| <USkeleton class="h-3 w-3/4" /> | |
| </div> | |
| </div> | |
| </div> | |
| </div> | |
| <div v-if="loading" class="grid place-content-center py-3"> | |
| <div class="space-y-4 py-3"> | |
| <div v-for="n in 3" :key="n" class="py-3"> | |
| <div class="flex items-center gap-2"> | |
| <USkeleton class="size-5 rounded-full" /> | |
| <USkeleton class="h-4 w-40" /> | |
| <div class="ml-auto"> | |
| <USkeleton class="h-3 w-24" /> | |
| </div> | |
| </div> | |
| <div class="mt-2"> | |
| <USkeleton class="h-3 w-3/4" /> | |
| </div> | |
| </div> | |
| </div> | |
| </div> |
🤖 Prompt for AI Agents
In web/src/components/Notifications/List.vue around lines 288 to 304, there is a
redundant v-if="loading" on the inner div (line ~290) while the outer div
already has v-if="loading"; remove the inner v-if attribute so the skeleton
markup is only conditionally rendered once by the outer wrapper, keeping the
same structure and classes otherwise to preserve layout and behavior.
| import { defineConfig } from 'vite'; | ||
| import removeConsole from 'vite-plugin-remove-console'; | ||
|
|
||
| import appConfig from './app.config'; |
There was a problem hiding this comment.
Missing .js extension in import statement.
Per the coding guidelines, TypeScript imports should include .js extensions for ESM compatibility.
🔎 Proposed fix
-import appConfig from './app.config';
+import appConfig from './app.config.js';Based on coding guidelines, always use TypeScript imports with .js extensions for ESM compatibility.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import appConfig from './app.config'; | |
| import appConfig from './app.config.js'; |
🤖 Prompt for AI Agents
In web/vite.config.ts around line 11, the import statement "import appConfig
from './app.config';" is missing the required .js extension for ESM-compatible
TypeScript imports; update the import to include the .js extension (import from
'./app.config.js'), ensure any TypeScript build/tsconfig settings still resolve
the file, and run a quick build to verify no import resolution errors.
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (12)
web/src/components/Notifications/CriticalNotifications.standalone.vue (1)
126-143: Consider displaying user-facing error message when dismiss fails.The current error handling logs to console but doesn't inform the user when the dismiss action fails. Based on learnings, when archive mutations fail, an error message should be shown below the action buttons to inform the user.
Consider adding a reactive error state to display dismissal failures to the user.
Based on learnings from previous reviews.
🔎 Suggested implementation
const { mutate: archiveNotificationMutate } = useMutation(archiveNotification); const dismissing = reactive(new Set<string>()); +const dismissErrors = reactive(new Map<string, string>()); const dismissNotification = async (notification: NotificationFragmentFragment) => { if (dismissing.has(notification.id)) { return; } + dismissErrors.delete(notification.id); dismissing.add(notification.id); try { await archiveNotificationMutate({ id: notification.id, }); criticalNotifications.value = criticalNotifications.value.filter( (current) => current.id !== notification.id ); } catch (dismissError) { console.error('[CriticalNotifications] Failed to dismiss notification', dismissError); + dismissErrors.set(notification.id, 'Failed to dismiss notification. Please try again.'); } finally { dismissing.delete(notification.id); } };Then in the template, after the action buttons:
{{ dismissing.has(notification.id) ? 'Dismissing…' : 'Dismiss' }} </button> </div> + <p v-if="dismissErrors.has(notification.id)" class="text-sm text-red-600"> + {{ dismissErrors.get(notification.id) }} + </p> </li>plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix/Notifications.page (4)
25-59: Consider refactoring to reduce repetition.The function contains significant repetition when computing composite flags and disabling checkboxes. A loop-based approach would be more maintainable.
🔎 Proposed refactoring
function prepareNotify(form) { - form.entity.value = form.normal1.checked | form.warning1.checked | form.alert1.checked; - form.normal.value = form.normal1.checked*1 + form.normal2.checked*2 + form.normal3.checked*4; - form.warning.value = form.warning1.checked*1 + form.warning2.checked*2 + form.warning3.checked*4; - form.alert.value = form.alert1.checked*1 + form.alert2.checked*2 + form.alert3.checked*4; - form.unraid.value = form.unraid1.checked*1 + form.unraid2.checked*2 + form.unraid3.checked*4; - form.plugin.value = form.plugin1.checked*1 + form.plugin2.checked*2 + form.plugin3.checked*4; - form.docker_notify.value = form.docker_notify1.checked*1 + form.docker_notify2.checked*2 + form.docker_notify3.checked*4; - form.language_notify.value = form.language_notify1.checked*1 + form.language_notify2.checked*2 + form.language_notify3.checked*4; - form.report.value = form.report1.checked*1 + form.report2.checked*2 + form.report3.checked*4; - form.normal1.disabled = true; - form.normal2.disabled = true; - form.normal3.disabled = true; - form.warning1.disabled = true; - form.warning2.disabled = true; - form.warning3.disabled = true; - form.alert1.disabled = true; - form.alert2.disabled = true; - form.alert3.disabled = true; - form.unraid1.disabled = true; - form.unraid2.disabled = true; - form.unraid3.disabled = true; - form.plugin1.disabled = true; - form.plugin2.disabled = true; - form.plugin3.disabled = true; - form.docker_notify1.disabled = true; - form.docker_notify2.disabled = true; - form.docker_notify3.disabled = true; - form.language_notify1.disabled = true; - form.language_notify2.disabled = true; - form.language_notify3.disabled = true; - form.report1.disabled = true; - form.report2.disabled = true; - form.report3.disabled = true; + const categories = ['normal', 'warning', 'alert', 'unraid', 'plugin', 'docker_notify', 'language_notify', 'report']; + + // Compute entity flag + form.entity.value = form.normal1.checked | form.warning1.checked | form.alert1.checked; + + // Compute composite flags and disable checkboxes + categories.forEach(function(category) { + form[category].value = (form[category + '1'].checked ? 1 : 0) + + (form[category + '2'].checked ? 2 : 0) + + (form[category + '3'].checked ? 4 : 0); + form[category + '1'].disabled = true; + form[category + '2'].disabled = true; + form[category + '3'].disabled = true; + }); }
60-62: Use strict equality and consider more specific selector.Line 61 uses loose equality (
==) and a generic.checkboxselector that could unintentionally affect other checkboxes.🔎 Proposed improvements
function prepareSystem(index) { - if (index==0) $('.checkbox').attr('disabled','disabled'); else $('.checkbox').removeAttr('disabled'); + if (index === 0) { + $('input[type="checkbox"].checkbox').attr('disabled', 'disabled'); + } else { + $('input[type="checkbox"].checkbox').removeAttr('disabled'); + } }
72-91: Consider consolidating duplicate show/hide functions.The five
prepare*functions (prepareUnraid, preparePlugin, prepareDocker, prepareLanguage, prepareReport) share identical logic and could be consolidated.🔎 Proposed refactoring
-function prepareUnraid(value) { - if (value=='') $('.unraid').hide(); else $('.unraid').show(); - prepareTitle(); -} -function preparePlugin(value) { - if (value=='') $('.plugin').hide(); else $('.plugin').show(); - prepareTitle(); -} -function prepareDocker(value) { - if (value=='') $('.docker').hide(); else $('.docker').show(); - prepareTitle(); -} -function prepareLanguage(value) { - if (value=='') $('.language').hide(); else $('.language').show(); - prepareTitle(); -} -function prepareReport(value) { - if (value=='') $('.report').hide(); else $('.report').show(); - prepareTitle(); -} +function prepareSection(sectionClass, value) { + if (value === '') { + $('.' + sectionClass).hide(); + } else { + $('.' + sectionClass).show(); + } + prepareTitle(); +} +function prepareUnraid(value) { prepareSection('unraid', value); } +function preparePlugin(value) { prepareSection('plugin', value); } +function prepareDocker(value) { prepareSection('docker', value); } +function prepareLanguage(value) { prepareSection('language', value); } +function prepareReport(value) { prepareSection('report', value); }
115-144: Ensure consistent null coalescing for all settings.The
expand,duration, andmaxfields use the null coalescing operator (??) for defaults, butposition(lines 117-122) does not. Consider adding a default for consistency.🔎 Proposed improvement
_(Display position)_: : <select name="position"> - <?=mk_option($notify['position'], "top-left", _("top-left"))?> - <?=mk_option($notify['position'], "top-right", _("top-right"))?> - <?=mk_option($notify['position'], "bottom-left", _("bottom-left"))?> - <?=mk_option($notify['position'], "bottom-right", _("bottom-right"))?> - <?=mk_option($notify['position'], "bottom-center", _("bottom-center"))?> - <?=mk_option($notify['position'], "top-center", _("top-center"))?> + <?=mk_option($notify['position'] ?? 'top-right', "top-left", _("top-left"))?> + <?=mk_option($notify['position'] ?? 'top-right', "top-right", _("top-right"))?> + <?=mk_option($notify['position'] ?? 'top-right', "bottom-left", _("bottom-left"))?> + <?=mk_option($notify['position'] ?? 'top-right', "bottom-right", _("bottom-right"))?> + <?=mk_option($notify['position'] ?? 'top-right', "bottom-center", _("bottom-center"))?> + <?=mk_option($notify['position'] ?? 'top-right', "top-center", _("top-center"))?> </select>plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix/include/Notify.php (1)
17-17: Consider renaming the $notify variable after modification.The
$notifyvariable starts as the script path (line 17) but gets arguments appended in the loop (line 38), then" add"is appended (line 46). While functional, this dual-purpose use could be clearer with a distinct variable name like$notifyCmdfor the built command.🔎 Optional clarity improvement
$notify = "$docroot/webGui/scripts/notify"; +$notifyCmd = $notify; switch ($_POST['cmd']??'') { ... case 'add': foreach ($_POST as $option => $value) { switch ($option) { case 'e': case 's': case 'd': case 'i': case 'm': case 'u': - $notify .= " -{$option} ".escapeshellarg($value); + $notifyCmd .= " -{$option} ".escapeshellarg($value); break; case 'x': case 't': - $notify .= " -{$option}"; + $notifyCmd .= " -{$option}"; break; } } - shell_exec("$notify add"); + shell_exec("$notifyCmd add"); break;Also applies to: 38-38, 46-46
web/src/composables/gql/graphql.ts (2)
1690-1712: NotificationSettings GraphQL shape and wiring look consistent
NotificationSettingsplus the non-nullnotifications.settingsfield andGetNotificationSettingsquery/document are all structurally consistent with the existing schema style and notification queries (fragments, overview, warningsAndAlerts). This should give the UI a clean, typed way to pull toaster configuration without impacting existing notification flows.You may want to document (in schema/docs, not here) the expected value space for
position(top-right, etc.) so downstream UI code can treat it as a constrained enum rather than a free-form string. Based on learnings, this ties into the unraid-toaster positioning config.Also applies to: 3191-3195, 3410-3410
3068-3074: Docker folder rename and single-container update mutations are correctly typedThe new
RenameDockerFolderMutation*andUpdateDockerContainerMutation*types and documents line up with the existingMutationRenameDockerFolderArgsandMutationUpdateContainerArgsdefinitions (stringfolderId/newName,PrefixedIDforid) and follow the same shape as the other Docker organizer/update mutations.No issues from the typing/codegen side.
If you find yourself duplicating the
ResolvedOrganizerV1selection set across multiple mutations, consider extracting a reusable fragment in the.graphqlsources to keep future changes DRY.Also applies to: 3135-3140, 3393-3393, 3403-3403
api/src/unraid-api/unraid-file-modifier/modifications/patches/notify-script.patch (2)
23-33: Filename sanitization and INI encode/decode helpers are solid and safer than the previous ad‑hoc format
safe_filename($string, $maxLength = 255)now:
- Drops dangerous characters (including path separators),
- Allows periods so real
.notifyextensions survive,- Normalizes spaces/hyphens to
_, and- Truncates to a path-segment–safe length.
This significantly reduces the risk of invalid or overlong filenames while still keeping them human-readable.
ini_encode_value,build_ini_string, andini_decode_valuegive you a proper, reversible encoding for fields that may contain spaces,=or quotes, which addresses edge cases the previouskey=valuelines could not safely handle.One minor caveat:
substroperates on bytes, so multibyte UTF‑8 characters can be cut mid-sequence when truncating, but since the filesystem treats names as byte strings and these filenames are primarily for internal use, that’s acceptable in practice.If you ever need user-facing display of these filenames, consider applying truncation earlier at the UI/API level, where you could be multibyte-aware.
Also applies to: 44-73
155-168: Robust read-path for notification files using boundedexplodeand INI decodingUpdating the
getcase to:
- Limit
explode('=', $field, 2)ensures values containing=remain intact.- Use
ini_decode_valuebefore assigning into$output[$i][…]keeps the JSON output compatible with pre-change consumers (they still see plain strings) while allowing you to safely store escaped/quoted values in the files.The
timestamphandling (first field per file) still works as before because you transform it into a formatted string before decoding, so there’s no behavioral regression there.If you later introduce booleans or numeric types that matter to the caller, consider special-casing those keys on the read side to cast them back to native PHP types before JSON encoding.
api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/notify.modified.snapshot (1)
1-333: Snapshot matches the patchednotifyscript and provides good regression coverageThe
notify.modified.snapshotcontent lines up with the changes innotify-script.patch:
- Updated usage with
-u.- New
safe_filename($string, $maxLength), INI helpers, and filename construction.- INI-style archive/unread writes and INI-aware reads.
This should give you solid regression coverage for the modification pipeline (downloaded file → apply patch → compare to snapshot). Just remember to update this snapshot whenever you tweak the patch or when the base
notifyscript changes upstream.If you later extend the notify format again (e.g., add new fields to the stored INI data), I can help draft updated fixtures and snapshots to keep the tests in lockstep.
api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/Notify.php.modified.snapshot.php (1)
39-42: Formatting: Remove unnecessary blank line.The empty line 42 between the 'u' case and the 'x' case is unnecessary and inconsistent with the formatting of other cases in the switch statement.
🔎 Proposed fix
case 'u': $notify .= " -{$option} ".escapeshellarg($value); break; - case 'x': case 't': $notify .= " -{$option}";
| table.array_status tr>td{padding-left:8px;white-space:normal} | ||
| table.array_status tr>td:nth-child(1){width:33%} | ||
| table.array_status tr>td:nth-child(2){width:22%} | ||
| able.array_status.noshift{margin-top:0} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and get line 141 content
if [ -f "api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/default-azure.css" ]; then
echo "=== File found. Line 141 content ==="
sed -n '141p' "api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/default-azure.css"
echo ""
echo "=== Context around line 141 (lines 138-143) ==="
sed -n '138,143p' "api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/default-azure.css"
else
echo "File not found at specified path"
# List what directories exist
find api/src/unraid-api -type d -name "__fixtures__" 2>/dev/null
fiRepository: unraid/api
Length of output: 491
🏁 Script executed:
# Search for any occurrences of "able" in CSS selectors that might be typos
echo "=== Searching for 'able.array_status' pattern ==="
rg "able\.array_status" --type css -n
echo ""
echo "=== Searching for similar patterns to understand context ==="
rg "array_status" --type css -n | head -20Repository: unraid/api
Length of output: 7975
🏁 Script executed:
# Check if there are any tests or comments about this fixture that explain its purpose
echo "=== Looking for test files related to this fixture ==="
find api/src/unraid-api/unraid-file-modifier/modifications -name "*test*" -o -name "*spec*" | head -10
echo ""
echo "=== Check for any README or documentation about fixtures ==="
find api/src/unraid-api/unraid-file-modifier/modifications/__test__ -type f \( -name "README*" -o -name "*.md" \) 2>/dev/nullRepository: unraid/api
Length of output: 417
Fix typo in CSS selector: able should be table
Line 141 contains a typo: able.array_status.noshift should be table.array_status.noshift. This is inconsistent with the same selector in all other theme fixtures (default-white.css, default-base.css, default-gray.css, default-black.css), which correctly use table.array_status.noshift.
🧰 Tools
🪛 Biome (2.1.2)
[error] 140-141: Unknown type selector is not allowed.
See MDN web docs for more details.
Consider replacing the unknown type selector with valid one.
(lint/correctness/noUnknownTypeSelector)
🤖 Prompt for AI Agents
In
api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/default-azure.css
around line 141, the CSS selector has a typo: "able.array_status.noshift" should
be "table.array_status.noshift"; replace the misspelled selector with the
correct "table.array_status.noshift" to match the other theme fixtures and
ensure the rule targets table elements as intended.
| $timestamp = time(); | ||
| $ticket = $timestamp; | ||
| $mailtest = false; | ||
| $overrule = false; | ||
| $noBrowser = false; | ||
| + $customFilename = false; | ||
|
|
||
| - $options = getopt("l:e:s:d:i:m:r:xtb"); | ||
| + $options = getopt("l:e:s:d:i:m:r:u:xtb"); | ||
| foreach ($options as $option => $value) { | ||
| switch ($option) { | ||
| case 'e': | ||
| $event = $value; | ||
| break; | ||
| @@ -213,19 +248,53 @@ | ||
| case 'l': | ||
| $nginx = (array)@parse_ini_file('/var/local/emhttp/nginx.ini'); | ||
| $link = $value; | ||
| $fqdnlink = (strpos($link,"http") === 0) ? $link : ($nginx['NGINX_DEFAULTURL']??'').$link; | ||
| break; | ||
| + case 'u': | ||
| + $customFilename = $value; | ||
| + break; | ||
| } | ||
| } | ||
|
|
||
| - $unread = "{$unread}/".safe_filename("{$event}-{$ticket}.notify"); | ||
| - $archive = "{$archive}/".safe_filename("{$event}-{$ticket}.notify"); | ||
| + if ($customFilename) { | ||
| + $filename = safe_filename($customFilename); | ||
| + } else { | ||
| + // suffix length: _{timestamp}.notify = 1+10+7 = 18 chars. | ||
| + $suffix = "_{$ticket}.notify"; | ||
| + $max_name_len = 255 - strlen($suffix); | ||
| + // sanitize event, truncating it to leave room for suffix | ||
| + $clean_name = safe_filename($event, $max_name_len); | ||
| + // construct filename with suffix (underscore separator matches safe_filename behavior) | ||
| + $filename = "{$clean_name}{$suffix}"; | ||
| + } | ||
| + | ||
| + $unread = "{$unread}/{$filename}"; | ||
| + $archive = "{$archive}/{$filename}"; |
There was a problem hiding this comment.
New -u custom filename flow works but relies on callers providing a .notify suffix
The new -u option and filename logic look correct overall:
getopt("l:e:s:d:i:m:r:u:xtb")cleanly introduces the-uargument.- When
-uis provided,$filename = safe_filename($customFilename)protects against path traversal and invalid characters. - When
-uis not provided, you now:- Build a suffix like
_<timestamp>.notify, - Compute
max_name_len = 255 - strlen($suffix), - Sanitize the
$eventinto$clean_namewith that max length, and - Append the suffix, guaranteeing a final basename within 255 characters and preserving the
.notifyextension.
- Build a suffix like
This also finally makes glob("$unread/*.notify") consistent with how filenames are created in the default path.
The only sharp edge is:
- If a caller passes
-uwithout including.notifyin the value, those files won’t match the*.notifyglob and will never be surfaced bynotify get.
Given the help text labels -u as “API use only”, that’s probably fine as long as your API layer always appends .notify explicitly.
If you want to make this more foolproof, you could normalize the custom name to ensure it ends with .notify (e.g., append it if missing) before passing it through safe_filename.
🤖 Prompt for AI Agents
In
api/src/unraid-api/unraid-file-modifier/modifications/patches/notify-script.patch
around lines 82 to 123, the new -u custom filename flow accepts arbitrary names
but relies on callers to include the ".notify" extension, which breaks
glob("*.notify") if omitted; before calling safe_filename on the provided custom
name, normalize it by appending ".notify" when missing (ensure you check
case-insensitively and only append if not already present), then pass the
normalized name to safe_filename and use that for $unread and $archive.
| */ | ||
| ?> | ||
| <? | ||
| $events = explode('|', $notify['events'] ?? ''); |
There was a problem hiding this comment.
Remove unused variable.
The $events variable is assigned but never used in the file.
🔎 Proposed fix
-$events = explode('|', $notify['events'] ?? '');
$disabled = $notify['system'] ? '' : 'disabled';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $events = explode('|', $notify['events'] ?? ''); | |
| $disabled = $notify['system'] ? '' : 'disabled'; |
🤖 Prompt for AI Agents
In
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix/Notifications.page
around line 21, the $events variable is assigned but never used; remove the
unused assignment line ($events = explode('|', $notify['events'] ?? '');) or, if
the intention was to use the parsed events, update the surrounding code to
reference $events where needed (e.g., iterate or check it) and ensure
$notify['events'] is handled safely.
| <span id="unraidTitle" class="unraid" style="display:none"> </span> | ||
| : <span class="unraid" style="display:none"> | ||
| <span class="notifications-checkbox-group-title">_(Unraid OS update)_</span> | ||
| <span class="inline-block"> | ||
| <label class="inline-block"> | ||
| <input type="checkbox" name="unraid1"<?=($notify['unraid'] & 1)==1 ? ' checked' : ''?>>_(Browser)_ | ||
| </label> | ||
| <label class="inline-block"> | ||
| <input type="checkbox" name="unraid2"<?=($notify['unraid'] & 2)==2 ? ' checked' : ''?>>_(Email)_ | ||
| </label> | ||
| <label class="inline-block"> | ||
| <input type="checkbox" name="unraid3"<?=($notify['unraid'] & 4)==4 ? ' checked' : ''?>>_(Agents)_ | ||
| </label> | ||
| </span> | ||
| </span> | ||
|
|
||
| <span id="pluginTitle" class="plugin" style="display:none"> </span> | ||
| : <span class="plugin" style="display:none"> | ||
| <span class="notifications-checkbox-group-title">_(Plugins update)_</span> | ||
| <span class="inline-block"> | ||
| <label class="inline-block"> | ||
| <input type="checkbox" name="plugin1"<?=($notify['plugin'] & 1)==1 ? ' checked' : ''?>>_(Browser)_ | ||
| </label> | ||
| <label class="inline-block"> | ||
| <input type="checkbox" name="plugin2"<?=($notify['plugin'] & 2)==2 ? ' checked' : ''?>>_(Email)_ | ||
| </label> | ||
| <label class="inline-block"> | ||
| <input type="checkbox" name="plugin3"<?=($notify['plugin'] & 4)==4 ? ' checked' : ''?>>_(Agents)_ | ||
| </label> | ||
| </span> | ||
| </span> | ||
|
|
||
| <span id="dockerTitle" class="docker" style="display:none"> </span> | ||
| : <span class="docker" style="display:none"> | ||
| <span class="notifications-checkbox-group-title">_(Docker update)_</span> | ||
| <span class="inline-block"> | ||
| <label class="inline-block"> | ||
| <input type="checkbox" name="docker_notify1"<?=($notify['docker_notify'] & 1)==1 ? ' checked' : ''?>>_(Browser)_ | ||
| </label> | ||
| <label class="inline-block"> | ||
| <input type="checkbox" name="docker_notify2"<?=($notify['docker_notify'] & 2)==2 ? ' checked' : ''?>>_(Email)_ | ||
| </label> | ||
| <label class="inline-block"> | ||
| <input type="checkbox" name="docker_notify3"<?=($notify['docker_notify'] & 4)==4 ? ' checked' : ''?>>_(Agents)_ | ||
| </label> | ||
| </span> | ||
| </span> | ||
|
|
||
| <span id="languageTitle" class="language" style="display:none"> </span> | ||
| : <span class="language" style="display:none"> | ||
| <span class="notifications-checkbox-group-title">_(Language update)_</span> | ||
| <span class="inline-block"> | ||
| <label class="inline-block"> | ||
| <input type="checkbox" name="language_notify1"<?=($notify['language_notify'] & 1)==1 ? ' checked' : ''?>>_(Browser)_ | ||
| </label> | ||
| <label class="inline-block"> | ||
| <input type="checkbox" name="language_notify2"<?=($notify['language_notify'] & 2)==2 ? ' checked' : ''?>>_(Email)_ | ||
| </label> | ||
| <label class="inline-block"> | ||
| <input type="checkbox" name="language_notify3"<?=($notify['language_notify'] & 4)==4 ? ' checked' : ''?>>_(Agents)_ | ||
| </label> | ||
| </span> | ||
| </span> | ||
|
|
||
| <span id="reportTitle" class="report" style="display:none"> </span> | ||
| : <span class="report" style="display:none"> | ||
| <span class="notifications-checkbox-group-title">_(Array status)_</span> | ||
| <span> | ||
| <span> | ||
| <input type="checkbox" name="report1"<?=($notify['report'] & 1)==1 ? ' checked' : ''?>>_(Browser)_ | ||
| </span> | ||
| <span> | ||
| <input type="checkbox" name="report2"<?=($notify['report'] & 2)==2 ? ' checked' : ''?>>_(Email)_ | ||
| </span> | ||
| <span> | ||
| <input type="checkbox" name="report3"<?=($notify['report'] & 4)==4 ? ' checked' : ''?>>_(Agents)_ | ||
| </span> | ||
| </span> | ||
| </span> | ||
| </span> | ||
|
|
||
| :notifications_agent_selection_help: |
There was a problem hiding this comment.
Standardize checkbox group HTML structure.
The report notification section (lines 306-316) has inconsistent HTML structure compared to other sections. It's missing <label> elements wrapping the checkboxes, which affects accessibility and may impact styling.
🔎 Proposed fix
<span id="reportTitle" class="report" style="display:none"> </span>
: <span class="report" style="display:none">
<span class="notifications-checkbox-group-title">_(Array status)_</span>
- <span>
- <span>
- <input type="checkbox" name="report1"<?=($notify['report'] & 1)==1 ? ' checked' : ''?>>_(Browser)_
- </span>
- <span>
- <input type="checkbox" name="report2"<?=($notify['report'] & 2)==2 ? ' checked' : ''?>>_(Email)_
- </span>
- <span>
- <input type="checkbox" name="report3"<?=($notify['report'] & 4)==4 ? ' checked' : ''?>>_(Agents)_
- </span>
- </span>
+ <span class="inline-block">
+ <label class="inline-block">
+ <input type="checkbox" name="report1"<?=($notify['report'] & 1)==1 ? ' checked' : ''?>>_(Browser)_
+ </label>
+ <label class="inline-block">
+ <input type="checkbox" name="report2"<?=($notify['report'] & 2)==2 ? ' checked' : ''?>>_(Email)_
+ </label>
+ <label class="inline-block">
+ <input type="checkbox" name="report3"<?=($notify['report'] & 4)==4 ? ' checked' : ''?>>_(Agents)_
+ </label>
</span>
</span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <span id="unraidTitle" class="unraid" style="display:none"> </span> | |
| : <span class="unraid" style="display:none"> | |
| <span class="notifications-checkbox-group-title">_(Unraid OS update)_</span> | |
| <span class="inline-block"> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="unraid1"<?=($notify['unraid'] & 1)==1 ? ' checked' : ''?>>_(Browser)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="unraid2"<?=($notify['unraid'] & 2)==2 ? ' checked' : ''?>>_(Email)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="unraid3"<?=($notify['unraid'] & 4)==4 ? ' checked' : ''?>>_(Agents)_ | |
| </label> | |
| </span> | |
| </span> | |
| <span id="pluginTitle" class="plugin" style="display:none"> </span> | |
| : <span class="plugin" style="display:none"> | |
| <span class="notifications-checkbox-group-title">_(Plugins update)_</span> | |
| <span class="inline-block"> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="plugin1"<?=($notify['plugin'] & 1)==1 ? ' checked' : ''?>>_(Browser)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="plugin2"<?=($notify['plugin'] & 2)==2 ? ' checked' : ''?>>_(Email)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="plugin3"<?=($notify['plugin'] & 4)==4 ? ' checked' : ''?>>_(Agents)_ | |
| </label> | |
| </span> | |
| </span> | |
| <span id="dockerTitle" class="docker" style="display:none"> </span> | |
| : <span class="docker" style="display:none"> | |
| <span class="notifications-checkbox-group-title">_(Docker update)_</span> | |
| <span class="inline-block"> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="docker_notify1"<?=($notify['docker_notify'] & 1)==1 ? ' checked' : ''?>>_(Browser)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="docker_notify2"<?=($notify['docker_notify'] & 2)==2 ? ' checked' : ''?>>_(Email)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="docker_notify3"<?=($notify['docker_notify'] & 4)==4 ? ' checked' : ''?>>_(Agents)_ | |
| </label> | |
| </span> | |
| </span> | |
| <span id="languageTitle" class="language" style="display:none"> </span> | |
| : <span class="language" style="display:none"> | |
| <span class="notifications-checkbox-group-title">_(Language update)_</span> | |
| <span class="inline-block"> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="language_notify1"<?=($notify['language_notify'] & 1)==1 ? ' checked' : ''?>>_(Browser)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="language_notify2"<?=($notify['language_notify'] & 2)==2 ? ' checked' : ''?>>_(Email)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="language_notify3"<?=($notify['language_notify'] & 4)==4 ? ' checked' : ''?>>_(Agents)_ | |
| </label> | |
| </span> | |
| </span> | |
| <span id="reportTitle" class="report" style="display:none"> </span> | |
| : <span class="report" style="display:none"> | |
| <span class="notifications-checkbox-group-title">_(Array status)_</span> | |
| <span> | |
| <span> | |
| <input type="checkbox" name="report1"<?=($notify['report'] & 1)==1 ? ' checked' : ''?>>_(Browser)_ | |
| </span> | |
| <span> | |
| <input type="checkbox" name="report2"<?=($notify['report'] & 2)==2 ? ' checked' : ''?>>_(Email)_ | |
| </span> | |
| <span> | |
| <input type="checkbox" name="report3"<?=($notify['report'] & 4)==4 ? ' checked' : ''?>>_(Agents)_ | |
| </span> | |
| </span> | |
| </span> | |
| </span> | |
| :notifications_agent_selection_help: | |
| <span id="unraidTitle" class="unraid" style="display:none"> </span> | |
| : <span class="unraid" style="display:none"> | |
| <span class="notifications-checkbox-group-title">_(Unraid OS update)_</span> | |
| <span class="inline-block"> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="unraid1"<?=($notify['unraid'] & 1)==1 ? ' checked' : ''?>>_(Browser)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="unraid2"<?=($notify['unraid'] & 2)==2 ? ' checked' : ''?>>_(Email)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="unraid3"<?=($notify['unraid'] & 4)==4 ? ' checked' : ''?>>_(Agents)_ | |
| </label> | |
| </span> | |
| </span> | |
| <span id="pluginTitle" class="plugin" style="display:none"> </span> | |
| : <span class="plugin" style="display:none"> | |
| <span class="notifications-checkbox-group-title">_(Plugins update)_</span> | |
| <span class="inline-block"> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="plugin1"<?=($notify['plugin'] & 1)==1 ? ' checked' : ''?>>_(Browser)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="plugin2"<?=($notify['plugin'] & 2)==2 ? ' checked' : ''?>>_(Email)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="plugin3"<?=($notify['plugin'] & 4)==4 ? ' checked' : ''?>>_(Agents)_ | |
| </label> | |
| </span> | |
| </span> | |
| <span id="dockerTitle" class="docker" style="display:none"> </span> | |
| : <span class="docker" style="display:none"> | |
| <span class="notifications-checkbox-group-title">_(Docker update)_</span> | |
| <span class="inline-block"> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="docker_notify1"<?=($notify['docker_notify'] & 1)==1 ? ' checked' : ''?>>_(Browser)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="docker_notify2"<?=($notify['docker_notify'] & 2)==2 ? ' checked' : ''?>>_(Email)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="docker_notify3"<?=($notify['docker_notify'] & 4)==4 ? ' checked' : ''?>>_(Agents)_ | |
| </label> | |
| </span> | |
| </span> | |
| <span id="languageTitle" class="language" style="display:none"> </span> | |
| : <span class="language" style="display:none"> | |
| <span class="notifications-checkbox-group-title">_(Language update)_</span> | |
| <span class="inline-block"> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="language_notify1"<?=($notify['language_notify'] & 1)==1 ? ' checked' : ''?>>_(Browser)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="language_notify2"<?=($notify['language_notify'] & 2)==2 ? ' checked' : ''?>>_(Email)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="language_notify3"<?=($notify['language_notify'] & 4)==4 ? ' checked' : ''?>>_(Agents)_ | |
| </label> | |
| </span> | |
| </span> | |
| <span id="reportTitle" class="report" style="display:none"> </span> | |
| : <span class="report" style="display:none"> | |
| <span class="notifications-checkbox-group-title">_(Array status)_</span> | |
| <span class="inline-block"> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="report1"<?=($notify['report'] & 1)==1 ? ' checked' : ''?>>_(Browser)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="report2"<?=($notify['report'] & 2)==2 ? ' checked' : ''?>>_(Email)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" name="report3"<?=($notify['report'] & 4)==4 ? ' checked' : ''?>>_(Agents)_ | |
| </label> | |
| </span> | |
| </span> | |
| :notifications_agent_selection_help: |
🤖 Prompt for AI Agents
In
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix/Notifications.page
around lines 239 to 320, the "report" (Array status) checkbox group uses bare
input elements without <label> wrappers unlike the other groups; update the
report section so each checkbox is wrapped in a <label class="inline-block">
containing the <input ...><?=... ? ' checked' : ''?> followed by the localized
text and spacing, and match the surrounding span structure (use the same
inline-block classes and markup pattern used by unraid/plugin/docker/language
groups) to restore consistent HTML structure, accessibility, and styling.
| | ||
| : <span> | ||
| <span class="notifications-checkbox-group-title">_(Alerts)_</span> | ||
| <span> | ||
| <span> | ||
| <input type="checkbox" class="checkbox" name="alert1"<?=($notify['alert'] & 1)==1 ? " checked $disabled" : $disabled?>>_(Browser)_ | ||
| </span> | ||
| <span> | ||
| <input type="checkbox" class="checkbox" name="alert2"<?=($notify['alert'] & 2)==2 ? " checked $disabled" : $disabled?>>_(Email)_ | ||
| </span> | ||
| <span> | ||
| <input type="checkbox" class="checkbox" name="alert3"<?=($notify['alert'] & 4)==4 ? " checked $disabled" : $disabled?>>_(Agents)_ | ||
| </span> | ||
| </span> | ||
|
|
There was a problem hiding this comment.
Standardize Alerts checkbox group HTML structure.
The Alerts notification section has inconsistent HTML structure compared to Notices and Warnings sections. It's missing <label> elements and inline-block classes.
🔎 Proposed fix
: <span>
<span class="notifications-checkbox-group-title">_(Alerts)_</span>
- <span>
- <span>
- <input type="checkbox" class="checkbox" name="alert1"<?=($notify['alert'] & 1)==1 ? " checked $disabled" : $disabled?>>_(Browser)_
- </span>
- <span>
- <input type="checkbox" class="checkbox" name="alert2"<?=($notify['alert'] & 2)==2 ? " checked $disabled" : $disabled?>>_(Email)_
- </span>
- <span>
- <input type="checkbox" class="checkbox" name="alert3"<?=($notify['alert'] & 4)==4 ? " checked $disabled" : $disabled?>>_(Agents)_
+ <span class="inline-block">
+ <label class="inline-block">
+ <input type="checkbox" class="checkbox" name="alert1"<?=($notify['alert'] & 1)==1 ? " checked $disabled" : $disabled?>>_(Browser)_
+ </label>
+ <label class="inline-block">
+ <input type="checkbox" class="checkbox" name="alert2"<?=($notify['alert'] & 2)==2 ? " checked $disabled" : $disabled?>>_(Email)_
+ </label>
+ <label class="inline-block">
+ <input type="checkbox" class="checkbox" name="alert3"<?=($notify['alert'] & 4)==4 ? " checked $disabled" : $disabled?>>_(Agents)_
+ </label>
</span>
</span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | |
| : <span> | |
| <span class="notifications-checkbox-group-title">_(Alerts)_</span> | |
| <span> | |
| <span> | |
| <input type="checkbox" class="checkbox" name="alert1"<?=($notify['alert'] & 1)==1 ? " checked $disabled" : $disabled?>>_(Browser)_ | |
| </span> | |
| <span> | |
| <input type="checkbox" class="checkbox" name="alert2"<?=($notify['alert'] & 2)==2 ? " checked $disabled" : $disabled?>>_(Email)_ | |
| </span> | |
| <span> | |
| <input type="checkbox" class="checkbox" name="alert3"<?=($notify['alert'] & 4)==4 ? " checked $disabled" : $disabled?>>_(Agents)_ | |
| </span> | |
| </span> | |
| | |
| : <span> | |
| <span class="notifications-checkbox-group-title">_(Alerts)_</span> | |
| <span class="inline-block"> | |
| <label class="inline-block"> | |
| <input type="checkbox" class="checkbox" name="alert1"<?=($notify['alert'] & 1)==1 ? " checked $disabled" : $disabled?>>_(Browser)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" class="checkbox" name="alert2"<?=($notify['alert'] & 2)==2 ? " checked $disabled" : $disabled?>>_(Email)_ | |
| </label> | |
| <label class="inline-block"> | |
| <input type="checkbox" class="checkbox" name="alert3"<?=($notify['alert'] & 4)==4 ? " checked $disabled" : $disabled?>>_(Agents)_ | |
| </label> | |
| </span> | |
| </span> |
🤖 Prompt for AI Agents
In
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix/Notifications.page
around lines 354-368, the Alerts checkbox group lacks the standardized HTML
structure used by Notices and Warnings: missing <label> wrappers and missing the
inline-block class on the checkbox container. Update each input to be paired
with a <label> that contains the input and text (so clicking text toggles the
box), add the same inline-block class to the surrounding span/container as other
sections, and ensure the checked/disabled PHP logic remains on the input
element; keep markup and spacing consistent with the other notification groups.
| onClick: () => { | ||
| if (notification.link) { | ||
| navigate(notification.link); | ||
| } | ||
| }, |
There was a problem hiding this comment.
Inconsistent navigation approach for notification links.
The toast action uses navigate(notification.link) (line 169), but the template uses a regular <a> tag with target="_blank" (lines 248-256). This creates an inconsistency in how notification links are opened.
Consider using the same navigation approach in both places for consistency. If navigate is intended for external link handling, apply it to both the toast action and the template link.
🔎 Suggested approach for consistent navigation
Option 1: Use navigate in both places:
- <a
- v-if="notification.link"
- :href="notification.link"
- class="inline-flex items-center gap-1 rounded-md border border-amber-500 px-3 py-1 text-sm font-medium text-amber-700 transition hover:bg-amber-50"
- target="_blank"
- rel="noreferrer"
- >
- View Details
- </a>
+ <button
+ v-if="notification.link"
+ type="button"
+ class="inline-flex items-center gap-1 rounded-md border border-amber-500 px-3 py-1 text-sm font-medium text-amber-700 transition hover:bg-amber-50"
+ @click="navigate(notification.link)"
+ >
+ View Details
+ </button>Option 2: Use standard <a> tags in both places (depending on what navigate does).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/src/components/Notifications/CriticalNotifications.standalone.vue around
lines 167-171, the toast action calls navigate(notification.link) while the
template uses a plain <a target="_blank"> (lines ~248-256), causing inconsistent
link behavior; pick a single navigation strategy and apply it in both
places—either call the same navigate(notification.link) helper from the template
link click handler (and ensure it opens in a new tab if that's required by the
current UX and preserves rel="noopener noreferrer") or replace the toast action
to use a standard anchor open (window.open or native link behavior) so both
places behave identically; update the template and the toast onClick accordingly
and add a null-check for notification.link before invoking navigation.
| export type GetDockerActiveContainerQueryVariables = Exact<{ | ||
| id: Scalars['PrefixedID']['input']; | ||
| }>; | ||
|
|
||
|
|
||
| export type GetDockerActiveContainerQuery = { __typename?: 'Query', docker: { __typename?: 'Docker', id: string, containers: Array<{ __typename?: 'DockerContainer', id: string, names: Array<string>, image: string, created: number, state: ContainerState, status: string, autoStart: boolean, networkSettings?: any | null, labels?: any | null, ports: Array<{ __typename?: 'ContainerPort', privatePort?: number | null, publicPort?: number | null, type: ContainerPortType }>, hostConfig?: { __typename?: 'ContainerHostConfig', networkMode: string } | null }> } }; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the actual GraphQL document definition for GetDockerActiveContainer
rg -A 10 "GetDockerActiveContainerDocument" web/src/composables/gql/Repository: unraid/api
Length of output: 43982
🏁 Script executed:
# Also search for the GraphQL query definition itself
rg -B 5 -A 15 "query GetDockerActiveContainer" web/src/composables/gql/Repository: unraid/api
Length of output: 32640
🏁 Script executed:
# Check DockerContainerManagement.vue usage
rg -B 5 -A 5 "GetDockerActiveContainer" web/src/components/ | head -100Repository: unraid/api
Length of output: 790
🏁 Script executed:
# Look at line 3381 to see what other issue applies
sed -n '3375,3390p' web/src/composables/gql/graphql.tsRepository: unraid/api
Length of output: 33656
🏁 Script executed:
sed -n '3375,3390p' web/src/composables/gql/graphql.tsRepository: unraid/api
Length of output: 33656
🏁 Script executed:
# Search for GraphQL schema or validation rules configuration
find web -name "*.graphql" -o -name "*schema*" -o -name "*graphql.config*" | head -20Repository: unraid/api
Length of output: 36
🏁 Script executed:
# Look for Apollo client configuration and any validation rules
rg -l "apollo|graphql" web/src --type ts --type tsx | xargs rg -l "validate|rule" | head -10Repository: unraid/api
Length of output: 352
🏁 Script executed:
# Check how GetDockerActiveContainerDocument is actually used
rg "GetDockerActiveContainer" web/src/components --type ts --type tsx -B 3 -A 3Repository: unraid/api
Length of output: 81
🏁 Script executed:
# Also check for NoUnusedVariables or validation rules in codegen config
rg -i "unused|validation|nounused" web/ --type ts --type js --type json | grep -i "rule\|valid"Repository: unraid/api
Length of output: 4498
🏁 Script executed:
# Get the exact line count to understand line numbers better
wc -l web/src/composables/gql/graphql.tsRepository: unraid/api
Length of output: 93
🏁 Script executed:
# Search for what's around 3381 in a different way
rg -n "export const" web/src/composables/gql/graphql.ts | grep -E "^(338|339|340|341)"Repository: unraid/api
Length of output: 50366
🏁 Script executed:
# Search for similar query pattern with unused variables
rg -A 3 "variableDefinitions.*\[\{" web/src/composables/gql/graphql.ts | head -50Repository: unraid/api
Length of output: 50366
🏁 Script executed:
# Look for codegen config that might control validation rules
find web -name "codegen.ts" -o -name "codegen.js" -o -name ".graphqlconfig*" | xargs cat 2>/dev/nullRepository: unraid/api
Length of output: 888
🏁 Script executed:
# Search for the actual source GraphQL files before codegen
find web -name "*.graphql" -o -path "*queries*" -name "*.ts" | xargs rg "GetDockerActiveContainer" 2>/dev/null | head -20Repository: unraid/api
Length of output: 36
Remove the unused $id variable from GetDockerActiveContainer query
The query declares $id: PrefixedID! but never uses it. The operation fetches all containers via docker { id containers { ... } } without filtering by the variable. Either integrate the variable into the query logic (e.g., docker { container(id: $id) { ... } }) or remove it entirely. Most GraphQL servers enforce the NoUnusedVariables validation rule, so this operation will fail validation.
🤖 Prompt for AI Agents
In web/src/composables/gql/graphql.ts around lines 2976 to 2982, the
GetDockerActiveContainer GraphQL operation declares a variable $id: PrefixedID!
that is never used; remove the unused $id variable from the operation signature
or, if the intent is to fetch a specific container, modify the query to pass $id
into the selection (e.g., call the appropriate field that accepts an id
argument) and update the TypeScript types accordingly so the operation no longer
violates NoUnusedVariables validation.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.