Refactor notification jobs to shared background service #1806
Conversation
WalkthroughIntroduces background job support for notifications: new BackgroundJobsService, NotificationJob types, async mutations (startArchiveAll/startDeleteAll) and polling-based frontend UI updates; existing delete mutation was nested under Changes
Sequence DiagramsequenceDiagram
actor User
participant Frontend as Frontend (Sidebar.vue)
participant Apollo as Apollo (GraphQL)
participant Resolver as Notifications Mutation Resolver
participant Service as Notifications Service
participant BgJobs as BackgroundJobsService
User->>Frontend: Trigger "Archive All"
Frontend->>Apollo: startArchiveAll(importance?)
Apollo->>Resolver: startArchiveAll mutation
Resolver->>Service: startArchiveAllJob(importance?)
Service->>BgJobs: createJob(...) & runJob(...)
BgJobs-->>Service: Job (QUEUED / RUNNING updates)
Service-->>Resolver: NotificationJob
Resolver-->>Apollo: NotificationJob (response)
Apollo-->>Frontend: Job returned (id,state,processed,total)
Frontend->>Frontend: start polling (notificationJobStatus(jobId))
loop Poll until SUCCEEDED/FAILED
Frontend->>Apollo: notificationJobStatus(jobId)
Apollo->>BgJobs: getJob(jobId)
BgJobs-->>Apollo: current job state
Apollo-->>Frontend: updated job
Frontend->>Frontend: update UI, stop when complete
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts (1)
3-12: Newnotificationsmutation namespace matches existing pattern; fix Prettier import issueThe new
notifications()mutation follows the existing namespaced style (array/docker/vm) and cleanly exposesNotificationMutations.CI is failing on a Prettier check complaining about
NotificationMutationsin the import block. Ensure the import list includes it with the expected formatting (and then rerunpnpm lint-staged):-import { - ApiKeyMutations, - ArrayMutations, - DockerMutations, - ParityCheckMutations, - RCloneMutations, - RootMutations, - VmMutations, -} from '@app/unraid-api/graph/resolvers/mutation/mutation.model.js'; +import { + ApiKeyMutations, + ArrayMutations, + DockerMutations, + NotificationMutations, + ParityCheckMutations, + RCloneMutations, + RootMutations, + VmMutations, +} from '@app/unraid-api/graph/resolvers/mutation/mutation.model.js';Also applies to: 46-49
🧹 Nitpick comments (11)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (2)
62-90: Job helper wrappers around BackgroundJobsService look good; consider nullability alignment
createJob,updateJob, andrunJobcleanly encapsulate the interaction withBackgroundJobsService, andgetJobprovides a simple lookup surface.If
backgroundJobs.getJobcan returnundefinedfor unknown IDs, consider changing the return type ofgetJob(and any callers) toNotificationJob | nullto better reflect actual behavior and avoid surprises at call sites.
359-381: Clarify queued IDs vs paths and make fire‑and‑forget intent explicitThe async job runners look correct, but two small refactors would make them less error‑prone:
- Use IDs instead of full paths in the delete queue
listFilesInFolderreturns absolute paths; pushing those intoqueue.idrelies onpath.join’s behavior with absolute paths indeleteNotification. To avoid this subtle coupling, derive IDs from the paths before queueing:- for (const targetType of targets) { - const ids = await this.listFilesInFolder(this.paths()[targetType]); - ids.forEach((id) => queue.push({ id, type: targetType })); - } + for (const targetType of targets) { + const paths = await this.listFilesInFolder(this.paths()[targetType]); + paths.forEach((path) => + queue.push({ id: this.getIdFromPath(path), type: targetType }) + ); + }
- Make ignored job Promise explicit
Since you’re intentionally fire‑and‑forget, prefix
runJobwithvoidin both starters to avoid unhandled‑promise‑lint noise:- setImmediate(() => - this.runJob(job, async () => { + setImmediate(() => + void this.runJob(job, async () => { // ... }) );Apply analogous
voidusage instartArchiveAllJob.Also applies to: 545-572
api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (1)
14-18: Alignjobfield nullability between GraphQL and TypeScriptThe new
jobresolve field cleanly exposesNotificationsService.getJob, but the decorator marks it asnullable: truewhile the TS return type isNotificationJob. If an unknown ID results inundefined/nullfrom the service (or if you later change it to do so), this will conflict with the non‑nullable TS type.Consider updating both the resolver and service signatures to return
NotificationJob | null(or making the schema non‑nullable) so the implementation, TypeScript types, and GraphQL schema all agree.Also applies to: 45-48
api/src/unraid-api/graph/resolvers/notifications/notifications.mutations.resolver.ts (1)
18-60: Resolver shape and permissions for notification mutations look appropriateThe
NotificationMutationsResolvercorrectly targetsNotificationMutationswith@Resolver, and exposesdelete,startArchiveAll, andstartDeleteAllas@ResolveFields. Each field is guarded withUsePermissions({ action: AuthAction.DELETE_ANY, resource: Resource.NOTIFICATIONS }), matching the expected authorization semantics for destructive notification operations. Delegation toNotificationsServicekeeps business logic out of the resolver.One note: given the background job implementation, the
startDeleteAllcheck forjob.state === NotificationJobState.FAILEDwill currently never trigger because the work runs asynchronously after the job is returned. It’s harmless, but you may consider either letting the service throw on synchronous failures or documenting that job failure is surfaced via polling the job state, not via this mutation.api/src/unraid-api/graph/services/background-jobs.service.ts (1)
5-93: BackgroundJobsService implementation is sound and matches notification job usageThe generic job model (
BackgroundJob), in‑memory storage viajobsMap, andcreateJob/updateJob/getJob/runJobAPIs all line up with howNotificationsServiceis using them (create + asyncrunJob+ latergetJob). State transitions and error logging inrunJobare handled cleanly, andAppError404 on unknown job IDs is appropriate for consumers.If you expect a high volume of background jobs over the lifetime of a node, consider adding an optional eviction mechanism (e.g., TTL-based cleanup of completed/failed jobs or a max-size LRU) to keep the in‑memory
jobsmap from growing unbounded.web/src/components/Notifications/graphql/notification.query.ts (1)
81-89: Delete-all mutation wiring looks correct, but confirm argument semantics
StartDeleteNotificationscallsnotifications.startDeleteAll(type: $type)and returns aNotificationJob. This matches the job-based delete flow; just make sure:
NotificationTypeis indeed the correct enum for thetypeargument.- Passing
null(if ever done) is supported by the server, since$typeis not marked non-null here.If the server requires a type, consider making
$type: NotificationType!to catch mistakes at compile time.web/src/components/Notifications/Sidebar.vue (5)
2-3: Explicit Vue imports are acceptable but not requiredGiven Nuxt’s auto-import setup, importing
computed,onBeforeUnmount,reactive, andrefis optional. Keeping them is fine for clarity and doesn’t introduce issues.
57-72: Job state containers are well-structured
jobPollersandactiveJobstrack polling intervals and job metadata per operation (archiveAll,deleteArchived,deleteUnread), andactiveStatescentralizes what counts as “in progress”. This layout makes the subsequent helpers straightforward.
109-123: Job activity and label helpers correctly drive UI state
isJobActiveandjobLabelboth derive fromactiveStatesandactiveJobs, andjobLabeldelegates to the newnotifications.sidebar.processingStatusstring when in progress, falling back to static button text otherwise. Once the i18n placeholder syntax is fixed, this will give clear live feedback.
246-248: Unmount hook correctly cleans up polling intervals
onBeforeUnmountiterates overjobPollersand clears any existing intervals, preventing timer leaks when the sidebar is closed or the component is destroyed. This is sufficient; no need to also null out the entries at this point.
287-316: Buttons correctly reflect job state and show progress labelsIn both tabs:
- Buttons are disabled when the corresponding mutation is loading or
isJobActive(...)indicates an in-progress job.- Labels use
jobLabel(...)to swap from static text to “Processing …” with live counts.Once the i18n string is fixed, users will see accurate progress feedback and won’t be able to start overlapping jobs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
api/src/unraid-api/graph/resolvers/mutation/mutation.model.ts(2 hunks)api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts(4 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.mutations.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts(4 hunks)api/src/unraid-api/graph/resolvers/resolvers.module.ts(3 hunks)api/src/unraid-api/graph/services/background-jobs.service.ts(1 hunks)web/src/components/Notifications/Item.vue(0 hunks)web/src/components/Notifications/Sidebar.vue(7 hunks)web/src/components/Notifications/graphql/notification.query.ts(3 hunks)web/src/locales/en.json(1 hunks)
💤 Files with no reviewable changes (1)
- web/src/components/Notifications/Item.vue
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
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-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/mutation/mutation.model.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.model.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/mutation/mutation.resolver.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/mutation/mutation.model.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.model.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.tsapi/src/unraid-api/graph/resolvers/mutation/mutation.resolver.tsweb/src/components/Notifications/graphql/notification.query.ts
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
Repo: unraid/api PR: 942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
api/src/unraid-api/graph/resolvers/mutation/mutation.model.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.mutations.resolver.tsapi/src/unraid-api/graph/resolvers/resolvers.module.tsapi/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
Repo: unraid/api PR: 1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Applied to files:
api/src/unraid-api/graph/resolvers/mutation/mutation.model.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.model.tsapi/src/unraid-api/graph/resolvers/mutation/mutation.resolver.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.mutations.resolver.tsweb/src/components/Notifications/Sidebar.vueweb/src/components/Notifications/graphql/notification.query.ts
📚 Learning: 2024-11-05T14:48:56.949Z
Learnt from: elibosley
Repo: unraid/api PR: 942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:41-47
Timestamp: 2024-11-05T14:48:56.949Z
Learning: Input types for GraphQL mutations can be pulled from the GraphQL codegen types, as they are generated into the API files. Therefore, explicit typing in the resolver files may be unnecessary.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.mutations.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.model.tsapi/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts
📚 Learning: 2024-11-06T20:59:06.096Z
Learnt from: mdatelle
Repo: unraid/api PR: 942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:87-89
Timestamp: 2024-11-06T20:59:06.096Z
Learning: Error handling in mutations like `addRoleForUser` in `auth.resolver.ts` is performed at the service level, so additional error handling in the resolver is unnecessary.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.mutations.resolver.ts
📚 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: 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-06-13T17:14:21.739Z
Learnt from: pujitm
Repo: unraid/api PR: 1417
File: web/components/ConnectSettings/ConnectSettings.ce.vue:11-18
Timestamp: 2025-06-13T17:14:21.739Z
Learning: The project’s build tooling auto-imports common Vue/Pinia helpers such as `storeToRefs`, so explicit import statements for them are not required.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-09T15:47:29.325Z
Learnt from: pujitm
Repo: unraid/api PR: 975
File: web/components/Notifications/TabList.vue:1-24
Timestamp: 2024-12-09T15:47:29.325Z
Learning: In our Nuxt setup using Vue.js, components defined within the codebase are autoloaded and do not require explicit import statements in the script section. For example, components like `TabsList` and `TabsTrigger` used in `web/components/Notifications/TabList.vue` are automatically available without imports.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-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/Sidebar.vue
📚 Learning: 2025-08-23T16:27:30.015Z
Learnt from: elibosley
Repo: unraid/api PR: 1609
File: web/components/ApiKey/ApiKeyCreate.vue:414-418
Timestamp: 2025-08-23T16:27:30.015Z
Learning: In Vue 3 templates, refs are automatically unwrapped for reading but NOT for assignment. Writing `formData = newValue` in a template replaces the ref object and breaks reactivity, while `formData.value = newValue` in script setup properly updates the ref's value.
Applied to files:
web/src/components/Notifications/Sidebar.vue
🧬 Code graph analysis (6)
api/src/unraid-api/graph/resolvers/mutation/mutation.model.ts (1)
api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts (5)
ObjectType(99-120)ObjectType(122-131)ObjectType(133-174)ObjectType(176-190)ObjectType(192-211)
api/src/unraid-api/graph/resolvers/notifications/notifications.mutations.resolver.ts (3)
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts (1)
Resolver(14-50)api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (3)
Resolver(21-172)overview(41-43)job(46-48)packages/unraid-shared/src/use-permissions.directive.ts (1)
UsePermissions(81-128)
api/src/unraid-api/graph/services/background-jobs.service.ts (1)
api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (1)
job(46-48)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (1)
api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (2)
job(46-48)notifications(34-38)
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts (1)
web/composables/gql/graphql.ts (1)
Mutation(1199-1239)
web/src/components/Notifications/graphql/notification.query.ts (2)
api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (1)
deleteNotification(71-79)api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (1)
deleteNotification(325-340)
🪛 GitHub Actions: CI - Main (API)
api/src/unraid-api/graph/resolvers/notifications/notifications.mutations.resolver.ts
[error] 12-12: "NotificationMutations" is not exported by "src/unraid-api/graph/resolvers/notifications/notifications.model.ts", imported by "src/unraid-api/graph/resolvers/notifications/notifications.mutations.resolver.ts".
web/src/components/Notifications/Sidebar.vue
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues in this file.
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts
[error] 7-7: Prettier formatting failed. Insert NotificationMutations, (prettier/prettier).
🪛 GitHub Check: Test API
api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts
[failure] 212-212:
Delete ⏎
🔇 Additional comments (12)
api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts (1)
17-27: Notification job enums and GraphQL registration look consistentEnum values and
registerEnumTypewiring match the existing notification enums pattern and should integrate cleanly with the new job service and schema.Also applies to: 38-45
api/src/unraid-api/graph/resolvers/resolvers.module.ts (1)
18-21: DI wiring for notification mutations and background jobs looks correctRegistering
NotificationMutationsResolverandBackgroundJobsServicehere matches how other mutation namespaces and shared services are exposed and should giveNotificationsServiceand related resolvers access to the shared job helper.Also applies to: 33-35, 62-66
api/src/unraid-api/graph/resolvers/mutation/mutation.model.ts (2)
43-47: NotificationMutations grouping type looks consistentThe new
NotificationMutationsobject type and description align with the existing mutation namespaces (array/docker/vm/etc.). No issues from a schema/typing standpoint.
68-69: RootMutations.notifications field wiring is correctThe
notificationsfield follows the same pattern as other grouped mutation fields (type, description, default instance). This will integrate cleanly withRootMutationsResolver.notifications().web/src/locales/en.json (1)
307-309: Confirm-delete-unread copy looks consistent with existing patternsTitle, description, and confirm text are clear and match the destructive nature and scope (“all unread notifications”) of the action. No changes needed.
web/src/components/Notifications/graphql/notification.query.ts (3)
26-35: Job fragment fields cover polling needs
NotificationJobFragmentexposes id, operation, state, processed, total, and error – everything the client-side poller needs without overfetching. Looks good.
71-79: I'll check the generated GraphQL schema to finalize the verification:Archive-all mutation correctly returns a job
In GraphQL inputs such as field arguments, nullable types are always optional, and the resolver confirms this:
importanceis marked asnullable: truein the NestJS@Args()decorator, and the TypeScript parameter is optional (importance?: NotificationImportance). The client-side mutation correctly expresses this nullability—$importance: NotificationImportancewithout a!suffix allows the argument to be omitted or null. The mutation callsnotifications.startArchiveAlland returnsNotificationJobFragment, matching the background job model.
110-118: Fix missing GraphQL type specification on job resolver argumentThe
jobfield resolver atapi/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts:46is missing the GraphQL type specification for theidargument.Currently:
public job(@Args('id') jobId: string): NotificationJobShould be:
public job(@Args('id', { type: () => ID }) jobId: string): NotificationJobThis inconsistency with other
@Args('id', { type: () => ... })usages in the same file (lines 72, 90, 116) may cause incorrect schema generation or type inference issues. The query contract expectsID!type, which must be explicitly declared in the decorator.⛔ Skipped due to learnings
Learnt from: elibosley Repo: unraid/api PR: 942 File: api/src/unraid-api/auth/api-key.service.ts:62-70 Timestamp: 2024-11-05T14:49:07.308Z Learning: In `api/src/unraid-api/auth/api-key.service.ts`, when handling read errors in the `findById` method, throw a `GraphQLError` instead of an `InternalServerErrorException`.Learnt from: elibosley Repo: unraid/api PR: 942 File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:41-47 Timestamp: 2024-11-05T14:48:56.949Z Learning: Input types for GraphQL mutations can be pulled from the GraphQL codegen types, as they are generated into the API files. Therefore, explicit typing in the resolver files may be unnecessary.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.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.web/src/components/Notifications/Sidebar.vue (4)
50-56: Perfect! The project uses@vue/apollo-composable: 4.2.2. Now let me verify theloadingref semantics for that specific version.Loading ref semantics verified—code is correct
The
loadingref tracks the progress of the mutation in @vue/apollo-composable, and your code correctly destructures and uses it. The pattern is standard and consistent with the documented API for version 4.2.2.
134-149: Archive-all confirmation correctly handles importance filtering
confirmAndArchiveAll:
- Uses the existing confirm dialog with archive-all translations.
- Calls
startArchiveAllJobwith the currentimportancefilter value.- When
importanceisnullor omitted, the mutation passes an empty filter object{}toloadNotificationsFromPaths, which processes all notification importances as intended.- Seeds job tracking via
setJob('archiveAll', job)and starts polling.This matches the background job model and disables the button while the job is active via other helpers.
151-183: Verification confirms consistent enum usageThe enum values match exactly across frontend and backend:
- Backend:
NotificationTypeenum defined inapi/src/unraid-api/graph/resolvers/notifications/notifications.model.tswithUNREAD = 'UNREAD'andARCHIVE = 'ARCHIVE'- Frontend: Generated from GraphQL schema in
web/src/composables/gql/graphql.tswith identical values- The
startDeleteAllresolver inapi/src/unraid-api/graph/resolvers/notifications/notifications.mutations.resolver.tsacceptstype?: NotificationType, and both function calls pass the correct enum members- Job state tracking with distinct keys (
deleteArchived,deleteUnread) aligns with the implementations
1-248: Request manual verification of Prettier formattingThe sandbox environment lacks the necessary dependencies to run Prettier, so I cannot verify the formatting issue. Please run the formatter locally in your development environment using one of these commands:
pnpm prettier --write web/src/components/Notifications/Sidebar.vueThen commit any formatting changes that result. This will ensure your code complies with the project's Prettier configuration before merging.
api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts
Outdated
Show resolved
Hide resolved
| import { | ||
| NotificationImportance, | ||
| NotificationJob, | ||
| NotificationJobState, | ||
| NotificationMutations, | ||
| NotificationOverview, | ||
| NotificationType, | ||
| } from '@app/unraid-api/graph/resolvers/notifications/notifications.model.js'; |
There was a problem hiding this comment.
Fix NotificationMutations import to resolve CI failure
CI error shows NotificationMutations is not exported from notifications.model.ts, and in this file it is imported from that module. The type is actually defined in mutation.model.ts, so this import is invalid and breaks the build.
Update the imports to pull NotificationMutations from the correct module:
-import {
- NotificationImportance,
- NotificationJob,
- NotificationJobState,
- NotificationMutations,
- NotificationOverview,
- NotificationType,
-} from '@app/unraid-api/graph/resolvers/notifications/notifications.model.js';
+import {
+ NotificationImportance,
+ NotificationJob,
+ NotificationJobState,
+ NotificationOverview,
+ NotificationType,
+} from '@app/unraid-api/graph/resolvers/notifications/notifications.model.js';
+import { NotificationMutations } from '@app/unraid-api/graph/resolvers/mutation/mutation.model.js';🧰 Tools
🪛 GitHub Actions: CI - Main (API)
[error] 12-12: "NotificationMutations" is not exported by "src/unraid-api/graph/resolvers/notifications/notifications.model.ts", imported by "src/unraid-api/graph/resolvers/notifications/notifications.mutations.resolver.ts".
🤖 Prompt for AI Agents
In
api/src/unraid-api/graph/resolvers/notifications/notifications.mutations.resolver.ts
around lines 8 to 15, the import list incorrectly pulls NotificationMutations
from notifications.model.js (which doesn't export it) causing CI to fail; remove
NotificationMutations from the import statement here and add an import of
NotificationMutations from the correct module (mutation.model.ts) using the
appropriate relative path, then run type-check/build to verify the error is
resolved.
| export const deleteNotification = graphql(/* GraphQL */ ` | ||
| mutation DeleteNotification($id: PrefixedID!, $type: NotificationType!) { | ||
| notifications { | ||
| delete(id: $id, type: $type) { | ||
| archive { | ||
| total | ||
| } | ||
| unread { | ||
| total | ||
| } | ||
| } | ||
| } | ||
| } | ||
| `); |
There was a problem hiding this comment.
🧩 Analysis chain
Ensure deleteNotification matches new schema and all call sites
This mutation now goes through the nested notifications { delete(...) } field and only fetches archive/unread totals. That’s fine as long as:
- The GraphQL schema actually exposes
Mutation.notifications.delete(id, type)with an object that hasarchive.totalandunread.total. - All TS types and call sites that previously expected a flat
deleteNotificationpayload (e.g., aNotificationFragmentor different shape) have been updated.
Please double-check codegen output and usages of deleteNotification in the web app to ensure there are no runtime GraphQL errors or type mismatches.
🏁 Script executed:
#!/bin/bash
# Inspect all usages of the deleteNotification mutation to ensure they match the new shape.
rg -n "deleteNotification\(" web -S -C3Length of output: 9084
I need to inspect the actual source file to verify the current state of the code.The web search didn't provide specific information about the file in the repository. Let me directly inspect the file to check the actual code.
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Directly inspect the file mentioned in the review comment
cat "web/src/components/Notifications/graphql/notification.query.ts" | head -100Length of output: 2090
I've found a critical discrepancy. The source mutation definition uses notifications { delete(...) } with both archive.total and unread.total, but the generated codegen in web/src/composables/gql/gql.ts shows a flat structure with only archive.total. Let me find where this mutation is actually called to assess the impact.
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Find actual usage of deleteNotification in Vue/TypeScript components and composables
rg -n "deleteNotification" web/src -A5 -B5 --type ts --type vue -g '!gql.ts' | head -150Length of output: 27
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Search for deleteNotification usage excluding the codegen file, broader search
rg -n "deleteNotification" web/src -A3 -B3 --type ts -g '!gql.ts' 2>/dev/null || rg -n "deleteNotification" web/src -A3 -B3 -g '!gql.ts' -g '!*.d.ts'Length of output: 11568
Now let me find where this mutation is actually called in components:
🏁 Script executed:
#!/bin/bash
# Search for where deleteNotification is used in Vue/component files
rg -n "deleteNotification" web/src/components -A5 -B5Length of output: 4219
Let me see the complete source mutation definition:
🏁 Script executed:
#!/bin/bash
# Get the complete deleteNotification mutation from source file
sed -n '56,70p' "web/src/components/Notifications/graphql/notification.query.ts"Length of output: 300
Regenerate codegen to match the updated mutation schema
The source mutation in web/src/components/Notifications/graphql/notification.query.ts (lines 56–69) has been updated to use a nested notifications { delete(...) } structure with both archive.total and unread.total fields. However, the generated TypeScript types in web/src/composables/gql/graphql.ts still reflect an older flat structure with only archive.total. This type mismatch will cause runtime GraphQL errors when the component in web/src/components/Notifications/Item.vue executes the mutation.
The codegen must be regenerated to produce types that match the actual GraphQL source mutation, and the Apollo cache handler at web/src/helpers/apollo-cache/index.ts:176 should be reviewed to ensure it properly handles the updated response structure.
| import ConfirmDialog from '~/components/ConfirmDialog.vue'; | ||
| import { | ||
| archiveAllNotifications, | ||
| deleteArchivedNotifications, | ||
| startDeleteNotifications, | ||
| NOTIFICATION_FRAGMENT, | ||
| NOTIFICATION_JOB_FRAGMENT, | ||
| notificationsOverview, | ||
| notificationJobStatus, | ||
| resetOverview, | ||
| } from '~/components/Notifications/graphql/notification.query'; |
There was a problem hiding this comment.
🧩 Analysis chain
GraphQL and Apollo imports align with new job-based flow
Pulling in startDeleteNotifications, NOTIFICATION_JOB_FRAGMENT, and notificationJobStatus plus NotificationImportance, NotificationJobState, NotificationType, and useApolloClient matches how the component now manages background jobs and polling. Import structure looks coherent.
🌐 Web query:
Confirm that the generated GraphQL typings expose `NotificationJobState`, `NotificationType`, and the new NotificationJob-related operations used here.
💡 Result:
I don’t have the generated typings or the code you’re referring to. Please either
- paste the generated typings (or the file path / repo link), or
- paste the GraphQL schema and the client-side code that uses NotificationJob-related operations,
and I’ll confirm whether NotificationJobState, NotificationType, and the new operations are exposed and match the usages.
I need to search the codebase for the GraphQL schema and generated typings to verify the exports. Let me investigate.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find GraphQL query files and typings in the notifications directory
find . -path "*/Notifications/*" -type f \( -name "*.query.*" -o -name "*.graphql*" -o -name "*.gql" -o -name "*types*" \) 2>/dev/null | head -20
Length of output: 64
🏁 Script executed:
#!/bin/bash
# Search for the notification.query file and related typings
fd -e ts -e js -e graphql "notification" --type f | grep -i "query\|type\|schema" | head -20Length of output: 104
🏁 Script executed:
#!/bin/bash
# Look for generated GraphQL types directory
fd -t d "generated\|__generated__\|types" --max-depth 3 | head -20Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine the notification.query.ts file to see actual exports
cat ./web/src/components/Notifications/graphql/notification.query.ts
Length of output: 2689
🏁 Script executed:
#!/bin/bash
# Look for TypeScript declaration files (.d.ts) and type definitions
fd -e "d.ts" --type f | head -20Length of output: 297
🏁 Script executed:
#!/bin/bash
# Search for NotificationJobState and NotificationType definitions in the codebase
rg -n "NotificationJobState|NotificationType" --type ts --type js -B2 -A2 | head -40Length of output: 4459
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for NotificationJobState with case-insensitive matching
rg -in "NotificationJobState" --type ts --type js -B1 -A1
Length of output: 5018
🏁 Script executed:
#!/bin/bash
# Search for NotificationImportance
rg -n "NotificationImportance" --type ts --type js -B1 -A1 | head -30Length of output: 1966
🏁 Script executed:
#!/bin/bash
# Check what types are exported from graphql.ts
grep -n "^export" ./web/composables/gql/graphql.ts | head -50Length of output: 2267
🏁 Script executed:
#!/bin/bash
# Look for the state field type in NOTIFICATION_JOB_FRAGMENT context
rg -n "NotificationJob\s*{" ./web/composables/gql/graphql.ts -A 10Length of output: 101
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for NotificationJobState in the generated graphql.ts file
grep -n "NotificationJobState" ./web/composables/gql/graphql.ts | head -20
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if useApolloClient is available (may be from Vue Apollo or local composable)
rg -n "useApolloClient" --type ts --type js | head -10Length of output: 556
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the exact import in Sidebar.vue
sed -n '40,50p' ./web/src/components/Notifications/Sidebar.vue
Length of output: 482
🏁 Script executed:
#!/bin/bash
# Search for any enum ending with State in graphql.ts
grep -n "export enum.*State" ./web/composables/gql/graphql.tsLength of output: 234
🏁 Script executed:
#!/bin/bash
# Look for NotificationJobState anywhere in web/src and composables directories
find ./web -name "*.ts" -o -name "*.vue" | xargs grep -l "export.*NotificationJobState" 2>/dev/nullLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check graphql codegen configuration
find . -maxdepth 2 -type f \( -name "codegen.yml" -o -name "codegen.yaml" -o -name "codegen.ts" \) 2>/dev/nullLength of output: 33
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the web codegen configuration
cat ./web/codegen.ts
Length of output: 834
🏁 Script executed:
#!/bin/bash
# Look for any reference to NotificationJobState in schema files
find . -name "*.graphql" -o -name "*.gql" | xargs grep -l "NotificationJobState" 2>/dev/nullLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check the tail of graphql.ts to see if NotificationJobState is at the end
tail -100 ./web/composables/gql/graphql.ts | grep -A5 -B5 "Notification"Length of output: 29228
---
🏁 Script executed:
```shell
#!/bin/bash
# Check if the generated schema file exists and contains NotificationJobState
if [ -f ./api/generated-schema.graphql ]; then
grep -n "NotificationJobState" ./api/generated-schema.graphql
else
echo "File not found"
fi
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for any graphql schema files
find . -name "*.graphql" -type f | head -20Length of output: 75
🏁 Script executed:
#!/bin/bash
# Check if there's a schema or sdl directory
find . -type d -name "schema" -o -type d -name "sdl" -o -type d -name "graphql" | head -10Length of output: 239
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the size and contents of the generated schema to see if it has notification types
wc -l ./api/generated-schema.graphql
echo "---"
grep -i "notification" ./api/generated-schema.graphql | head -20
Length of output: 737
🏁 Script executed:
#!/bin/bash
# Check what NotificationJobState references exist across the codebase
rg "NotificationJobState" --type ts --type vue -lLength of output: 27
🏁 Script executed:
#!/bin/bash
# Get exact line from Sidebar.vue showing how NotificationJobState is used
sed -n '40,80p' ./web/src/components/Notifications/Sidebar.vueLength of output: 1501
Missing GraphQL type definition for NotificationJobState
The import of NotificationJobState will fail: it doesn't exist in web/composables/gql/graphql.ts because the backend GraphQL schema (api/generated-schema.graphql) doesn't expose it. While the backend defines NotificationJobState in the API layer, it hasn't been registered in the GraphQL schema or queries. The component uses it at lines 64 and 71 but the type is unavailable.
Verify that:
NotificationJobStateenum is registered and exposed in the backend GraphQL schema- The schema is regenerated and graphql.ts is updated to include the new types
🤖 Prompt for AI Agents
In web/src/components/Notifications/Sidebar.vue around lines 24 to 33,
Sidebar.vue imports and uses NotificationJobState but the backend GraphQL schema
does not expose that enum so graphql.ts has no generated type; fix by
adding/registering the NotificationJobState enum to the backend GraphQL schema
(ensure the enum is exported/declared in api/generated-schema.graphql or the
server schema definitions), re-run the schema generation and GraphQL Code
Generator to regenerate web/composables/gql/graphql.ts so NotificationJobState
is present, then update the import in Sidebar.vue if the generated module path
changed.
| const stopPolling = (key: keyof typeof jobPollers) => { | ||
| const interval = jobPollers[key]; | ||
| if (interval) { | ||
| clearInterval(interval); | ||
| jobPollers[key] = null; | ||
| } | ||
| }; | ||
| const setJob = (key: keyof typeof jobPollers, job?: unknown) => { | ||
| const parsed = job ? useFragment(NOTIFICATION_JOB_FRAGMENT, job) : null; | ||
| activeJobs[key] = parsed; | ||
| if (!parsed || !activeStates.includes(parsed.state)) { | ||
| stopPolling(key); | ||
| } | ||
| }; | ||
| const pollJob = (key: keyof typeof jobPollers) => { | ||
| const job = activeJobs[key]; | ||
| if (!job) return; | ||
| stopPolling(key); | ||
| if (!activeStates.includes(job.state)) return; | ||
| jobPollers[key] = setInterval(async () => { | ||
| const { data } = await client.query({ | ||
| query: notificationJobStatus, | ||
| variables: { id: job.id }, | ||
| fetchPolicy: 'network-only', | ||
| }); | ||
| const updated = data?.notifications.job; | ||
| if (updated) { | ||
| setJob(key, updated); | ||
| } | ||
| }, 750); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Strengthen polling robustness: handle errors and missing jobs
The polling helpers mostly look solid, but there are a couple of edge cases worth tightening:
- The
setIntervalcallback isasyncand any rejectedclient.querywill surface as an unhandled promise rejection. That can spam the console or trigger global handlers. - If the server ever starts returning
job: null(e.g., job expired/purged),updatedwill be falsy,setJobwon’t be called, and the interval keeps running indefinitely against a missing job.
Consider something like:
- jobPollers[key] = setInterval(async () => {
- const { data } = await client.query({
+ jobPollers[key] = setInterval(async () => {
+ try {
+ const { data } = await client.query({
query: notificationJobStatus,
variables: { id: job.id },
fetchPolicy: 'network-only',
- });
- const updated = data?.notifications.job;
- if (updated) {
- setJob(key, updated);
- }
+ });
+ const updated = data?.notifications.job;
+ // Treat a missing job as terminal to avoid polling forever
+ setJob(key, updated ?? undefined);
+ } catch {
+ // Optional: stop polling on repeated errors, or surface a toast
+ stopPolling(key);
+ }
}, 750);This keeps the interval from running forever on a missing job and prevents unhandled rejections.
🌐 Web query:
Check how Apollo Client surfaces network and GraphQL errors in `client.query` for the version used here, to confirm whether unhandled rejections may be thrown from this polling loop.
💡 Result:
Short answer: Yes — client.query returns a Promise that will reject on errors (network errors and, with the default errorPolicy "none", GraphQL errors), so if your polling loop calls client.query and you don't catch/reject-handler those promises you can get unhandled promise rejections. [1][2]
Key points and options:
- Promise-based APIs (client.query / client.mutate) either reject or resolve depending on errorPolicy. With errorPolicy: "none" (the default) the promise rejects for errors; with errorPolicy: "all" the promise resolves and the returned result contains an error field. [1][2]
- Network errors always cause the promise to reject. [1][3]
- Apollo notes that aborted queries are handled so they don't produce an unhandled rejection if you don't attach a handler, but that does not apply to ordinary network/GraphQL errors — you must handle them yourself (try/catch or .catch). [2]
Practical fixes:
- Add .catch(...) or try/catch around every client.query used by the polling loop; or
- Use errorPolicy: "all" and check result.error instead of relying on promise rejection.
Sources:
[1] Apollo Client — Handling operation errors / Error behavior for promise-based APIs.
[2] Apollo Client — Queries docs (client.query behavior and error handling).
[3] Apollo Client migration notes (network errors always reject).
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the Sidebar.vue file and show the polling code section
fd -type f -name "Sidebar.vue" --path "*/Notifications/*" | head -5
Length of output: 170
🏁 Script executed:
#!/bin/bash
# Examine lines 73-107 of the Sidebar.vue file to see current state
find . -type f -name "Sidebar.vue" -path "*/Notifications/*" -exec sed -n '73,107p' {} +Length of output: 916
🏁 Script executed:
#!/bin/bash
# Also check the entire polling-related functions in Sidebar.vue
find . -type f -name "Sidebar.vue" -path "*/Notifications/*" -exec sed -n '50,150p' {} +Length of output: 3299
🏁 Script executed:
#!/bin/bash
# Check package.json to see which Apollo Client version is used
fd -type f -name "package.json" | head -1 | xargs grep -A2 -B2 "@apollo/client" || echo "Could not find @apollo/client version"Length of output: 208
🏁 Script executed:
#!/bin/bash
# Search for all uses of setInterval with async callbacks in the codebase to understand pattern
rg -n "setInterval\s*\(\s*async" --type vue --type ts --type js | head -20Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check if try/catch is already around client.query in the file
grep -n "try\|catch" web/src/components/Notifications/Sidebar.vue 2>/dev/null || echo "File not found or no try/catch blocks"Length of output: 37
🏁 Script executed:
#!/bin/bash
# Alternative: just show the whole Sidebar.vue file to assess state
cat "web/src/components/Notifications/Sidebar.vue" 2>/dev/null | head -200Length of output: 6643
🏁 Script executed:
#!/bin/bash
# Look for jobPollers in the file
rg -A 30 "jobPollers\[key\]\s*=\s*setInterval" web/src/components/Notifications/Sidebar.vueLength of output: 787
🏁 Script executed:
#!/bin/bash
# Simple web search to understand the latest Apollo Client behavior docs
echo "Checking Apollo Client error handling patterns..."Length of output: 49
Add error handling to polling loop to prevent unhandled rejections and infinite polling on missing jobs
The polling helpers have two edge cases that need tightening:
- The
setIntervalcallback isasyncbut does not catch errors fromclient.query(). Apollo Client rejects promises on network errors and GraphQL errors (with default errorPolicy), so any rejection will surface as an unhandled promise rejection and may spam the console or trigger global error handlers. - If the server returns
job: null(e.g., job expired/purged),updatedwill be falsy,setJobwon't be called, and the interval will run indefinitely against a missing job.
Wrap the query in try/catch and call stopPolling(key) on error; also treat a missing job response as terminal:
jobPollers[key] = setInterval(async () => {
+ try {
const { data } = await client.query({
query: notificationJobStatus,
variables: { id: job.id },
fetchPolicy: 'network-only',
});
const updated = data?.notifications.job;
- if (updated) {
- setJob(key, updated);
- }
+ setJob(key, updated ?? undefined);
+ } catch {
+ stopPolling(key);
+ }
}, 750);🤖 Prompt for AI Agents
In web/src/components/Notifications/Sidebar.vue around lines 73 to 107, the
polling callback can cause unhandled promise rejections and may poll forever
when the server returns job: null; wrap the async setInterval callback in a
try/catch, call stopPolling(key) in the catch (and optionally log the error with
context), and also treat a falsy updated response as terminal by calling
stopPolling(key) before returning instead of leaving the interval running.
| "notifications.sidebar.confirmDeleteUnread.description": "Are you sure you want to permanently delete all unread notifications? This cannot be undone.", | ||
| "notifications.sidebar.confirmDeleteUnread.title": "Delete unread notifications", | ||
| "notifications.sidebar.deleteAllAction": "Delete All", | ||
| "notifications.sidebar.processingStatus": "Processing {{processed}} / {{total}}", |
There was a problem hiding this comment.
Fix interpolation syntax in processingStatus
vue-i18n expects single-brace placeholders (e.g. {processed}), but this string uses {{processed}} / {{total}}, which will likely render the braces literally instead of substituting values.
Recommend:
- "notifications.sidebar.processingStatus": "Processing {{processed}} / {{total}}",
+ "notifications.sidebar.processingStatus": "Processing {processed} / {total}",🤖 Prompt for AI Agents
In web/src/locales/en.json around line 311 the translation uses double-brace
interpolation "Processing {{processed}} / {{total}}", but vue-i18n expects
single-brace placeholders; update the string to use single braces "Processing
{processed} / {total}" and confirm any calls to $t/$tc pass named params {
processed, total } so values are substituted correctly.
…e unused import in notifications model
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
web/src/components/Notifications/Sidebar.vue (2)
2-2: Imports align with the new job-based polling implementation.The additions support:
- Lifecycle cleanup with
onBeforeUnmount- Manual GraphQL queries via
useApolloClient- Job fragments and status queries
- Type safety with enum imports
Note: The past review comment about
NotificationJobStatebeing missing from generated types should be addressed first.Also applies to: 4-4, 24-46
100-118: Polling implementation needs error handling as flagged in previous review.The previous review comment correctly identifies that the polling loop lacks error handling and can run indefinitely if the server returns
job: null. The suggested fix adds try-catch for error handling and treats missing jobs as terminal.
🧹 Nitpick comments (2)
api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts (2)
17-22: Consider adding a CANCELLED state for user-initiated job termination.The current state machine covers the happy path and failure scenarios, but lacks support for user-initiated cancellation. If users can cancel long-running archive/delete operations, adding a
CANCELLEDstate would improve the state model completeness.Would you like me to propose the addition of a CANCELLED state and the corresponding cancellation mutation?
192-211: Consider adding validation decorators for consistency with other ObjectTypes.While validation decorators are not strictly required for output types (ObjectType), other classes in this file (
Notification,NotificationCounts, etc.) include them (@IsEnum,@IsInt,@IsString, etc.). Adding them toNotificationJobwould maintain consistency and provide an extra layer of type safety.Apply this diff to add validation decorators:
@ObjectType() export class NotificationJob { @Field(() => ID) + @IsString() + @IsNotEmpty() id!: string; @Field(() => NotificationJobOperation) + @IsEnum(NotificationJobOperation) + @IsNotEmpty() operation!: NotificationJobOperation; @Field(() => NotificationJobState) + @IsEnum(NotificationJobState) + @IsNotEmpty() state!: NotificationJobState; @Field(() => Int) + @IsInt() + @Min(0) + @IsNotEmpty() processed!: number; @Field(() => Int) + @IsInt() + @Min(0) + @IsNotEmpty() total!: number; @Field({ nullable: true }) + @IsString() + @IsOptional() error?: string | null; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts(4 hunks)web/src/components/Notifications/Sidebar.vue(7 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
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-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/mutation/mutation.resolver.ts
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
Repo: unraid/api PR: 942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.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/mutation/mutation.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.model.ts
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
Repo: unraid/api PR: 1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Applied to files:
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.model.tsweb/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-11-05T14:48:56.949Z
Learnt from: elibosley
Repo: unraid/api PR: 942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:41-47
Timestamp: 2024-11-05T14:48:56.949Z
Learning: Input types for GraphQL mutations can be pulled from the GraphQL codegen types, as they are generated into the API files. Therefore, explicit typing in the resolver files may be unnecessary.
Applied to files:
api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts
📚 Learning: 2024-11-01T17:52:55.311Z
Learnt from: pujitm
Repo: unraid/api PR: 941
File: web/components/Notifications/Item.vue:0-0
Timestamp: 2024-11-01T17:52:55.311Z
Learning: In the `web/components/Notifications/Item.vue` component, ensure that when delete or archive mutations fail, the notification remains displayed, and an error message is shown below the action buttons to inform the user.
Applied to files:
web/src/components/Notifications/Sidebar.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/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: 2024-12-09T15:45:46.492Z
Learnt from: pujitm
Repo: unraid/api PR: 975
File: web/components/Notifications/TabList.vue:1-4
Timestamp: 2024-12-09T15:45:46.492Z
Learning: In our Nuxt.js setup for the `web` project, it's not necessary to explicitly import `computed` from `vue` in Vue components, as it's globally available.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-09T15:47:29.325Z
Learnt from: pujitm
Repo: unraid/api PR: 975
File: web/components/Notifications/TabList.vue:1-24
Timestamp: 2024-12-09T15:47:29.325Z
Learning: In our Nuxt setup using Vue.js, components defined within the codebase are autoloaded and do not require explicit import statements in the script section. For example, components like `TabsList` and `TabsTrigger` used in `web/components/Notifications/TabList.vue` are automatically available without imports.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2025-06-13T17:14:21.739Z
Learnt from: pujitm
Repo: unraid/api PR: 1417
File: web/components/ConnectSettings/ConnectSettings.ce.vue:11-18
Timestamp: 2025-06-13T17:14:21.739Z
Learning: The project’s build tooling auto-imports common Vue/Pinia helpers such as `storeToRefs`, so explicit import statements for them are not required.
Applied to files:
web/src/components/Notifications/Sidebar.vue
📚 Learning: 2024-12-17T13:55:42.068Z
Learnt from: elibosley
Repo: unraid/api PR: 972
File: web/components/ColorSwitcher.ce.vue:1-2
Timestamp: 2024-12-17T13:55:42.068Z
Learning: In this Nuxt.js project, components used in templates are automatically imported by Nuxt, so explicit import statements for components are unnecessary.
Applied to files:
web/src/components/Notifications/Sidebar.vue
🧬 Code graph analysis (1)
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts (1)
web/composables/gql/graphql.ts (1)
Mutation(1199-1239)
🔇 Additional comments (18)
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts (2)
7-7: LGTM!The import follows the established pattern and is correctly placed alphabetically within the imports block.
46-49: LGTM!The implementation correctly follows the established nested mutation pattern used throughout this resolver. The
notifications()method appropriately creates a namespace for notification-related mutations, consistent with howarray,docker,vm, and other domain mutations are structured.api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts (4)
1-1: LGTM - ID import supports the new job identifier field.The addition of
IDto the imports is necessary for typing theidfield in theNotificationJobclass.
24-27: LGTM - Operation enum covers the job types introduced in this PR.The enum values align with the archive and delete operations mentioned in the PR objectives.
38-44: LGTM - Enum registrations follow the established pattern.The
registerEnumTypecalls properly expose the new enums to the GraphQL schema.
188-189: LGTM - Optional job field enables status queries through the notifications resolver.Making the field nullable is appropriate since a job may not be active at query time.
web/src/components/Notifications/Sidebar.vue (12)
49-51: LGTM - Mutation bindings and Apollo client setup support the job-based workflow.The renamed
startArchiveAllJobmutation and newstartDeleteAllmutation align with the background job pattern. The Apollo client instance enables manual polling queries.
56-80: LGTM - Job tracking state is well-structured and type-safe.The separate tracking of polling intervals (
jobPollers) and job state (activeJobs) with explicit keys for each operation provides clear separation of concerns and type safety.
82-82: LGTM - Active states correctly identify non-terminal job states.Limiting active states to
QUEUEDandRUNNINGensures polling stops once jobs reach terminal states (SUCCEEDEDorFAILED).
84-90: LGTM - Polling cleanup is correctly implemented.The function properly clears the interval and resets the reference to
null.
92-98: LGTM - Job state management includes automatic polling cleanup.The function correctly handles job updates and automatically stops polling when the job reaches a terminal state or is missing.
120-123: LGTM - Job activity check is straightforward and correct.The helper correctly determines if a job is in progress by checking its state against the active states array.
125-134: LGTM - Dynamic job labels enhance user experience with progress feedback.The helper provides real-time progress updates when jobs are active and falls back to static labels otherwise.
145-160: LGTM - Archive operation properly initiates job tracking after confirmation.The function follows a good pattern: confirm → mutate → track job → start polling.
162-177: LGTM - Delete archives operation follows the established job pattern.The function correctly uses
NotificationType.ARCHIVEto target archived notifications and initiates proper job tracking.
179-194: LGTM - Unread deletion feature follows the consistent job-based pattern.The new function extends deletion capabilities to unread notifications with proper job tracking.
257-259: LGTM - Lifecycle cleanup prevents resource leaks.The
onBeforeUnmounthook properly clears all active polling intervals when the component is destroyed.
300-327: LGTM - UI properly integrates job state with button states and labels.The template changes correctly:
- Disable buttons during mutations and active jobs
- Display dynamic progress labels via
jobLabel- Maintain separate job tracking for archive and delete operations
Summary
Testing
Codex Task
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.