feat(api): restructure versioning information in GraphQL schema #1600
Conversation
- Introduced new CoreVersions and PackageVersions types to encapsulate core system and software package version details. - Updated InfoVersions type to include references to CoreVersions and PackageVersions. - Modified the versions resolver to return structured version information. - Added a new query for fetching version information in the frontend. - Updated the HeaderOsVersion component to display the new version information. - Created a ReleaseNotesModal component for viewing OS release notes.
WalkthroughRefactors InfoVersions into nested CoreVersions and PackageVersions, adds VersionsResolver and CoreVersionsResolver, simplifies VersionsService to return a stub id, updates resolver signatures and tests, adds frontend InfoVersions query, header dropdown + ReleaseNotes modal, bumps API config version, and tweaks notifications default. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client as GraphQL Client
participant Query as Query.info
participant IR as InfoResolver
participant VR as VersionsResolver
participant CR as CoreVersionsResolver
participant SI as systeminformation
Client->>Query: query info { versions { id core { unraid api kernel } packages { ... } } }
Query->>IR: resolve info
IR->>VR: versions() -> { id: "info/versions" } %% stub returned synchronously
VR->>VR: ResolveField core() -> read config for unraid/api (kernel deferred)
VR->>SI: ResolveField packages() -> versions()
SI-->>VR: software versions map
VR-->>Client: packages fields resolved
CR->>SI: kernel() field resolver -> versions()
SI-->>CR: { kernel }
CR-->>Client: kernel field resolved
sequenceDiagram
autonumber
actor User
participant Header as HeaderOsVersion.ce.vue
participant GQL as useQuery(INFO_VERSIONS_QUERY)
participant Modal as ReleaseNotesModal
User->>Header: Open header menu
Header->>GQL: Fetch info { versions { core { unraid api } } }
GQL-->>Header: Data or fallback
Header-->>User: Dropdown shows OS and API versions
User->>Header: Click "View OS Release Notes"
Header->>Modal: open(version)
Modal-->>User: Iframe with release notes (loads)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Claude finished @elibosley's task —— View job Code Review - Critical Issues FoundReviewing ONLY the diff/changeset for critical issues:
🚨 CRITICAL ISSUES FOUND:
Recommendations:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/composables/api/use-notifications.ts (1)
74-80: Bug: “No unread notifications” case leaves haveSeenNotifications stuck at falseWith the new default
falseand the early return whenlatestNotificationTimestampis unset, users with zero unread notifications will never havehaveSeenNotificationsset totrue. Previously this may have been ambiguous (null); now it becomes an incorrect persisted false.Derive
trueexplicitly when the unread list is empty, and remove the console log.Apply this diff:
// derive haveSeenNotifications by comparing last seen's timestamp to latest's timestamp watchEffect(() => { - if (!latestNotificationTimestamp.value) { - return; - } - haveSeenNotifications.value = isBeforeLastSeen(latestNotificationTimestamp.value); - console.log('[use-notifications] set haveSeenNotifications to', haveSeenNotifications.value); + const list = latestNotifications.value?.notifications.list; + if (Array.isArray(list) && list.length === 0) { + // No unread notifications -> user has seen everything + haveSeenNotifications.value = true; + return; + } + if (!latestNotificationTimestamp.value) { + // Data not ready yet; wait for timestamp derivation + return; + } + haveSeenNotifications.value = isBeforeLastSeen(latestNotificationTimestamp.value); });api/generated-schema.graphql (1)
1049-1104: Regenerate generated GraphQL clients/types to update InfoVersions shapeThe generated definitions for InfoVersions in multiple clients still use the old flat fields (e.g. api, kernel, openssl) instead of the new nested
core/packagesstructure. This will break any consumers relying on the updated schema:• web/composables/gql/graphql.ts (L1048–1052):
export type InfoVersionsis still flat and doesn’t include acorefield.
• api/src/unraid-api/cli/generated/graphql.ts (L1038–1042):
export type InfoVersionsuses the old flat version fields.
• web/composables/gql/gql.ts (L90–92):
TheInfoVersionsquery selection only requestscore.unraid/core.api, but the generated document still references the old structure.
• api/src/unraid-api/cli/generated/graphql.ts (L2558–2560):
SystemReportQuerystill selectskernelandopensslat the top level under versions.Action: run your GraphQL codegen (CLI or npm script) to regenerate all clients/types so that
InfoVersionsreflects the new nestedcoreandpackagesshape.
🧹 Nitpick comments (16)
web/composables/api/use-notifications.ts (3)
42-42: Defaulting tofalseis fine, but it won’t coerce existing localStoragenullvalues
useStorageonly uses the default when the key is missing. If existing users have"null"persisted,JSON.parsewill returnnull, so the ref can still start asnullat runtime. Make sure you migrate legacy values to a boolean at least once (see derivation fix below) or perform a one-time migration on init.Would you like me to draft a small migration snippet that normalizes the stored value at app bootstrap?
63-66: Differentiate “not loaded yet” vs “no unread” with an explicit null sentinelInitialize the ref to
nullso you can distinguish “unknown/not loaded” from “empty/no unread” (which sets the ref toundefinedvia the watch). This complements the fix above and prevents premature derivation.Apply this diff:
- const latestNotificationTimestamp = ref<string | null>(); + const latestNotificationTimestamp = ref<string | null>(null);
4-9: ESM import specifiers: add .js extensions for TS source filesPer the coding guidelines, TypeScript source files should use .js extensions in import specifiers for ESM compatibility (for internal modules).
Apply this diff:
import { getNotifications, NOTIFICATION_FRAGMENT, -} from '~/components/Notifications/graphql/notification.query'; -import { useFragment } from '~/composables/gql/fragment-masking'; -import { NotificationType } from '../gql/graphql'; +} from '~/components/Notifications/graphql/notification.query.js'; +import { useFragment } from '~/composables/gql/fragment-masking.js'; +import { NotificationType } from '../gql/graphql.js';api/src/unraid-api/graph/resolvers/info/versions/get-api-version.ts (2)
12-13: Consider resolving package.json relative to this file, not process.cwd()process.cwd() can differ depending on how the process is launched. Resolving package.json using the module location (e.g., via import.meta.url or by walking up directories) is more robust.
If
resolveJsonModuleis enabled, you can simplify to a static import and avoid IO:// tsconfig: "resolveJsonModule": true, "esModuleInterop": true import pkg from '../../../../../../package.json' assert { type: 'json' }; export function getApiVersion(): string { return typeof pkg.version === 'string' && pkg.version.trim() ? pkg.version : 'unknown'; }Or use a safe “find nearest package.json” approach (e.g., pkg-up) and cache the result once at startup.
17-17: Prefer a consistent logger over console.errorThe codebase appears to use a structured logger (e.g., LogService/Nest Logger). Using console.error here bypasses centralized logging and log levels.
If possible, route this error through the shared logger; otherwise, consider a TODO linking a follow-up to integrate logging here.
api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts (1)
168-173: Test expectation update is correct; consider adding coverage for nested resolversSwitching the assertion to treat versions() as a stub aligns with field-level resolvers for core/packages. To catch regressions, add tests for CoreVersionsResolver and VersionsResolver ensuring non-null objects and expected fields are resolved.
I can sketch an integration-style test that queries info { versions { core { unraid api } packages { docker } } } if helpful.
web/components/UserProfile/versions.query.ts (1)
7-17: Trim unused fields from the query to reduce payloadIf you don’t use
info.osin consumers of this query, consider removing it to minimize network and cache footprint.Example:
query InfoVersions { info { id - os { - id - hostname - } versions { id core { unraid api } } } }web/components/HeaderOsVersion.ce.vue (3)
39-41: Harden window.open against reverse tabnabbingPass the noopener/noreferrer features when opening new tabs.
- window.open('https://github.com/unraid/api/releases', '_blank'); + window.open('https://github.com/unraid/api/releases', '_blank', 'noopener,noreferrer');
35-37: Ensure a visible trigger label when OS version is unavailableIf
displayOsVersionis null initially, the trigger renders empty text. Show a friendly fallback (you already use “Unknown” in the menu).- {{ displayOsVersion }} + {{ displayOsVersion || t('Unknown') }}Alternatively, compute a
displayOsVersionTextand reuse it in both places.Also applies to: 121-121
29-35: Optional: remove obvious commentsThe comments “Query for version information” / “Use versions endpoint as primary source” repeat what the code says; consider trimming to reduce noise per guidelines.
web/components/ReleaseNotesModal.vue (3)
69-69: Tighten iframe sandbox permissionsDropping
allow-same-originreduces risk when combined withallow-scripts, since that combo gives the framed page full same-origin capabilities. Docs likely don’t need same-origin privileges.- sandbox="allow-scripts allow-same-origin" + sandbox="allow-scripts"
48-48: Localize user-visible strings with t()You’re already passing
tinto the modal; localize these strings.- :title="`Unraid OS ${version} Release Notes`" + :title="t('Unraid OS {0} Release Notes', [version])" @@ - <p>Loading release notes…</p> + <p>{{ t('Loading release notes…') }}</p> @@ - Open in New Tab + {{ t('Open in New Tab') }}Also applies to: 60-60, 88-88
20-21: Remove unused iframeRef
iframeRefis declared but unused; remove it and the corresponding ref attribute to keep things tidy.-const iframeRef = ref<HTMLIFrameElement | null>(null);And:
- ref="iframeRef"Also applies to: 66-66
api/src/unraid-api/graph/resolvers/info/versions/core-versions.resolver.ts (1)
9-13: Cache systeminformation.versions() per request to avoid duplicate work and add a safe fallback.Kernel and packages resolvers both call versions(); cache it in the GraphQL context and handle failures gracefully.
Apply this diff within the selected lines:
- async kernel(): Promise<string | undefined> { - const softwareVersions = await versions(); - return softwareVersions.kernel; - } + async kernel(@Context() ctx: any): Promise<string | undefined> { + try { + const softwareVersions = ctx.softwareVersions ??= await versions(); + return softwareVersions.kernel; + } catch { + return undefined; + } + }And outside the selected range, update imports:
// at line 1 import { Context, ResolveField, Resolver } from '@nestjs/graphql';api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts (2)
21-25: Nit: no need to set kernel: undefined.Field resolvers will handle kernel on CoreVersions; omitting it reduces noise.
return { unraid, api, - kernel: undefined, // Will be resolved separately if requested };
47-48: Tiny type-safety improvement: avoid any for apache.Use a generic Record cast to limit the surface area of any.
- apache: (softwareVersions as any).apache, + apache: (softwareVersions as Record<string, string | undefined>).apache,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**
📒 Files selected for processing (16)
api/dev/configs/api.json(1 hunks)api/generated-schema.graphql(2 hunks)api/src/unraid-api/graph/resolvers/info/info.module.ts(3 hunks)api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/versions/core-versions.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/versions/get-api-version.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/versions/versions.model.ts(2 hunks)api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/versions/versions.service.ts(1 hunks)web/components/HeaderOsVersion.ce.vue(4 hunks)web/components/ReleaseNotesModal.vue(1 hunks)web/components/UserProfile/versions.query.ts(1 hunks)web/composables/api/use-notifications.ts(1 hunks)web/composables/gql/gql.ts(3 hunks)web/composables/gql/graphql.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
TypeScript source files must use import specifiers with .js extensions for ESM compatibility
Files:
api/src/unraid-api/graph/resolvers/info/versions/get-api-version.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.resolver.tsapi/src/unraid-api/graph/resolvers/info/info.resolver.tsapi/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.tsweb/composables/api/use-notifications.tsweb/composables/gql/gql.tsapi/src/unraid-api/graph/resolvers/info/versions/core-versions.resolver.tsweb/components/UserProfile/versions.query.tsapi/src/unraid-api/graph/resolvers/info/info.module.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.service.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.model.tsweb/composables/gql/graphql.ts
api/src/unraid-api/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place new API source files in api/src/unraid-api/ rather than legacy locations
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
api/src/unraid-api/graph/resolvers/info/versions/get-api-version.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.resolver.tsapi/src/unraid-api/graph/resolvers/info/info.resolver.tsapi/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/info/versions/core-versions.resolver.tsapi/src/unraid-api/graph/resolvers/info/info.module.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.service.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.model.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Avoid unnecessary or obvious comments; only add comments when needed for clarity
Files:
api/src/unraid-api/graph/resolvers/info/versions/get-api-version.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.resolver.tsapi/src/unraid-api/graph/resolvers/info/info.resolver.tsapi/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.tsweb/components/HeaderOsVersion.ce.vueweb/composables/api/use-notifications.tsweb/composables/gql/gql.tsapi/src/unraid-api/graph/resolvers/info/versions/core-versions.resolver.tsweb/components/UserProfile/versions.query.tsapi/src/unraid-api/graph/resolvers/info/info.module.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.service.tsweb/components/ReleaseNotesModal.vueapi/src/unraid-api/graph/resolvers/info/versions/versions.model.tsweb/composables/gql/graphql.ts
api/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Prefer not to mock simple dependencies in API tests
Files:
api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{test,spec}.{js,ts,jsx,tsx}: General testing: use .rejects.toThrow() without arguments when asserting thrown errors
General testing: focus on behavior, not exact error message wording
General testing: avoid brittle tests tied to non-essential details (exact messages, log formats)
General testing: use mocks as nouns (objects), not verbs (behavioral overuse)
Files:
api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts
api/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*.{test,spec}.{js,jsx,ts,tsx}: Use Vitest for tests in the api; do not use Jest
Prefer not to mock simple dependencies in tests
For error testing, use.rejects.toThrow()without arguments; avoid asserting exact error messages unless the message format is the subject under test
Files:
api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts
**/components/**/*.vue
📄 CodeRabbit Inference Engine (.cursor/rules/web-testing-rules.mdc)
Some Vue files may require explicit imports like ref or computed due to Nuxt auto-imports not applying in tests
Files:
web/components/HeaderOsVersion.ce.vueweb/components/ReleaseNotesModal.vue
web/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
All web GraphQL code must follow the .query.ts/.mutation.ts filename conventions (do not place operations in other file types)
Files:
web/composables/api/use-notifications.tsweb/composables/gql/gql.tsweb/components/UserProfile/versions.query.tsweb/composables/gql/graphql.ts
web/**/*.query.ts
📄 CodeRabbit Inference Engine (.cursor/rules/web-graphql.mdc)
GraphQL queries must be placed in
.query.tsfilesIn the web app, GraphQL queries must live in files ending with .query.ts
Files:
web/components/UserProfile/versions.query.ts
web/**/*.{query,mutation}.ts
📄 CodeRabbit Inference Engine (.cursor/rules/web-graphql.mdc)
All GraphQL under
web/must follow this naming convention
Files:
web/components/UserProfile/versions.query.ts
🧠 Learnings (9)
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/{store,stores}/**/*.ts : In store files, explicitly import Vue reactivity utilities (e.g., computed, ref, watchEffect); do not rely on Nuxt auto-imports
Applied to files:
web/components/HeaderOsVersion.ce.vue
📚 Learning: 2025-03-27T23:52:57.888Z
Learnt from: zackspear
PR: unraid/api#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/components/HeaderOsVersion.ce.vue
📚 Learning: 2025-03-27T23:52:57.888Z
Learnt from: zackspear
PR: unraid/api#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/components/HeaderOsVersion.ce.vue
📚 Learning: 2025-03-27T23:33:13.215Z
Learnt from: zackspear
PR: unraid/api#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/components/HeaderOsVersion.ce.vue
📚 Learning: 2025-06-13T17:14:21.739Z
Learnt from: pujitm
PR: unraid/api#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/components/HeaderOsVersion.ce.vue
📚 Learning: 2024-12-09T15:45:46.492Z
Learnt from: pujitm
PR: unraid/api#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/components/HeaderOsVersion.ce.vue
📚 Learning: 2024-12-09T15:47:29.325Z
Learnt from: pujitm
PR: unraid/api#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/components/HeaderOsVersion.ce.vue
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
PR: unraid/api#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:
web/composables/gql/gql.ts
📚 Learning: 2025-07-21T14:00:25.140Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-graphql.mdc:0-0
Timestamp: 2025-07-21T14:00:25.140Z
Learning: Applies to web/**/*.query.ts : GraphQL queries must be placed in `.query.ts` files
Applied to files:
web/composables/gql/gql.ts
🧬 Code Graph Analysis (8)
api/src/unraid-api/graph/resolvers/info/versions/get-api-version.ts (1)
api/src/unraid-api/cli/log.service.ts (1)
error(65-69)
api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts (4)
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (2)
Resolver(26-100)versions(97-99)api/src/unraid-api/graph/resolvers/info/versions/core-versions.resolver.ts (1)
Resolver(7-14)api/src/unraid-api/cli/generated/graphql.ts (1)
InfoVersions(1040-1105)web/composables/gql/graphql.ts (3)
InfoVersions(1050-1057)CoreVersions(523-531)PackageVersions(1483-1541)
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (2)
api/src/unraid-api/cli/generated/graphql.ts (1)
InfoVersions(1040-1105)web/composables/gql/graphql.ts (1)
InfoVersions(1050-1057)
api/src/unraid-api/graph/resolvers/info/versions/core-versions.resolver.ts (3)
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (2)
Resolver(26-100)versions(97-99)api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts (1)
Resolver(12-63)web/composables/gql/graphql.ts (1)
CoreVersions(523-531)
web/components/UserProfile/versions.query.ts (1)
web/composables/gql/gql.ts (1)
graphql(282-284)
api/src/unraid-api/graph/resolvers/info/versions/versions.service.ts (2)
api/src/unraid-api/cli/generated/graphql.ts (1)
InfoVersions(1040-1105)web/composables/gql/graphql.ts (1)
InfoVersions(1050-1057)
api/src/unraid-api/graph/resolvers/info/versions/versions.model.ts (3)
api/src/unraid-api/graph/resolvers/info/info.model.ts (1)
ObjectType(13-44)web/composables/gql/graphql.ts (4)
CoreVersions(523-531)PackageVersions(1483-1541)Node(1338-1340)InfoVersions(1050-1057)api/src/unraid-api/cli/generated/graphql.ts (2)
Node(1386-1388)InfoVersions(1040-1105)
web/composables/gql/graphql.ts (1)
api/src/unraid-api/cli/generated/graphql.ts (3)
Maybe(3-3)Scalars(11-75)Exact(5-5)
🪛 GitHub Actions: CI - Main (API)
api/src/unraid-api/graph/resolvers/info/versions/get-api-version.ts
[error] 15-15: TypeScript error TS2322: Type 'string | undefined' is not assignable to type 'string'.
web/components/HeaderOsVersion.ce.vue
[error] 103-103: vitest error: No "DropdownMenuRoot" export is defined on the "@unraid/ui" mock. Did you forget to return it from "vi.mock"?
🔇 Additional comments (17)
api/dev/configs/api.json (1)
2-2: Version consistency confirmed for API configBoth
package.jsonandapi/dev/configs/api.jsonreport version 4.14.0, so the bump is intentional and aligned.api/generated-schema.graphql (2)
1504-1513: Schema restructure: CoreVersions addition looks coherentAdding CoreVersions with unraid, api, and kernel fields reads cleanly and matches the PR intent to group core versioning.
1601-1609: InfoVersions.core/packages resolvers correctly return non-null objectsBoth
core()andpackages()field resolvers inVersionsResolveralways return an object (nevernull), satisfying theCoreVersions!andPackageVersions!non-null GraphQL constraints.• api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts
–core()returns{ unraid, api, kernel: undefined }
–packages()returns aPackageVersionsobject with all expected fieldsNo changes required.
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (1)
96-99: Returning a stub for InfoVersions via field resolvers is a good moveKeeping versions() lean and delegating population to field resolvers reduces coupling and aligns with the new schema.
api/src/unraid-api/graph/resolvers/info/info.module.ts (1)
11-12: LGTM: Module wires new resolvers correctlyProviders and exports are updated to include VersionsResolver and CoreVersionsResolver, and ConfigModule remains imported for downstream needs. Looks good.
Also applies to: 24-26, 35-35
api/src/unraid-api/graph/resolvers/info/versions/versions.service.ts (1)
7-11: Stub service returning only id is appropriate for field resolversReturning a Partial with just
idis fine when field resolvers populatecoreandpackages. Keep as-is.api/src/unraid-api/graph/resolvers/info/versions/core-versions.resolver.ts (1)
5-5: LGTM: ESM-compatible import path.Using .js extension on the local import aligns with our ESM guideline.
web/composables/gql/gql.ts (3)
47-47: LGTM: Documents map entry added for InfoVersions.Entry shape matches the generated InfoVersionsDocument; consistent with codegen patterns.
90-90: LGTM: Runtime documents map includes InfoVersionsDocument.Consistent with the type map addition and expected by the graphql() overloads.
237-241: LGTM: graphql() overload for InfoVersions.Overload matches the query string and maintains type inference. Note: duplicates here are intentional per our codegen setup.
api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts (1)
12-12: InfoModule wiring correctBoth
VersionsResolverandCoreVersionsResolverare imported, registered under sub-resolvers, and exported inapi/src/unraid-api/graph/resolvers/info/info.module.ts. No further action required.api/src/unraid-api/graph/resolvers/info/versions/versions.model.ts (2)
5-15: LGTM: CoreVersions type added cleanly.Fields are nullable as expected, and import paths use .js extensions per ESM guideline.
104-111: LGTM: InfoVersions now composes core and packages.Non-null grouping fields are appropriate; combined with resolver-side try/catch, this will keep info.versions stable even if a probe fails.
web/composables/gql/graphql.ts (4)
523-531: LGTM: CoreVersions type exposed to the client.Matches server-side structure and field nullability.
1052-1056: LGTM: InfoVersions now nests core and packages.Aligns with API refactor.
1483-1541: LGTM: PackageVersions type looks complete.Fields align with systeminformation.versions() shape.
2724-2728: LGTM: InfoVersionsQuery types match the query and new schema.Query selects core.unraid/api as intended.
| import { getReleaseNotesUrl, WEBGUI_TOOLS_DOWNGRADE, WEBGUI_TOOLS_UPDATE } from '~/helpers/urls'; | ||
| import { BellAlertIcon, ExclamationTriangleIcon, InformationCircleIcon, DocumentTextIcon, ArrowTopRightOnSquareIcon } from '@heroicons/vue/24/solid'; | ||
| import { Badge, DropdownMenuRoot, DropdownMenuTrigger, DropdownMenuContent, DropdownMenuItem, DropdownMenuLabel, DropdownMenuSeparator } from '@unraid/ui'; | ||
| import { WEBGUI_TOOLS_DOWNGRADE, WEBGUI_TOOLS_UPDATE } from '~/helpers/urls'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Honor ESM import specifiers with .js for local modules
Per guidelines, local TypeScript imports should use .js extensions.
Apply:
-import { WEBGUI_TOOLS_DOWNGRADE, WEBGUI_TOOLS_UPDATE } from '~/helpers/urls';
+import { WEBGUI_TOOLS_DOWNGRADE, WEBGUI_TOOLS_UPDATE } from '~/helpers/urls.js';
@@
-import { INFO_VERSIONS_QUERY } from './UserProfile/versions.query';
+import { INFO_VERSIONS_QUERY } from './UserProfile/versions.query.js';If paths differ, confirm actual files:
Also applies to: 15-16
🏁 Script executed:
#!/bin/bash
fd -a 'urls.ts' web/helpers
fd -a 'versions.query.ts' web/components/UserProfileLength of output: 183
Add .js extensions to local ESM imports
To preserve ESM compatibility, all local imports of TypeScript modules must include the .js extension. In web/components/HeaderOsVersion.ce.vue, update:
• Line 9:
-import { WEBGUI_TOOLS_DOWNGRADE, WEBGUI_TOOLS_UPDATE } from '~/helpers/urls';
+import { WEBGUI_TOOLS_DOWNGRADE, WEBGUI_TOOLS_UPDATE } from '~/helpers/urls.js';• Lines 15–16:
-import { INFO_VERSIONS_QUERY } from './UserProfile/versions.query';
+import { INFO_VERSIONS_QUERY } from './UserProfile/versions.query.js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { WEBGUI_TOOLS_DOWNGRADE, WEBGUI_TOOLS_UPDATE } from '~/helpers/urls'; | |
| import { WEBGUI_TOOLS_DOWNGRADE, WEBGUI_TOOLS_UPDATE } from '~/helpers/urls.js'; | |
| import { INFO_VERSIONS_QUERY } from './UserProfile/versions.query.js'; |
🤖 Prompt for AI Agents
In web/components/HeaderOsVersion.ce.vue around lines 9 and 15-16, local ESM
imports are missing the .js extension; update the import statements that
reference local TypeScript modules (the import on line 9 and the imports on
lines 15–16) to include the .js extension (e.g., change './module' or
'~/helpers/urls' style local imports to './module.js' or '~/helpers/urls.js') so
they remain ESM-compatible, preserving the same relative/alias paths and only
appending .js.
| import { ArrowTopRightOnSquareIcon } from '@heroicons/vue/24/solid'; | ||
| import { BrandButton, BrandLoading } from '@unraid/ui'; | ||
| import Modal from '~/components/Modal.vue'; | ||
| import { getReleaseNotesUrl } from '~/helpers/urls'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use .js extension in local import
Align with ESM import specifier guideline.
-import { getReleaseNotesUrl } from '~/helpers/urls';
+import { getReleaseNotesUrl } from '~/helpers/urls.js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { getReleaseNotesUrl } from '~/helpers/urls'; | |
| import { getReleaseNotesUrl } from '~/helpers/urls.js'; |
🤖 Prompt for AI Agents
In web/components/ReleaseNotesModal.vue around line 7, the local import uses a
bare specifier without a file extension ("~/helpers/urls"); update the import to
include the .js extension (e.g. import { getReleaseNotesUrl } from
'~/helpers/urls.js') to comply with ESM import specifier guidelines and ensure
the resolved file path matches your build/alias configuration.
| const openInNewTab = () => { | ||
| window.open(releaseNotesUrl.value, '_blank'); | ||
| }; | ||
| </script> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid double navigation: don’t combine href with window.open click handler
With :external="true" and :href, BrandButton likely renders an anchor that already opens in a new tab. The additional @click calling window.open can cause two tabs to open.
Apply:
-const openInNewTab = () => {
- window.open(releaseNotesUrl.value, '_blank');
-};
+// Clicking the BrandButton anchor handles opening in a new tab via :external and :hrefAnd in the template:
- :external="true"
- :href="releaseNotesUrl"
+ :external="true"
+ :href="releaseNotesUrl"
:icon="ArrowTopRightOnSquareIcon"
aria-label="View on Docs"
- @click="openInNewTab"If BrandButton does not handle target="_blank", keep the click handler but add features: window.open(releaseNotesUrl.value, '_blank', 'noopener,noreferrer').
Also applies to: 80-87
🏁 Script executed:
#!/bin/bash
# Locate ReleaseNotesModal.vue
modal=$(fd ReleaseNotesModal.vue)
echo "ReleaseNotesModal.vue path: $modal"
# Show lines around the current function definition and template usage
echo "---- ReleaseNotesModal.vue (lines 1-120) ----"
sed -n '1,120p' "$modal"
echo "---- ReleaseNotesModal.vue (lines 120-200) ----"
sed -n '120,200p' "$modal"
# Locate BrandButton.vue
btn=$(fd BrandButton.vue)
echo "BrandButton.vue path: $btn"
# Show props and initial implementation of external/href
echo "---- BrandButton.vue (first 200 lines) ----"
sed -n '1,200p' "$btn"
echo "---- Searching for 'external' usage in BrandButton.vue ----"
grep -n "external" -C3 "$btn"
echo "---- Searching for 'href' usage in BrandButton.vue ----"
grep -n "href" -C3 "$btn"Length of output: 8164
Remove redundant click handler to prevent double navigation
BrandButton’s :external="true" + :href already renders an <a> with target="_blank" and rel="noopener noreferrer". The @click="openInNewTab" plus its window.open(...) function causes two tabs to open.
Please refactor in web/components/ReleaseNotesModal.vue:
- Delete the
openInNewTabfunction (lines 35–38). - Remove
@click="openInNewTab"from the<BrandButton>(around lines 80–87).
Diff example:
--- a/web/components/ReleaseNotesModal.vue
+++ b/web/components/ReleaseNotesModal.vue
@@ <script setup lang="ts"> (around line 35)
-const openInNewTab = () => {
- window.open(releaseNotesUrl.value, '_blank');
-};
@@ <template #footer> (around line 80)
- <BrandButton
+ <BrandButton
variant="underline"
:external="true"
:href="releaseNotesUrl"
:icon="ArrowTopRightOnSquareIcon"
aria-label="View on Docs"
- @click="openInNewTab"
>
Open in New Tab
</BrandButton>📝 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.
| const openInNewTab = () => { | |
| window.open(releaseNotesUrl.value, '_blank'); | |
| }; | |
| </script> | |
| ++ b/web/components/ReleaseNotesModal.vue | |
| @@ <script setup lang="ts"> (around line 35) | |
| -const openInNewTab = () => { | |
| - window.open(releaseNotesUrl.value, '_blank'); | |
| -}; | |
| @@ <template #footer> (around line 80) | |
| - <BrandButton | |
| - variant="underline" | |
| - :external="true" | |
| - :href="releaseNotesUrl" | |
| - :icon="ArrowTopRightOnSquareIcon" | |
| - aria-label="View on Docs" | |
| - @click="openInNewTab" | |
| <BrandButton | |
| variant="underline" | |
| :external="true" | |
| :href="releaseNotesUrl" | |
| :icon="ArrowTopRightOnSquareIcon" | |
| aria-label="View on Docs" | |
| > | |
| Open in New Tab | |
| </BrandButton> |
🤖 Prompt for AI Agents
In web/components/ReleaseNotesModal.vue around lines 35–38 and 80–87, remove the
redundant openInNewTab click handler and its function: delete the openInNewTab
function (lines ~35–38) and remove the @click="openInNewTab" attribute from the
<BrandButton> so the component relies on BrandButton's :external and :href
behavior (target="_blank" and rel="noopener noreferrer") to open the link once.
There was a problem hiding this comment.
Good suggestion. The click handler here is redundant with the presence of href & external props - as the DOM element will be an <a>.
There was a problem hiding this comment.
@zackspear Thank you for confirming! Yes, exactly - when BrandButton receives both href and external props, it renders as an anchor element with target="_blank" and rel="noopener noreferrer", making the additional window.open() call unnecessary and causing the double navigation issue.
The fix I suggested will clean up the component by removing the redundant openInNewTab function and @click handler, letting the native anchor behavior handle the new tab opening.
- Refactored the getApiVersion function to store the version in a local variable before caching it. - Enhanced readability by separating the version assignment from the caching logic.
- Removed deprecated version fields from PackageVersions type. - Introduced CoreVersions type to encapsulate core system version details. - Updated InfoVersions type to include references to CoreVersions and PackageVersions. - Modified SystemReport query to return structured version information. - Enhanced versions resolver to align with the new schema structure.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts (1)
28-42: Harden packages() to avoid breaking a non-null field and cache versions() per request.If systeminformation.versions() throws, GraphQL will null the non-null packages field, potentially failing Info.versions or the entire info query. Also, kernel resolution in CoreVersionsResolver calls versions() again — cache it in the request context to avoid duplicate OS probes.
Apply this diff:
-import { ResolveField, Resolver } from '@nestjs/graphql'; +import { Context, ResolveField, Resolver } from '@nestjs/graphql'; @@ - @ResolveField(() => PackageVersions) - async packages(): Promise<PackageVersions> { - const softwareVersions = await versions(); - - return { - openssl: softwareVersions.openssl, - node: softwareVersions.node, - npm: softwareVersions.npm, - pm2: softwareVersions.pm2, - git: softwareVersions.git, - nginx: softwareVersions.nginx, - php: softwareVersions.php, - docker: softwareVersions.docker, - }; - } + @ResolveField(() => PackageVersions) + async packages(@Context() ctx: any): Promise<PackageVersions> { + try { + const softwareVersions = (ctx.softwareVersions ||= await versions()); + return { + openssl: softwareVersions.openssl, + node: softwareVersions.node, + npm: softwareVersions.npm, + pm2: softwareVersions.pm2, + git: softwareVersions.git, + nginx: softwareVersions.nginx, + php: softwareVersions.php, + docker: softwareVersions.docker, + }; + } catch { + // Non-null field: return an empty object instead of erroring the entire query + return {} as PackageVersions; + } + }Follow-up: mirror the same context caching in CoreVersionsResolver.kernel() so both fields share a single versions() call per request.
🧹 Nitpick comments (2)
web/__test__/components/HeaderOsVersion.test.ts (1)
26-55: UI stubs are solid; consider adding DropdownMenuPortal to prevent potential unresolved component errorsIf HeaderOsVersion uses a Portal component (common with DropdownMenu libs), tests can fail unless it’s stubbed too. Add a simple passthrough stub for DropdownMenuPortal.
vi.mock('@unraid/ui', () => ({ Badge: { name: 'Badge', template: '<div><slot /></div>', }, DropdownMenuRoot: { name: 'DropdownMenuRoot', template: '<div><slot /></div>', }, DropdownMenuTrigger: { name: 'DropdownMenuTrigger', template: '<div><slot /></div>', }, DropdownMenuContent: { name: 'DropdownMenuContent', template: '<div><slot /></div>', }, DropdownMenuItem: { name: 'DropdownMenuItem', template: '<div><slot /></div>', }, DropdownMenuLabel: { name: 'DropdownMenuLabel', template: '<div><slot /></div>', }, DropdownMenuSeparator: { name: 'DropdownMenuSeparator', template: '<div />', }, + DropdownMenuPortal: { + name: 'DropdownMenuPortal', + template: '<div><slot /></div>', + }, }));api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts (1)
16-26: Minor: omit redundant kernel placeholder and keep core lean.Since CoreVersionsResolver resolves kernel via a field resolver regardless of parent shape, explicitly returning kernel: undefined is unnecessary.
Apply this small cleanup:
@ResolveField(() => CoreVersions) core(): CoreVersions { const unraid = this.configService.get<string>('store.emhttp.var.version') || 'unknown'; const api = this.configService.get<string>('api.version') || 'unknown'; return { unraid, api, - kernel: undefined, // Will be resolved separately if requested }; }Additionally, consider centralizing API version resolution with the get-api-version helper (if available synchronously) to avoid config drift.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
api/src/unraid-api/cli/generated/gql.tsis excluded by!**/generated/**api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**
📒 Files selected for processing (6)
api/generated-schema.graphql(1 hunks)api/src/unraid-api/cli/queries/system-report.query.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/versions/get-api-version.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/versions/versions.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts(1 hunks)web/__test__/components/HeaderOsVersion.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/graph/resolvers/info/versions/get-api-version.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
TypeScript source files must use import specifiers with .js extensions for ESM compatibility
Files:
api/src/unraid-api/cli/queries/system-report.query.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.resolver.tsweb/__test__/components/HeaderOsVersion.test.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.model.ts
api/src/unraid-api/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place new API source files in api/src/unraid-api/ rather than legacy locations
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
api/src/unraid-api/cli/queries/system-report.query.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.resolver.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.model.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Avoid unnecessary or obvious comments; only add comments when needed for clarity
Files:
api/src/unraid-api/cli/queries/system-report.query.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.resolver.tsweb/__test__/components/HeaderOsVersion.test.tsapi/src/unraid-api/graph/resolvers/info/versions/versions.model.ts
web/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
All web GraphQL code must follow the .query.ts/.mutation.ts filename conventions (do not place operations in other file types)
Files:
web/__test__/components/HeaderOsVersion.test.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{test,spec}.{js,ts,jsx,tsx}: General testing: use .rejects.toThrow() without arguments when asserting thrown errors
General testing: focus on behavior, not exact error message wording
General testing: avoid brittle tests tied to non-essential details (exact messages, log formats)
General testing: use mocks as nouns (objects), not verbs (behavioral overuse)
Files:
web/__test__/components/HeaderOsVersion.test.ts
web/__test__/**/*.{test,spec}.{ts,js,tsx,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
web/__test__/**/*.{test,spec}.{ts,js,tsx,jsx}: Web (Vue) component tests should use mount from Vue Test Utils
Web (Vue) component tests should stub complex child components not under test
Web (Vue) component tests should mock external dependencies and services
Web (Vue) component tests should query elements semantically (e.g., find('button')) rather than using data-test IDs
Web (Vue) component tests should use createTestingPinia() for mocking stores in components
Web (Vue) component tests should use await nextTick() for DOM updates and await all async operations before assertions
Files:
web/__test__/components/HeaderOsVersion.test.ts
web/**/*.{test,spec}.{ts,js,tsx,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
web/**/*.{test,spec}.{ts,js,tsx,jsx}: Store testing with Pinia: use createPinia() and setActivePinia; only use createTestingPinia when you need its features
Store testing with Pinia: let stores initialize with natural defaults; do not mock the store under test
Store testing with Pinia: place all mock declarations at top level; use factory functions for module mocks; clear mocks between tests
Files:
web/__test__/components/HeaderOsVersion.test.ts
web/__test__/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Tests for the web app are located under web/test
Files:
web/__test__/components/HeaderOsVersion.test.ts
{**/*.test.ts,**/__test__/{components,store}/**/*.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/web-testing-rules.mdc)
{**/*.test.ts,**/__test__/{components,store}/**/*.ts}: Use .rejects.toThrow() without arguments when asserting that async functions throw; avoid checking exact error message strings unless the message format is explicitly under test
Focus tests on observable behavior and outcomes, not implementation details such as exact error messages
Use await nextTick() for DOM update assertions and flushPromises() for complex async chains; always await async operations before asserting
Place module mock declarations (vi.mock) at the top level of the test file to avoid hoisting issues
Use factory functions in vi.mock calls to define mocks and avoid hoisting pitfalls
Use vi.spyOn() to specify return values or behavior of methods under test
Reset/clear mocks between tests using vi.clearAllMocks() (and vi.resetAllMocks() when appropriate) to ensure isolation
Do not rely on Nuxt auto-imports in tests; import required Vue utilities explicitly in test files
Remember that vi.mock calls are hoisted; avoid mixing mock declarations and module mocks incorrectly
Files:
web/__test__/components/HeaderOsVersion.test.ts
**/__test__/components/**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/web-testing-rules.mdc)
**/__test__/components/**/*.ts: Component tests should use mount from @vue/test-utils
Stub complex child components that are not the focus of a component test
Mock external dependencies and services in component tests (e.g., vi.mock for helper modules)
Use createTestingPinia() to mock Pinia stores when testing Vue components
Test that expected elements are rendered and verify component output rather than internals
Test component interactions (clicks, inputs) using await element.trigger and setValue, and verify emitted events via wrapper.emitted()
Prefer semantic queries like find('button') or CSS selectors; avoid relying on data-test IDs unless necessary
Assert rendered text with wrapper.text(), check attributes via element.attributes(), and verify element existence with expect(element.exists()).toBe(true)
In component tests that use real store implementations, configure createTestingPinia with stubActions: false when you need real action behavior
Use findComponent(ComponentName) for locating child components and findAll for collections
Files:
web/__test__/components/HeaderOsVersion.test.ts
🧠 Learnings (12)
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Test that expected elements are rendered and verify component output rather than internals
Applied to files:
web/__test__/components/HeaderOsVersion.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Use factory functions in vi.mock calls to define mocks and avoid hoisting pitfalls
Applied to files:
web/__test__/components/HeaderOsVersion.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/store/**/*.ts : In store tests, mock external dependencies used by the store and verify interactions with those mocks
Applied to files:
web/__test__/components/HeaderOsVersion.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Remember that vi.mock calls are hoisted; avoid mixing mock declarations and module mocks incorrectly
Applied to files:
web/__test__/components/HeaderOsVersion.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Mock external dependencies and services in component tests (e.g., vi.mock for helper modules)
Applied to files:
web/__test__/components/HeaderOsVersion.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Place module mock declarations (vi.mock) at the top level of the test file to avoid hoisting issues
Applied to files:
web/__test__/components/HeaderOsVersion.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Use createTestingPinia() to mock Pinia stores when testing Vue components
Applied to files:
web/__test__/components/HeaderOsVersion.test.ts
📚 Learning: 2025-08-11T15:06:30.877Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-11T15:06:30.877Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,js,tsx,jsx} : Web (Vue) component tests should mock external dependencies and services
Applied to files:
web/__test__/components/HeaderOsVersion.test.ts
📚 Learning: 2025-08-11T15:06:30.877Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-11T15:06:30.877Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,js,tsx,jsx} : Web (Vue) component tests should use createTestingPinia() for mocking stores in components
Applied to files:
web/__test__/components/HeaderOsVersion.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Reset/clear mocks between tests using vi.clearAllMocks() (and vi.resetAllMocks() when appropriate) to ensure isolation
Applied to files:
web/__test__/components/HeaderOsVersion.test.ts
📚 Learning: 2025-08-11T15:06:30.877Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-11T15:06:30.877Z
Learning: Applies to web/**/*.{test,spec}.{ts,js,tsx,jsx} : Store testing with Pinia: place all mock declarations at top level; use factory functions for module mocks; clear mocks between tests
Applied to files:
web/__test__/components/HeaderOsVersion.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Assert rendered text with wrapper.text(), check attributes via element.attributes(), and verify element existence with expect(element.exists()).toBe(true)
Applied to files:
web/__test__/components/HeaderOsVersion.test.ts
🧬 Code Graph Analysis (2)
api/src/unraid-api/graph/resolvers/info/versions/versions.resolver.ts (3)
api/src/unraid-api/graph/resolvers/info/versions/core-versions.resolver.ts (1)
Resolver(7-14)api/src/unraid-api/graph/resolvers/info/info.resolver.ts (2)
Resolver(26-100)versions(97-99)api/src/unraid-api/cli/generated/graphql.ts (3)
InfoVersions(1050-1057)CoreVersions(523-531)PackageVersions(1483-1499)
api/src/unraid-api/graph/resolvers/info/versions/versions.model.ts (3)
api/src/unraid-api/graph/resolvers/info/info.model.ts (1)
ObjectType(13-44)api/src/unraid-api/cli/generated/graphql.ts (4)
CoreVersions(523-531)PackageVersions(1483-1499)Node(1338-1340)InfoVersions(1050-1057)web/composables/gql/graphql.ts (4)
CoreVersions(523-531)PackageVersions(1483-1541)Node(1338-1340)InfoVersions(1050-1057)
🔇 Additional comments (7)
web/__test__/components/HeaderOsVersion.test.ts (1)
155-159: LGTM: Assertions match the new button-based “Version Information” UIThe test semantically queries the button by title and verifies that the displayed text includes the OS version. It also ensures no update status is shown initially—aligned with the PR’s UI changes.
api/src/unraid-api/cli/queries/system-report.query.ts (1)
17-23: Query shape update aligns with new schema (core/packages). LGTM.The nested selection matches CoreVersions/PackageVersions. Import uses .js extension per ESM guidelines.
api/generated-schema.graphql (2)
1504-1513: CoreVersions addition looks correct and consistent with model/resolvers.The fields and docstrings align with the new nested design and the CoreVersionsResolver for kernel.
1541-1549: InfoVersions implements Node with non-null core/packages. LGTM.This enforces the nested structure at the API boundary while allowing nullable individual fields within the objects.
api/src/unraid-api/graph/resolvers/info/versions/versions.model.ts (3)
5-15: CoreVersions object type is well-scoped and documented.Fields and nullability align with schema. ESM import of Node uses .js extension per guideline.
17-42: PackageVersions mapping is clear and matches the schema.Good separation from core fields; descriptions help introspection.
44-51: InfoVersions implements Node and nests core/packages.Matches the generated schema and resolver structure.
…ckageVersions type - Changed version retrieval in ApiReportService to use core.unraid instead of unraid. - Added new fields for nginx, node, npm, and pm2 versions in PackageVersions type. - Removed deprecated fields for perl, python, and python3 from PackageVersions type. - Ensured consistent export formatting in generated index file.
…te InfoResolver tests - Restructured version information in ApiReportService to separate core and package versions. - Updated InfoResolver integration tests to reflect changes in version retrieval, now returning a stub object for field resolvers.
…ields - Removed several deprecated fields from the PackageVersions type to streamline version management. - Ensured consistent export formatting in the index file.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
web/composables/gql/graphql.ts (1)
1052-1056: Regenerate CLI artifacts so InfoVersions exposes nested core/packages.This Web generated shape is nested. If the CLI’s generated
InfoVersionsremains flat, you’ll get API/client drift. Please re-run codegen for the CLI package.#!/bin/bash set -euo pipefail WEB_FILE="web/composables/gql/graphql.ts" CLI_FILE="$(fd -a 'graphql.ts' api | grep -E 'cli/generated/.*/graphql\.ts$' | head -n1 || true)" echo "---- Web InfoVersions ----" awk '/export type InfoVersions = Node & {/,/};/' "$WEB_FILE" || true if [ -n "${CLI_FILE:-}" ] && [ -f "$CLI_FILE" ]; then echo echo "---- CLI InfoVersions ----" awk '/export type InfoVersions = Node & {/,/};/' "$CLI_FILE" || true else echo "CLI generated graphql.ts not found; ensure codegen has been run for the CLI package." fi
🧹 Nitpick comments (5)
api/src/unraid-api/cli/__test__/api-report.service.test.ts (2)
67-74: Type your mocks against generated GraphQL query types to catch schema drift earlyAnnotate the mocks with
SystemReportQuery,ConnectStatusQuery, andServicesQueryso changes in the generated types surface as compile-time errors instead of silent test passes.Add these type-only imports (ESM-compliant with .js extension):
import type { ConnectStatusQuery, ServicesQuery, SystemReportQuery, } from '@app/unraid-api/cli/generated/graphql.js';Then annotate the mocks:
const mockSystemData: SystemReportQuery = { /* ...existing object... */ }; const mockConnectData: ConnectStatusQuery = { /* ...existing object... */ }; const mockServicesData: ServicesQuery = { /* ...existing object... */ };
67-74: Add a test for the 'Unknown' fallback when versions path is missingEnsure we don’t regress on the runtime safety for
versions.core.unraid. This complements the service change to use optional chaining.Example test to add:
it('falls back to "Unknown" when versions.core.unraid is missing', async () => { const mockSystemDataNoUnraid = { ...mockSystemData, info: { ...mockSystemData.info, versions: { // simulate missing core.unraid; cast to any to mimic older/partial data // eslint-disable-next-line @typescript-eslint/no-explicit-any core: {} as any, }, }, }; mockClient.query.mockImplementation(({ query }) => { if (query === SYSTEM_REPORT_QUERY) { return Promise.resolve({ data: mockSystemDataNoUnraid }); } else if (query === CONNECT_STATUS_QUERY) { return Promise.resolve({ data: mockConnectData }); } else if (query === SERVICES_QUERY) { return Promise.resolve({ data: mockServicesData }); } return Promise.reject(new Error('Unknown query')); }); const result = await apiReportService.generateReport(true); expect(result.system.version).toBe('Unknown'); });If you prefer to avoid
any, we can factory a minimal typed helper aroundSystemReportQueryto construct partial shapes safely—happy to draft it.api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts (1)
168-174: Refine theversions()stub test for resilienceWe’ve confirmed that:
InfoResolver.versions()is synchronous and delegates toVersionsService.generateVersions().generateVersions()currently returns only{ id: 'info/versions' }.To guard against future benign additions (e.g. metadata fields), update the spec to assert shape and key count rather than exact key lists.
• File:
api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts(around lines 168–174)Apply this diff:
- it('should return versions stub for field resolvers', () => { - const result = infoResolver.versions(); - - expect(result).toHaveProperty('id', 'info/versions'); - // Versions now returns a stub object, with actual data resolved via field resolvers - expect(Object.keys(result)).toEqual(['id']); - }); + it('should return a minimal versions stub for field resolvers', () => { + const result = infoResolver.versions(); + + // Stub only contains the identifier; actual data is resolved via field resolvers. + expect(result).toMatchObject({ id: 'info/versions' }); + expect(Object.keys(result)).toHaveLength(1); + // Guard against prematurely including data fields + expect(result).not.toHaveProperty('core'); + expect(result).not.toHaveProperty('packages'); + });web/composables/gql/graphql.ts (2)
2684-2688: Query selects only core.unraid/api—confirm that’s sufficient for current UI.If HeaderOsVersion or ReleaseNotesModal will soon need
core.kernelor anypackages.*, add them now to avoid another codegen churn. Also consider selecting__typenamefor better cache behavior if you use normalized caching.Would you like me to update
web/components/UserProfile/versions.query.tsto include kernel and packages now? Example:// web/components/UserProfile/versions.query.ts import { graphql } from '~/composables/gql/gql.js'; export const INFO_VERSIONS_QUERY = graphql(` query InfoVersions { info { id os { id hostname } versions { id core { unraid api kernel } packages { docker git nginx node npm openssl php pm2 } } } } `);
72-74: Unify PrefixedID scalar mappings across GraphQL codegen configsWe’ve confirmed that the Web client defines PrefixedID as
PrefixedID: { input: string; output: string }whereas the CLI generator maps it to
PrefixedID: { input: any; output: any }(package api/src/unraid-api/cli/generated/graphql.ts:62) – leading to inconsistent type safety between runtimes.
Please update your codegen configurations so that all GraphQL clients use the same mapping for PrefixedID:
• In the CLI codegen config (used to produce api/src/unraid-api/cli/generated/graphql.ts), map PrefixedID → string instead of any, then re-run generation.
• In the Web codegen setup (e.g. web/codegen.ts), ensure PrefixedID isn’t overridden to any.
• In the Connect plugin client (packages/unraid-api-plugin-connect/src/graphql/generated/client/graphql.ts), confirm it picks up the same scalar mapping (add PrefixedID if missing).After aligning these configs, regenerate all GraphQL outputs to restore consistent typing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**
📒 Files selected for processing (4)
api/src/unraid-api/cli/__test__/api-report.service.test.ts(1 hunks)api/src/unraid-api/cli/api-report.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts(1 hunks)web/composables/gql/graphql.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
TypeScript source files must use import specifiers with .js extensions for ESM compatibility
Files:
api/src/unraid-api/cli/api-report.service.tsapi/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.tsapi/src/unraid-api/cli/__test__/api-report.service.test.tsweb/composables/gql/graphql.ts
api/src/unraid-api/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place new API source files in api/src/unraid-api/ rather than legacy locations
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
api/src/unraid-api/cli/api-report.service.tsapi/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.tsapi/src/unraid-api/cli/__test__/api-report.service.test.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Avoid unnecessary or obvious comments; only add comments when needed for clarity
Files:
api/src/unraid-api/cli/api-report.service.tsapi/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.tsapi/src/unraid-api/cli/__test__/api-report.service.test.tsweb/composables/gql/graphql.ts
api/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Prefer not to mock simple dependencies in API tests
Files:
api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.tsapi/src/unraid-api/cli/__test__/api-report.service.test.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{test,spec}.{js,ts,jsx,tsx}: General testing: use .rejects.toThrow() without arguments when asserting thrown errors
General testing: focus on behavior, not exact error message wording
General testing: avoid brittle tests tied to non-essential details (exact messages, log formats)
General testing: use mocks as nouns (objects), not verbs (behavioral overuse)
Files:
api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.tsapi/src/unraid-api/cli/__test__/api-report.service.test.ts
api/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*.{test,spec}.{js,jsx,ts,tsx}: Use Vitest for tests in the api; do not use Jest
Prefer not to mock simple dependencies in tests
For error testing, use.rejects.toThrow()without arguments; avoid asserting exact error messages unless the message format is the subject under test
Files:
api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.tsapi/src/unraid-api/cli/__test__/api-report.service.test.ts
{**/*.test.ts,**/__test__/{components,store}/**/*.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/web-testing-rules.mdc)
{**/*.test.ts,**/__test__/{components,store}/**/*.ts}: Use .rejects.toThrow() without arguments when asserting that async functions throw; avoid checking exact error message strings unless the message format is explicitly under test
Focus tests on observable behavior and outcomes, not implementation details such as exact error messages
Use await nextTick() for DOM update assertions and flushPromises() for complex async chains; always await async operations before asserting
Place module mock declarations (vi.mock) at the top level of the test file to avoid hoisting issues
Use factory functions in vi.mock calls to define mocks and avoid hoisting pitfalls
Use vi.spyOn() to specify return values or behavior of methods under test
Reset/clear mocks between tests using vi.clearAllMocks() (and vi.resetAllMocks() when appropriate) to ensure isolation
Do not rely on Nuxt auto-imports in tests; import required Vue utilities explicitly in test files
Remember that vi.mock calls are hoisted; avoid mixing mock declarations and module mocks incorrectly
Files:
api/src/unraid-api/cli/__test__/api-report.service.test.ts
web/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
All web GraphQL code must follow the .query.ts/.mutation.ts filename conventions (do not place operations in other file types)
Files:
web/composables/gql/graphql.ts
🧠 Learnings (1)
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
PR: unraid/api#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:
web/composables/gql/graphql.ts
🧬 Code Graph Analysis (1)
web/composables/gql/graphql.ts (1)
api/src/unraid-api/cli/generated/graphql.ts (5)
CoreVersions(523-531)Maybe(3-3)Scalars(11-75)PackageVersions(1483-1501)Exact(5-5)
🔇 Additional comments (5)
api/src/unraid-api/cli/__test__/api-report.service.test.ts (1)
67-74: LGTM: mocks updated to the new nested versions shapeThe test data now reflects
versions.core.unraidand aligns with the schema refactor. Expectations still validate the top-levelsystem.versioncorrectly.web/composables/gql/graphql.ts (4)
523-531: CoreVersions type addition looks correct.Shape and nullability match the new schema; field docs are consistent. LGTM.
1483-1501: PackageVersions type addition looks good.Field names match the schema and are appropriately nullable.
2772-2772: No action needed: InfoVersionsDocument correctly generated and wiredAll verifications passed:
- Export of
InfoVersionsDocumentexists inweb/composables/gql/graphql.ts(line 2772).- Mappings in
web/composables/gql/gql.tsinclude bothtypeof types.InfoVersionsDocumentandtypes.InfoVersionsDocumententries for theInfoVersionsoperation (lines 47 and 90).- Overload in the
graphqlhelper covers theInfoVersionsquery.- UI query file follows the
.query.tsconvention:web/components/UserProfile/versions.query.tsexists.Duplicate GraphQL strings in generated files are intentional for type safety.
1052-1056: All ‘core’ and ‘packages’ resolvers return non-null objects as requiredI’ve reviewed the
VersionsResolverandCoreVersionsResolverimplementations:
VersionsResolver.core()always returns an object literal ({ unraid, api, kernel: undefined }), nevernull.VersionsResolver.packages()always returns an object literal with all package fields populated (possibly asundefined), nevernull.- The separate
CoreVersionsResolver.kernel()field resolver returnsstring | undefined(mapped to a nullable GraphQL field), but does not affect the non-nullability of thecoreobject itself.Because none of these resolvers ever return
nullforcoreorpackages, there’s no risk of a hard GraphQL error at runtime. You can safely resolve this comment.
|
| kernel | ||
| } | ||
| packages { | ||
| openssl |
There was a problem hiding this comment.
did you want to include the other packages here?
| const openInNewTab = () => { | ||
| window.open(releaseNotesUrl.value, '_blank'); | ||
| }; | ||
| </script> |
There was a problem hiding this comment.
Good suggestion. The click handler here is redundant with the presence of href & external props - as the DOM element will be an <a>.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores
Tests