-
Notifications
You must be signed in to change notification settings - Fork 16
fix: notification display and functionality + nuxtui refactor #1860
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
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
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/components/Notifications/Item.vue (1)
65-78: Potential SSR issue withnavigator.languageaccess.
navigatoris a browser-only API. If this component runs during SSR, accessing it will throw a ReferenceError. Consider guarding with a client-side check:🔎 Suggested fix
const reformattedTimestamp = computed<string>(() => { if (!props.timestamp) return ''; - const userLocale = navigator.language ?? 'en-US'; + const userLocale = typeof navigator !== 'undefined' ? navigator.language : 'en-US'; const reformattedDate = new Intl.DateTimeFormat(userLocale, {
♻️ Duplicate comments (2)
api/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.ts (2)
25-32: UnusedisBelow70variable should be removed or used.
isBelow70is computed on line 26 but both parameters toapplyToSourcereceiveisBelow71. Either removeisBelow70if not needed, or use it forremoveDisplayOptionif version-specific behavior differs between 7.0 and 7.1.🔎 Proposed fix (if isBelow70 is not needed)
protected async generatePatch(overridePath?: string): Promise<string> { const fileContent = await readFile(this.filePath, 'utf-8'); const isBelow71 = !(await this.isUnraidVersionGreaterThanOrEqualTo('7.1.0')); - const isBelow70 = !(await this.isUnraidVersionGreaterThanOrEqualTo('7.0.0')); const newContent = NotificationsPageModification.applyToSource( fileContent, isBelow71, isBelow71 );
52-80: Remove unused variable and redundant regex.Two issues flagged previously remain:
- Line 66:
autoCloseAnchoris declared but never used.- Lines 76-79: The second, more generic regex is redundant if the first specific regex (lines 59-62) succeeds.
Additionally, the extensive comments (lines 64-74) could be condensed per coding guidelines.
🔎 Proposed fix
if (removeAutoClose) { // Remove _(Auto-close)_ section - // Looks like: - // _(Auto-close)_ (_(seconds)_): - // : <input type="number" name="life" class="a" min="0" max="60" value="<?=$notify['life']?>"> _(a value of zero means no automatic closure)_ - // - // :notifications_auto_close_help: newContent = newContent.replace( /^\s*_\(Auto-close\)_ \(?_\(seconds\)_?\)?:\s*\n\s*:\s*<input[^>]+>.*?\n\s*\n\s*:notifications_auto_close_help:\s*/gm, '' ); - - // Try generic regex if the detailed one fails/is too brittle, but specific is better for safety - // Let's rely on the anchor strings - const autoCloseAnchor = '_((Auto-close))_ (_((seconds))_):'; - // Since regex with multiple lines and potential slight variations is better handled loosely or with concrete bounds - // Let's use a simpler approach if the specific one is tricky without seeing exact whitespace chars - // The user provided snippet has: - // _(Auto-close)_ (_(seconds)_): - // : <input type="number" name="life" class="a" min="0" max="60" value="<?=$notify['life']?>"> _(a value of zero means no automatic closure)_ - // - // :notifications_auto_close_help: - - // Using a regex that captures from start to the help tag - newContent = newContent.replace( - /_\(Auto-close\)_[^:]*:(?:[^\n]*\n)*?\s*:notifications_auto_close_help:/gm, - '' - ); }
🧹 Nitpick comments (3)
api/src/utils.ts (1)
181-217: Well-designed function with appropriate scope.The implementation correctly handles Unraid's date/time formats with an explicit mapping for common cases and a sensible heuristic fallback. The check for existing '%' characters (line 204) properly prevents double-conversion of strftime formats.
Optional: Consider adding test coverage for the heuristic fallback path
The current tests (lines 84-92 in the test file) only exercise formats that exist in the explicit mapping. Consider adding tests for formats that would use the heuristic fallback path to ensure the word-boundary replacements work as expected for edge cases.
For example:
it('handles formats via heuristic fallback', () => { // Format not in explicit mapping const result = formatDatetime(testDate, { dateFormat: 'd.m.Y' }); expect(result).toContain('14.02.2024'); });This would help validate the fallback logic, though it's not critical given the documented scope.
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (2)
124-139: Consider extracting retry configuration to constants.The retry logic correctly handles race conditions when notification files are being written. However, the magic numbers (5 retries, 100ms delay) could be extracted to class-level or module-level constants for easier maintenance and testing.
🔎 Suggested refactor
export class NotificationsService { private logger = new Logger(NotificationsService.name); private static watcher: FSWatcher | null = null; + private static readonly NOTIFICATION_LOAD_RETRIES = 5; + private static readonly NOTIFICATION_LOAD_RETRY_DELAY_MS = 100;Then update the retry loop:
- for (let i = 0; i < 5; i++) { + for (let i = 0; i < NotificationsService.NOTIFICATION_LOAD_RETRIES; i++) { try { notification = await this.loadNotificationFile(path, NotificationType[type]); this.logger.debug( `[handleNotificationAdd] Successfully loaded ${path} on attempt ${i + 1}` ); break; } catch (error) { lastError = error; this.logger.warn( `[handleNotificationAdd] Attempt ${i + 1} failed for ${path}: ${error}` ); - await new Promise((resolve) => setTimeout(resolve, 100)); + await new Promise((resolve) => setTimeout(resolve, NotificationsService.NOTIFICATION_LOAD_RETRY_DELAY_MS)); } }
766-787: Consider extracting BATCH_SIZE to a constant.The generator-based batching approach is excellent for memory efficiency and error handling. The batch size of 10 is reasonable, but extracting it to a class-level constant would improve maintainability.
🔎 Suggested refactor
export class NotificationsService { private logger = new Logger(NotificationsService.name); private static watcher: FSWatcher | null = null; + private static readonly NOTIFICATION_BATCH_SIZE = 10;Then update the generator:
private async *getNotificationsGenerator( files: string[], type: NotificationType ): AsyncGenerator<{ success: true; value: Notification } | { success: false; reason: unknown }> { - const BATCH_SIZE = 10; - for (let i = 0; i < files.length; i += BATCH_SIZE) { - const batch = files.slice(i, i + BATCH_SIZE); + for (let i = 0; i < files.length; i += NotificationsService.NOTIFICATION_BATCH_SIZE) { + const batch = files.slice(i, i + NotificationsService.NOTIFICATION_BATCH_SIZE);
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
api/src/__test__/utils.test.tsapi/src/core/types/ini.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/Notifications.page.modified.snapshot.phpapi/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.tsapi/src/unraid-api/unraid-file-modifier/modifications/patches/notifications-page.patchapi/src/utils.tsweb/src/components/Notifications/Item.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/unraid-file-modifier/modifications/test/snapshots/Notifications.page.modified.snapshot.php
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{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/utils.tsapi/src/core/types/ini.tsapi/src/__test__/utils.test.tsapi/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.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/utils.tsapi/src/core/types/ini.tsapi/src/__test__/utils.test.tsapi/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.tsapi/src/unraid-api/unraid-file-modifier/modifications/patches/notifications-page.patchapi/src/unraid-api/graph/resolvers/notifications/notifications.service.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:
api/src/utils.tsapi/src/core/types/ini.tsapi/src/__test__/utils.test.tsapi/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.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/utils.tsapi/src/core/types/ini.tsapi/src/__test__/utils.test.tsapi/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
api/src/utils.tsweb/src/components/Notifications/Item.vueapi/src/core/types/ini.tsapi/src/__test__/utils.test.tsapi/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.tsapi/src/unraid-api/unraid-file-modifier/modifications/patches/notifications-page.patchapi/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/src/components/Notifications/Item.vue
**/*.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/__test__/utils.test.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/__test__/utils.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/__test__/utils.test.ts
🧠 Learnings (20)
📓 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.
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.
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-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/Item.vueapi/src/unraid-api/graph/resolvers/notifications/notifications.service.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: 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.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/Item.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.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/Item.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.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.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 : Test component behavior and output, not implementation details
Applied to files:
api/src/__test__/utils.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/__test__/utils.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.tsapi/src/unraid-api/unraid-file-modifier/modifications/patches/notifications-page.patch
📚 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.tsapi/src/unraid-api/unraid-file-modifier/modifications/patches/notifications-page.patch
📚 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.tsapi/src/unraid-api/unraid-file-modifier/modifications/patches/notifications-page.patch
📚 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.tsapi/src/unraid-api/unraid-file-modifier/modifications/patches/notifications-page.patch
📚 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/unraid-file-modifier/modifications/patches/notifications-page.patchapi/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
📚 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: Notifications are implemented on a separate page rather than in the DefaultPageLayout.php file.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/patches/notifications-page.patch
📚 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/patches/notifications-page.patch
📚 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
🧬 Code graph analysis (2)
api/src/__test__/utils.test.ts (2)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (1)
formatDatetime(974-1000)api/src/utils.ts (1)
formatDatetime(138-179)
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)
🪛 GitHub Check: CodeQL
api/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.ts
[notice] 26-26: Unused variable, import, function or class
Unused variable isBelow70.
[notice] 66-66: Unused variable, import, function or class
Unused variable autoCloseAnchor.
🔇 Additional comments (22)
api/src/unraid-api/unraid-file-modifier/modifications/patches/notifications-page.patch (1)
1-82: Patch structure looks correct.The patch properly restructures the Notifications.page UI:
- Moves "Store notifications to flash" before "Display position"
- Replaces legacy "center" position with "bottom-center"/"top-center" options
- Adds new Stack/Duration/Max notification popup settings
- Removes deprecated "Notifications display" and "Auto-close" sections
This aligns well with the NuxtUI toast migration and the new notification settings model.
api/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.ts (4)
12-21: Version gating logic is correct.The
shouldApplymethod properly skips modifications for Unraid 7.4+ where the refactored notifications page is native, and delegates to the base class for earlier versions withcheckOsVersion: false.
37-50: Conditional removal logic is appropriate.The method signature and conditional section removal using regex are well-structured for version-specific transformations.
82-111: Block extraction and repositioning logic is sound.The code correctly:
- Replaces the legacy "center" position with "bottom-center"/"top-center"
- Safely extracts the "Store notifications to flash" block with null checking
- Repositions it before "Display position"
113-144: New settings insertion is well-guarded.The duplication check (
!newContent.includes('_(Stack notification popups)_:')) prevents multiple insertions if the method is called repeatedly or the content already contains these settings.api/src/utils.ts (1)
143-144: LGTM! Clean integration of PHP format conversion.The conversion step is properly placed before strftime calls, and maintains backward compatibility with existing strftime formats (since formats containing '%' pass through unchanged).
Also applies to: 146-146, 176-176
api/src/__test__/utils.test.ts (1)
72-93: Excellent test coverage for PHP format conversion.The test suite effectively validates the conversion and formatting of PHP-style date/time formats. The parameterized tests using
.eachprovide good coverage of the mapped formats, and the use of.toContain()appropriately validates the formatted output.web/src/components/Notifications/Item.vue (8)
1-17: LGTM!Imports are well-organized with clear separation between Vue/composables, helpers, types, and local modules. The migration to NuxtUI's icon system via constants is a clean abstraction.
23-30: LGTM!Good defensive error handling in
descriptionMarkup— catches markdown parsing failures and falls back to raw description gracefully.
32-41: LGTM!Clean null-safe implementation with good separation of icon/color mapping via constants.
43-57: LGTM!Error handling is correctly implemented per the project requirements — mutations preserve the notification on failure and
mutationErroris displayed below action buttons (lines 142-144). Based on learnings, this aligns with the expected behavior.
59-63: LGTM!Clean implementation using the centralized navigation helper with appropriate null check.
80-94: LGTM!Correctly prioritizes the API-provided
formattedTimestampto respect user date-format settings. The regex-based splitting has a clean fallback when patterns don't match.
146-180: LGTM!Action buttons are well-structured with:
- Correct conditional rendering based on notification type
- Proper loading states wired to mutation status
- Consistent use of NuxtUI button variants and icons
The migration from Shadcn to NuxtUI components is clean.
136-139: No action required. Thev-htmlusage withdescriptionMarkupis safe—Markdown.parsealready sanitizes all output using DOMPurify in its postprocess hook, explicitly forbiddingstyletags and attributes.Likely an incorrect or invalid review comment.
api/src/core/types/ini.ts (1)
96-98: LGTM! Union types appropriate for INI config parsing.The union types (
string | boolean,string | number) correctly handle the flexibility of INI file parsing where values may be stored as strings or typed values. ThegetSettings()method in the service layer properly handles type coercion withparseBooleanandparsePositiveInthelpers.api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (6)
175-193: LGTM! Well-structured settings extraction.The
getSettings()method correctly exposes notification UI settings with proper type coercion and sensible defaults. The inline helper functions (parseBoolean,parsePositiveInt) are simple, well-scoped, and handle edge cases appropriately.
301-315: LGTM! Clean backward-compatible signature extension.The optional
idparameter is properly handled and only passed to the legacy notifier when provided. This maintains backward compatibility while enabling the new ID propagation feature.
490-545: LGTM! Critical fix for notification count accuracy.The refactored logic correctly handles the edge case where a notification file exists in both UNREAD and ARCHIVE directories. By checking for existence before incrementing archive counts, this prevents double-counting that would break the frontend's "Archived Only" calculation
(Archive Total - Unread Total).The explicit stat updates and dual-case handling (delete vs. move) ensure consistency with the frontend's counting logic.
584-595: LGTM! Improved consistency by reusing archiveNotification.The refactor to call
archiveNotificationper-item ensures consistent handling of the duplicate-file edge case across all archive operations. ReturninggetOverview()at the end provides the current state after all operations, which is more reliable than maintaining a snapshot.
801-839: LGTM! Excellent refactor to generator-based streaming.The generator-based approach with offset/limit handling is a significant improvement:
- Memory efficient: Processes notifications in batches instead of loading all at once
- Correct pagination: Properly skips
offsetitems and collects up tolimititems- Early termination: Breaks as soon as the limit is reached, avoiding unnecessary file reads
- Error resilience: Collects errors without failing the entire operation
The offset/limit logic correctly handles the filter-then-paginate flow.
102-107: Initial notification state is not loaded at startup.With
ignoreInitial: true, the watcher ignores existing notification files and only reacts to new files added after startup. However,recalculateOverview()is never called during service initialization, and there are no lifecycle hooks to load the initial state. This means the notification overview will display zero notifications at startup even if notification files exist on disk.Either enable
ignoreInitial: false(like DockerEventService) to detect existing files, or add a lifecycle hook to callrecalculateOverview()duringonModuleInit/onApplicationBootstrap.⛔ Skipped due to learnings
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.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.
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
unraid-ui/package.json (1)
49-69: Revert vue-sonner removal from dependencies—the package is still imported and used inuseToast.ts.Removing
vue-sonnerfrompackage.jsonwhile keeping active imports inunraid-ui/src/composables/useToast.tscreates a broken module that will fail at runtime. TheuseToast()composable actively callssonnerToast()throughout all its methods but the dependency is no longer declared.Either restore
vue-sonnerto dependencies or updateuseToast.tsto remove all vue-sonner imports and replace with the intended toast implementation.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
unraid-ui/package.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
unraid-ui/package.json
🧠 Learnings (3)
📓 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: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: 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.
📚 Learning: 2025-02-04T17:21:39.710Z
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.
Applied to files:
unraid-ui/package.json
📚 Learning: 2025-03-14T19:18:23.324Z
Learnt from: pujitm
Repo: unraid/api PR: 1211
File: unraid-ui/package.json:43-43
Timestamp: 2025-03-14T19:18:23.324Z
Learning: The internationalized/number package is a dependency of reka-ui, which is used for number formatting in the Unraid UI components.
Applied to files:
unraid-ui/package.json
🪛 GitHub Actions: CI - Main (API)
unraid-ui/package.json
[error] 1-1: pnpm install --frozen-lockfile failed: lockfile is out of date. Specifiers in the lockfile don't match package.json (1 dependencies were removed: vue-sonner@2.0.8). Run 'pnpm install' to update the lockfile.
66c655e to
c93d4f2
Compare
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
…s working yesterday and i didn't need to make these changes Here's what the ai has to say: build: fix type-check failures and dependency resolution * deps: Pin `jiti` to `2.6.1` in root overrides to resolve a diamond dependency conflict. This fixes the "Type X is not assignable to Type X" errors caused by multiple incompatible versions of Vite's types being loaded by `tailwindcss` and `vite`. * web: Replace non-standard `import.meta.client` (Nuxt-specific) with `!import.meta.env.SSR` (Vite standard) in `ResizableSlideover.vue`. * web: Add `vite/client` to `tsconfig.json` types to ensure Vite global definitions are correctly loaded.
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
…7.3.0-beta-0.5 notify-script changes
api/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts
Fixed
Show fixed
Hide fixed
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Fix all issues with AI Agents 🤖
In @api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts:
- Around line 521-538: Wrap the rename(unreadPath, archivePath) call in a
try/catch similar to moveNotification: attempt the rename, and if it throws
revert the earlier unread decrement by calling
this.increment(notification.importance, NotificationsService.overview.unread)
and, if snapshot exists, this.increment(notification.importance,
snapshot.unread), then rethrow or log the error; keep the existing branch logic
that only increments archive counts when the move succeeds (this.increment(...,
NotificationsService.overview.archive) and snapshot.archive) so you avoid double
counting.
In
@api/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts:
- Around line 236-246: The fallback contains a dead no-op variable
intermediateWriteBlockSingleQuotes and incomplete logic; remove the unused
assignment and either implement a real relaxed-matching fallback or delete the
entire fallback block. Specifically, remove the line creating
intermediateWriteBlockSingleQuotes and surrounding placeholder comments in
notify-script.modification.ts (the else branch that references
intermediateWriteBlock) and either (a) add a proper relaxed/regex-based
replacement routine that uses intermediateWriteBlock to attempt broader
matching, or (b) delete the else branch so only the strict replace path remains.
In
@api/src/unraid-api/unraid-file-modifier/modifications/patches/notify-script.patch:
- Around line 126-161: The new unconditional debug writes to
/var/log/notify_debug.log (calls to file_put_contents with notify_debug.log)
should be removed or gated behind a runtime debug flag; wrap every debug
file_put_contents added in the patch (the writes before archive existence check,
before Writing archive, before Writing unread, and the Skipping unread log) in a
conditional like if (is_notify_debug_enabled()) or a $notify_debug variable, or
remove them entirely for production, and ensure the helper clean_subject() and
build_ini_string() usage remains unchanged when you gate the logging so only the
log statements are conditional.
♻️ Duplicate comments (3)
api/src/unraid-api/unraid-file-modifier/modifications/patches/notify-script.patch (1)
111-121: Custom filename path should normalize.notifyextension.As noted in a previous review, when
-uis provided without a.notifyextension, the resulting file won't match theglob("*.notify")pattern in thegetcase, making those notifications invisible.api/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts (1)
54-61: Empty else block is dead code.The else block contains only comments explaining intent but no actual implementation. Either implement the resilient fallback logic or remove the else block entirely.
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (1)
987-989: Fix type casting to avoidanytype (previously flagged).Lines 988-989 still use
(any)type casting, which violates the coding guideline: "Never use the any type. Always prefer proper typing."The
dateandtimefields should be properly typed in theNotifyandDisplayinterfaces, eliminating the need for type casting.🔎 Proposed fix
Since
api/src/core/types/ini.tsalready defines these fields with proper types:
Notify.date: 'd-m-Y' | 'm-d-Y' | 'Y-m-d'Notify.time: 'h:i A' | 'H:i'Display.date: string(strftime format)Display.time?: string(strftime format)You can safely access them without casting:
// Prioritize notification-specific settings, fall back to global display settings - const dateFormat = (notifySettings as any).date || (settings as any).date || null; - const timeFormat = (notifySettings as any).time || (settings as any).time || null; + const dateFormat = notifySettings.date || settings.date || null; + const timeFormat = notifySettings.time || settings.time || null;
🧹 Nitpick comments (3)
web/src/store/notifications.ts (1)
17-17: Consider removing debug console.log before production.This console.log appears to be for development/debugging purposes. Console logs in production code can clutter browser consoles and may impact performance. Consider either:
- Removing this before the final merge, or
- Using a proper logging framework that can be disabled in production environments
api/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts (1)
23-271: Patch generation approach using string replacements is fragile.The
generatePatchmethod relies on exact string matching of multi-line PHP code blocks. While there are fallbacks for "original" and "intermediate" versions, subtle differences in whitespace, escaping, or minor code changes could cause all replacements to silently fail, resulting in an incomplete patch.Consider adding validation that verifies critical replacements actually occurred (e.g., checking that the new content differs from original in expected ways), and logging or throwing when expected patterns aren't found.
🔎 Example validation approach
// After all replacements, verify critical changes were applied const criticalPatterns = [ 'function safe_filename($string, $maxLength=255)', 'function build_ini_string', '$customFilename = false;', "case 'u':", ]; for (const pattern of criticalPatterns) { if (!newContent.includes(pattern)) { throw new Error(`NotifyScriptModification: Failed to apply patch - missing: ${pattern}`); } }api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (1)
124-139: Consider exponential backoff for retry logic.The fixed 100ms delay between retries works for filesystem operations, but exponential backoff would be more resilient for edge cases where the file system is under heavy load.
🔎 Proposed enhancement with exponential backoff
- for (let i = 0; i < 5; i++) { + const MAX_RETRIES = 5; + for (let i = 0; i < MAX_RETRIES; i++) { try { notification = await this.loadNotificationFile(path, NotificationType[type]); this.logger.debug( `[handleNotificationAdd] Successfully loaded ${path} on attempt ${i + 1}` ); break; } catch (error) { lastError = error; this.logger.warn( `[handleNotificationAdd] Attempt ${i + 1} failed for ${path}: ${error}` ); - // wait 100ms before retrying - await new Promise((resolve) => setTimeout(resolve, 100)); + // Exponential backoff: 100ms, 200ms, 400ms, 800ms, 1600ms + if (i < MAX_RETRIES - 1) { + await new Promise((resolve) => setTimeout(resolve, 100 * Math.pow(2, i))); + } } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
api/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/notify-script.modification.tsapi/src/unraid-api/unraid-file-modifier/modifications/patches/notify-script.patchweb/src/store/notifications.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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/notifications.tsapi/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/src/store/notifications.ts
web/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Files:
web/src/store/notifications.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/notifications.tsapi/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
web/src/store/notifications.tsapi/src/unraid-api/unraid-file-modifier/modifications/notify-script.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.patch
**/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/notifications.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/unraid-file-modifier/modifications/notify-script.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.patch
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/unraid-file-modifier/modifications/notify-script.modification.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
🧠 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: 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: 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.
📚 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-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/notify-script.modification.tsapi/src/unraid-api/unraid-file-modifier/modifications/patches/notify-script.patch
📚 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: 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.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-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.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: 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.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-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/patches/notify-script.patch
🧬 Code graph analysis (1)
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)
🪛 GitHub Check: CodeQL
api/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts
[notice] 240-240: Unused variable, import, function or class
Unused variable intermediateWriteBlockSingleQuotes.
🔇 Additional comments (13)
api/src/unraid-api/unraid-file-modifier/modifications/patches/notify-script.patch (2)
44-73: INI helper functions look correct.The
ini_encode_value,build_ini_string, andini_decode_valuefunctions properly handle:
- Numeric types (unquoted)
- Boolean types (as 'true'/'false')
- String escaping (backslashes and quotes)
- Decoding with proper unescaping via
stripslashesThis is a solid approach for INI-compatible serialization.
167-184: Get case update correctly handles values containing=characters.The change to
explode('=', $field, 2)ensures that values containing=(like URLs with query strings) are parsed correctly. Theini_decode_valuecall properly unescapes the stored values before JSON output.api/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts (1)
12-21: Version check and fallback logic look correct.The
shouldApplymethod properly skips modification for Unraid 7.4+ where the refactored script is native, and falls back to the parent implementation for earlier versions. ThecheckOsVersion: falseparameter in the super call is appropriate since version checking is handled explicitly above.api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/notify.modified.snapshot (3)
1-345: Snapshot correctly represents the expected modified state.The snapshot properly captures:
- Updated
safe_filenamefunction with$maxLengthparameter- New INI helper functions (
ini_encode_value,build_ini_string,ini_decode_value)- Extended CLI options including
-ufor custom filename- Updated filename construction logic with proper length handling
- Modified
getcase usingini_decode_valuefor proper value unescapingThe snapshot is consistent with both the patch file and the modification TypeScript class.
274-307: Debug logging in snapshot confirms it will be present in production.The snapshot confirms that debug logging statements writing to
/var/log/notify_debug.logwill be included in the modified script. These should be addressed as mentioned in the patch review.
259-269: Custom filename handling doesn't enforce.notifyextension.The snapshot confirms the implementation where custom filenames are passed directly to
safe_filenamewithout ensuring the.notifyextension. This is consistent with the patch but carries the same risk noted in previous reviews.api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (7)
102-107: LGTM: Watcher configuration hardening.The addition of
usePollingandignoreInitial: trueprevents duplicate processing of existing files on startup, which is essential for maintaining accurate notification counts.
175-193: LGTM: Robust settings parsing with sensible defaults.The inline helper functions handle type coercion safely, and the fallback values ensure the notification system remains functional even when configuration is incomplete.
274-280: LGTM: Improved logging levels for observability.Promoting the execution message to
loglevel and usingwarnfor legacy notifier failures provides better visibility into the notification creation flow and potential issues.
302-316: LGTM: Backward-compatible extension of legacy script support.The optional
idparameter allows callers to specify a unique identifier for the legacy notifier without breaking existing calls that don't provide it.
585-595: LGTM: Improved consistency by reusing archiveNotification.Refactoring to call
archiveNotificationfor each item ensures that the complex existence-checking and double-counting prevention logic is consistently applied, rather than being duplicated.
767-788: LGTM: Memory-efficient generator-based loading.The batched generator pattern prevents loading all notifications into memory at once while preserving individual error handling. The discriminated union return type provides type-safe result processing.
802-840: LGTM: Efficient streaming implementation with early termination.The refactored approach using the generator with offset/limit handling prevents loading unnecessary files when pagination is used, significantly improving performance for large notification directories.
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
Show resolved
Hide resolved
api/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts
Show resolved
Hide resolved
| } else { | ||
| // Fallback: try to replace partial bits if possible or regex? | ||
| // For now, assume one of these matches. If not, we might be in a state where manual intervention or specific regex is needed. | ||
| // Let's rely on strict matching for safety, but check for single quotes vs double quotes in intermediate block just in case user paste had slightly different escaping | ||
| const intermediateWriteBlockSingleQuotes = intermediateWriteBlock.replace(/'/g, "'"); // no-op but reminder | ||
| // If the user's file has slightly different spacing, we might fail. | ||
| // But we'll try this for now. | ||
|
|
||
| // Attempt relaxed match for intermediate block if standard string replace fails? | ||
| // Not easily done without reliable anchors. | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable and incomplete fallback logic.
The intermediateWriteBlockSingleQuotes variable is assigned but never used (confirmed by static analysis). The fallback logic at lines 237-246 is essentially a no-op with only comments.
🔎 Suggested fix: remove dead code
} else if (newContent.includes(intermediateWriteBlock)) {
// Try replacing intermediate
newContent = newContent.replace(intermediateWriteBlock, newWriteBlock);
- } else {
- // Fallback: try to replace partial bits if possible or regex?
- // For now, assume one of these matches. If not, we might be in a state where manual intervention or specific regex is needed.
- // Let's rely on strict matching for safety, but check for single quotes vs double quotes in intermediate block just in case user paste had slightly different escaping
- const intermediateWriteBlockSingleQuotes = intermediateWriteBlock.replace(/'/g, "'"); // no-op but reminder
- // If the user's file has slightly different spacing, we might fail.
- // But we'll try this for now.
-
- // Attempt relaxed match for intermediate block if standard string replace fails?
- // Not easily done without reliable anchors.
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: CodeQL
[notice] 240-240: Unused variable, import, function or class
Unused variable intermediateWriteBlockSingleQuotes.
🤖 Prompt for AI Agents
In
@api/src/unraid-api/unraid-file-modifier/modifications/notify-script.modification.ts
around lines 236-246, The fallback contains a dead no-op variable
intermediateWriteBlockSingleQuotes and incomplete logic; remove the unused
assignment and either implement a real relaxed-matching fallback or delete the
entire fallback block. Specifically, remove the line creating
intermediateWriteBlockSingleQuotes and surrounding placeholder comments in
notify-script.modification.ts (the else branch that references
intermediateWriteBlock) and either (a) add a proper relaxed/regex-based
replacement routine that uses intermediateWriteBlock to attempt broader
matching, or (b) delete the else branch so only the strict replace path remains.
api/src/unraid-api/unraid-file-modifier/modifications/patches/notify-script.patch
Outdated
Show resolved
Hide resolved
…o prevent double counting. Fixes: #1859
…r.vue to prevent double counting.
…r.vue to prevent double counting.
…r.vue to prevent double counting.
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Fix all issues with AI Agents 🤖
In @api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts:
- Around line 1010-1011: The code uses `(notifySettings as any).date` and
`(settings as any).date` (and same for `.time`) in the computation of dateFormat
and timeFormat; remove the `as any` casts by ensuring the Notify and Display
interfaces include properly typed optional `date` and `time` fields (e.g.,
string | null or a specific enum), update the declarations of `notifySettings`
and `settings` to be typed as those interfaces, and then change the expressions
to access `notifySettings.date ?? settings.date ?? null` and
`notifySettings.time ?? settings.time ?? null` (or equivalent optional chaining)
so no `any` is required and typings are preserved for `dateFormat` and
`timeFormat`.
In @web/src/components/Notifications/List.vue:
- Around line 206-210: The current updateQuery block calls setTimeout(() =>
debouncedRefetch(), 0) when pagination drift is detected, which can cause
repeated refetch loops; add a short cooldown guard (e.g., a ref/boolean
driftDetectedRecently or a counter) alongside the debouncedRefetch call in
updateQuery so that you only schedule a refetch if the guard is false, set the
guard true when scheduling, and reset it after a reasonable delay (e.g.,
~2000ms) to prevent excessive cycles; keep the original return previousResult
behavior and reference debouncedRefetch and the updateQuery drift condition to
locate where to add the guard and reset logic.
- Around line 232-236: In the catch block inside the component's load-more
routine (e.g., the fetchMore / onLoadMore function) stop re-throwing the error
to avoid an unhandled promise rejection: keep the console.error('[Notifications]
fetchMore Error:', error) and set canLoadMore.value = false, then either return
or resolve gracefully instead of throw error so the infinite-scroll caller won't
receive an uncaught rejection.
- Line 325: Replace the hardcoded paragraph text in Notifications/List.vue (<p
class="text-muted-foreground text-center text-sm">Loading Notifications...</p>)
with the i18n translation call (e.g., use $t or useI18n().t such as
$t('notifications.loading') or t('notifications.loading') depending on your
setup) and add the corresponding "notifications.loading" key and translations to
the project's i18n files.
In @web/src/components/Notifications/Sidebar.vue:
- Around line 115-128: The toast action's onClick (created in createOpener)
calls navigate(notif.link as string) without awaiting or catching errors and
uses a non-null assertion; update createOpener so onClick is async, validate
notif.link before using it (remove the unsafe "as string" cast), await
navigate(notif.link) and wrap it in try/catch to handle and log navigation
failures (or show a fallback toast) before returning; keep usage with toast.add
(actions: notif.link ? [createOpener()] : undefined) unchanged but ensure
createOpener returns a safe action when notif.link is present.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (1)
509-568: Add error handling to prevent stat corruption on file operation failures.Stats are decremented (lines 533-536) before attempting file operations, but the
unlink(line 541) andrename(line 551) calls lack error handling. If either operation fails, the stats will be corrupted—the unread count has been decremented but the file wasn't moved or removed.Apply the same error-handling pattern used in
moveNotification(lines 492-500): wrap the file operations in try-catch and revert the stat decrements if they fail.🔎 Proposed fix
// Update stats this.decrement(notification.importance, NotificationsService.overview.unread); if (snapshot) { this.decrement(notification.importance, snapshot.unread); } - 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); - } - } else { - // File not in archive, move it there - await rename(unreadPath, archivePath); - - // We moved a file to archive that wasn't there. - // We DO need to increment the stats. - this.increment(notification.importance, NotificationsService.overview.archive); - if (snapshot) { - this.increment(notification.importance, snapshot.archive); - } - } + try { + 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); + } + } else { + // File not in archive, move it there + await rename(unreadPath, archivePath); + + // We moved a file to archive that wasn't there. + // We DO need to increment the stats. + this.increment(notification.importance, NotificationsService.overview.archive); + if (snapshot) { + this.increment(notification.importance, snapshot.archive); + } + } + } catch (err) { + // Revert the earlier decrements + this.increment(notification.importance, NotificationsService.overview.unread); + if (snapshot) { + this.increment(notification.importance, snapshot.unread); + } + throw err; + }web/src/components/Notifications/Sidebar.vue (1)
56-78: Missing error handling for archive/delete mutations.Both
confirmAndArchiveAllandconfirmAndDeleteArchivescall their respective mutations without error handling. Based on retrieved learnings, when mutations fail, users should be informed. Consider adding try-catch with toast error feedback.🔎 Proposed fix
const confirmAndArchiveAll = async () => { const confirmed = await confirm({ title: t('notifications.sidebar.confirmArchiveAll.title'), description: t('notifications.sidebar.confirmArchiveAll.description'), confirmText: t('notifications.sidebar.confirmArchiveAll.confirmText'), confirmVariant: 'primary', }); if (confirmed) { - await archiveAll(); + try { + await archiveAll(); + } catch (e) { + console.error('[Notifications] Archive all failed:', e); + toast.add({ + title: t('notifications.sidebar.archiveAllError'), + color: 'error', + }); + } } };Apply similar pattern to
confirmAndDeleteArchives.
🧹 Nitpick comments (3)
web/src/components/Notifications/List.vue (2)
86-93: Remove or guard debug console.log statements for production.There are numerous
console.logandconsole.warncalls scattered throughout the component. While useful during development, these should be removed or guarded behind a debug flag before release to avoid polluting the browser console in production.🔎 Suggested approach
Either remove these statements entirely or wrap them in a conditional:
const DEBUG = import.meta.env.DEV; // Then use: if (DEBUG) console.log('[Notifications] ...');Affected lines include: 90, 110, 128, 143, 166, 169, 189-191, 196, 203-204, 224-226, 229.
Also applies to: 109-114, 122-129, 143-143, 166-166, 189-191, 196-196, 203-204, 223-230
139-145: Redundant client-side filtering when server already filters by type.The query at line 68 already passes
type: props.typeto the server filter. The computed property then filters again byn.type === props.typeat line 142. This is redundant unless the server response could include mismatched types.🔎 Suggested simplification
If the server guarantees only matching types are returned, simplify to:
const notifications = computed(() => { if (!result.value?.notifications.list) return []; const list = useFragment(NOTIFICATION_FRAGMENT, result.value?.notifications.list); - const filtered = list.filter((n) => n.type === props.type); - console.log('[Notifications] Computed list updated. Length:', filtered.length); - return filtered; + return list; });web/src/components/Notifications/Sidebar.vue (1)
94-101: Refetch triggered on every 'online' status change.The watcher at lines 94-101 triggers
handleRefetch()wheneverunraidApiStatusbecomes'online'. If the status flickers (e.g., during connection instability), this could cause rapid refetches. Consider debouncing or adding a cooldown.🔎 Suggested improvement
import { useDebounceFn } from '@vueuse/core'; const debouncedHandleRefetch = useDebounceFn(() => { void refetch(); }, 500); watch( () => unraidApiStore.unraidApiStatus, (status) => { if (status === 'online') { debouncedHandleRefetch(); } } );
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.tsweb/src/components/Notifications/List.vueweb/src/components/Notifications/Sidebar.vue
🧰 Additional context used
📓 Path-based instructions (6)
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/src/components/Notifications/Sidebar.vueweb/src/components/Notifications/List.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.vueapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsweb/src/components/Notifications/List.vue
**/*.{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/graph/resolvers/notifications/notifications.service.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.service.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:
api/src/unraid-api/graph/resolvers/notifications/notifications.service.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.service.ts
🧠 Learnings (35)
📓 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: 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.
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-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: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.vueapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsweb/src/components/Notifications/List.vue
📚 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.vueapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsweb/src/components/Notifications/List.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.vueweb/src/components/Notifications/List.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.vueweb/src/components/Notifications/List.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.vueweb/src/components/Notifications/List.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.vueweb/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/Sidebar.vueweb/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/Sidebar.vueweb/src/components/Notifications/List.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.vueweb/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 : Check for expected prop handling and event emissions in Vue components
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/Sidebar.vue
📚 Learning: 2025-02-19T17:30:55.418Z
Learnt from: pujitm
Repo: unraid/api PR: 1148
File: web/composables/gql/graphql.ts:1812-1821
Timestamp: 2025-02-19T17:30:55.418Z
Learning: When using Apollo Vue's subscribeToMore, throttling can be implemented by wrapping the update function with lodash's throttle or using RxJS's throttleTime operator to prevent flooding the client with updates.
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: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.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.tsweb/src/components/Notifications/List.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-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-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-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-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: 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
🧬 Code graph analysis (1)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (6)
api/src/environment.ts (1)
CHOKIDAR_USEPOLLING(75-75)packages/unraid-shared/src/util/file.ts (1)
fileExists(12-15)api/src/unraid-api/cli/generated/graphql.ts (2)
Notification(1640-1653)NotificationSettings(1690-1696)web/src/composables/gql/graphql.ts (2)
Notification(1640-1653)NotificationSettings(1690-1696)api/src/utils.ts (2)
batchProcess(58-71)formatDatetime(138-179)web/scripts/extract-translations.mjs (1)
result(77-77)
🔇 Additional comments (7)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (3)
115-170: LGTM - Robust handling for notification additions.The archive shadowing check (lines 119-128) correctly prevents double-counting when files exist in both directories, and the retry logic (lines 133-148) handles race conditions during file writes. Publishing overview updates for archive additions (lines 166-169) ensures counts remain synchronized.
184-202: Excellent type-safe implementation.The
getSettings()method uses proper type guards (parseBoolean,parsePositiveInt) to safely parse configuration values with sensible defaults, avoiding any type assertions.
789-862: Excellent refactoring to generator-based batched loading.The
getNotificationsGenerator(lines 789-810) with batch size of 10 improves memory efficiency by avoiding loading all notifications at once. The discriminated union return type and proper offset/limit handling inloadNotificationsFromPaths(lines 824-862) maintain correctness while enabling early termination when the limit is reached.web/src/components/Notifications/Sidebar.vue (4)
251-264: Both NotificationsList instances render simultaneously but only one is visible.Using
v-ifon theNotificationsListcomponents means only the active tab's list is mounted. This is likely intentional to avoid duplicate subscriptions and data fetching, which is good. However, switching tabs will re-mount and refetch data each time.Consider if
v-showorkeep-alivewould provide a better UX for frequently switching users, at the cost of maintaining both subscriptions.
183-269: Good migration to USlideover with clean tab/filter structure.The refactored UI using
USlideover,UTabs, andUTooltipprovides a clean, modular structure. The separation of concerns between tabs (unread/archived) and filters (importance) is well organized. The use of computed properties fortabsandfilterTabsensures reactivity.
30-32:useToast()is correctly auto-imported from@nuxt/uivia Nuxt's auto-import configuration. No explicit import is required.
170-170: Correct the browser compatibility information forcolor-mix()withoklab.The class
hover:bg-[color-mix(in_oklab,hsl(var(--accent))_20%,transparent)]usescolor-mix()withoklabcolor space, which is well-supported: Chrome 116+, Safari 16.2+, Firefox 113+. Browser support for this feature is broadly available and does not present a significant compatibility concern.Likely an incorrect or invalid review comment.
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
Outdated
Show resolved
Hide resolved
| // Trigger refetch asynchronously to avoid side-effects during render cycle | ||
| setTimeout(() => { | ||
| debouncedRefetch(); | ||
| }, 0); | ||
| return previousResult; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential infinite loop with setTimeout + debouncedRefetch.
When pagination drift is detected, setTimeout(() => debouncedRefetch(), 0) is called. If the drift condition persists (e.g., items keep being added), this could trigger repeated refetches. The debounce helps, but consider adding a drift-detection cooldown or counter to prevent excessive refetch cycles.
🔎 Suggested safeguard
const driftDetectedRecently = ref(false);
// In updateQuery:
if (incomingList.length > 0 && newUniqueItems.length === 0) {
if (!driftDetectedRecently.value) {
driftDetectedRecently.value = true;
setTimeout(() => {
debouncedRefetch();
// Reset after a delay
setTimeout(() => { driftDetectedRecently.value = false; }, 2000);
}, 0);
}
return previousResult;
}🤖 Prompt for AI Agents
In @web/src/components/Notifications/List.vue around lines 206-210, The current
updateQuery block calls setTimeout(() => debouncedRefetch(), 0) when pagination
drift is detected, which can cause repeated refetch loops; add a short cooldown
guard (e.g., a ref/boolean driftDetectedRecently or a counter) alongside the
debouncedRefetch call in updateQuery so that you only schedule a refetch if the
guard is false, set the guard true when scheduling, and reset it after a
reasonable delay (e.g., ~2000ms) to prevent excessive cycles; keep the original
return previousResult behavior and reference debouncedRefetch and the
updateQuery drift condition to locate where to add the guard and reset logic.
| const color = NOTIFICATION_TOAST_COLORS[notif.importance]; | ||
| const createOpener = () => ({ | ||
| label: t('notifications.sidebar.toastOpen'), | ||
| onClick: () => window.location.assign(notif.link as string), | ||
| onClick: () => navigate(notif.link as string), | ||
| }); | ||
| requestAnimationFrame(() => | ||
| toast(notif.title, { | ||
| toast.add({ | ||
| title: notif.title, | ||
| description: notif.subject, | ||
| action: notif.link ? createOpener() : undefined, | ||
| color, | ||
| actions: notif.link ? [createOpener()] : undefined, | ||
| }) | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Toast action onClick doesn't await navigation or handle errors.
The onClick handler calls navigate(notif.link as string) without error handling. If notif.link is malformed or navigation fails, the error will be silently swallowed. Also, the type assertion as string suggests the link could be null despite the conditional at line 126.
🔎 Proposed fix
const createOpener = () => ({
label: t('notifications.sidebar.toastOpen'),
- onClick: () => navigate(notif.link as string),
+ onClick: () => {
+ if (notif.link) {
+ navigate(notif.link);
+ }
+ },
});🤖 Prompt for AI Agents
In @web/src/components/Notifications/Sidebar.vue around lines 115-128, The toast
action's onClick (created in createOpener) calls navigate(notif.link as string)
without awaiting or catching errors and uses a non-null assertion; update
createOpener so onClick is async, validate notif.link before using it (remove
the unsafe "as string" cast), await navigate(notif.link) and wrap it in
try/catch to handle and log navigation failures (or show a fallback toast)
before returning; keep usage with toast.add (actions: notif.link ?
[createOpener()] : undefined) unchanged but ensure createOpener returns a safe
action when notif.link is present.
This commit addresses several code review suggestions to improve type safety, error handling, and production readiness. Changes: - api: Prevent stat corruption in archiveNotification by reverting stat decrements if the file rename operation fails. - api: Refactor formatDatetime in `notifications.service` to usage optional chaining and remove unnecessary `any` casting. - file-modifier: Remove debug logging (`notify_debug.log`) and unused variables from the `notify-script` modification to ensure it is clean for production. - web: Guard `navigator` access in Item.vue to prevent potential SSR reference errors. - web: Improve `timeAgo` reactivity and conditional rendering to gracefully handle missing timestamps without hydration mismatches. - web: Add error handling and toast feedback to bulk actions (Archive All/Delete All) in the Sidebar to inform users of failures.
- 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 |
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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
✏️ Tip: You can customize this high-level summary in your review settings.