fix: notification display and functionality + nuxtui refactor #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
- refactor: centralized unraidApiStatus management to store - fix(web): ssr-safety for navigator.language in Item.vue - fix(web): prevent 'just now' flash on missing timestamps - fix(web): remove unsafe error re-throw in infinite scroll - fix(web): guard toast navigation actions in Sidebar - feat(web): add error handling & feedback for bulk actions - chore(web): externalize loading string to i18n - fix(api): transactional error handling for notification stats - refactor(api): remove any casts in date formatting - chore(api): remove debug logs from notify-script modification - test: update snapshots and fix server store tests
…build was working yesterday and i didn't need to make these changes" This reverts commit 5c742e4.
- unraid-ui: Remove unused variables in utils.test.ts to satisfy `vue-tsc`. - web: Add safety check for `document` availability in ResizableSlideover.vue to prevent SSR errors.
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI Agents
In @api/src/unraid-api/config/api-config.module.ts:
- Around line 32-50: The try/catch currently swallows all errors and can hide
failures from apiHandler.loadConfig and apiHandler.writeConfigFile; change it to
log errors and avoid letting writeConfigFile failures discard a successfully
loaded diskConfig: call apiHandler.loadConfig() inside the try as you do, but
wrap the writeConfigFile(...) call in its own try/catch that logs the error (use
the module's logger) so a failing write does not trigger the outer fallback; in
the outer catch log the caught exception (include error details) before
returning defaultConfig; reference apiHandler.loadConfig,
isConnectPluginInstalled, apiHandler.writeConfigFile, defaultConfig and
API_VERSION to locate the relevant logic.
- Line 39: The diskConfig variable is typed Partial<ApiConfig> but is being
unsafely cast to ApiConfig when calling apiHandler.writeConfigFile(diskConfig as
ApiConfig); ensure the config is complete before writing by either merging
diskConfig with a default ApiConfig (e.g., defaultConfig) and passing the merged
result to apiHandler.writeConfigFile, or add a runtime type guard/validation
function that checks required fields on diskConfig and only casts and writes
after validation; update the call site to use the merged/validated object
instead of the direct cast.
In @api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts:
- Around line 539-548: The unlink(unreadPath) call inside the block that checks
await fileExists(archivePath) needs a try/catch and rollback like the rename
handling: perform the unlink in a try block, and if it throws, restore the
unread stats you decremented earlier (i.e., call the same
NotificationsService.decrement/restore logic for notification.importance and
snapshot.unread if applicable) and rethrow or log the error; only after a
successful unlink should you increment archive stats via
this.increment(notification.importance, NotificationsService.overview.archive)
and snapshot.archive. Ensure you reference the existing symbols unreadPath,
archivePath, notification.importance, snapshot, unlink, and
NotificationsService.increment/decrement so the failure leaves stats consistent.
In @web/src/components/Notifications/List.vue:
- Around line 288-304: The inner redundant v-if="loading" on the nested <div>
should be removed because the parent wrapper already guards rendering; locate
the nested <div class="space-y-4 py-3"> (the one that currently has
v-if="loading") in the Notifications List component and delete the
v-if="loading" attribute so only the outer container retains the conditional.
🧹 Nitpick comments (8)
web/src/locales/en.json (1)
297-297: LGTM! Clear and consistent localization key.The new loading message follows the established naming pattern and integrates well with the notification system refactor.
Optional: Consider standardizing ellipsis usage across the file. Some keys use the ellipsis character "…" (U+2026) while others use three periods "...". For example, line 34 (
common.loading2) and line 445 (updateOs.changelogModal.loadingChangelog) use "…", while this line and others (lines 253, 274, 479) use "...". Standardizing on the Unicode ellipsis character would improve typographic consistency.plugin/plugins/dynamix.unraid.net.plg (1)
417-440: Indentation inconsistency in restore block.Lines 417-419 use 4-space indentation while the surrounding code uses 6-space indentation. This appears to be a copy-paste issue that breaks the visual consistency of the shell script.
🔎 Proposed fix for indentation
if [ -d "$DIR-" ]; then - mv "$DIR-" "$DIR" - echo "Restored directory: $DIR- to $DIR" - fi + mv "$DIR-" "$DIR" + echo "Restored directory: $DIR- to $DIR" + fiThe version-based cache cleanup logic is well-implemented—using PHP's
version_comparefor semantic version checking and defaulting to cache cleanup when version is unknown is the safe approach.api/src/unraid-api/config/api-config.test.ts (2)
197-202: Consider improving type safety of the mock.The
as anycast on line 202 suppresses type checking. Consider defining a proper type or interface for the file handler mock to catch potential mismatches between the mock and actual implementation.🔎 Proposed fix
+type FileHandlerMock = { + loadConfig: ReturnType<typeof vi.fn>; + readConfigFile: ReturnType<typeof vi.fn>; + writeConfigFile: ReturnType<typeof vi.fn>; + updateConfig: ReturnType<typeof vi.fn>; +}; + beforeEach(async () => { vi.clearAllMocks(); vi.spyOn(ApiConfigPersistence.prototype, 'getFileHandler').mockReturnValue({ loadConfig: vi.fn().mockResolvedValue({}), readConfigFile: vi.fn().mockResolvedValue({}), writeConfigFile: vi.fn().mockResolvedValue(true), updateConfig: vi.fn().mockResolvedValue(true), - } as any); + } as FileHandlerMock); });
218-223: LGTM with same type safety suggestion.The error handling test correctly validates behavior (returns default config on failure) rather than implementation details. However, the same
as anycast issue applies here as in the beforeEach block.api/src/unraid-api/config/api-config.module.ts (1)
35-37: Remove redundant optional chaining.Line 35 already ensures
diskConfig.pluginsis defined, making the optional chaining on line 36 unnecessary.🔎 Proposed fix
if (!isConnectPluginInstalled() && diskConfig.plugins) { - diskConfig.plugins = diskConfig.plugins?.filter( + diskConfig.plugins = diskConfig.plugins.filter( (plugin) => plugin !== 'unraid-api-plugin-connect' );web/src/components/Notifications/Item.vue (2)
59-83: Consider consolidating offline detection logic.The offline detection checks in
handleMutation(lines 66-75) are duplicated in themutationErrorcomputed (lines 89-93). Both checkunraidApiStore.unraidApiStatus === 'offline'and attempt to extract/translate error messages. Consider extracting this into a shared helper function to reduce duplication and ensure consistent offline handling across both paths.💡 Consolidation approach
const getOfflineAwareErrorMessage = (e: unknown): string => { if (unraidApiStore.unraidApiStatus === 'offline') { const key = 'notifications.item.apiOfflineError'; const text = t(key); return text !== key ? text : 'The Unraid API is unreachable.'; } return extractGraphQLErrorMessage(e); };Then use it in both
handleMutationandmutationError.
104-117: Locale assumption in hour12 detection may not work universally.Line 114 detects 12-hour format by checking if
props.formattedTimestampcontains 'AM' or 'PM', with a fallback to 'AM'. This assumes English-language period markers and may fail for other locales that use different period indicators (e.g., 'vorm.'/'nachm.' in German). IfformattedTimestampis always English, this is fine; otherwise, consider a more robust locale-aware detection or explicit user preference.web/src/components/Notifications/Sidebar.vue (1)
164-168: Remove historical context comment.Line 164's comment explains why the code changed rather than clarifying functionality. The code is self-explanatory.
As per coding guidelines, comments should only be added when needed for clarity of function.
🔎 Proposed fix
-/** The archived count is now correctly reported by the API. */ const readArchivedCount = computed(() => { if (!overview.value) return 0; return overview.value.archive.total; });
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
api/dev/configs/api.jsonapi/src/unraid-api/config/api-config.module.tsapi/src/unraid-api/config/api-config.test.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/notify.modified.snapshotapi/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.tsapi/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.tsapi/src/unraid-api/unraid-file-modifier/modifications/patches/notify-script.patchpackage.jsonplugin/plugins/dynamix.unraid.net.plgunraid-ui/src/lib/utils.test.tsweb/__test__/store/server.test.tsweb/src/components/Common/ResizableSlideover.vueweb/src/components/Notifications/Item.vueweb/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vueweb/src/locales/en.jsonweb/src/store/server.tsweb/src/store/unraidApi.tsweb/tsconfig.json
💤 Files with no reviewable changes (1)
- unraid-ui/src/lib/utils.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- web/tsconfig.json
- web/src/components/Common/ResizableSlideover.vue
- package.json
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use TypeScript imports with.jsextensions for ESM compatibility
Never add comments unless they are needed for clarity of function
Never add comments for obvious things, and avoid commenting when starting and ending code blocks
Files:
web/src/store/unraidApi.tsweb/src/store/server.tsapi/src/unraid-api/config/api-config.test.tsapi/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/config/api-config.module.tsweb/__test__/store/server.test.tsapi/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/src/store/unraidApi.tsweb/src/locales/en.jsonweb/src/store/server.tsweb/src/components/Notifications/List.vueweb/__test__/store/server.test.tsweb/src/components/Notifications/Sidebar.vueweb/src/components/Notifications/Item.vue
web/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Files:
web/src/store/unraidApi.tsweb/src/store/server.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use theanytype. Always prefer proper typing
Avoid using casting whenever possible, prefer proper typing from the start
Files:
web/src/store/unraidApi.tsweb/src/store/server.tsapi/src/unraid-api/config/api-config.test.tsapi/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/config/api-config.module.tsweb/__test__/store/server.test.tsapi/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
web/src/store/unraidApi.tsweb/src/locales/en.jsonweb/src/store/server.tsapi/dev/configs/api.jsonapi/src/unraid-api/config/api-config.test.tsapi/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.tsplugin/plugins/dynamix.unraid.net.plgweb/src/components/Notifications/List.vueapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/notify.modified.snapshotapi/src/unraid-api/unraid-file-modifier/modifications/patches/notify-script.patchapi/src/unraid-api/config/api-config.module.tsweb/__test__/store/server.test.tsapi/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.tsweb/src/components/Notifications/Sidebar.vueweb/src/components/Notifications/Item.vue
**/store/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
Ensure Vue reactivity imports like
computed,ref, andwatchEffectare added to store files even with Nuxt auto-imports enabled
Files:
web/src/store/unraidApi.tsweb/src/store/server.tsweb/__test__/store/server.test.ts
api/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer adding new files to the NestJS repo located at
api/src/unraid-api/instead of the legacy code
Files:
api/dev/configs/api.jsonapi/src/unraid-api/config/api-config.test.tsapi/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/notify.modified.snapshotapi/src/unraid-api/unraid-file-modifier/modifications/patches/notify-script.patchapi/src/unraid-api/config/api-config.module.tsapi/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx,js,jsx}: Use VITEST for test suite, not jest
Use.rejects.toThrow()without arguments to test that functions throw errors, not exact error message strings
Files:
api/src/unraid-api/config/api-config.test.tsweb/__test__/store/server.test.ts
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 expects TTL values in milliseconds, not seconds (e.g., 600000 for 10 minutes, not 600)
Files:
api/src/unraid-api/config/api-config.test.tsapi/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/config/api-config.module.tsapi/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts
api/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
api/**/*.test.{ts,tsx}: Use Vitest for the test suite, not Jest
Prefer to not mock simple dependencies
For error testing, use.rejects.toThrow()without arguments; do not test exact error message strings unless the message format is specifically what you're testing
Files:
api/src/unraid-api/config/api-config.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
**/*.test.ts: Use.rejects.toThrow()without arguments to test that functions throw errors. Don't test exact error message strings unless the message format is specifically what you're testing
Test what the code does, not implementation details like exact error message wording
Mock external services and API calls
Usevi.mock()for module-level mocks
Specify return values for component methods withvi.spyOn()
Reset mocks between tests withvi.clearAllMocks()
Always await async operations before making assertions
Files:
api/src/unraid-api/config/api-config.test.tsweb/__test__/store/server.test.ts
web/__test__/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/__test__/**/*.test.{ts,tsx}: Test component behavior and output, not implementation details
Usemountfrom Vue Test Utils for component testing
Stub complex child components that aren't the focus of the test
Find elements with semantic queries likefind('button')rather than data-test IDs
UsecreatePinia()andsetActivePiniawhen testing Store files, only usecreateTestingPiniaif you specifically need its testing features
Files:
web/__test__/store/server.test.ts
**/__test__/store/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
**/__test__/store/**/*.ts: UsecreatePiniaandsetActivePiniawhen testing Pinia store files
UsecreatePinia()instead ofcreateTestingPinia()for most Pinia store tests
Only usecreateTestingPiniaif you specifically need its testing features for Pinia stores
Let stores initialize with their natural default state instead of forcing initial state
Do not mock the store being tested in the test file when usingcreatePinia()
Place all mock declarations at the top level in Pinia store tests
Use factory functions for module mocks in Pinia store tests to avoid hoisting issues
Test Pinia action side effects and state changes
Verify Pinia actions are called with correct parameters
Mock external dependencies appropriately in Pinia store tests
Test computed properties in Pinia stores by accessing them directly
Verify state changes after Pinia store actions
Test Pinia store getter dependencies are properly mocked
Test Pinia store interactions with other stores
Verify proper error handling in Pinia store tests
Test async operations completely in Pinia store tests
Override specific Pinia action implementations when needed in tests
Set initial state for focused Pinia store testing
Files:
web/__test__/store/server.test.ts
🧠 Learnings (70)
📓 Common learnings
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:24-27
Timestamp: 2025-01-31T22:01:22.708Z
Learning: The Unraid UI uses a modern notification system with a custom `unraid-toaster` component replacing the legacy jGrowl notifications. The system is backed by a comprehensive GraphQL API with real-time subscription support for notification updates.
Learnt from: pujitm
Repo: unraid/api PR: 941
File: web/components/Notifications/Item.vue:0-0
Timestamp: 2024-11-01T17:52:55.311Z
Learning: In the `web/components/Notifications/Item.vue` component, ensure that when delete or archive mutations fail, the notification remains displayed, and an error message is shown below the action buttons to inform the user.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch removes the old jGrowl notification system and is complemented by the unraid-toaster component implementation. The new system is added through the DefaultPageLayout modification which inserts the toaster component with proper position configuration based on user preferences.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:30-54
Timestamp: 2025-01-31T22:01:02.725Z
Learning: The removal of jGrowl notifications from DefaultPageLayout.php is intentional as notifications are now handled on a separate page as part of the architectural design to override existing Unraid pages.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch is used to remove the old jGrowl notification system from Unraid pages, as notifications are handled by a new system implemented on a different page.
Learnt from: pujitm
Repo: unraid/api PR: 1075
File: api/src/unraid-api/unraid-file-modifier/modifications/default-page-layout.modification.ts:0-0
Timestamp: 2025-01-30T20:15:25.614Z
Learning: In Unraid's DefaultPageLayout.php, the $notify['position'] variable is a safe system configuration variable that doesn't require additional sanitization when used in templates.
Learnt from: mdatelle
Repo: unraid/api PR: 1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Learnt from: elibosley
Repo: unraid/api PR: 1746
File: web/src/components/Activation/ActivationTimezoneStep.vue:78-91
Timestamp: 2025-10-11T02:09:16.853Z
Learning: In Unraid setup/activation flows (e.g., ActivationTimezoneStep.vue), don't block user progression when non-critical configuration steps like timezone updates fail. The design prioritizes allowing users to access Unraid over enforcing complete setup step success.
📚 Learning: 2025-03-27T23:52:57.888Z
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. The `setActivePinia(createPinia())` call at the module level in store files is intentional and ensures all web components share a single Pinia store instance, which is the desired behavior. This shared state approach is critical for the application's architecture to function correctly.
Applied to files:
web/src/store/unraidApi.tsweb/src/components/Notifications/List.vueweb/__test__/store/server.test.ts
📚 Learning: 2025-03-27T23:33:13.215Z
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:33:13.215Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files would break the build by causing all web components to share a singular Pinia store instance. Each web component needs its own Pinia store instance to maintain proper isolation and encapsulation.
Applied to files:
web/src/store/unraidApi.tsweb/src/components/Notifications/List.vueweb/__test__/store/server.test.ts
📚 Learning: 2025-01-23T21:26:17.766Z
Learnt from: elibosley
Repo: unraid/api PR: 1063
File: api/src/unraid-api/cli/sso/validate-token.command.ts:76-81
Timestamp: 2025-01-23T21:26:17.766Z
Learning: In the unraid/api codebase, store dispatches using Redux Toolkit's `unwrap()` method already handle errors appropriately and don't require additional try-catch blocks.
Applied to files:
web/src/store/server.tsweb/src/components/Notifications/List.vue
📚 Learning: 2025-01-22T18:34:06.925Z
Learnt from: elibosley
Repo: unraid/api PR: 1068
File: api/src/unraid-api/auth/api-key.service.ts:122-137
Timestamp: 2025-01-22T18:34:06.925Z
Learning: The store in app/store is implemented using Redux's configureStore, where dispatch operations for config updates are synchronous in-memory state updates that cannot fail, making transaction-like patterns unnecessary.
Applied to files:
web/src/store/server.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Mock external services and API calls
Applied to files:
api/src/unraid-api/config/api-config.test.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Applies to api/**/*.test.{ts,tsx} : Prefer to not mock simple dependencies
Applied to files:
api/src/unraid-api/config/api-config.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Mock external dependencies appropriately in Pinia store tests
Applied to files:
api/src/unraid-api/config/api-config.test.tsweb/__test__/store/server.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Test Pinia store getter dependencies are properly mocked
Applied to files:
api/src/unraid-api/config/api-config.test.tsweb/__test__/store/server.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Use factory functions for module mocks in Pinia store tests to avoid hoisting issues
Applied to files:
api/src/unraid-api/config/api-config.test.tsweb/__test__/store/server.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Do not mock the store being tested in the test file when using `createPinia()`
Applied to files:
api/src/unraid-api/config/api-config.test.tsweb/__test__/store/server.test.ts
📚 Learning: 2025-01-27T14:57:46.617Z
Learnt from: elibosley
Repo: unraid/api PR: 1063
File: web/_data/serverState.ts:137-147
Timestamp: 2025-01-27T14:57:46.617Z
Learning: The values in `web/_data/serverState.ts` are used for testing purposes and should remain as hardcoded mock data to facilitate testing different scenarios.
Applied to files:
api/src/unraid-api/config/api-config.test.tsweb/__test__/store/server.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Verify proper error handling in Pinia store tests
Applied to files:
api/src/unraid-api/config/api-config.test.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Test what the code does, not implementation details like exact error message wording
Applied to files:
api/src/unraid-api/config/api-config.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Override specific Pinia action implementations when needed in tests
Applied to files:
api/src/unraid-api/config/api-config.test.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Specify return values for component methods with `vi.spyOn()`
Applied to files:
api/src/unraid-api/config/api-config.test.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Use `vi.mock()` for module-level mocks
Applied to files:
api/src/unraid-api/config/api-config.test.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Reset mocks between tests with `vi.clearAllMocks()`
Applied to files:
api/src/unraid-api/config/api-config.test.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Applies to api/**/*.test.{ts,tsx} : For error testing, use `.rejects.toThrow()` without arguments; do not test exact error message strings unless the message format is specifically what you're testing
Applied to files:
api/src/unraid-api/config/api-config.test.ts
📚 Learning: 2025-01-31T22:01:41.842Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch removes the old jGrowl notification system and is complemented by the unraid-toaster component implementation. The new system is added through the DefaultPageLayout modification which inserts the toaster component with proper position configuration based on user preferences.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.tsweb/src/components/Notifications/Sidebar.vueweb/src/components/Notifications/Item.vue
📚 Learning: 2025-01-31T22:01:41.842Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch is used to remove the old jGrowl notification system from Unraid pages, as notifications are handled by a new system implemented on a different page.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.tsweb/src/components/Notifications/List.vueapi/src/unraid-api/unraid-file-modifier/modifications/patches/notify-script.patchweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-01-31T22:01:02.725Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:30-54
Timestamp: 2025-01-31T22:01:02.725Z
Learning: The removal of jGrowl notifications from DefaultPageLayout.php is intentional as notifications are now handled on a separate page as part of the architectural design to override existing Unraid pages.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.tsweb/src/components/Notifications/Sidebar.vueweb/src/components/Notifications/Item.vue
📚 Learning: 2025-02-03T17:21:26.738Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/DefaultPageLayout.php.modified.snapshot.php:203-207
Timestamp: 2025-02-03T17:21:26.738Z
Learning: The project uses patches to override existing Unraid pages rather than modifying them directly.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.ts
📚 Learning: 2025-09-04T18:42:53.531Z
Learnt from: pujitm
Repo: unraid/api PR: 1658
File: plugin/plugins/dynamix.unraid.net.plg:73-79
Timestamp: 2025-09-04T18:42:53.531Z
Learning: In the dynamix.unraid.net plugin, versions 6.12.1-6.12.14 and 6.12.15 prereleases are intentionally allowed to install with warnings (rather than immediate cleanup) to provide users with a grace period and notice before functionality is completely removed. This is a deliberate UX decision to avoid immediately breaking existing setups while encouraging upgrades.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-09-04T15:26:34.416Z
Learnt from: elibosley
Repo: unraid/api PR: 1657
File: web/scripts/deploy-dev.sh:37-41
Timestamp: 2025-09-04T15:26:34.416Z
Learning: In web/scripts/deploy-dev.sh, the command `rm -rf /usr/local/emhttp/plugins/dynamix.my.servers/unraid-components/*` intentionally removes all contents of the unraid-components directory before deploying standalone components. This broader cleanup is desired behavior according to the maintainer elibosley.
Applied to files:
plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2024-11-01T17:52:55.311Z
Learnt from: pujitm
Repo: unraid/api PR: 941
File: web/components/Notifications/Item.vue:0-0
Timestamp: 2024-11-01T17:52:55.311Z
Learning: In the `web/components/Notifications/Item.vue` component, ensure that when delete or archive mutations fail, the notification remains displayed, and an error message is shown below the action buttons to inform the user.
Applied to files:
web/src/components/Notifications/List.vueapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsweb/src/components/Notifications/Sidebar.vueweb/src/components/Notifications/Item.vue
📚 Learning: 2025-01-31T22:01:22.708Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:24-27
Timestamp: 2025-01-31T22:01:22.708Z
Learning: The Unraid UI uses a modern notification system with a custom `unraid-toaster` component replacing the legacy jGrowl notifications. The system is backed by a comprehensive GraphQL API with real-time subscription support for notification updates.
Applied to files:
web/src/components/Notifications/List.vueapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsweb/src/components/Notifications/Sidebar.vueweb/src/components/Notifications/Item.vue
📚 Learning: 2025-02-06T17:24:58.784Z
Learnt from: mdatelle
Repo: unraid/api PR: 1122
File: web/components/UserProfile/DropdownLaunchpad.vue:38-42
Timestamp: 2025-02-06T17:24:58.784Z
Learning: In the DropdownLaunchpad.vue component, the current implementation of conditional icon rendering using `h(BrandLoading, { variant: 'white' })` is a temporary fix to maintain build stability, with a proper refactoring being handled in a separate branch.
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Item.vue
📚 Learning: 2024-12-06T17:38:40.999Z
Learnt from: elibosley
Repo: unraid/api PR: 974
File: web/components/Loading/Error.vue:1-3
Timestamp: 2024-12-06T17:38:40.999Z
Learning: In Nuxt.js projects, components are automatically imported, so explicit import statements for components like `LoadingSpinner` in `web/components/Loading/Error.vue` are not necessary.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2025-02-18T18:38:24.981Z
Learnt from: zackspear
Repo: unraid/api PR: 1143
File: web/pages/webComponents.vue:28-29
Timestamp: 2025-02-18T18:38:24.981Z
Learning: In Vue components, when there's a repeating pattern of heading + content + divider, prefer using a wrapper component that takes the heading as a prop and content via slots, rather than duplicating the structure. This improves maintainability and reduces code duplication.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2025-10-11T02:09:16.853Z
Learnt from: elibosley
Repo: unraid/api PR: 1746
File: web/src/components/Activation/ActivationTimezoneStep.vue:78-91
Timestamp: 2025-10-11T02:09:16.853Z
Learning: In Unraid setup/activation flows (e.g., ActivationTimezoneStep.vue), don't block user progression when non-critical configuration steps like timezone updates fail. The design prioritizes allowing users to access Unraid over enforcing complete setup step success.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2025-03-27T13:34:53.438Z
Learnt from: pujitm
Repo: unraid/api PR: 1252
File: api/src/environment.ts:56-56
Timestamp: 2025-03-27T13:34:53.438Z
Learning: For critical components in the Unraid API, such as retrieving version information from package.json, failing fast (allowing crashes) is preferred over graceful degradation with fallback values.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
Repo: unraid/api PR: 942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2025-08-11T15:41:23.573Z
Learnt from: elibosley
Repo: unraid/api PR: 1563
File: web/pages/login.vue:31-48
Timestamp: 2025-08-11T15:41:23.573Z
Learning: In the Unraid API codebase, debug/test views (such as the debug modal in `web/pages/login.vue`) may intentionally display sensitive information like passwords in plain text for debugging and testing purposes. This is an accepted practice for development tools in this project.
Applied to files:
web/src/components/Notifications/List.vueapi/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts
📚 Learning: 2024-11-27T15:30:02.252Z
Learnt from: elibosley
Repo: unraid/api PR: 969
File: web/justfile:7-9
Timestamp: 2024-11-27T15:30:02.252Z
Learning: In the Unraid Connect project, the different implementations of the `setup` commands in `web/justfile` and `api/justfile` are intentional and correct behavior.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2025-03-14T19:22:11.839Z
Learnt from: pujitm
Repo: unraid/api PR: 1211
File: api/src/unraid-api/graph/connect/connect-settings.service.ts:0-0
Timestamp: 2025-03-14T19:22:11.839Z
Learning: In the Unraid API project, input validation should happen at the store layer, not in service methods like ConnectSettingsService, to maintain proper separation of concerns.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2024-12-06T14:43:04.876Z
Learnt from: pujitm
Repo: unraid/api PR: 974
File: web/components/Notifications/List.vue:31-31
Timestamp: 2024-12-06T14:43:04.876Z
Learning: In the Vue.js application using `useQuery` from Vue Apollo, the `error` reactive variable captures errors from both the initial query and any `fetchMore` calls, so wrapping `fetchMore` in a try-catch block is unnecessary.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2025-01-31T14:42:06.752Z
Learnt from: pujitm
Repo: unraid/api PR: 1075
File: web/components/Notifications/Sidebar.vue:41-58
Timestamp: 2025-01-31T14:42:06.752Z
Learning: When handling errors in code that uses toast notifications, avoid using toast in the error handler itself since toast functionality might be the source of the error. Use console.error or other reliable logging mechanisms instead.
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-06-13T17:14:21.739Z
Learnt from: pujitm
Repo: unraid/api PR: 1417
File: web/components/ConnectSettings/ConnectSettings.ce.vue:11-18
Timestamp: 2025-06-13T17:14:21.739Z
Learning: The project’s build tooling auto-imports common Vue/Pinia helpers such as `storeToRefs`, so explicit import statements for them are not required.
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-03-27T23:52:57.888Z
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files ensures that all web components share a single Pinia store instance, which is the desired behavior. Without this initialization, each web component would have its own isolated store, breaking the intended architecture.
Applied to files:
web/src/components/Notifications/List.vueweb/__test__/store/server.test.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Use `createTestingPinia()` for mocking stores in Vue components
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Nuxt is set to auto import so some Vue files may need `computed` or `ref` imported explicitly in store files
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/store/**/*.ts : Ensure Vue reactivity imports like `computed`, `ref`, and `watchEffect` are added to store files even with Nuxt auto-imports enabled
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-03-27T23:33:13.215Z
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:33:13.215Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files would break the build by causing all web components to share a singular Pinia store instance. Each web component needs its own Pinia store instance to function correctly.
Applied to files:
web/src/components/Notifications/List.vueweb/__test__/store/server.test.ts
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to web/src/**/*.ts : Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-09T15:47:29.325Z
Learnt from: pujitm
Repo: unraid/api PR: 975
File: web/components/Notifications/TabList.vue:1-24
Timestamp: 2024-12-09T15:47:29.325Z
Learning: In our Nuxt setup using Vue.js, components defined within the codebase are autoloaded and do not require explicit import statements in the script section. For example, components like `TabsList` and `TabsTrigger` used in `web/components/Notifications/TabList.vue` are automatically available without imports.
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vueweb/src/components/Notifications/Item.vue
📚 Learning: 2024-12-06T17:34:16.133Z
Learnt from: pujitm
Repo: unraid/api PR: 974
File: web/components/Loading/Error.vue:50-50
Timestamp: 2024-12-06T17:34:16.133Z
Learning: In this project, the `Button` component from `~/components/shadcn/Button.vue` is autoloaded and does not need to be imported manually in components like `web/components/Loading/Error.vue`.
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to **/*.{ts,tsx} : Avoid using casting whenever possible, prefer proper typing from the start
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
📚 Learning: 2025-03-07T19:29:12.540Z
Learnt from: pujitm
Repo: unraid/api PR: 1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to **/*.{ts,tsx} : Never use the `any` type. Always prefer proper typing
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
📚 Learning: 2025-09-09T14:34:07.189Z
Learnt from: pujitm
Repo: unraid/api PR: 1582
File: packages/unraid-shared/src/util/processing.ts:69-71
Timestamp: 2025-09-09T14:34:07.189Z
Learning: In TypeScript code, pujitm considers it acceptable to use `any` type for private/internal fields when the type is not exposed through the public API and is masked by properly typed method declarations. The focus is on ensuring the public interface maintains type safety rather than enforcing strict typing on all internal implementation details.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
📚 Learning: 2025-01-28T16:39:50.278Z
Learnt from: elibosley
Repo: unraid/api PR: 1078
File: api/src/unraid-api/unraid-file-modifier/modifications/sso.modification.ts:35-36
Timestamp: 2025-01-28T16:39:50.278Z
Learning: When reviewing TypeScript files containing PHP code snippets intended for file modifications (like in `SSOFileModification`), PHP functions that appear undefined should not be flagged as issues since they will be available in the target PHP environment where the code is injected.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts
📚 Learning: 2025-01-30T19:56:42.055Z
Learnt from: pujitm
Repo: unraid/api PR: 1075
File: web/components/Notifications/Sidebar.vue:52-57
Timestamp: 2025-01-30T19:56:42.055Z
Learning: Notification links in the Unraid API are validated at creation time on the server side, making additional client-side validation unnecessary when receiving notifications through GraphQL subscriptions.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
📚 Learning: 2025-04-07T14:34:47.255Z
Learnt from: elibosley
Repo: unraid/api PR: 1334
File: api/src/unraid-api/graph/resolvers/docker/docker-event.service.ts:63-66
Timestamp: 2025-04-07T14:34:47.255Z
Learning: In DockerEventService, the chokidar file watcher is configured with `ignoreInitial: false` to ensure that existing files (like the Docker socket) are detected and processed at application startup, not just when files change after the watcher is initialized.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
📚 Learning: 2025-01-30T20:15:25.614Z
Learnt from: pujitm
Repo: unraid/api PR: 1075
File: api/src/unraid-api/unraid-file-modifier/modifications/default-page-layout.modification.ts:0-0
Timestamp: 2025-01-30T20:15:25.614Z
Learning: In Unraid's DefaultPageLayout.php, the $notify['position'] variable is a safe system configuration variable that doesn't require additional sanitization when used in templates.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/notify.modified.snapshotapi/src/unraid-api/unraid-file-modifier/modifications/patches/notify-script.patch
📚 Learning: 2025-02-03T18:57:53.577Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts:80-80
Timestamp: 2025-02-03T18:57:53.577Z
Learning: In the unraid/api codebase, patch files generated for file modifications always use the .patch extension, regardless of the source file type being modified.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/patches/notify-script.patchapi/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts
📚 Learning: 2025-05-07T16:07:47.236Z
Learnt from: elibosley
Repo: unraid/api PR: 1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/setup_api.sh:107-113
Timestamp: 2025-05-07T16:07:47.236Z
Learning: The Unraid API is designed to handle missing configuration files gracefully with smart internal fallbacks rather than requiring installation scripts to create default configurations.
Applied to files:
api/src/unraid-api/config/api-config.module.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Verify state changes after Pinia store actions
Applied to files:
web/__test__/store/server.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Test Pinia action side effects and state changes
Applied to files:
web/__test__/store/server.test.ts
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to web/__test__/**/*.test.{ts,tsx} : Use `createPinia()` and `setActivePinia` when testing Store files, only use `createTestingPinia` if you specifically need its testing features
Applied to files:
web/__test__/store/server.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Test Pinia store interactions with other stores
Applied to files:
web/__test__/store/server.test.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Place all mock declarations at the top level in Pinia store tests
Applied to files:
web/__test__/store/server.test.ts
📚 Learning: 2025-01-29T16:35:43.699Z
Learnt from: elibosley
Repo: unraid/api PR: 1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:39-41
Timestamp: 2025-01-29T16:35:43.699Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations, including detailed error logging with stack traces and modification IDs. Individual FileModification implementations should focus on their core functionality without duplicating error handling.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts
📚 Learning: 2025-02-04T18:45:23.106Z
Learnt from: elibosley
Repo: unraid/api PR: 1111
File: api/src/unraid-api/unraid-file-modifier/file-modification.ts:182-187
Timestamp: 2025-02-04T18:45:23.106Z
Learning: In the FileModification class's patch handling:
- `results === false` indicates patch application failure
- Empty string (`results === ''`) is a valid patch result indicating the file should be deleted
- These are distinct conditions and should be handled differently
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts
📚 Learning: 2024-11-18T20:50:19.224Z
Learnt from: elibosley
Repo: unraid/api PR: 963
File: web/helpers/markdown.ts:10-13
Timestamp: 2024-11-18T20:50:19.224Z
Learning: In `web/helpers/markdown.ts`, the `safeParseMarkdown` function does not require additional input validation, error handling, or caching as per the user's preference.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts
📚 Learning: 2024-12-09T15:45:46.492Z
Learnt from: pujitm
Repo: unraid/api PR: 975
File: web/components/Notifications/TabList.vue:1-4
Timestamp: 2024-12-09T15:45:46.492Z
Learning: In our Nuxt.js setup for the `web` project, it's not necessary to explicitly import `computed` from `vue` in Vue components, as it's globally available.
Applied to files:
web/src/components/Notifications/Sidebar.vueweb/src/components/Notifications/Item.vue
📚 Learning: 2024-12-17T13:55:42.068Z
Learnt from: elibosley
Repo: unraid/api PR: 972
File: web/components/ColorSwitcher.ce.vue:1-2
Timestamp: 2024-12-17T13:55:42.068Z
Learning: In this Nuxt.js project, components used in templates are automatically imported by Nuxt, so explicit import statements for components are unnecessary.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Use `mount` from Vue Test Utils for component testing
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Check for expected prop handling and event emissions in Vue components
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-11-22T15:47:54.695Z
Learnt from: pujitm
Repo: unraid/api PR: 967
File: web/components/Notifications/graphql/notification.query.ts:13-13
Timestamp: 2024-11-22T15:47:54.695Z
Learning: The `formattedTimestamp` field in the `Notification` type is already provided via `NotificationsResolver::list`.
Applied to files:
web/src/components/Notifications/Item.vue
🧬 Code graph analysis (3)
api/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.ts (1)
api/src/unraid-api/unraid-file-modifier/file-modification.ts (1)
ShouldApplyWithReason(15-24)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (4)
api/src/environment.ts (1)
CHOKIDAR_USEPOLLING(75-75)packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)web/src/composables/gql/graphql.ts (2)
Notification(1640-1653)NotificationSettings(1690-1696)api/src/utils.ts (2)
batchProcess(58-71)formatDatetime(138-179)
api/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts (1)
api/src/unraid-api/unraid-file-modifier/file-modification.ts (1)
ShouldApplyWithReason(15-24)
| try { | ||
| const diskConfig: Partial<ApiConfig> = await apiHandler.loadConfig(); | ||
| // Hack: cleanup stale connect plugin entry if necessary | ||
| if (!isConnectPluginInstalled() && diskConfig.plugins) { | ||
| diskConfig.plugins = diskConfig.plugins?.filter( | ||
| (plugin) => plugin !== 'unraid-api-plugin-connect' | ||
| ); | ||
| await apiHandler.writeConfigFile(diskConfig as ApiConfig); | ||
| } | ||
|
|
||
| return { | ||
| ...defaultConfig, | ||
| ...diskConfig, | ||
| // diskConfig's version may be older, but we still want to use the correct version | ||
| version: API_VERSION, | ||
| }; | ||
| return { | ||
| ...defaultConfig, | ||
| ...diskConfig, | ||
| // diskConfig's version may be older, but we still want to use the correct version | ||
| version: API_VERSION, | ||
| }; | ||
| } catch (e) { | ||
| return defaultConfig; | ||
| } |
There was a problem hiding this comment.
Add error logging and improve error handling.
The try/catch block silently swallows all errors and returns the default config. While graceful degradation is appropriate per the learnings about Unraid API handling missing configs, errors should be logged for debugging and operational visibility.
Additionally, the writeConfigFile call on line 39 could fail and throw an error that would be caught by the outer catch block, potentially hiding the actual config that was successfully loaded.
🔎 Proposed fix
try {
const diskConfig: Partial<ApiConfig> = await apiHandler.loadConfig();
- // Hack: cleanup stale connect plugin entry if necessary
- if (!isConnectPluginInstalled() && diskConfig.plugins) {
- diskConfig.plugins = diskConfig.plugins?.filter(
- (plugin) => plugin !== 'unraid-api-plugin-connect'
- );
- await apiHandler.writeConfigFile(diskConfig as ApiConfig);
+
+ if (!isConnectPluginInstalled() && diskConfig.plugins) {
+ diskConfig.plugins = diskConfig.plugins.filter(
+ (plugin) => plugin !== 'unraid-api-plugin-connect'
+ );
+
+ try {
+ await apiHandler.writeConfigFile(diskConfig as ApiConfig);
+ } catch (writeError) {
+ console.error('Failed to write cleaned config, proceeding with loaded config:', writeError);
+ }
}
return {
...defaultConfig,
...diskConfig,
- // diskConfig's version may be older, but we still want to use the correct version
version: API_VERSION,
};
} catch (e) {
+ console.error('Failed to load API config, using defaults:', e);
return defaultConfig;
}Based on learnings, the Unraid API handles missing configs gracefully with fallbacks.
🤖 Prompt for AI Agents
In @api/src/unraid-api/config/api-config.module.ts around lines 32 - 50, The
try/catch currently swallows all errors and can hide failures from
apiHandler.loadConfig and apiHandler.writeConfigFile; change it to log errors
and avoid letting writeConfigFile failures discard a successfully loaded
diskConfig: call apiHandler.loadConfig() inside the try as you do, but wrap the
writeConfigFile(...) call in its own try/catch that logs the error (use the
module's logger) so a failing write does not trigger the outer fallback; in the
outer catch log the caught exception (include error details) before returning
defaultConfig; reference apiHandler.loadConfig, isConnectPluginInstalled,
apiHandler.writeConfigFile, defaultConfig and API_VERSION to locate the relevant
logic.
| diskConfig.plugins = diskConfig.plugins?.filter( | ||
| (plugin) => plugin !== 'unraid-api-plugin-connect' | ||
| ); | ||
| await apiHandler.writeConfigFile(diskConfig as ApiConfig); |
There was a problem hiding this comment.
Unsafe type cast from Partial to ApiConfig.
The cast assumes diskConfig contains all required ApiConfig fields, but it's typed as Partial<ApiConfig>. If the disk config is incomplete, this could cause issues downstream.
Consider either:
- Merging with defaultConfig before the write operation to ensure all required fields are present
- Using a type guard to validate the config structure before casting
🔎 Proposed fix
if (!isConnectPluginInstalled() && diskConfig.plugins) {
diskConfig.plugins = diskConfig.plugins.filter(
(plugin) => plugin !== 'unraid-api-plugin-connect'
);
- await apiHandler.writeConfigFile(diskConfig as ApiConfig);
+
+ const configToWrite: ApiConfig = {
+ ...defaultConfig,
+ ...diskConfig,
+ version: API_VERSION,
+ };
+ await apiHandler.writeConfigFile(configToWrite);
}📝 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.
| await apiHandler.writeConfigFile(diskConfig as ApiConfig); | |
| const configToWrite: ApiConfig = { | |
| ...defaultConfig, | |
| ...diskConfig, | |
| version: API_VERSION, | |
| }; | |
| await apiHandler.writeConfigFile(configToWrite); |
🤖 Prompt for AI Agents
In @api/src/unraid-api/config/api-config.module.ts at line 39, The diskConfig
variable is typed Partial<ApiConfig> but is being unsafely cast to ApiConfig
when calling apiHandler.writeConfigFile(diskConfig as ApiConfig); ensure the
config is complete before writing by either merging diskConfig with a default
ApiConfig (e.g., defaultConfig) and passing the merged result to
apiHandler.writeConfigFile, or add a runtime type guard/validation function that
checks required fields on diskConfig and only casts and writes after validation;
update the call site to use the merged/validated object instead of the direct
cast.
| if (await fileExists(archivePath)) { | ||
| // File already in archive, just delete the unread one | ||
| await unlink(unreadPath); | ||
|
|
||
| // Since we previously ignored this file in the archive definition (because it was in unread), | ||
| // we must now increment the archive stats because it has been "revealed" as an archived notification. | ||
| this.increment(notification.importance, NotificationsService.overview.archive); | ||
| if (snapshot) { | ||
| this.increment(notification.importance, snapshot.archive); | ||
| } |
There was a problem hiding this comment.
Add error handling around unlink to prevent stat corruption.
Similar to the error handling added for rename (lines 551-562), the unlink operation at line 541 needs try-catch with rollback. If unlink fails after decrementing unread stats (lines 533-537), the notification will still exist in unread but the count will be incorrect.
🔎 Proposed fix
if (await fileExists(archivePath)) {
// File already in archive, just delete the unread one
- await unlink(unreadPath);
+ try {
+ await unlink(unreadPath);
+ } catch (err) {
+ // Revert the earlier unread decrement
+ this.increment(notification.importance, NotificationsService.overview.unread);
+ if (snapshot) {
+ this.increment(notification.importance, snapshot.unread);
+ }
+ throw err;
+ }
// Since we previously ignored this file in the archive definition (because it was in unread),
// we must now increment the archive stats because it has been "revealed" as an archived notification.🤖 Prompt for AI Agents
In @api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
around lines 539 - 548, The unlink(unreadPath) call inside the block that checks
await fileExists(archivePath) needs a try/catch and rollback like the rename
handling: perform the unlink in a try block, and if it throws, restore the
unread stats you decremented earlier (i.e., call the same
NotificationsService.decrement/restore logic for notification.importance and
snapshot.unread if applicable) and rethrow or log the error; only after a
successful unlink should you increment archive stats via
this.increment(notification.importance, NotificationsService.overview.archive)
and snapshot.archive. Ensure you reference the existing symbols unreadPath,
archivePath, notification.importance, snapshot, unlink, and
NotificationsService.increment/decrement so the failure leaves stats consistent.
| <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.
Remove redundant nested loading check.
The inner v-if="loading" at line 290 is redundant since the parent <div> at line 288 already has v-if="loading". Remove the inner check to avoid unnecessary conditional nesting.
🔎 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 - 304, The inner
redundant v-if="loading" on the nested <div> should be removed because the
parent wrapper already guards rendering; locate the nested <div class="space-y-4
py-3"> (the one that currently has v-if="loading") in the Notifications List
component and delete the v-if="loading" attribute so only the outer container
retains the conditional.
- Introduce `NOTIFICATION_EVENT` pubsub channel and `NotificationEventType` enum to support granular event broadcasting. - Update NotificationsService to publish `ADDED`, `UPDATED`, `DELETED`, and `CLEARED` events for all notification lifecycle actions. - Add notificationEvent subscription to the GraphQL schema and resolvers. - Update List.vue to subscribe to notificationEvent. This ensures the list updates in real-time when notifications are archived, deleted, or cleared from other tabs/users. - Update Sidebar.vue to use the unified notificationEvent subscription for triggering toast notifications and tracking the latest message. - Fix `ReferenceError` in notifications.model.ts by ensuring Notification class is defined before it is referenced by NotificationEvent.
- Refactored archiveAll in the backend to perform file operations in a batch and emit a single `CLEARED` event, significantly reducing network traffic and UI lag. - Implemented `processingArchives` tracking to suppress the file watcher during batch operations, preventing race conditions and double-counting of stats. - Re-enabled `notificationOverviewSubscription` in the Sidebar to ensure unread/archived counts update correctly regardless of the active tab.
|
🚀 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: 1
🤖 Fix all issues with AI agents
In @api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts:
- Around line 683-707: batchCreateArchive currently adds archivePath to
this.processingArchives before calling rename but never removes it if rename
fails; update batchCreateArchive so the call to
this.processingArchives.add(archivePath) is paired with a removal on failure —
e.g., wrap the rename in its own try/catch (inside the existing else) and call
this.processingArchives.delete(archivePath) in the catch before rethrowing or
handling the error, or use a try/finally around rename that deletes the
archivePath only when rename throws, ensuring successful renames keep the entry
as designed and failures do not leave stale entries that suppress future
handleNotificationAdd checks.
🧹 Nitpick comments (5)
web/src/components/Notifications/List.vue (4)
11-11: Remove commented-out debug import.Per coding guidelines, avoid leaving commented-out code. If this debug utility is needed in the future, it can be re-added.
Suggested fix
-// import { dbgApolloError } from '~/helpers/functions';
129-136: Remove commented-out debug code and consider removing verbose console.log statements.The commented-out watcher (lines 129-132) should be removed. Additionally, the console.log statements throughout (lines 88, 101, 134-136, 150-151, 173, 176, 196-198, 203, 210-211, 231-233, 236, 240) are quite verbose for production code. Consider gating these behind a debug flag or removing them before release.
Suggested fix for commented code
-// for debugging purposes: -// watch(error, (e) => dbgApolloError('useQuery error', e as ApolloError | null | undefined), { -// immediate: true, -// }); - -watch(offlineError, (o) => { - if (o) console.log('[Notifications] offlineError:', o); -}); +watch(offlineError, (o) => { + if (o) console.warn('[Notifications] offlineError:', o.message); +});
297-313: Redundantv-if="loading"check.The inner
v-if="loading"on line 299 is unnecessary since the parent<div v-if="loading">on line 297 already guards this block.Suggested 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">
334-344: Hardcoded strings should use i18n.Lines 340 and 343 use hardcoded strings ("Error" and "Try Again") while the rest of the component uses
t()for internationalization. Consider using translation keys for consistency.Suggested fix
<div class="text-center"> - <h3 class="font-bold">Error</h3> + <h3 class="font-bold">{{ t('notifications.list.errorTitle') }}</h3> <p>{{ displayErrorMessage }}</p> </div> - <UButton block @click="() => void refetch()">Try Again</UButton> + <UButton block @click="() => void refetch()">{{ t('notifications.list.tryAgain') }}</UButton> </div>api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts (1)
80-87: Consider adding validators for consistency.The
typefield lacks@IsEnum()and@IsNotEmpty()validators, which are applied to other enum fields in this file. While validators on output types are optional, adding them maintains consistency and provides defense-in-depth for any future use cases.♻️ Suggested addition
@ObjectType('NotificationEvent') export class NotificationEvent { @Field(() => NotificationEventType) + @IsEnum(NotificationEventType) + @IsNotEmpty() type!: NotificationEventType; @Field(() => Notification, { nullable: true }) notification?: Notification; }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**
📒 Files selected for processing (11)
api/dev/configs/api.jsonapi/generated-schema.graphqlapi/src/unraid-api/graph/resolvers/notifications/notifications.model.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tspackages/unraid-shared/src/pubsub/graphql.pubsub.tsweb/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vueweb/src/components/Notifications/graphql/notification.subscription.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.ts
✅ Files skipped from review due to trivial changes (2)
- api/dev/configs/api.json
- web/src/composables/gql/graphql.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- api/generated-schema.graphql
- web/src/composables/gql/gql.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use TypeScript imports with.jsextensions for ESM compatibility
Never add comments unless they are needed for clarity of function
Never add comments for obvious things, and avoid commenting when starting and ending code blocks
Files:
web/src/components/Notifications/graphql/notification.subscription.tspackages/unraid-shared/src/pubsub/graphql.pubsub.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.model.ts
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/src/components/Notifications/graphql/notification.subscription.tsweb/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vue
web/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Files:
web/src/components/Notifications/graphql/notification.subscription.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use theanytype. Always prefer proper typing
Avoid using casting whenever possible, prefer proper typing from the start
Files:
web/src/components/Notifications/graphql/notification.subscription.tspackages/unraid-shared/src/pubsub/graphql.pubsub.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.model.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
web/src/components/Notifications/graphql/notification.subscription.tsweb/src/components/Notifications/List.vuepackages/unraid-shared/src/pubsub/graphql.pubsub.tsweb/src/components/Notifications/Sidebar.vueapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.model.ts
api/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer adding new files to the NestJS repo located at
api/src/unraid-api/instead of the legacy code
Files:
api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.model.ts
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 expects TTL values in milliseconds, not seconds (e.g., 600000 for 10 minutes, not 600)
Files:
api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.model.ts
🧠 Learnings (36)
📓 Common learnings
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:24-27
Timestamp: 2025-01-31T22:01:22.708Z
Learning: The Unraid UI uses a modern notification system with a custom `unraid-toaster` component replacing the legacy jGrowl notifications. The system is backed by a comprehensive GraphQL API with real-time subscription support for notification updates.
Learnt from: pujitm
Repo: unraid/api PR: 941
File: web/components/Notifications/Item.vue:0-0
Timestamp: 2024-11-01T17:52:55.311Z
Learning: In the `web/components/Notifications/Item.vue` component, ensure that when delete or archive mutations fail, the notification remains displayed, and an error message is shown below the action buttons to inform the user.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:30-54
Timestamp: 2025-01-31T22:01:02.725Z
Learning: The removal of jGrowl notifications from DefaultPageLayout.php is intentional as notifications are now handled on a separate page as part of the architectural design to override existing Unraid pages.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch removes the old jGrowl notification system and is complemented by the unraid-toaster component implementation. The new system is added through the DefaultPageLayout modification which inserts the toaster component with proper position configuration based on user preferences.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch is used to remove the old jGrowl notification system from Unraid pages, as notifications are handled by a new system implemented on a different page.
Learnt from: mdatelle
Repo: unraid/api PR: 1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
📚 Learning: 2025-01-30T19:56:42.055Z
Learnt from: pujitm
Repo: unraid/api PR: 1075
File: web/components/Notifications/Sidebar.vue:52-57
Timestamp: 2025-01-30T19:56:42.055Z
Learning: Notification links in the Unraid API are validated at creation time on the server side, making additional client-side validation unnecessary when receiving notifications through GraphQL subscriptions.
Applied to files:
web/src/components/Notifications/graphql/notification.subscription.tspackages/unraid-shared/src/pubsub/graphql.pubsub.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.model.ts
📚 Learning: 2024-11-01T17:52:55.311Z
Learnt from: pujitm
Repo: unraid/api PR: 941
File: web/components/Notifications/Item.vue:0-0
Timestamp: 2024-11-01T17:52:55.311Z
Learning: In the `web/components/Notifications/Item.vue` component, ensure that when delete or archive mutations fail, the notification remains displayed, and an error message is shown below the action buttons to inform the user.
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vueapi/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
📚 Learning: 2025-10-11T02:09:16.853Z
Learnt from: elibosley
Repo: unraid/api PR: 1746
File: web/src/components/Activation/ActivationTimezoneStep.vue:78-91
Timestamp: 2025-10-11T02:09:16.853Z
Learning: In Unraid setup/activation flows (e.g., ActivationTimezoneStep.vue), don't block user progression when non-critical configuration steps like timezone updates fail. The design prioritizes allowing users to access Unraid over enforcing complete setup step success.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
Repo: unraid/api PR: 942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2025-03-27T13:34:53.438Z
Learnt from: pujitm
Repo: unraid/api PR: 1252
File: api/src/environment.ts:56-56
Timestamp: 2025-03-27T13:34:53.438Z
Learning: For critical components in the Unraid API, such as retrieving version information from package.json, failing fast (allowing crashes) is preferred over graceful degradation with fallback values.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2024-11-27T15:30:02.252Z
Learnt from: elibosley
Repo: unraid/api PR: 969
File: web/justfile:7-9
Timestamp: 2024-11-27T15:30:02.252Z
Learning: In the Unraid Connect project, the different implementations of the `setup` commands in `web/justfile` and `api/justfile` are intentional and correct behavior.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2024-12-06T14:43:04.876Z
Learnt from: pujitm
Repo: unraid/api PR: 974
File: web/components/Notifications/List.vue:31-31
Timestamp: 2024-12-06T14:43:04.876Z
Learning: In the Vue.js application using `useQuery` from Vue Apollo, the `error` reactive variable captures errors from both the initial query and any `fetchMore` calls, so wrapping `fetchMore` in a try-catch block is unnecessary.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2025-01-31T14:42:06.752Z
Learnt from: pujitm
Repo: unraid/api PR: 1075
File: web/components/Notifications/Sidebar.vue:41-58
Timestamp: 2025-01-31T14:42:06.752Z
Learning: When handling errors in code that uses toast notifications, avoid using toast in the error handler itself since toast functionality might be the source of the error. Use console.error or other reliable logging mechanisms instead.
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-02-06T17:24:58.784Z
Learnt from: mdatelle
Repo: unraid/api PR: 1122
File: web/components/UserProfile/DropdownLaunchpad.vue:38-42
Timestamp: 2025-02-06T17:24:58.784Z
Learning: In the DropdownLaunchpad.vue component, the current implementation of conditional icon rendering using `h(BrandLoading, { variant: 'white' })` is a temporary fix to maintain build stability, with a proper refactoring being handled in a separate branch.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2024-12-06T17:38:40.999Z
Learnt from: elibosley
Repo: unraid/api PR: 974
File: web/components/Loading/Error.vue:1-3
Timestamp: 2024-12-06T17:38:40.999Z
Learning: In Nuxt.js projects, components are automatically imported, so explicit import statements for components like `LoadingSpinner` in `web/components/Loading/Error.vue` are not necessary.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2025-02-18T18:38:24.981Z
Learnt from: zackspear
Repo: unraid/api PR: 1143
File: web/pages/webComponents.vue:28-29
Timestamp: 2025-02-18T18:38:24.981Z
Learning: In Vue components, when there's a repeating pattern of heading + content + divider, prefer using a wrapper component that takes the heading as a prop and content via slots, rather than duplicating the structure. This improves maintainability and reduces code duplication.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2025-06-13T17:14:21.739Z
Learnt from: pujitm
Repo: unraid/api PR: 1417
File: web/components/ConnectSettings/ConnectSettings.ce.vue:11-18
Timestamp: 2025-06-13T17:14:21.739Z
Learning: The project’s build tooling auto-imports common Vue/Pinia helpers such as `storeToRefs`, so explicit import statements for them are not required.
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-03-27T23:52:57.888Z
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files ensures that all web components share a single Pinia store instance, which is the desired behavior. Without this initialization, each web component would have its own isolated store, breaking the intended architecture.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Use `createTestingPinia()` for mocking stores in Vue components
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2025-03-27T23:52:57.888Z
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. The `setActivePinia(createPinia())` call at the module level in store files is intentional and ensures all web components share a single Pinia store instance, which is the desired behavior. This shared state approach is critical for the application's architecture to function correctly.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Nuxt is set to auto import so some Vue files may need `computed` or `ref` imported explicitly in store files
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-03-27T23:33:13.215Z
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:33:13.215Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files would break the build by causing all web components to share a singular Pinia store instance. Each web component needs its own Pinia store instance to maintain proper isolation and encapsulation.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/store/**/*.ts : Ensure Vue reactivity imports like `computed`, `ref`, and `watchEffect` are added to store files even with Nuxt auto-imports enabled
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-03-27T23:33:13.215Z
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:33:13.215Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files would break the build by causing all web components to share a singular Pinia store instance. Each web component needs its own Pinia store instance to function correctly.
Applied to files:
web/src/components/Notifications/List.vue
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to web/src/**/*.ts : Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-09T15:47:29.325Z
Learnt from: pujitm
Repo: unraid/api PR: 975
File: web/components/Notifications/TabList.vue:1-24
Timestamp: 2024-12-09T15:47:29.325Z
Learning: In our Nuxt setup using Vue.js, components defined within the codebase are autoloaded and do not require explicit import statements in the script section. For example, components like `TabsList` and `TabsTrigger` used in `web/components/Notifications/TabList.vue` are automatically available without imports.
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-01-31T22:01:22.708Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:24-27
Timestamp: 2025-01-31T22:01:22.708Z
Learning: The Unraid UI uses a modern notification system with a custom `unraid-toaster` component replacing the legacy jGrowl notifications. The system is backed by a comprehensive GraphQL API with real-time subscription support for notification updates.
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vueapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.model.ts
📚 Learning: 2024-12-06T17:34:16.133Z
Learnt from: pujitm
Repo: unraid/api PR: 974
File: web/components/Loading/Error.vue:50-50
Timestamp: 2024-12-06T17:34:16.133Z
Learning: In this project, the `Button` component from `~/components/shadcn/Button.vue` is autoloaded and does not need to be imported manually in components like `web/components/Loading/Error.vue`.
Applied to files:
web/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-01-31T22:01:41.842Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch removes the old jGrowl notification system and is complemented by the unraid-toaster component implementation. The new system is added through the DefaultPageLayout modification which inserts the toaster component with proper position configuration based on user preferences.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-01-31T22:01:02.725Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:30-54
Timestamp: 2025-01-31T22:01:02.725Z
Learning: The removal of jGrowl notifications from DefaultPageLayout.php is intentional as notifications are now handled on a separate page as part of the architectural design to override existing Unraid pages.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-01-31T22:01:41.842Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch is used to remove the old jGrowl notification system from Unraid pages, as notifications are handled by a new system implemented on a different page.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-09T15:45:46.492Z
Learnt from: pujitm
Repo: unraid/api PR: 975
File: web/components/Notifications/TabList.vue:1-4
Timestamp: 2024-12-09T15:45:46.492Z
Learning: In our Nuxt.js setup for the `web` project, it's not necessary to explicitly import `computed` from `vue` in Vue components, as it's globally available.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-17T13:55:42.068Z
Learnt from: elibosley
Repo: unraid/api PR: 972
File: web/components/ColorSwitcher.ce.vue:1-2
Timestamp: 2024-12-17T13:55:42.068Z
Learning: In this Nuxt.js project, components used in templates are automatically imported by Nuxt, so explicit import statements for components are unnecessary.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Use `mount` from Vue Test Utils for component testing
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Check for expected prop handling and event emissions in Vue components
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-03-14T16:10:38.562Z
Learnt from: pujitm
Repo: unraid/api PR: 1211
File: api/src/graphql/schema/types/connect/connect.graphql:142-146
Timestamp: 2025-03-14T16:10:38.562Z
Learning: The updateApiSettings mutation in api/src/unraid-api/graph/connect/connect.resolver.ts is protected with the UsePermissions decorator that requires UPDATE permission on the CONFIG resource.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to **/*.{ts,tsx} : Avoid using casting whenever possible, prefer proper typing from the start
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
📚 Learning: 2025-03-07T19:29:12.540Z
Learnt from: pujitm
Repo: unraid/api PR: 1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to **/*.{ts,tsx} : Never use the `any` type. Always prefer proper typing
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
📚 Learning: 2025-04-07T14:34:47.255Z
Learnt from: elibosley
Repo: unraid/api PR: 1334
File: api/src/unraid-api/graph/resolvers/docker/docker-event.service.ts:63-66
Timestamp: 2025-04-07T14:34:47.255Z
Learning: In DockerEventService, the chokidar file watcher is configured with `ignoreInitial: false` to ensure that existing files (like the Docker socket) are detected and processed at application startup, not just when files change after the watcher is initialized.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
🧬 Code graph analysis (4)
web/src/components/Notifications/graphql/notification.subscription.ts (1)
web/src/composables/gql/gql.ts (1)
graphql(456-458)
api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (4)
web/src/composables/gql/graphql.ts (3)
NotificationSettings(1703-1709)Subscription(2310-2326)NotificationEvent(1671-1675)web/composables/gql/graphql.ts (1)
Subscription(2032-2044)packages/unraid-shared/src/use-permissions.directive.ts (1)
UsePermissions(81-128)api/src/core/pubsub.ts (1)
createSubscription(19-23)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (4)
packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)web/src/composables/gql/graphql.ts (2)
Notification(1640-1653)NotificationSettings(1703-1709)api/src/core/pubsub.ts (1)
pubsub(12-12)api/src/utils.ts (2)
batchProcess(58-71)formatDatetime(138-179)
api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts (1)
web/composables/gql/graphql.ts (4)
Node(1357-1359)Notification(1361-1374)NotificationOverview(1405-1409)NotificationCounts(1376-1382)
🔇 Additional comments (29)
web/src/components/Notifications/graphql/notification.subscription.ts (1)
31-41: LGTM!The new
notificationEventSubscriptionfollows the established pattern of other subscriptions in this file, correctly uses the sharedNotificationFragment, and aligns with the real-time notification event system introduced in this PR.web/src/components/Notifications/List.vue (4)
40-50: LGTM!The increased
pageSizeof 50 with documented rationale for minimizing pagination drift is a reasonable trade-off. The newrefetchedemit is properly typed and integrates well with parent component coordination.
267-272: LGTM!The error message computation correctly prioritizes offline errors over GraphQL errors and uses the helper function for consistent error extraction.
93-127: The DELETED event handling is correct as-is.The backend implementation always includes the notification object when publishing DELETED events (notifications.service.ts:426), so the condition
notification && notification.type === props.typeon lines 118-123 will not silently fail. Only CLEARED events publish withnotification: null, and those are already handled with unconditional refetch on line 105.Likely an incorrect or invalid review comment.
209-218: Pagination drift detection is handled correctly with VueUse's automatic cleanup.The
setTimeoutcallback safely callsdebouncedRefetch(), which is created usinguseDebounceFnfrom@vueuse/core. VueUse composables automatically integrate with Vue 3's lifecycle and cancel pending debounced calls when the component unmounts, so no additional ref-guarding or explicit cleanup is required.web/src/components/Notifications/Sidebar.vue (7)
42-54: LGTM!The writable computed for
activeFiltercleanly handles the 'all' ↔undefinedmapping, and the filter tabs are properly typed.
64-72: LGTM!The error handling with
console.errorfor logging andtoast.addfor user feedback aligns with the learned pattern of keeping notifications visible and informing users when mutations fail. Based on learnings, this approach is appropriate since the error source is the mutation, not the toast system itself.
110-127: LGTM!The
handleRefetchfunction properly sequences the overview recalculation before refetching, and the watcher correctly triggers recovery when the API comes back online.
130-166: LGTM!The subscription handler correctly filters for new unread notifications, applies importance-based toast colors, and uses
requestAnimationFrameto ensure smooth toast rendering. The conditional link navigation is properly guarded.
220-306: LGTM!The USlideover-based layout is well-structured with proper tab management, filter controls, and settings navigation. Both NotificationsList instances correctly wire up the
@refetchedevent tohandleRefetchfor coordinated state updates.
216-217: Good accessibility practice.The
sr-onlyspan provides proper screen reader context for the notification button trigger.
30-32: The code is correct.useToast()is auto-imported by Nuxt and does not require an explicit import statement, as confirmed by consistent usage throughout the codebase (e.g.,web/src/components/Notifications/Item.vue,web/src/composables/useClipboardWithToast.ts,web/src/components/RClone/RCloneOverview.vue). This aligns with the project's established pattern of auto-importing common helpers and composables.packages/unraid-shared/src/pubsub/graphql.pubsub.ts (1)
23-23: LGTM!The new
NOTIFICATION_EVENTchannel is correctly added and follows the existing naming conventions for notification-related pubsub channels.api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts (4)
26-35: LGTM!The
NotificationEventTypeenum is well-defined with clear semantic values for the notification lifecycle events, and properly registered with GraphQL.
37-78: LGTM!The
Notificationclass is well-structured with proper validation decorators and field definitions that align with the client-side GraphQL types.
165-188: LGTM!The
NotificationSettingsclass has appropriate validators. Using@Min(1)fordurationandmaxcorrectly prevents invalid zero or negative values.
190-222: LGTM!The
NotificationOverviewand updatedNotificationsclass are properly structured with correct field decorators and validators.api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (2)
46-49: LGTM!The
settingsresolver field correctly delegates to the service and follows the existing pattern for resolve fields.
202-209: LGTM!The
notificationEventsubscription follows the established pattern, with proper permissions and correct pubsub channel usage.api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (10)
147-170: LGTM with minor observation.The retry logic is appropriate for handling file system race conditions. The 5 attempts with 100ms delay provides reasonable resilience without excessive waiting.
210-228: LGTM!The
getSettingsmethod properly parses configuration with defensive defaults and handles various input formats gracefully.
574-612: LGTM!The shadow handling logic correctly manages the edge case where a notification exists in both unread and archive directories. The stat update reversion on failure is properly implemented.
901-922: LGTM!The async generator pattern with batched loading is a good improvement for memory efficiency. The discriminated union return type cleanly separates success and failure cases.
936-974: LGTM!The refactored method correctly uses the generator for memory-efficient loading while properly handling pagination with offset/limit and maintaining the sort order.
1109-1133: LGTM!The updated datetime formatting correctly prioritizes notification-specific settings over global display settings, with a safe ISO fallback when formats are unavailable.
280-293: LGTM!The shadow filtering in
recalculateOverviewcorrectly prevents double-counting notifications that exist in both unread and archive directories, using an efficient Set-based lookup.
458-465: LGTM!The
CLEAREDevent is appropriately used for bulk delete operations withnullnotification payload.
346-361: LGTM!The
getLegacyScriptArgsupdate is backward-compatible, correctly adding the-uflag only when an id is provided.
719-727: CLEARED event handling is correctly implemented.The client-side code in
List.vueproperly interprets theCLEAREDevent as a refetch signal. WhennotificationEventwithtype: 'CLEARED'is received, it invokesdebouncedRefetch()to refresh the list, which applies to both unread and archived notifications depending on the active filter. The 500ms debounce batches events efficiently. The semantic meaning is well-documented in the backend comment explaining thatCLEAREDsignals "unread list cleared" from the frontend perspective, not actual deletion.
|
🚀 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: 1
🤖 Fix all issues with AI agents
In @web/src/components/Notifications/Sidebar.vue:
- Line 19: Remove the unused import symbol notificationOverviewSubscription from
the import list in the Sidebar.vue component (it is declared but never
referenced), i.e., update the import statement that currently includes
notificationOverviewSubscription to omit that symbol; if the subscription is
intended to be used, instead wire it into the component logic (e.g., in setup or
created hook) and reference it, otherwise simply delete the import to resolve
the TS6133 unused import error.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/components/Notifications/Sidebar.vue
🧰 Additional context used
📓 Path-based instructions (2)
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/src/components/Notifications/Sidebar.vue
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
web/src/components/Notifications/Sidebar.vue
🧠 Learnings (16)
📓 Common learnings
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:24-27
Timestamp: 2025-01-31T22:01:22.708Z
Learning: The Unraid UI uses a modern notification system with a custom `unraid-toaster` component replacing the legacy jGrowl notifications. The system is backed by a comprehensive GraphQL API with real-time subscription support for notification updates.
Learnt from: pujitm
Repo: unraid/api PR: 941
File: web/components/Notifications/Item.vue:0-0
Timestamp: 2024-11-01T17:52:55.311Z
Learning: In the `web/components/Notifications/Item.vue` component, ensure that when delete or archive mutations fail, the notification remains displayed, and an error message is shown below the action buttons to inform the user.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:30-54
Timestamp: 2025-01-31T22:01:02.725Z
Learning: The removal of jGrowl notifications from DefaultPageLayout.php is intentional as notifications are now handled on a separate page as part of the architectural design to override existing Unraid pages.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch removes the old jGrowl notification system and is complemented by the unraid-toaster component implementation. The new system is added through the DefaultPageLayout modification which inserts the toaster component with proper position configuration based on user preferences.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch is used to remove the old jGrowl notification system from Unraid pages, as notifications are handled by a new system implemented on a different page.
Learnt from: mdatelle
Repo: unraid/api PR: 1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
📚 Learning: 2024-11-01T17:52:55.311Z
Learnt from: pujitm
Repo: unraid/api PR: 941
File: web/components/Notifications/Item.vue:0-0
Timestamp: 2024-11-01T17:52:55.311Z
Learning: In the `web/components/Notifications/Item.vue` component, ensure that when delete or archive mutations fail, the notification remains displayed, and an error message is shown below the action buttons to inform the user.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-01-31T22:01:22.708Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:24-27
Timestamp: 2025-01-31T22:01:22.708Z
Learning: The Unraid UI uses a modern notification system with a custom `unraid-toaster` component replacing the legacy jGrowl notifications. The system is backed by a comprehensive GraphQL API with real-time subscription support for notification updates.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-01-31T22:01:41.842Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch removes the old jGrowl notification system and is complemented by the unraid-toaster component implementation. The new system is added through the DefaultPageLayout modification which inserts the toaster component with proper position configuration based on user preferences.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-01-31T22:01:02.725Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:30-54
Timestamp: 2025-01-31T22:01:02.725Z
Learning: The removal of jGrowl notifications from DefaultPageLayout.php is intentional as notifications are now handled on a separate page as part of the architectural design to override existing Unraid pages.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-01-31T14:42:06.752Z
Learnt from: pujitm
Repo: unraid/api PR: 1075
File: web/components/Notifications/Sidebar.vue:41-58
Timestamp: 2025-01-31T14:42:06.752Z
Learning: When handling errors in code that uses toast notifications, avoid using toast in the error handler itself since toast functionality might be the source of the error. Use console.error or other reliable logging mechanisms instead.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-09T15:47:29.325Z
Learnt from: pujitm
Repo: unraid/api PR: 975
File: web/components/Notifications/TabList.vue:1-24
Timestamp: 2024-12-09T15:47:29.325Z
Learning: In our Nuxt setup using Vue.js, components defined within the codebase are autoloaded and do not require explicit import statements in the script section. For example, components like `TabsList` and `TabsTrigger` used in `web/components/Notifications/TabList.vue` are automatically available without imports.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-09T15:45:46.492Z
Learnt from: pujitm
Repo: unraid/api PR: 975
File: web/components/Notifications/TabList.vue:1-4
Timestamp: 2024-12-09T15:45:46.492Z
Learning: In our Nuxt.js setup for the `web` project, it's not necessary to explicitly import `computed` from `vue` in Vue components, as it's globally available.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Nuxt is set to auto import so some Vue files may need `computed` or `ref` imported explicitly in store files
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-06T17:34:16.133Z
Learnt from: pujitm
Repo: unraid/api PR: 974
File: web/components/Loading/Error.vue:50-50
Timestamp: 2024-12-06T17:34:16.133Z
Learning: In this project, the `Button` component from `~/components/shadcn/Button.vue` is autoloaded and does not need to be imported manually in components like `web/components/Loading/Error.vue`.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to web/src/**/*.ts : Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/store/**/*.ts : Ensure Vue reactivity imports like `computed`, `ref`, and `watchEffect` are added to store files even with Nuxt auto-imports enabled
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-17T13:55:42.068Z
Learnt from: elibosley
Repo: unraid/api PR: 972
File: web/components/ColorSwitcher.ce.vue:1-2
Timestamp: 2024-12-17T13:55:42.068Z
Learning: In this Nuxt.js project, components used in templates are automatically imported by Nuxt, so explicit import statements for components are unnecessary.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Use `mount` from Vue Test Utils for component testing
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-06-13T17:14:21.739Z
Learnt from: pujitm
Repo: unraid/api PR: 1417
File: web/components/ConnectSettings/ConnectSettings.ce.vue:11-18
Timestamp: 2025-06-13T17:14:21.739Z
Learning: The project’s build tooling auto-imports common Vue/Pinia helpers such as `storeToRefs`, so explicit import statements for them are not required.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Check for expected prop handling and event emissions in Vue components
Applied to files:
web/src/components/Notifications/Sidebar.vue
🪛 GitHub Actions: CI - Main (API)
web/src/components/Notifications/Sidebar.vue
[error] 19-19: vue-tsc --noEmit: TS6133: 'notificationOverviewSubscription' is declared but its value is never read.
🔇 Additional comments (7)
web/src/components/Notifications/Sidebar.vue (7)
64-73: LGTM: Error handling with toast feedback.The error handling correctly uses
console.errorfor logging and provides user feedback via toast notifications. This pattern appropriately distinguishes between logging (console) and user communication (toast).
84-93: LGTM: Consistent error handling pattern.Error handling follows the same reliable pattern as
confirmAndArchiveAll.
96-116: LGTM: Refetch logic replaces subscribeToMore.The manual refetch triggered by API status changes appropriately replaces the previous
subscribeToMorepattern. Thewatchensures overview data is refreshed when the API comes online, while individual notification events are still handled in real-time vianotificationEventSubscription.
163-167: LGTM: Archived count now uses correct API field.The updated logic correctly retrieves the archived count from
overview.value.archive.totalas reported by the API.
173-187: LGTM: Clean reactive state management.The reactive state and computed tabs are well-structured with proper TypeScript typing and derive their badge counts from the overview data.
191-206: LGTM: Header button implementation.The notification bell button correctly integrates with the theme store and triggers the slide-over panel with proper visibility management.
208-297: LGTM: Comprehensive slide-over UI implementation.The refactored UI properly uses NuxtUI components (USlideover, UTabs, UButton, UTooltip) with correct two-way binding, conditional rendering, and event handling. The
@refetched="handleRefetch"emissions properly synchronize the list and overview data.
|
🚀 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: 0
🧹 Nitpick comments (1)
web/src/components/Notifications/Sidebar.vue (1)
273-286: Consider consolidating duplicate NotificationsList instances.Two
NotificationsListcomponents are conditionally rendered with nearly identical props. This could be simplified to a single instance with a computed:typeprop.♻️ Optional consolidation
- <NotificationsList - v-if="activeTab === 'unread'" - :importance="importance" - :type="NotificationType.UNREAD" - class="flex-1" - @refetched="handleRefetch" - /> - <NotificationsList - v-else - :importance="importance" - :type="NotificationType.ARCHIVE" - class="flex-1" - @refetched="handleRefetch" - /> + <NotificationsList + :importance="importance" + :type="activeTab === 'unread' ? NotificationType.UNREAD : NotificationType.ARCHIVE" + :key="activeTab" + class="flex-1" + @refetched="handleRefetch" + />The
:keyensures the component remounts when switching tabs, preserving any scroll state reset behavior.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/components/Notifications/Sidebar.vue
🧰 Additional context used
📓 Path-based instructions (2)
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/src/components/Notifications/Sidebar.vue
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
web/src/components/Notifications/Sidebar.vue
🧠 Learnings (15)
📓 Common learnings
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:24-27
Timestamp: 2025-01-31T22:01:22.708Z
Learning: The Unraid UI uses a modern notification system with a custom `unraid-toaster` component replacing the legacy jGrowl notifications. The system is backed by a comprehensive GraphQL API with real-time subscription support for notification updates.
Learnt from: pujitm
Repo: unraid/api PR: 941
File: web/components/Notifications/Item.vue:0-0
Timestamp: 2024-11-01T17:52:55.311Z
Learning: In the `web/components/Notifications/Item.vue` component, ensure that when delete or archive mutations fail, the notification remains displayed, and an error message is shown below the action buttons to inform the user.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch removes the old jGrowl notification system and is complemented by the unraid-toaster component implementation. The new system is added through the DefaultPageLayout modification which inserts the toaster component with proper position configuration based on user preferences.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch is used to remove the old jGrowl notification system from Unraid pages, as notifications are handled by a new system implemented on a different page.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:30-54
Timestamp: 2025-01-31T22:01:02.725Z
Learning: The removal of jGrowl notifications from DefaultPageLayout.php is intentional as notifications are now handled on a separate page as part of the architectural design to override existing Unraid pages.
Learnt from: mdatelle
Repo: unraid/api PR: 1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
📚 Learning: 2024-11-01T17:52:55.311Z
Learnt from: pujitm
Repo: unraid/api PR: 941
File: web/components/Notifications/Item.vue:0-0
Timestamp: 2024-11-01T17:52:55.311Z
Learning: In the `web/components/Notifications/Item.vue` component, ensure that when delete or archive mutations fail, the notification remains displayed, and an error message is shown below the action buttons to inform the user.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-01-31T22:01:22.708Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:24-27
Timestamp: 2025-01-31T22:01:22.708Z
Learning: The Unraid UI uses a modern notification system with a custom `unraid-toaster` component replacing the legacy jGrowl notifications. The system is backed by a comprehensive GraphQL API with real-time subscription support for notification updates.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-01-31T22:01:41.842Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch removes the old jGrowl notification system and is complemented by the unraid-toaster component implementation. The new system is added through the DefaultPageLayout modification which inserts the toaster component with proper position configuration based on user preferences.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-01-31T22:01:02.725Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:30-54
Timestamp: 2025-01-31T22:01:02.725Z
Learning: The removal of jGrowl notifications from DefaultPageLayout.php is intentional as notifications are now handled on a separate page as part of the architectural design to override existing Unraid pages.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-01-31T14:42:06.752Z
Learnt from: pujitm
Repo: unraid/api PR: 1075
File: web/components/Notifications/Sidebar.vue:41-58
Timestamp: 2025-01-31T14:42:06.752Z
Learning: When handling errors in code that uses toast notifications, avoid using toast in the error handler itself since toast functionality might be the source of the error. Use console.error or other reliable logging mechanisms instead.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-09T15:47:29.325Z
Learnt from: pujitm
Repo: unraid/api PR: 975
File: web/components/Notifications/TabList.vue:1-24
Timestamp: 2024-12-09T15:47:29.325Z
Learning: In our Nuxt setup using Vue.js, components defined within the codebase are autoloaded and do not require explicit import statements in the script section. For example, components like `TabsList` and `TabsTrigger` used in `web/components/Notifications/TabList.vue` are automatically available without imports.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/store/**/*.ts : Ensure Vue reactivity imports like `computed`, `ref`, and `watchEffect` are added to store files even with Nuxt auto-imports enabled
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Nuxt is set to auto import so some Vue files may need `computed` or `ref` imported explicitly in store files
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to web/src/**/*.ts : Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-09T15:45:46.492Z
Learnt from: pujitm
Repo: unraid/api PR: 975
File: web/components/Notifications/TabList.vue:1-4
Timestamp: 2024-12-09T15:45:46.492Z
Learning: In our Nuxt.js setup for the `web` project, it's not necessary to explicitly import `computed` from `vue` in Vue components, as it's globally available.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-06T17:34:16.133Z
Learnt from: pujitm
Repo: unraid/api PR: 974
File: web/components/Loading/Error.vue:50-50
Timestamp: 2024-12-06T17:34:16.133Z
Learning: In this project, the `Button` component from `~/components/shadcn/Button.vue` is autoloaded and does not need to be imported manually in components like `web/components/Loading/Error.vue`.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-06-13T17:14:21.739Z
Learnt from: pujitm
Repo: unraid/api PR: 1417
File: web/components/ConnectSettings/ConnectSettings.ce.vue:11-18
Timestamp: 2025-06-13T17:14:21.739Z
Learning: The project’s build tooling auto-imports common Vue/Pinia helpers such as `storeToRefs`, so explicit import statements for them are not required.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-17T13:55:42.068Z
Learnt from: elibosley
Repo: unraid/api PR: 972
File: web/components/ColorSwitcher.ce.vue:1-2
Timestamp: 2024-12-17T13:55:42.068Z
Learning: In this Nuxt.js project, components used in templates are automatically imported by Nuxt, so explicit import statements for components are unnecessary.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-04-02T21:21:29.168Z
Learnt from: elibosley
Repo: unraid/api PR: 1308
File: unraid-ui/src/components/common/loading/Error.vue:2-2
Timestamp: 2025-04-02T21:21:29.168Z
Learning: Components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other parts of the project that may use Nuxt.js autoloading features.
Applied to files:
web/src/components/Notifications/Sidebar.vue
🔇 Additional comments (8)
web/src/components/Notifications/Sidebar.vue (8)
1-37: LGTM on imports and setup.The imports are well-organized, and the use of auto-imported
useToastis consistent with Nuxt conventions. Store initialization and composable usage follow expected patterns.
39-51: Clean implementation of filter state management.The writable computed pattern with normalization (
'all'→undefined) is appropriate for bridging the UI's "all" concept with the API's optional importance filter.
53-91: Error handling follows established patterns.The try/catch blocks correctly log errors to console before displaying toast notifications. This aligns with the guidance to use
console.errorfor reliable logging when toast might be unavailable. Based on learnings, previous guidance suggested showing errors below action buttons, but using toasts is consistent with the NuxtUI migration approach in this PR.
93-112: Solid refetch strategy with API status awareness.The
handleRefetchfunction correctly ensuresrefetch()runs regardless of whetherrecalculateOverviewsucceeds, using thefinallyblock. The watcher on API status properly triggers a refresh when the connection is restored, addressing the PR objective of fixing the infinite API-request loop issue.
114-151: Subscription handling is well-structured.The event subscription correctly filters for
ADDEDevents ofUNREADtype before processing. TherequestAnimationFramewrapper appropriately defers toast rendering. The conditional action creation on line 148 ensures the "Open" button only appears when a link exists.
153-184: Overview and tab state management is correctly implemented.The computed properties properly handle undefined states with optional chaining. The
tabscomputed provides dynamic badge values that will update reactively when notification counts change.
187-203: Accessible notification button with theme integration.The button includes proper screen reader text via
sr-only. The theme-aware styling usingthemeStore.theme.textColorintegrates with the broader theming system mentioned in the PR objectives.
292-294: LGTM!The
ConfirmDialogis correctly placed at the component level to support theuseConfirmcomposable used by the archive/delete confirmation flows.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09a81de1b1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| NotificationsService.watcher = watch(basePath, { | ||
| usePolling: CHOKIDAR_USEPOLLING, | ||
| ignoreInitial: true, // Only watch for new files | ||
| }).on('add', (path) => { |
There was a problem hiding this comment.
With ignoreInitial: true the watcher no longer emits add events for the existing notification files at startup. Because NotificationsService doesn’t perform a startup scan/recalculate, the in-memory overview stays at zero until a new notification arrives or a client explicitly calls recalculateOverview. This means notificationsOverview (and the UI badge) will be wrong after an API restart when unread/archive files already exist. Consider seeding the overview on bootstrap (or removing ignoreInitial) so clients querying immediately after restart get correct counts.
Useful? React with 👍 / 👎.
| if (type === NotificationType.UNREAD) { | ||
| this.publishOverview(); | ||
| pubsub.publish(PUBSUB_CHANNEL.NOTIFICATION_ADDED, { | ||
| notificationAdded: notification, | ||
| }); | ||
| pubsub.publish(PUBSUB_CHANNEL.NOTIFICATION_EVENT, { | ||
| notificationEvent: { | ||
| type: NotificationEventType.ADDED, | ||
| notification, | ||
| }, | ||
| }); | ||
| void this.publishWarningsAndAlerts(); | ||
| } | ||
| // Also publish overview updates for archive adds, so counts stay in sync | ||
| if (type === NotificationType.ARCHIVE) { | ||
| pubsub.publish(PUBSUB_CHANNEL.NOTIFICATION_EVENT, { | ||
| notificationEvent: { | ||
| type: NotificationEventType.ADDED, | ||
| notification, | ||
| }, | ||
| }); | ||
| this.publishOverview(); | ||
| } |
There was a problem hiding this comment.
seems like NOTIFICATION_EVENT & publishOverview happen regardless of condition, so they might not need to be in if blocks. also, it seems redundant to have both a NOTIFICATION_ADDED and NOTIFICATION_EVENT -> type: ADDED channel -- why keep the former around? backwards compat?
There was a problem hiding this comment.
I did keep it around, but I just removed it because it's confusing (and our frontend doesn't use it), so that's a breaking change we'll need to inform ppl who use our existing graphql api about.
| public getSettings(): NotificationSettings { | ||
| const { notify } = getters.dynamix(); | ||
| const parseBoolean = (value: unknown, defaultValue: boolean) => { | ||
| if (value === undefined || value === null || value === '') return defaultValue; | ||
| const s = String(value).toLowerCase(); | ||
| return s === 'true' || s === '1' || s === 'yes'; | ||
| }; | ||
| const parsePositiveInt = (value: unknown, defaultValue: number) => { | ||
| const n = Number(value); | ||
| return !isNaN(n) && n > 0 ? n : defaultValue; | ||
| }; | ||
|
|
||
| return { | ||
| position: notify?.position ?? 'top-right', | ||
| expand: parseBoolean(notify?.expand, true), | ||
| duration: parsePositiveInt(notify?.duration, 5000), | ||
| max: parsePositiveInt(notify?.max, 3), | ||
| }; | ||
| } |
There was a problem hiding this comment.
are you certain that this value parsing doesn't already happen in the notify getter? iiirc it did
There was a problem hiding this comment.
parsing does happen in the notify getter, but further parsing is indeed required. this was intentional.
| // Update stats | ||
| this.decrement(notification.importance, NotificationsService.overview.unread); | ||
|
|
||
| if (snapshot) { | ||
| this.decrement(notification.importance, snapshot.unread); | ||
| } |
There was a problem hiding this comment.
the point of the snapshot was to update the overview atomically. if that's no longer working/needed, maybe we should get rid of the snapshot + its modifications!
| if (matches(notification)) { | ||
| if (skipped < offset) { | ||
| skipped++; | ||
| } else { | ||
| notifications.push(notification); | ||
| if (notifications.length >= limit) { | ||
| break; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
confused about what's going on here, esp what the skipped vs. offset is trying to do
There was a problem hiding this comment.
Oh this is the fix for the bug where we were paginating by like 15 and then you filtered by importance (Warning, Alert, Info) so for ex. when the user filtered by "Alerts" it only showed the user 3 Alerts because after giving the first page of the first 15 notifications (from all notifications), only 3 notifications were alerts, and since 3 < 15, it didn't ask for another page. When in reality, if you got the next page, there might be another 5 Alert notifications
So this solution basically ensures that we actually paginate the filtered alerts. Since we're using a file-system as a db and not a regular db, we have to add a workaround to determine if a notification matches our filter while we are reading the files.
skipped < offset: Ignores the matches that belong to previous pages.
and
break: Stops it from reading the rest of the files on disk once we have filled the current page (limit)."
| const text = t(key); | ||
| return text !== key ? text : 'The Unraid API is unreachable.'; |
There was a problem hiding this comment.
Is text ever supposed to equal key here? @elibosley
not doubting that it does/did, just wondering whether a more fundamental fix would be more appropriate
| let message = extractGraphQLErrorMessage(e); | ||
| if ( | ||
| unraidApiStore.unraidApiStatus === 'offline' || | ||
| message === 'Error message not found' || | ||
| message === 'Failed to fetch' || | ||
| (e as { message?: string })?.message === 'Failed to fetch' |
There was a problem hiding this comment.
my concern here would be brittleness -- very easy to accidentally introduce a regression/different error display behavior accidentally after modifying the backend error message
There was a problem hiding this comment.
i added a code to the graphql extensions and use that instead. Lmk if you don't like the way I implemented it or smth
There was a problem hiding this comment.
cool updates, ty for fixing infinite scroll :)
| 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; | ||
| } |
There was a problem hiding this comment.
so, uh, the map's keys and values are the same! oh no!
There was a problem hiding this comment.
oh this was cuz the requirements changed, and then changed again. I kept it there just in case it changed again.
But now it seems like we know what we want, so I'll just change the map into a array.
Good catch.
| try { | ||
| const { data } = await apolloClient.query<NotificationSettingsResponse>({ | ||
| query: getNotificationSettings, | ||
| fetchPolicy: 'network-only', | ||
| }); |
There was a problem hiding this comment.
will the mounting of all vue components be blocked until this query responds? if so, i'd move this elsewhere, maybe into an onMount for a notification component instead.
There was a problem hiding this comment.
workaround im thinking of is maybe using .then() instead of await and also use Vue's reactivity system
…cription - Remove the redundant `NOTIFICATION_ADDED` pubsub event and `notificationAdded` GraphQL subscription. - Standardize all notification lifecycle updates (Add, Archive, Delete) to use the unified `NOTIFICATION_EVENT` channel. - Refactor handleNotificationAdd in NotificationsService to hoist common logic, reducing code duplication between Unread and Archive handling. - Ensure `overview` updates are published consistent for all notification types.
- Refactor Item.vue to use stable error codes (`NOTIFICATION_NOT_FOUND`) instead of brittle string matching. - Update AppError and global exception filter to properly propagate error codes to GraphQL `extensions`. - Add `NOTIFICATION_NOT_FOUND` error code to backend delete/archive operations. - distinct handling for "Notification not found" (user friendly) vs "API Offline" (network error) in frontend. - Fix horizontal overflow on error messages in notification items. - Migrate `CriticalNotifications` component to use `notificationEvent` subscription (replacing deprecated `notificationAdded`). - Add helper functions extractGraphQLErrorCode and isGraphQLNetworkError. - Add backend tests for non-existent notification scenarios.
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
Replaced the redundant key-value map in `mount-engine.ts` with a direct array inclusion check to validate `ToastPosition` values from settings.
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
- Refactor mountUnifiedApp to fetch notification settings in the background instead of `await`ing them. - Wrap `toasterSettings` in a Vue `reactive` object so the UI updates automatically when settings arrive. - Eliminate the startup delay caused by blocking on the network request before mounting components.
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @api/generated-schema.graphql:
- Around line 2912-2916: Add a deprecated alias field named notificationAdded to
the Subscription type so external clients keep working: declare
notificationAdded with the same GraphQL return type as the old subscription and
mark it @deprecated, then implement a resolver for notificationAdded that
forwards/events from the new notificationEvent (or maps NotificationEvent
payload to the old Notification payload) so the behavior is identical; update
the Subscription type definition (Subscription) and the notificationEvent
resolver to ensure notificationAdded reuses the new event stream and is
documented as deprecated.
In @api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts:
- Around line 653-677: The batchCreateArchive function can run concurrently for
the same notification id causing race conditions when updating the shared
NotificationsService.overview via decrement/increment; add a guard so operations
for the same notification id are serialized (e.g., a per-id lock or a
Map<string, Promise> to chain operations) or wrap the decrement/increment pair
in a mutex to ensure atomic updates; locate batchCreateArchive, batchProcess,
NotificationsService.overview, processingArchives, decrement and increment and
implement the per-id locking or global mutex around the stats update, or
document the limitation if you prefer not to change runtime behavior.
- Around line 60-65: The Set processingArchives can retain stale paths if the
rename in batchCreateArchive fails; wrap the rename and any subsequent work that
depends on it in a try/finally (or ensure each catch removes the path) so the
path is removed from processingArchives on all error paths; update
batchCreateArchive to add the path, attempt rename inside try, and always call
processingArchives.delete(path) in finally (or before each rethrow) so the
watcher is not permanently suppressed.
🧹 Nitpick comments (8)
api/src/unraid-api/exceptions/graphql-exceptions.filter.ts (1)
21-29: Avoidanycasts; use a type guard or interface instead.The repeated
(exception.originalError as any)casts violate the coding guidelines prohibitingany. Consider defining an interface for errors that carrycodeandextensions:♻️ Suggested refactor
+interface ErrorWithExtensions extends Error { + code?: string; + extensions?: Record<string, unknown>; +} + +function hasExtensions(error: unknown): error is ErrorWithExtensions { + return error instanceof Error; +} + @Catch(GraphQLError) export class GraphQLExceptionsFilter<T extends GraphQLError> implements ExceptionFilter { catch(exception: T, host: ArgumentsHost) { const ctx = host.switchToHttp(); const response: FastifyReply<any> = ctx.getResponse<FastifyReply>(); + const originalError = exception.originalError; if (response.code) { response.code(200).send({ data: null, errors: [ { message: exception.message, locations: exception.locations, path: exception.path, extensions: { ...exception.extensions, - ...((exception.originalError as any)?.code - ? { code: (exception.originalError as any).code } - : {}), - ...((exception.originalError as any)?.extensions - ? (exception.originalError as any).extensions - : {}), + ...(hasExtensions(originalError) && originalError.code + ? { code: originalError.code } + : {}), + ...(hasExtensions(originalError) && originalError.extensions + ? originalError.extensions + : {}), }, }, ], }); } } }api/src/core/errors/app-error.ts (1)
14-15: Replaceanywithunknownin extensions type.Per coding guidelines, avoid using
any. UseRecord<string, unknown>for safer typing.♻️ Suggested fix
- /** GraphQL Extensions compatibility */ - public extensions?: Record<string, any>; + /** GraphQL Extensions compatibility */ + public extensions?: Record<string, unknown>;web/src/helpers/functions.ts (1)
25-57: Consider simplifying the redundant type assertion on line 46-47.The
e.networkErroris already typed with{ message?: string }on line 27, so the cast on lines 46-47 is redundant.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;web/src/components/Notifications/Item.vue (1)
87-104: Consider extracting shared error message logic.The logic for determining error messages (NOTIFICATION_NOT_FOUND, offline check) is duplicated between
handleMutationandmutationError. Consider extracting to a shared helper function.Proposed refactor
const getErrorMessage = (err: unknown): string => { const code = extractGraphQLErrorCode(err); if (code === 'NOTIFICATION_NOT_FOUND') { return t('notifications.item.notFoundError', 'Notification no longer exists.'); } if (unraidApiStore.unraidApiStatus === 'offline' || isGraphQLNetworkError(err)) { const key = 'notifications.item.apiOfflineError'; const text = t(key); return text !== key ? text : 'The Unraid API is unreachable.'; } return extractGraphQLErrorMessage(err); };Then use in both
handleMutationandmutationError.api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts (1)
266-290: Avoidanytype and consider using structured error assertions.The
error: anytype annotation violates the coding guideline to never useany. Consider using a more specific type or restructuring the assertions.Proposed refactor using toMatchObject
- it('throws NOTIFICATION_NOT_FOUND when archiving a non-existent notification', async ({ - expect, - }) => { - const id = 'non-existent-id.notify'; - try { - await service.archiveNotification({ id }); - expect.fail('Should have thrown an error'); - } catch (error: any) { - expect(error.code).toBe('NOTIFICATION_NOT_FOUND'); - expect(error.status).toBe(404); - } - }); + it('throws NOTIFICATION_NOT_FOUND when archiving a non-existent notification', async ({ + expect, + }) => { + const id = 'non-existent-id.notify'; + await expect(service.archiveNotification({ id })).rejects.toMatchObject({ + code: 'NOTIFICATION_NOT_FOUND', + status: 404, + }); + }); - it('throws NOTIFICATION_NOT_FOUND when unarchiving a non-existent notification', async ({ - expect, - }) => { - const id = 'non-existent-id.notify'; - try { - await service.markAsUnread({ id }); - expect.fail('Should have thrown an error'); - } catch (error: any) { - expect(error.code).toBe('NOTIFICATION_NOT_FOUND'); - expect(error.status).toBe(404); - } - }); + it('throws NOTIFICATION_NOT_FOUND when unarchiving a non-existent notification', async ({ + expect, + }) => { + const id = 'non-existent-id.notify'; + await expect(service.markAsUnread({ id })).rejects.toMatchObject({ + code: 'NOTIFICATION_NOT_FOUND', + status: 404, + }); + });This approach:
- Eliminates the
anytype- Is more idiomatic for async error testing in Vitest
- Maintains the specific property assertions needed
api/generated-schema.graphql (1)
1418-1448: New notification schema types look coherent; keep an eye on nullability + type-safety.
NotificationEvent.notificationbeing nullable is a good fit forCLEARED.NotificationSettings.position: String!is flexible, but if the set of positions is finite, consider an enum later to prevent invalid values leaking through.api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (1)
917-921: Type default inconsistency:typedefaults toUNREADin two places.The
passesFiltercheck on line 918 defaults toUNREAD, and the generator call on line 921 also defaults toUNREAD. This is consistent, but consider extracting a constant or documenting this default behavior.web/src/components/Notifications/Sidebar.vue (1)
135-140: Navigation action onClick has no feedback ifnotif.linkis falsy.The
createOpenerfunction is only included whennotif.linkexists (line 147), but the internal onClick checksnotif.linkagain. The outer check makes the inner check redundant but harmless.♻️ Simplify onClick - the link is guaranteed to exist
const createOpener = () => ({ label: t('notifications.sidebar.toastOpen'), onClick: () => { - if (notif.link) { - navigate(notif.link); - } + navigate(notif.link!); }, });
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**
📒 Files selected for processing (14)
api/generated-schema.graphqlapi/src/core/errors/app-error.tsapi/src/unraid-api/exceptions/graphql-exceptions.filter.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsweb/src/components/Notifications/CriticalNotifications.standalone.vueweb/src/components/Notifications/Item.vueweb/src/components/Notifications/Sidebar.vueweb/src/components/Notifications/graphql/notification.subscription.tsweb/src/components/Wrapper/mount-engine.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.tsweb/src/helpers/functions.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/components/Wrapper/mount-engine.ts
- web/src/components/Notifications/CriticalNotifications.standalone.vue
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use TypeScript imports with.jsextensions for ESM compatibility
Never add comments unless they are needed for clarity of function
Never add comments for obvious things, and avoid commenting when starting and ending code blocks
Files:
api/src/unraid-api/exceptions/graphql-exceptions.filter.tsapi/src/core/errors/app-error.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsweb/src/helpers/functions.tsweb/src/composables/gql/gql.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.tsweb/src/composables/gql/graphql.tsweb/src/components/Notifications/graphql/notification.subscription.ts
api/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer adding new files to the NestJS repo located at
api/src/unraid-api/instead of the legacy code
Files:
api/src/unraid-api/exceptions/graphql-exceptions.filter.tsapi/src/core/errors/app-error.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.tsapi/generated-schema.graphql
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use theanytype. Always prefer proper typing
Avoid using casting whenever possible, prefer proper typing from the start
Files:
api/src/unraid-api/exceptions/graphql-exceptions.filter.tsapi/src/core/errors/app-error.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsweb/src/helpers/functions.tsweb/src/composables/gql/gql.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.tsweb/src/composables/gql/graphql.tsweb/src/components/Notifications/graphql/notification.subscription.ts
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 expects TTL values in milliseconds, not seconds (e.g., 600000 for 10 minutes, not 600)
Files:
api/src/unraid-api/exceptions/graphql-exceptions.filter.tsapi/src/core/errors/app-error.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
api/src/unraid-api/exceptions/graphql-exceptions.filter.tsapi/src/core/errors/app-error.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsweb/src/helpers/functions.tsweb/src/composables/gql/gql.tsweb/src/components/Notifications/Item.vueapi/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.tsweb/src/composables/gql/graphql.tsapi/generated-schema.graphqlweb/src/components/Notifications/Sidebar.vueweb/src/components/Notifications/graphql/notification.subscription.ts
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/src/helpers/functions.tsweb/src/composables/gql/gql.tsweb/src/components/Notifications/Item.vueweb/src/composables/gql/graphql.tsweb/src/components/Notifications/Sidebar.vueweb/src/components/Notifications/graphql/notification.subscription.ts
web/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Files:
web/src/helpers/functions.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.tsweb/src/components/Notifications/graphql/notification.subscription.ts
🧠 Learnings (37)
📓 Common learnings
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:24-27
Timestamp: 2025-01-31T22:01:22.708Z
Learning: The Unraid UI uses a modern notification system with a custom `unraid-toaster` component replacing the legacy jGrowl notifications. The system is backed by a comprehensive GraphQL API with real-time subscription support for notification updates.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch removes the old jGrowl notification system and is complemented by the unraid-toaster component implementation. The new system is added through the DefaultPageLayout modification which inserts the toaster component with proper position configuration based on user preferences.
Learnt from: pujitm
Repo: unraid/api PR: 941
File: web/components/Notifications/Item.vue:0-0
Timestamp: 2024-11-01T17:52:55.311Z
Learning: In the `web/components/Notifications/Item.vue` component, ensure that when delete or archive mutations fail, the notification remains displayed, and an error message is shown below the action buttons to inform the user.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:30-54
Timestamp: 2025-01-31T22:01:02.725Z
Learning: The removal of jGrowl notifications from DefaultPageLayout.php is intentional as notifications are now handled on a separate page as part of the architectural design to override existing Unraid pages.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch is used to remove the old jGrowl notification system from Unraid pages, as notifications are handled by a new system implemented on a different page.
Learnt from: mdatelle
Repo: unraid/api PR: 1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/DefaultPageLayout.php.modified.snapshot.php:203-207
Timestamp: 2025-02-03T17:21:26.738Z
Learning: The project uses patches to override existing Unraid pages rather than modifying them directly.
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
Repo: unraid/api PR: 942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
api/src/unraid-api/exceptions/graphql-exceptions.filter.tsweb/src/components/Notifications/Item.vue
📚 Learning: 2024-11-05T14:49:07.308Z
Learnt from: elibosley
Repo: unraid/api PR: 942
File: api/src/unraid-api/auth/api-key.service.ts:62-70
Timestamp: 2024-11-05T14:49:07.308Z
Learning: In `api/src/unraid-api/auth/api-key.service.ts`, when handling read errors in the `findById` method, throw a `GraphQLError` instead of an `InternalServerErrorException`.
Applied to files:
api/src/unraid-api/exceptions/graphql-exceptions.filter.tsweb/src/helpers/functions.ts
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
Repo: unraid/api PR: 1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Applied to files:
api/src/unraid-api/exceptions/graphql-exceptions.filter.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.ts
📚 Learning: 2025-01-31T22:01:22.708Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:24-27
Timestamp: 2025-01-31T22:01:22.708Z
Learning: The Unraid UI uses a modern notification system with a custom `unraid-toaster` component replacing the legacy jGrowl notifications. The system is backed by a comprehensive GraphQL API with real-time subscription support for notification updates.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.tsweb/src/components/Notifications/Item.vueweb/src/composables/gql/graphql.tsweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to **/*.{ts,tsx} : Avoid using casting whenever possible, prefer proper typing from the start
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
📚 Learning: 2025-03-07T19:29:12.540Z
Learnt from: pujitm
Repo: unraid/api PR: 1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to **/*.{ts,tsx} : Never use the `any` type. Always prefer proper typing
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
📚 Learning: 2024-11-01T17:52:55.311Z
Learnt from: pujitm
Repo: unraid/api PR: 941
File: web/components/Notifications/Item.vue:0-0
Timestamp: 2024-11-01T17:52:55.311Z
Learning: In the `web/components/Notifications/Item.vue` component, ensure that when delete or archive mutations fail, the notification remains displayed, and an error message is shown below the action buttons to inform the user.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.tsweb/src/components/Notifications/Item.vueapi/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.tsweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-04-07T14:34:47.255Z
Learnt from: elibosley
Repo: unraid/api PR: 1334
File: api/src/unraid-api/graph/resolvers/docker/docker-event.service.ts:63-66
Timestamp: 2025-04-07T14:34:47.255Z
Learning: In DockerEventService, the chokidar file watcher is configured with `ignoreInitial: false` to ensure that existing files (like the Docker socket) are detected and processed at application startup, not just when files change after the watcher is initialized.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
📚 Learning: 2025-01-30T19:56:42.055Z
Learnt from: pujitm
Repo: unraid/api PR: 1075
File: web/components/Notifications/Sidebar.vue:52-57
Timestamp: 2025-01-30T19:56:42.055Z
Learning: Notification links in the Unraid API are validated at creation time on the server side, making additional client-side validation unnecessary when receiving notifications through GraphQL subscriptions.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.tsweb/src/components/Notifications/Item.vueapi/generated-schema.graphqlweb/src/components/Notifications/graphql/notification.subscription.ts
📚 Learning: 2024-12-11T17:20:09.604Z
Learnt from: pujitm
Repo: unraid/api PR: 978
File: web/helpers/apollo-cache/index.ts:133-157
Timestamp: 2024-12-11T17:20:09.604Z
Learning: In `web/helpers/apollo-cache/index.ts`, when updating the cache in the `cache.updateQuery` function, it's sufficient to check if `data` is present, as type checking inside `data` is handled at the GraphQL layer.
Applied to files:
web/src/helpers/functions.ts
📚 Learning: 2024-12-13T17:28:15.384Z
Learnt from: pujitm
Repo: unraid/api PR: 982
File: web/helpers/apollo-cache/index.ts:142-142
Timestamp: 2024-12-13T17:28:15.384Z
Learning: In `web/helpers/apollo-cache/index.ts`, it's acceptable to use `structuredClone` since the project targets Node.js v20 and browsers where `structuredClone` is supported.
Applied to files:
web/src/helpers/functions.ts
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to web/**/*.query.ts : GraphQL queries must be placed in `.query.ts` files
Applied to files:
web/src/composables/gql/gql.ts
📚 Learning: 2025-11-24T17:52:00.572Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-graphql.mdc:0-0
Timestamp: 2025-11-24T17:52:00.572Z
Learning: Applies to web/**/*.{query.ts,mutation.ts} : All GraphQL under `web/` must follow the `.query.ts` and `.mutation.ts` naming convention
Applied to files:
web/src/composables/gql/gql.ts
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to web/**/*.mutation.ts : GraphQL mutations must be placed in `.mutation.ts` files
Applied to files:
web/src/composables/gql/gql.ts
📚 Learning: 2025-02-06T17:24:58.784Z
Learnt from: mdatelle
Repo: unraid/api PR: 1122
File: web/components/UserProfile/DropdownLaunchpad.vue:38-42
Timestamp: 2025-02-06T17:24:58.784Z
Learning: In the DropdownLaunchpad.vue component, the current implementation of conditional icon rendering using `h(BrandLoading, { variant: 'white' })` is a temporary fix to maintain build stability, with a proper refactoring being handled in a separate branch.
Applied to files:
web/src/components/Notifications/Item.vue
📚 Learning: 2025-01-31T22:01:41.842Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch removes the old jGrowl notification system and is complemented by the unraid-toaster component implementation. The new system is added through the DefaultPageLayout modification which inserts the toaster component with proper position configuration based on user preferences.
Applied to files:
web/src/components/Notifications/Item.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-04-21T18:04:56.755Z
Learnt from: pujitm
Repo: unraid/api PR: 1352
File: packages/unraid-api-plugin-connect/src/connect.resolver.ts:14-24
Timestamp: 2025-04-21T18:04:56.755Z
Learning: Error handling in GraphQL resolvers: pujitm prefers to let errors bubble up naturally to the client rather than adding explicit try/catch blocks in each resolver method.
Applied to files:
web/src/components/Notifications/Item.vue
📚 Learning: 2024-12-09T15:47:29.325Z
Learnt from: pujitm
Repo: unraid/api PR: 975
File: web/components/Notifications/TabList.vue:1-24
Timestamp: 2024-12-09T15:47:29.325Z
Learning: In our Nuxt setup using Vue.js, components defined within the codebase are autoloaded and do not require explicit import statements in the script section. For example, components like `TabsList` and `TabsTrigger` used in `web/components/Notifications/TabList.vue` are automatically available without imports.
Applied to files:
web/src/components/Notifications/Item.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-09T15:45:46.492Z
Learnt from: pujitm
Repo: unraid/api PR: 975
File: web/components/Notifications/TabList.vue:1-4
Timestamp: 2024-12-09T15:45:46.492Z
Learning: In our Nuxt.js setup for the `web` project, it's not necessary to explicitly import `computed` from `vue` in Vue components, as it's globally available.
Applied to files:
web/src/components/Notifications/Item.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Nuxt is set to auto import so some Vue files may need `computed` or `ref` imported explicitly in store files
Applied to files:
web/src/components/Notifications/Item.vueweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-11-22T15:47:54.695Z
Learnt from: pujitm
Repo: unraid/api PR: 967
File: web/components/Notifications/graphql/notification.query.ts:13-13
Timestamp: 2024-11-22T15:47:54.695Z
Learning: The `formattedTimestamp` field in the `Notification` type is already provided via `NotificationsResolver::list`.
Applied to files:
web/src/components/Notifications/Item.vue
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Verify proper error handling in Pinia store tests
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Test what the code does, not implementation details like exact error message wording
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Mock external services and API calls
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Test Pinia action side effects and state changes
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Applies to api/**/*.test.{ts,tsx} : For error testing, use `.rejects.toThrow()` without arguments; do not test exact error message strings unless the message format is specifically what you're testing
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts
📚 Learning: 2025-01-31T22:01:02.725Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:30-54
Timestamp: 2025-01-31T22:01:02.725Z
Learning: The removal of jGrowl notifications from DefaultPageLayout.php is intentional as notifications are now handled on a separate page as part of the architectural design to override existing Unraid pages.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-01-31T14:42:06.752Z
Learnt from: pujitm
Repo: unraid/api PR: 1075
File: web/components/Notifications/Sidebar.vue:41-58
Timestamp: 2025-01-31T14:42:06.752Z
Learning: When handling errors in code that uses toast notifications, avoid using toast in the error handler itself since toast functionality might be the source of the error. Use console.error or other reliable logging mechanisms instead.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/store/**/*.ts : Ensure Vue reactivity imports like `computed`, `ref`, and `watchEffect` are added to store files even with Nuxt auto-imports enabled
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to web/src/**/*.ts : Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-06T17:34:16.133Z
Learnt from: pujitm
Repo: unraid/api PR: 974
File: web/components/Loading/Error.vue:50-50
Timestamp: 2024-12-06T17:34:16.133Z
Learning: In this project, the `Button` component from `~/components/shadcn/Button.vue` is autoloaded and does not need to be imported manually in components like `web/components/Loading/Error.vue`.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-06-13T17:14:21.739Z
Learnt from: pujitm
Repo: unraid/api PR: 1417
File: web/components/ConnectSettings/ConnectSettings.ce.vue:11-18
Timestamp: 2025-06-13T17:14:21.739Z
Learning: The project’s build tooling auto-imports common Vue/Pinia helpers such as `storeToRefs`, so explicit import statements for them are not required.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-17T13:55:42.068Z
Learnt from: elibosley
Repo: unraid/api PR: 972
File: web/components/ColorSwitcher.ce.vue:1-2
Timestamp: 2024-12-17T13:55:42.068Z
Learning: In this Nuxt.js project, components used in templates are automatically imported by Nuxt, so explicit import statements for components are unnecessary.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-04-02T21:21:29.168Z
Learnt from: elibosley
Repo: unraid/api PR: 1308
File: unraid-ui/src/components/common/loading/Error.vue:2-2
Timestamp: 2025-04-02T21:21:29.168Z
Learning: Components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other parts of the project that may use Nuxt.js autoloading features.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-08-29T19:48:45.517Z
Learnt from: elibosley
Repo: unraid/api PR: 1618
File: web/components/Logs/SingleLogViewer.vue:248-299
Timestamp: 2025-08-29T19:48:45.517Z
Learning: In Vue Apollo's composition API, subscribeToMore automatically handles subscription cleanup when the related query is stopped or the component unmounts, unlike React Apollo where manual cleanup is required. Manual unsubscribe is only needed when changing subscription variables, not for lifecycle management.
Applied to files:
web/src/components/Notifications/graphql/notification.subscription.ts
🧬 Code graph analysis (3)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (3)
api/src/core/pubsub.ts (1)
pubsub(12-12)api/src/core/errors/app-error.ts (1)
AppError(4-47)api/src/utils.ts (2)
batchProcess(58-71)formatDatetime(138-179)
web/src/composables/gql/graphql.ts (3)
api/src/unraid-api/cli/generated/graphql.ts (6)
NotificationEvent(1671-1675)Maybe(3-3)Notification(1640-1653)NotificationSettings(1703-1709)Scalars(11-75)Exact(5-5)web/composables/gql/graphql.ts (5)
Maybe(3-3)Notification(1361-1374)Scalars(11-75)Exact(5-5)NotificationFragmentFragment(2664-2664)packages/unraid-api-plugin-connect/src/graphql/generated/client/graphql.ts (4)
Maybe(3-3)Notification(414-422)Scalars(11-31)Exact(5-5)
web/src/components/Notifications/graphql/notification.subscription.ts (1)
web/src/composables/gql/gql.ts (1)
graphql(450-452)
🔇 Additional comments (34)
api/src/core/errors/app-error.ts (1)
17-32: LGTM on the constructor changes.The extension of the constructor to accept an optional
codeparameter and the conditional population ofextensionsis well-implemented. This cleanly integrates with the GraphQL error handling in the exception filter.web/src/helpers/functions.ts (3)
7-20: LGTM! Clean debug helper for GraphQL errors.The
dbgApolloErrorfunction provides structured logging with proper null-checking and safe JSON serialization fallback.
62-77: LGTM! Error code extraction is well-structured.Handles both
extensions.codeandextensions.error.codepaths, which aligns with how NestJS GraphQL can return error codes.
79-84: LGTM! Simple and effective network error check.web/src/components/Notifications/Item.vue (5)
63-85: Well-structured mutation error handling with user feedback.The
handleMutationwrapper properly:
- Logs errors for debugging
- Extracts meaningful error codes and messages
- Handles offline/network errors gracefully
- Shows toast notifications for failures
This aligns with the learning that notifications should remain displayed when mutations fail. Based on learnings, the error message is shown below action buttons via
mutationErrorcomputed.
129-141: LGTM! Robust timestamp parsing with graceful fallback.The regex handles common time patterns (12h/24h formats) and gracefully falls back to displaying the full string when parsing fails.
147-238: LGTM! Clean template structure with good UX patterns.The template properly:
- Shows loading states on action buttons
- Conditionally renders actions based on notification type
- Displays errors below actions per the retrieved learning
- Uses appropriate icon sizing and color mapping
186-189: Markdown content is properly sanitized viaMarkdown.parse().
Markdown.parse()uses DOMPurify to sanitize HTML output through a postprocess hook, as documented inweb/src/helpers/markdown.ts. However, the error fallback at line 38 returns unsanitizedprops.description, which would bypass sanitization if parsing fails. Sanitize the fallback value as well.⛔ Skipped due to learnings
Learnt from: elibosley Repo: unraid/api PR: 963 File: web/helpers/markdown.ts:10-13 Timestamp: 2024-11-18T20:50:19.224Z Learning: In `web/helpers/markdown.ts`, the `safeParseMarkdown` function does not require additional input validation, error handling, or caching as per the user's preference.Learnt from: elibosley Repo: unraid/api PR: 963 File: web/helpers/markdown.ts:1-13 Timestamp: 2024-11-18T20:52:47.800Z Learning: All configurations and direct usage of `marked` should be centralized in the `safeParseMarkdown` function in `web/helpers/markdown.ts` to maintain security and consistency.Learnt from: pujitm Repo: unraid/api PR: 963 File: web/helpers/markdown.ts:1-41 Timestamp: 2024-11-19T16:34:44.804Z Learning: In `web/store/updateOsChangelog.ts`, when custom logic is required, `Markdown.create` is used to obtain a custom `marked` instance. This is the correct and intended usage of the `Markdown` helper in such cases.
28-29: No action needed —useToastis properly auto-imported.
useToastis correctly configured for auto-import via the @nuxt/ui integration (visible inauto-imports.d.ts). The code follows the established pattern in the Nuxt.js setup and requires no changes.api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (2)
46-49: LGTM! Settings resolver field properly exposes notification configuration.The synchronous
settings()method correctly delegates to the service layer and returnsNotificationSettings.
193-200: LGTM! Unified notification event subscription.The
notificationEventsubscription properly:
- Returns
NotificationEventtype which includes event type and notification data- Has correct
READ_ANYpermission onNOTIFICATIONSresource- Uses the consistent
createSubscriptionpatternThis replaces the previous
notificationAddedsubscription with a more comprehensive event-based approach that can handle add/update/delete events. Based on learnings, the new notification system uses GraphQL subscriptions for real-time updates.web/src/components/Notifications/graphql/notification.subscription.ts (1)
24-33: Well-structured notification event subscription with properly generated types.The
notificationEventSubscriptionis correctly implemented:
- Includes both
typefield for event classification andnotificationwithNotificationFragment- GraphQL codegen has been run and types are properly generated in
web/src/composables/gql/graphql.tsNotificationEventSubSubscriptionandNotificationEventSubDocumentare correctly exportedapi/generated-schema.graphql (1)
1449-1461:Notifications.settings: NotificationSettings!requires a guaranteed default.Because it’s non-null, the resolver must always return settings (including for older Unraid versions / missing persisted prefs) or the entire
notificationsquery will error.web/src/composables/gql/graphql.ts (6)
1671-1682:NotificationEventtypings align with the schema (nullablenotification, non-nulltype).
1703-1725:Notifications.settingsplumbs through cleanly; good separation from the list query.
2310-2325: Subscription switch tonotificationEventis reflected correctly in generated TS.
3182-3186:GetNotificationSettingsquery shape is tight (only pulls settings), which should help avoid over-fetching.
3268-3275:NotificationEventSubtypes correctly modelnotificationas nullable forCLEARED.
3398-3399: Generated documents added—please confirm codegen is the single source of truth.Given this file is generated, make sure
web/codegen was run and committed consistently (and that no hand-edits are expected/required). As per coding guidelines, runpnpm codegeninweb/.Also applies to: 3409-3410
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (6)
147-162: Retry loop with fixed delay and count is acceptable, but consider exponential backoff.The 5-retry loop with 100ms delay handles file-system timing issues gracefully. For robustness, exponential backoff could reduce unnecessary retries when the file is truly inaccessible.
198-216: LGTM:getSettings()with defensive parsing.The helper functions
parseBooleanandparsePositiveInthandle various input types gracefully with sensible defaults.
547-551: Stable error codeNOTIFICATION_NOT_FOUNDis a good practice.Using a stable error code enables programmatic handling on the frontend. This aligns with the
AppErrorpattern shown in the relevant snippets.
871-892: Generator-based batching improves memory efficiency.The
getNotificationsGeneratorwithBATCH_SIZE = 10provides bounded concurrency and streaming results. This is a solid pattern for large notification sets.
1079-1103: Fallback to ISO format when settings are missing is appropriate.The prioritization of
notifysettings overdisplaysettings aligns with the PR objective of notification-specific formatting.
334-349: Legacy script supports the-uflag for custom notification filenames.The
/usr/local/emhttp/webGui/scripts/notifyscript fully supports the-uflag. The implementation is documented in the script's usage information (use -u to specify a custom filename (API use only)) and properly handles the flag through thegetoptcall and switch case handler, allowing the API to pass custom notification identifiers as filenames.web/src/components/Notifications/Sidebar.vue (6)
61-69: Error handling uses toast for mutation failures - verify this is safe.Based on retrieved learnings, when handling errors in code that uses toast notifications, avoid using toast in the error handler if toast functionality might be the source of the error. Here, the
archiveAllmutation failure is unrelated to toast, so this usage is appropriate.
95-112: Refetch strategy on API status change is well-implemented.The
handleRefetchfunction properly chainsrecalculateOverviewwithrefetch, and the watch onunraidApiStore.unraidApiStatustriggers refresh when the API comes back online.
118-150: Toast creation for new notifications handles optional link correctly.The
onNotificationEventhandler properly guards forADDEDtype and creates toasts with conditional navigation actions. UsingrequestAnimationFrameensures smooth UI updates.
159-163: Archived count now correctly uses API-reported value.The comment and implementation align:
overview.value.archive.totaldirectly uses the backend-calculated count rather than local computation.
272-285: No action needed. The@refetched="handleRefetch"binding is safe. The child component emitsrefetchedwhen its query completes, and the parent'shandleRefetchrefetches a separate query (notificationsOverview). These are different queries, so there's no infinite loop. Additionally, the List component uses debounced refetch logic to prevent rapid cascading updates.
27-29: No action required.useToastis correctly auto-imported by Nuxt.js from@nuxt/uiand does not require an explicit import statement in the web project.Likely an incorrect or invalid review comment.
web/src/composables/gql/gql.ts (3)
60-71: New notification documents properly added to Documents type.
GetNotificationSettingsDocumentandNotificationEventSubDocumentare correctly mapped to their respective type definitions.
345-348: New function overloads for notification queries and subscriptions are correctly structured.The overloads follow the established pattern in this file, mapping source strings to their document types.
Also applies to: 384-392
1-3: This is a GraphQL CodeGen generated file.The
eslint-disablecomment and structure confirm this file is auto-generated. TheGetNotificationSettingsDocumentandNotificationEventSubDocumentare properly exported ingraphql.ts. Per learnings, duplicate GraphQL strings are intentional for the type system. Ensurepnpm codegenhas been run in the web directory.
| type Subscription { | ||
| notificationAdded: Notification! | ||
| notificationsOverview: NotificationOverview! | ||
| notificationsWarningsAndAlerts: [Notification!]! | ||
| notificationEvent: NotificationEvent! | ||
| ownerSubscription: Owner! |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify no remaining clients still use the old subscription field.
rg -n --hidden -S '\bnotificationAdded\b' .
rg -n --hidden -S '\bNotificationAddedSub\b' .Repository: unraid/api
Length of output: 4407
🏁 Script executed:
#!/bin/bash
# Check for actual usage of NotificationAddedSub in Vue/component files
rg -n 'NotificationAddedSub' web --type vue --type ts --type js -A 2
# Check if these are generated files (look for generation comments)
head -20 web/composables/gql/graphql.tsRepository: unraid/api
Length of output: 1369
🏁 Script executed:
#!/bin/bash
# Search for actual usage of NotificationAddedSub in web components
rg -n 'NotificationAddedSub' web --type ts --type js -B 2 -A 2
# Also check for usage in .vue files directly
rg -n 'NotificationAddedSub' webRepository: unraid/api
Length of output: 12592
🏁 Script executed:
#!/bin/bash
# Search for actual component imports of the old subscription
rg -n 'NotificationAddedSubDocument' web --type ts --type js
# Also search for imports from graphql.ts or gql.ts
rg -n 'import.*notificationAdded' web --type ts --type jsRepository: unraid/api
Length of output: 1765
🏁 Script executed:
#!/bin/bash
# Search for subscription usage in components - look for useSubscription or subscription patterns
rg -n 'useSubscription\|graphql\(' web -A 3 --type ts --type js | head -100
# Also check if there's any notification-related subscription in components
rg -n 'notificationEvent\|notificationsOverview\|notificationsWarningsAndAlerts' web --type ts --type js | head -50Repository: unraid/api
Length of output: 36
🏁 Script executed:
#!/bin/bash
# Find GraphQL query/subscription files
find web -name "*.graphql" -o -name "*.gql" | head -20
# Also check composables directory structure
ls -la web/composables/
# Search for any Vue files that might use subscriptions
find web -name "*.vue" -type f | head -20Repository: unraid/api
Length of output: 1112
🏁 Script executed:
#!/bin/bash
# Find Notification-related Vue components
find web -name "*otification*.vue" -type f
# Also search for components that might use notification subscriptions
rg -l 'notification' web/src/components --type vue -i | head -20Repository: unraid/api
Length of output: 168
🏁 Script executed:
#!/bin/bash
# Search for notification-related Vue components
find web -name "*otification*.vue" -o -name "*otification*.ts"
# Check the CriticalNotifications component
cat -n web/src/components/Notifications/CriticalNotifications.standalone.vue | head -100Repository: unraid/api
Length of output: 4244
🏁 Script executed:
#!/bin/bash
# Check the subscription file
cat -n web/src/components/Notifications/graphql/notification.subscription.tsRepository: unraid/api
Length of output: 1030
Breaking change confirmed: notificationAdded subscription removed without backward compatibility.
The old notificationAdded subscription has been replaced with notificationEvent and related subscriptions. While internal webgui components have already migrated to the new subscriptions (notificationEventSubscription, warningsAndAlertsSubscription), any external plugins or shipped clients still subscribing to notificationAdded will fail at runtime.
To maintain backward compatibility with external clients, consider adding notificationAdded as a deprecated alias to the schema.
🤖 Prompt for AI Agents
In @api/generated-schema.graphql around lines 2912 - 2916, Add a deprecated
alias field named notificationAdded to the Subscription type so external clients
keep working: declare notificationAdded with the same GraphQL return type as the
old subscription and mark it @deprecated, then implement a resolver for
notificationAdded that forwards/events from the new notificationEvent (or maps
NotificationEvent payload to the old Notification payload) so the behavior is
identical; update the Subscription type definition (Subscription) and the
notificationEvent resolver to ensure notificationAdded reuses the new event
stream and is documented as deprecated.
| /** | ||
| * Tracks archive file paths that are currently being processed by batch operations. | ||
| * This allows us to suppress the file watcher's 'add' event for these files | ||
| * to prevent double counting stats and flooding pubsub events. | ||
| */ | ||
| private processingArchives = new Set<string>(); |
There was a problem hiding this comment.
Potential memory leak in processingArchives Set on error paths.
If rename fails in batchCreateArchive (lines 666), the path is added to processingArchives but never removed. The file watcher won't fire for a failed rename, leaving stale entries in the Set.
🐛 Proposed fix
} else {
// Otherwise move it to archive
// We must register this path to be ignored by the file watcher's 'add' event
// to avoid double counting stats and flooding pubsub events.
this.processingArchives.add(archivePath);
- await rename(unreadPath, archivePath);
+ try {
+ await rename(unreadPath, archivePath);
+ } catch (error) {
+ this.processingArchives.delete(archivePath);
+ throw error;
+ }
}📝 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.
| /** | |
| * Tracks archive file paths that are currently being processed by batch operations. | |
| * This allows us to suppress the file watcher's 'add' event for these files | |
| * to prevent double counting stats and flooding pubsub events. | |
| */ | |
| private processingArchives = new Set<string>(); | |
| } else { | |
| // Otherwise move it to archive | |
| // We must register this path to be ignored by the file watcher's 'add' event | |
| // to avoid double counting stats and flooding pubsub events. | |
| this.processingArchives.add(archivePath); | |
| try { | |
| await rename(unreadPath, archivePath); | |
| } catch (error) { | |
| this.processingArchives.delete(archivePath); | |
| throw error; | |
| } | |
| } |
🤖 Prompt for AI Agents
In @api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
around lines 60 - 65, The Set processingArchives can retain stale paths if the
rename in batchCreateArchive fails; wrap the rename and any subsequent work that
depends on it in a try/finally (or ensure each catch removes the path) so the
path is removed from processingArchives on all error paths; update
batchCreateArchive to add the path, attempt rename inside try, and always call
processingArchives.delete(path) in finally (or before each rethrow) so the
watcher is not permanently suppressed.
| const batchCreateArchive = async (notification: Notification) => { | ||
| const unreadPath = join(UNREAD, notification.id); | ||
| const archivePath = join(ARCHIVE, notification.id); | ||
|
|
||
| try { | ||
| // If file already exists in archive, delete the unread copy | ||
| if (await fileExists(archivePath)) { | ||
| await unlink(unreadPath); | ||
| } else { | ||
| // Otherwise move it to archive | ||
| // We must register this path to be ignored by the file watcher's 'add' event | ||
| // to avoid double counting stats and flooding pubsub events. | ||
| this.processingArchives.add(archivePath); | ||
| await rename(unreadPath, archivePath); | ||
| } | ||
|
|
||
| // Update in-memory stats | ||
| this.decrement(notification.importance, NotificationsService.overview.unread); | ||
| this.increment(notification.importance, NotificationsService.overview.archive); | ||
| } catch (error) { | ||
| this.logger.error( | ||
| `[archiveAll] Failed to archive ${notification.id}: ${error instanceof Error ? error.message : error}` | ||
| ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Race condition: stats may become inconsistent if batch operations interleave.
batchProcess runs operations in parallel. If two batchCreateArchive calls overlap on the same notification, decrement/increment on the shared static overview could race. While unlikely in practice, consider documenting this limitation or adding guards.
🤖 Prompt for AI Agents
In @api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
around lines 653 - 677, The batchCreateArchive function can run concurrently for
the same notification id causing race conditions when updating the shared
NotificationsService.overview via decrement/increment; add a guard so operations
for the same notification id are serialized (e.g., a per-id lock or a
Map<string, Promise> to chain operations) or wrap the decrement/increment pair
in a mutex to ensure atomic updates; locate batchCreateArchive, batchProcess,
NotificationsService.overview, processingArchives, decrement and increment and
implement the per-id locking or global mutex around the stats update, or
document the limitation if you prefer not to change runtime behavior.
Purpose
Related PRs
Warning
Please coordinate changes with webgui and CA teams.
webgui:
- PR (#2504)
- PR (#2503)
community apps:
- PR (#15)
Key Changes
NuxtUI Notifications Migration:
Nuxt UI Toast Migration:
NotificationSettingsto the graphql api model to sync toast position with legacy settingsNotification Hardening & Optimization:
Theming Changes
Warning
These changes will need to be merged into the webgui repo for v7.4. The file-modifications for these theming changes assume that they will be merged into Unraid version 7.4.0
:where(:not(.unapi *)with a more robust, developer-friendly approach to solving CSS specificity issues while excluding legacy webgui styles and preventing conflicts with the modern UI.@scopeand@layerscoping rules tofont-awesome.cssto prevents icon sizing regressions in other tabs (like Community Applications).Legacy Integration (File Modifiers):
default-white.css,default-black.css,default-azure.css,default-gray.cssand created a file-modifier fordefault-base.cssin Unraid versions > 7.1.Translation.phpto patch a stale.dotfile for whenhelpertext.txtdiffers from existing version on unraid server and updated the.plgscript to delete the newer.dotfiles when the connect plugin is removed, forcing a clean cache rebuild (as.dotis generated and thus cannot be included in theFILES_TO_REMOVE/BACKUP).dynamix.unraid.net.plgto reflect these file-modification changes.Dev Cleanup & Maintenance
predocker:build-and-runSome Notes
window.location.assignnot the:toprop as there is no vue router in a standalone component enviornment.Unfixed Bugs
Future Changes/Recommendations
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.