-
Notifications
You must be signed in to change notification settings - Fork 16
feat: add docker -> organizer query #1555
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (6)
WalkthroughThis update introduces a new Docker organizer system, including backend services, DTOs, and GraphQL schema extensions for hierarchical container organization. It also adds a comprehensive UPS management API to the GraphQL schema. Supporting tests, utilities, and configuration mechanisms are included. Module providers and test setups are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GraphQL API
participant DockerResolver
participant DockerOrganizerService
participant DockerService
participant DockerConfigService
Client->>GraphQL API: Query Docker.organizer
GraphQL API->>DockerResolver: organizer()
DockerResolver->>DockerOrganizerService: getResolvedOrganizer()
DockerOrganizerService->>DockerConfigService: getConfig()
DockerOrganizerService->>DockerService: listContainers()
DockerOrganizerService->>DockerOrganizerService: containerListToResourcesObject()
DockerOrganizerService->>DockerOrganizerService: syncDefaultView()
DockerOrganizerService->>DockerOrganizerService: resolveOrganizer()
DockerOrganizerService-->>DockerResolver: ResolvedOrganizerV1
DockerResolver-->>GraphQL API: ResolvedOrganizerV1
GraphQL API-->>Client: ResolvedOrganizerV1
sequenceDiagram
participant Client
participant GraphQL API
Client->>GraphQL API: Query upsDevices / upsConfiguration
GraphQL API-->>Client: UPSDevice[] / UPSConfiguration
Client->>GraphQL API: Mutation configureUps(config)
GraphQL API-->>Client: Boolean (success)
Client-->>GraphQL API: Subscribe upsUpdates
GraphQL API-->>Client: UPSDevice (on update)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Suggested reviewers
Poem
Tip CodeRabbit can generate a title for your PR based on the changes with custom instructions.Add the ✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Claude finished @pujitm's task —— View job Code Review Complete ✅I've completed a thorough review of this PR which adds docker organizer query functionality to the Unraid API. Here's my analysis: Todo List:
SummaryThis PR introduces a well-architected organizer system for Docker containers that allows users to organize containers into custom views with folders and references. The implementation follows NestJS best practices and includes comprehensive validation and testing. ✅ StrengthsArchitecture & Design
Code Quality
Test Coverage
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (2)
api/src/unraid-api/organizer/organizer.ts (1)
13-22: Consider stricter handling of the refId function result.The nullish coalescing operator on line 18 suggests that
refId(resource)might return null/undefined, but the function signature indicates it should return a string. This could mask bugs in the caller's implementation.Consider either:
- Making the return type explicit in the function signature:
- refId: (resource: OrganizerResource) => string + refId: (resource: OrganizerResource) => string | null | undefined
- Or throwing an error if refId returns null/undefined:
- id: refId(resource) ?? resource.id, + id: refId(resource) || throw new Error(`refId function returned invalid value for resource ${resource.id}`),api/src/unraid-api/organizer/organizer.dto.ts (1)
95-98: Consider enhancing validation for the entries dictionary.The current
@Type(() => Object)decorator won't provide deep validation for the nestedOrganizerFolderandOrganizerResourceRefobjects within the entries dictionary. Since class-validator doesn't natively support validating dictionary values with union types, you may need a custom validator to ensure each entry conforms to eitherOrganizerFolderorOrganizerResourceRefschema.Consider implementing a custom validator:
import { ValidatorConstraint, ValidatorConstraintInterface, ValidationArguments, Validate } from 'class-validator'; @ValidatorConstraint({ name: 'isOrganizerEntries', async: false }) export class IsOrganizerEntriesConstraint implements ValidatorConstraintInterface { validate(entries: any, args: ValidationArguments) { if (!entries || typeof entries !== 'object') return false; for (const entry of Object.values(entries)) { if (!entry || typeof entry !== 'object') return false; if (entry.type === 'folder') { // Validate OrganizerFolder structure if (!entry.id || !entry.name || !Array.isArray(entry.children)) return false; } else if (entry.type === 'ref') { // Validate OrganizerResourceRef structure if (!entry.id || !entry.target) return false; } else { return false; } } return true; } defaultMessage(args: ValidationArguments) { return 'Each entry must be a valid OrganizerFolder or OrganizerResourceRef'; } } // Then in your class: @Validate(IsOrganizerEntriesConstraint) entries!: { [id: string]: OrganizerFolder | OrganizerResourceRef };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
api/dev/configs/api.json(1 hunks)api/dev/connectStatus.json(1 hunks)api/generated-schema.graphql(7 hunks)api/src/unraid-api/graph/resolvers/docker/docker-config.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/docker/docker-organizer.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts(3 hunks)api/src/unraid-api/graph/resolvers/docker/docker.module.ts(2 hunks)api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts(2 hunks)api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts(2 hunks)api/src/unraid-api/organizer/organizer.dto.ts(5 hunks)api/src/unraid-api/organizer/organizer.resolution.test.ts(1 hunks)api/src/unraid-api/organizer/organizer.test.ts(1 hunks)api/src/unraid-api/organizer/organizer.ts(1 hunks)packages/unraid-shared/src/services/config-file.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
api/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*: Use pnpm as the only package manager
Always run scripts from api/package.json unless specifically requested otherwise
Files:
api/src/unraid-api/graph/resolvers/docker/docker.module.tsapi/dev/configs/api.jsonapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/dev/connectStatus.jsonapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker-config.service.tsapi/src/unraid-api/organizer/organizer.test.tsapi/src/unraid-api/organizer/organizer.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/organizer/organizer.dto.tsapi/src/unraid-api/organizer/organizer.resolution.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.tsapi/generated-schema.graphql
api/src/unraid-api/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the NestJS repository located at api/src/unraid-api/ instead of the legacy code
Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
Files:
api/src/unraid-api/graph/resolvers/docker/docker.module.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker-config.service.tsapi/src/unraid-api/organizer/organizer.test.tsapi/src/unraid-api/organizer/organizer.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/organizer/organizer.dto.tsapi/src/unraid-api/organizer/organizer.resolution.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.ts
api/**/*.{test,spec}.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*.{test,spec}.{js,ts,tsx}: Use Vitest as the test suite; do not use Jest
Prefer not to mock simple dependencies in tests
Files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/organizer/organizer.test.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/organizer/organizer.resolution.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts
api/**/*.{test,spec}.{ts,js}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Test suite is VITEST, do not use jest in the API package
Files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/organizer/organizer.test.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/organizer/organizer.resolution.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts
🧠 Learnings (64)
📓 Common learnings
Learnt from: pujitm
PR: unraid/api#1352
File: packages/unraid-api-plugin-generator/src/create-plugin.ts:91-112
Timestamp: 2025-04-21T18:27:36.482Z
Learning: For utility functions like file operations in api plugin generators, pujitm prefers minimal error handling rather than verbose try/catch blocks for each operation, as errors will propagate properly to higher level error handlers.
📚 Learning: applies to api/src/unraid-api/**/* : prefer adding new files to the nestjs repository located at api...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-07-21T14:00:06.258Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repository located at api/src/unraid-api/ instead of the legacy code
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.tsapi/src/unraid-api/graph/resolvers/docker/docker-config.service.tsapi/src/unraid-api/organizer/organizer.tspackages/unraid-shared/src/services/config-file.ts
📚 Learning: applies to api/src/unraid-api/**/* : prefer adding new files to the nestjs repo located at api/src/u...
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.tspackages/unraid-shared/src/services/config-file.ts
📚 Learning: in web/components/docker/edit.vue, the hardcoded configuration values in the config ref are intentio...
Learnt from: mdatelle
PR: unraid/api#1533
File: web/components/Docker/Edit.vue:16-32
Timestamp: 2025-07-24T18:48:44.035Z
Learning: In web/components/Docker/Edit.vue, the hardcoded configuration values in the config ref are intentional temporary/dummy data used during initial UI development phase while building out the real Docker components, as clarified by mdatelle in PR #1533.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.tsapi/src/unraid-api/graph/resolvers/docker/docker-config.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
📚 Learning: the project uses a custom or extended implementation of nestjs configservice that includes a `set()`...
Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.tsapi/src/unraid-api/graph/resolvers/docker/docker-config.service.tspackages/unraid-shared/src/services/config-file.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
📚 Learning: in the unraid api project, error handling for mutations is handled at the service level rather than ...
Learnt from: mdatelle
PR: unraid/api#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/docker/docker.module.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/organizer/organizer.dto.ts
📚 Learning: components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other par...
Learnt from: elibosley
PR: unraid/api#1308
File: unraid-ui/src/components/common/loading/Error.vue:2-2
Timestamp: 2025-04-02T21:21:29.168Z
Learning: Components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other parts of the project that may use Nuxt.js autoloading features.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.ts
📚 Learning: this codebase uses esm (ecmascript modules) and requires .js extensions in import statements, even w...
Learnt from: elibosley
PR: unraid/api#1408
File: web/components/ApiKey/PermissionCounter.vue:6-6
Timestamp: 2025-05-23T21:59:29.632Z
Learning: This codebase uses ESM (ECMAScript Modules) and requires .js extensions in import statements, even when importing from TypeScript files, as the imports refer to the compiled JavaScript output.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.ts
📚 Learning: in nestjs services, dynamic imports should be wrapped in try/catch blocks with proper error logging ...
Learnt from: elibosley
PR: unraid/api#1211
File: api/src/unraid-api/graph/connect/connect-settings.service.ts:48-53
Timestamp: 2025-03-13T16:17:55.820Z
Learning: In NestJS services, dynamic imports should be wrapped in try/catch blocks with proper error logging and fallback handling to prevent unhandled rejections from crashing the application.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.ts
📚 Learning: in the unraid api project, the duplicate graphql query and mutation strings in gql.ts files are inte...
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:
api/src/unraid-api/graph/resolvers/docker/docker.module.tsapi/src/unraid-api/organizer/organizer.dto.ts
📚 Learning: the project uses a custom configservice implementation that includes a `set()` method for runtime co...
Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom ConfigService implementation that includes a `set()` method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.tspackages/unraid-shared/src/services/config-file.ts
📚 Learning: in dockereventservice, the chokidar file watcher is configured with `ignoreinitial: false` to ensure...
Learnt from: elibosley
PR: unraid/api#1334
File: api/src/unraid-api/graph/resolvers/docker/docker-event.service.ts:63-66
Timestamp: 2025-04-07T14:34:47.255Z
Learning: In DockerEventService, the chokidar file watcher is configured with `ignoreInitial: false` to ensure that existing files (like the Docker socket) are detected and processed at application startup, not just when files change after the watcher is initialized.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.ts
📚 Learning: the updateapisettings mutation in api/src/unraid-api/graph/connect/connect.resolver.ts is protected ...
Learnt from: pujitm
PR: unraid/api#1211
File: api/src/graphql/schema/types/connect/connect.graphql:142-146
Timestamp: 2025-03-14T16:10:38.562Z
Learning: The updateApiSettings mutation in api/src/unraid-api/graph/connect/connect.resolver.ts is protected with the @UsePermissions decorator that requires UPDATE permission on the CONFIG resource.
Applied to files:
api/dev/configs/api.jsonapi/src/unraid-api/graph/resolvers/docker/docker.resolver.ts
📚 Learning: helmet security configuration in the unraid api is intentionally relaxed (with disabled csp, cors po...
Learnt from: mdatelle
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:32-55
Timestamp: 2025-03-07T17:35:50.406Z
Learning: Helmet security configuration in the Unraid API is intentionally relaxed (with disabled CSP, CORS policies, and HSTS) to maintain compatibility with existing Unraid plugins. Stricter security settings might break current plugin functionality.
Applied to files:
api/dev/configs/api.json
📚 Learning: for the unraid connect plugin, the script `/etc/rc.d/rc.unraid-api` is bundled with the plugin packa...
Learnt from: pujitm
PR: unraid/api#1415
File: plugin/plugins/dynamix.unraid.net.plg:234-236
Timestamp: 2025-06-11T14:14:30.348Z
Learning: For the Unraid Connect plugin, the script `/etc/rc.d/rc.unraid-api` is bundled with the plugin package itself, so its presence on the target system is guaranteed during installation.
Applied to files:
api/dev/configs/api.json
📚 Learning: the cors configuration in the unraid api is intentionally set to allow all origins (`origin: true`)....
Learnt from: mdatelle
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:57-63
Timestamp: 2025-03-07T17:36:52.790Z
Learning: The CORS configuration in the unraid API is intentionally set to allow all origins (`origin: true`). This was a deliberate architectural decision when removing the previous custom CORS implementation and implementing helmet security headers instead.
Applied to files:
api/dev/configs/api.json
📚 Learning: for critical components in the unraid api, such as retrieving version information from package.json,...
Learnt from: pujitm
PR: unraid/api#1252
File: api/src/environment.ts:56-56
Timestamp: 2025-03-27T13:34:53.438Z
Learning: For critical components in the Unraid API, such as retrieving version information from package.json, failing fast (allowing crashes) is preferred over graceful degradation with fallback values.
Applied to files:
api/dev/configs/api.json
📚 Learning: the directory `/usr/local/emhttp/plugins/dynamix.my.servers` is a valid directory that exists as par...
Learnt from: elibosley
PR: unraid/api#1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh:19-24
Timestamp: 2025-05-08T19:28:54.365Z
Learning: The directory `/usr/local/emhttp/plugins/dynamix.my.servers` is a valid directory that exists as part of the Unraid API plugin installation and should be included in verification checks.
Applied to files:
api/dev/configs/api.json
📚 Learning: the unraid api is designed to handle missing configuration files gracefully with smart internal fall...
Learnt from: elibosley
PR: unraid/api#1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/setup_api.sh:107-113
Timestamp: 2025-05-07T16:07:47.236Z
Learning: The Unraid API is designed to handle missing configuration files gracefully with smart internal fallbacks rather than requiring installation scripts to create default configurations.
Applied to files:
api/dev/configs/api.json
📚 Learning: in the unraid api web components, sso-related props are intentionally provided in both camelcase (`s...
Learnt from: elibosley
PR: unraid/api#1063
File: web/components/SsoButton.ce.vue:5-8
Timestamp: 2025-01-27T14:31:42.305Z
Learning: In the Unraid API web components, SSO-related props are intentionally provided in both camelCase (`ssoEnabled`) and lowercase (`ssoenabled`) variants to support interchangeable usage across different contexts (e.g., HTML attributes vs Vue props).
Applied to files:
api/dev/configs/api.json
📚 Learning: applies to web/__test__/**/*.{test,spec}.{ts,js} : mock external dependencies and services in web/__...
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,js} : Mock external dependencies and services in web/__test__
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
📚 Learning: applies to web/__test__/store/**/*.ts : mock external dependencies appropriately in store tests...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Mock external dependencies appropriately in store tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
📚 Learning: applies to web/__test__/{components,store,mocks}/**/*.ts : mock external dependencies and services i...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Mock external dependencies and services in tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
📚 Learning: applies to api/**/*.{test,spec}.{js,ts,tsx} : prefer not to mock simple dependencies in tests...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-07-21T14:00:06.258Z
Learning: Applies to api/**/*.{test,spec}.{js,ts,tsx} : Prefer not to mock simple dependencies in tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
📚 Learning: applies to web/__test__/store/**/*.ts : test getter dependencies are properly mocked in store tests...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Test getter dependencies are properly mocked in store tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
📚 Learning: applies to web/__test__/{components,store,mocks}/**/*.ts : clear mocks between tests to ensure isola...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Clear mocks between tests to ensure isolation
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts
📚 Learning: applies to web/__test__/{components,store,mocks}/**/*.ts : remember that `vi.mock()` calls are hoist...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Remember that `vi.mock()` calls are hoisted
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/organizer/organizer.test.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
📚 Learning: applies to web/__test__/{components,store,mocks}/**/*.ts : use `vi.mock()` for module-level mocks...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Use `vi.mock()` for module-level mocks
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/organizer/organizer.test.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
📚 Learning: applies to web/__test__/{components,store,mocks}/**/*.ts : use factory functions for module mocks to...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Use factory functions for module mocks to avoid hoisting issues
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
📚 Learning: applies to web/__test__/**/*store*.{test,spec}.{ts,js} : use factory functions for module mocks to a...
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/__test__/**/*store*.{test,spec}.{ts,js} : Use factory functions for module mocks to avoid hoisting issues in store tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts
📚 Learning: applies to api/**/*.{test,spec}.{ts,js} : test suite is vitest, do not use jest in the api package...
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to api/**/*.{test,spec}.{ts,js} : Test suite is VITEST, do not use jest in the API package
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/organizer/organizer.test.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/organizer/organizer.resolution.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts
📚 Learning: applies to api/**/*.{test,spec}.{js,ts,tsx} : use vitest as the test suite; do not use jest...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-07-21T14:00:06.258Z
Learning: Applies to api/**/*.{test,spec}.{js,ts,tsx} : Use Vitest as the test suite; do not use Jest
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/organizer/organizer.resolution.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts
📚 Learning: applies to web/__test__/{components,store,mocks}/**/*.ts : reset mocks between tests with `vi.cleara...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Reset mocks between tests with `vi.clearAllMocks()`
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
📚 Learning: applies to web/__test__/{components,store,mocks}/**/*.ts : specify return values for component metho...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Specify return values for component methods with `vi.spyOn()`
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
📚 Learning: applies to web/__test__/store/**/*.ts : verify state changes by updating the store in store tests...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Verify state changes by updating the store in store tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/organizer/organizer.test.tsapi/src/unraid-api/organizer/organizer.resolution.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts
📚 Learning: applies to web/__test__/components/**/*.ts : use `mount` from vue test utils for component testing...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/components/**/*.ts : Use `mount` from Vue Test Utils for component testing
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts
📚 Learning: connect configuration files like api/dev/configs/connect.json use their own naming conventions (e.g....
Learnt from: elibosley
PR: unraid/api#1425
File: api/dev/configs/connect.json:5-13
Timestamp: 2025-06-20T20:50:14.050Z
Learning: Connect configuration files like api/dev/configs/connect.json use their own naming conventions (e.g., apikey, accesstoken, idtoken, refreshtoken) as implementation details, and these should not be changed to follow general camelCase patterns.
Applied to files:
api/dev/connectStatus.json
📚 Learning: the json scalar type in web/codegen.ts was temporarily changed from 'string' to 'any' for compatibil...
Learnt from: pujitm
PR: unraid/api#1211
File: web/codegen.ts:14-14
Timestamp: 2025-03-12T13:35:43.900Z
Learning: The JSON scalar type in web/codegen.ts was temporarily changed from 'string' to 'any' for compatibility with JsonForms integration. This change facilitates the implementation of the Connect settings web component.
Applied to files:
api/dev/connectStatus.jsonapi/src/unraid-api/organizer/organizer.dto.ts
📚 Learning: error handling in mutations like `addroleforuser` in `auth.resolver.ts` is performed at the service ...
Learnt from: mdatelle
PR: unraid/api#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/docker/docker.resolver.ts
📚 Learning: applies to web/__test__/store/**/*.ts : verify state changes after actions in store tests...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Verify state changes after actions in store tests
Applied to files:
api/src/unraid-api/organizer/organizer.test.tsapi/src/unraid-api/organizer/organizer.resolution.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts
📚 Learning: applies to web/__test__/store/**/*.ts : verify proper error handling in store tests...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Verify proper error handling in store tests
Applied to files:
api/src/unraid-api/organizer/organizer.test.tsapi/src/unraid-api/organizer/organizer.resolution.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts
📚 Learning: applies to web/__test__/store/**/*.ts : test action side effects and state changes in store tests...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Test action side effects and state changes in store tests
Applied to files:
api/src/unraid-api/organizer/organizer.test.tsapi/src/unraid-api/organizer/organizer.resolution.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts
📚 Learning: applies to web/__test__/components/**/*.ts : verify that the expected elements are rendered in compo...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/components/**/*.ts : Verify that the expected elements are rendered in component tests
Applied to files:
api/src/unraid-api/organizer/organizer.test.ts
📚 Learning: applies to web/__test__/store/**/*.ts : test async operations completely in store tests...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Test async operations completely in store tests
Applied to files:
api/src/unraid-api/organizer/organizer.test.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/organizer/organizer.resolution.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts
📚 Learning: applies to web/__test__/store/**/*.ts : test store interactions with other stores...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Test store interactions with other stores
Applied to files:
api/src/unraid-api/organizer/organizer.test.tsapi/src/unraid-api/organizer/organizer.resolution.test.ts
📚 Learning: applies to web/__test__/**/*.{test,spec}.{ts,js} : test component behavior and output, not implement...
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,js} : Test component behavior and output, not implementation details in web/__test__
Applied to files:
api/src/unraid-api/organizer/organizer.test.tsapi/src/unraid-api/organizer/organizer.resolution.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts
📚 Learning: applies to web/__test__/store/**/*.ts : verify actions are called with correct parameters in store t...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Verify actions are called with correct parameters in store tests
Applied to files:
api/src/unraid-api/organizer/organizer.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts
📚 Learning: applies to web/__test__/components/**/*.ts : test component behavior and output, not implementation ...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/components/**/*.ts : Test component behavior and output, not implementation details
Applied to files:
api/src/unraid-api/organizer/organizer.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts
📚 Learning: applies to web/__test__/components/**/*.ts : verify element existence with `expect(element.exists())...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/components/**/*.ts : Verify element existence with `expect(element.exists()).toBe(true)`
Applied to files:
api/src/unraid-api/organizer/organizer.test.ts
📚 Learning: applies to web/__test__/store/**/*.ts : test action side effects if not stubbed in store tests...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/store/**/*.ts : Test action side effects if not stubbed in store tests
Applied to files:
api/src/unraid-api/organizer/organizer.test.ts
📚 Learning: when modifying `apikey.roles` in `removerolefromapikey` and `addroletoapikey` within `api/src/unraid...
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:0-0
Timestamp: 2024-11-04T20:44:46.432Z
Learning: When modifying `apiKey.roles` in `removeRoleFromApiKey` and `addRoleToApiKey` within `api/src/unraid-api/auth/auth.service.ts`, concurrency issues are not a concern because the keys are stored in the file system.
Applied to files:
packages/unraid-shared/src/services/config-file.ts
📚 Learning: applies to web/__test__/{components,store,mocks}/**/*.ts : don't mix mock declarations and module mo...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Don't mix mock declarations and module mocks incorrectly
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
📚 Learning: applies to web/__test__/{components,store,mocks}/**/*.ts : avoid relying on nuxt's auto-imports in t...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Avoid relying on Nuxt's auto-imports in test environment
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
📚 Learning: applies to web/__test__/{components,store,mocks}/**/*.ts : don't rely on nuxt auto-imports in tests...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Don't rely on Nuxt auto-imports in tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
📚 Learning: applies to web/__test__/components/**/*.ts : use `flushpromises()` for more complex promise chains i...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/components/**/*.ts : Use `flushPromises()` for more complex promise chains in async tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/organizer/organizer.resolution.test.ts
📚 Learning: applies to web/__test__/{components,store,mocks}/**/*.ts : always await async operations before maki...
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Always await async operations before making assertions
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
📚 Learning: the project prefers using class-validator decorators directly on entity classes rather than separate...
Learnt from: elibosley
PR: unraid/api#1352
File: packages/unraid-api-plugin-connect/src/config.entity.ts:37-39
Timestamp: 2025-04-21T18:44:24.383Z
Learning: The project prefers using class-validator decorators directly on entity classes rather than separate Zod schemas to maintain type consistency between GraphQL entities and validation rules.
Applied to files:
api/src/unraid-api/organizer/organizer.dto.ts
📚 Learning: applies to web/**/*.{query,mutation}.ts : all graphql under `web/` must follow this naming conventio...
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,mutation}.ts : All GraphQL under `web/` must follow this naming convention
Applied to files:
api/src/unraid-api/organizer/organizer.dto.ts
📚 Learning: applies to web/**/*.{query,mutation}.ts : all graphql under web/ must follow this naming convention ...
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/**/*.{query,mutation}.ts : All GraphQL under web/ must follow this naming convention (.query.ts for queries, .mutation.ts for mutations)
Applied to files:
api/src/unraid-api/organizer/organizer.dto.ts
📚 Learning: applies to web/**/*.mutation.ts : graphql mutations must be placed in `.mutation.ts` files...
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/**/*.mutation.ts : GraphQL mutations must be placed in `.mutation.ts` files
Applied to files:
api/src/unraid-api/organizer/organizer.dto.ts
📚 Learning: applies to web/**/*.query.ts : graphql queries must be placed in `.query.ts` files...
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:
api/src/unraid-api/organizer/organizer.dto.ts
📚 Learning: input types for graphql mutations can be pulled from the graphql codegen types, as they are generate...
Learnt from: elibosley
PR: unraid/api#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/organizer/organizer.dto.ts
📚 Learning: applies to web/**/*.mutation.ts : graphql mutations must be placed in .mutation.ts files under web/...
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/**/*.mutation.ts : GraphQL mutations must be placed in .mutation.ts files under web/
Applied to files:
api/src/unraid-api/organizer/organizer.dto.ts
📚 Learning: for the unraid api project, class-validator should be used for validation to avoid mismatches betwee...
Learnt from: elibosley
PR: unraid/api#1352
File: packages/unraid-api-plugin-connect/src/config.entity.ts:16-26
Timestamp: 2025-04-21T18:44:15.414Z
Learning: For the Unraid API project, class-validator should be used for validation to avoid mismatches between different validation schemas (like Zod).
Applied to files:
api/src/unraid-api/organizer/organizer.dto.ts
🪛 GitHub Actions: CI - Main (API)
api/src/unraid-api/organizer/organizer.resolution.test.ts
[error] 77-77: TypeScript error TS2339: Property 'children' does not exist on type 'ResolvedOrganizerEntryType'.
🔇 Additional comments (36)
packages/unraid-shared/src/services/config-file.ts (2)
69-89: LGTM! Well-implemented type-safe config retrieval.The method overloads provide excellent type safety, and the implementation correctly handles both error cases and immutability through
structuredClone. The fallback todefaultConfig()whenassertExistsis false is a sensible design choice.
91-99: LGTM! Clean config replacement with immediate persistence.The method correctly uses the custom ConfigService's
set()method and ensures immediate persistence. The implementation is straightforward and follows the class's established patterns.api/dev/configs/api.json (1)
2-6: LGTM! Configuration updates align with new features.The version bump to 4.12.0, sandbox enablement, and addition of the
unraid-api-plugin-connectplugin all align well with the new Docker organizer and UPS management features introduced in this PR.api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts (2)
6-6: LGTM! Import added for new service dependency.The import statement correctly brings in the new
DockerOrganizerServicethat needs to be mocked in the test setup.
26-31: LGTM! Proper mock setup for new service dependency.The mock provider for
DockerOrganizerServicefollows the established pattern in the test file and correctly stubs thegetResolvedOrganizermethod using Vitest'svi.fn().api/src/unraid-api/graph/resolvers/docker/docker.module.ts (2)
3-4: LGTM! Proper imports for new services.The import statements correctly bring in the new
DockerConfigServiceandDockerOrganizerServicewith proper ESM .js extensions.
13-14: LGTM! Clean integration of new services.The new services are properly added to the providers array following the existing organization pattern. They're correctly scoped as internal services (not exported) which is appropriate for their usage within the Docker resolver.
api/dev/connectStatus.json (1)
1-7: LGTM! Well-structured connection status configuration.The JSON structure appropriately captures connection state information with logical fields for status tracking, error reporting, and diagnostics. The placement in the
dev/directory indicates this is for development/testing purposes, which aligns with the error state shown.api/src/unraid-api/graph/resolvers/docker/docker-config.service.ts (1)
29-38: LGTM! Default organizer structure is well-defined.The addition of a default organizer view with proper folder structure provides a solid foundation for the new organizer system. The structure correctly defines a root folder entry with the expected properties.
api/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts (3)
1-1: LGTM! New service imports are correctly added.The imports for
DockerConfigServiceandDockerOrganizerServiceare properly added to support the expanded Docker module functionality.Also applies to: 6-6, 8-8
21-22: LGTM! Mock override for DockerConfigService is appropriate.The mock implementation provides the necessary
getConfigmethod for testing the module compilation.
60-64: LGTM! DockerResolver test properly mocks new dependency.The test correctly includes the
DockerOrganizerServicemock provider alongside the existingDockerServicemock, following the coding guideline to use simple mocks for dependencies.api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (3)
10-10: LGTM! Imports for organizer functionality are correct.The imports for
DockerOrganizerServiceandResolvedOrganizerV1properly support the new organizer field.Also applies to: 17-17
21-24: LGTM! Constructor properly injects the new service.The constructor correctly injects the
DockerOrganizerServicefollowing NestJS dependency injection patterns.
62-70: LGTM! Organizer resolve field is properly implemented.The new
organizer()method correctly:
- Uses the same READ permissions on DOCKER resource as other fields
- Returns the appropriate
ResolvedOrganizerV1type- Delegates to the service method following the established pattern
api/src/unraid-api/organizer/organizer.test.ts (5)
1-11: LGTM! Test imports and setup are correct.The test file properly imports Vitest testing utilities and the necessary organizer types and functions.
12-31: LGTM! Mock resource setup is well-structured.The mock resources provide good test data coverage with different resource types (container, vm) and varying metadata.
32-62: LGTM! Empty view test case is comprehensive.This test properly verifies that missing resources are added to an empty view, including:
- Root folder creation with correct structure
- Resource reference entries with proper targets
- Correct folder children array
184-204: LGTM! Immutability test is crucial.This test ensures the function doesn't mutate the original view object, which is important for preventing side effects. The assertions verify both object identity and structural integrity.
238-265: LGTM! Edge case handling for different key/id is excellent.This test covers the important scenario where the resource key differs from the resource's internal ID, ensuring the reference target points to the correct resource ID.
api/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts (4)
1-9: LGTM! Test imports are correct.The test file properly imports Vitest utilities and the necessary Docker-related types and functions.
11-62: LGTM! Comprehensive transformation test.This test thoroughly verifies the
containerToResourcetransformation with full container data, ensuring all properties are correctly mapped fromDockerContainertoOrganizerResource.
64-83: LGTM! Edge case for empty names array is well-tested.This test correctly verifies the fallback behavior when the names array is empty, using the image name as the resource name.
119-157: LGTM! Multiple ports test provides good coverage.This test verifies handling of containers with complex port configurations and multiple labels, ensuring the transformation maintains all port and metadata information.
api/generated-schema.graphql (1)
1160-1163: LGTM! Schema changes align with the implementation.The auto-generated schema correctly reflects the new organizer types and the Docker type extension. The use of JSON scalar for the views field is appropriate for the dynamic structure.
Also applies to: 1233-1233
api/src/unraid-api/organizer/organizer.dto.ts (11)
1-17: LGTM! Imports are well-organized and properly used.The imports follow a logical order and all imported items are utilized in the code. The use of
GraphQLJSONscalar is appropriate for flexible object fields.
20-38: Well-structured resource DTO with proper validation.The
OrganizerResourceclass is properly defined with appropriate GraphQL decorators and validation rules. The use ofGraphQLJSONfor the optional meta field is correct for flexible metadata storage.
41-59: Folder DTO correctly implements type constraints and array validation.Good use of
@IsIn(['folder'])to enforce the literal type and@IsString({ each: true })to validate each array element. The GraphQL array field declaration is properly done.
61-74: Resource reference DTO follows consistent patterns.The class properly implements a reference type with appropriate validation. The structure is consistent with other DTOs in the file.
76-77: Union type provides good TypeScript type safety.The
OrganizerEntryunion type is well-defined for compile-time type checking. The comment appropriately clarifies its purpose.
107-125: Root organizer structure is well-designed with proper versioning.Good use of
@Equals(1)for version constraint, which will help with future schema evolution. The nested validation setup for resources and views dictionaries is correctly implemented.
132-150: Resolved folder structure correctly implements nested object pattern.The transformation from ID-based to object-based children is well-implemented. The use of the GraphQL union type in the Field decorator is correct.
153-162: GraphQL union type is correctly implemented with proper type resolution.The
resolveTypefunction correctly identifies the concrete type based on thetypefield. Good use ofas constfor type safety.
164-165: TypeScript union type correctly mirrors the GraphQL union.This type definition ensures type safety in TypeScript code when working with resolved entries.
168-186: Resolved view structure properly simplifies to a root object pattern.Good design decision to remove the
entriesdictionary in the resolved version since all data is accessible through the nestedrootstructure. The GraphQL union type is correctly applied.
189-201: Final resolved structure correctly eliminates redundancy.Smart design to remove the
resourcesfield in the resolved version since all resources are now embedded within the resolved view structures. This eliminates data duplication and simplifies the API.
api/src/unraid-api/graph/resolvers/docker/docker-organizer.service.ts
Outdated
Show resolved
Hide resolved
| export function resolveOrganizer(organizer: OrganizerV1): ResolvedOrganizerV1 { | ||
| const resolvedViews: { [id: string]: ResolvedOrganizerView } = {}; | ||
|
|
||
| for (const [viewId, view] of Object.entries(organizer.views)) { | ||
| resolvedViews[viewId] = resolveOrganizerView(view, organizer.resources); | ||
| } | ||
|
|
||
| return { | ||
| version: 1, | ||
| views: resolvedViews, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Preserve the version from the input organizer.
The function hardcodes version: 1 in the output, but it should preserve the version from the input organizer for consistency.
return {
- version: 1,
+ version: organizer.version,
views: resolvedViews,
};📝 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.
| export function resolveOrganizer(organizer: OrganizerV1): ResolvedOrganizerV1 { | |
| const resolvedViews: { [id: string]: ResolvedOrganizerView } = {}; | |
| for (const [viewId, view] of Object.entries(organizer.views)) { | |
| resolvedViews[viewId] = resolveOrganizerView(view, organizer.resources); | |
| } | |
| return { | |
| version: 1, | |
| views: resolvedViews, | |
| }; | |
| } | |
| export function resolveOrganizer(organizer: OrganizerV1): ResolvedOrganizerV1 { | |
| const resolvedViews: { [id: string]: ResolvedOrganizerView } = {}; | |
| for (const [viewId, view] of Object.entries(organizer.views)) { | |
| resolvedViews[viewId] = resolveOrganizerView(view, organizer.resources); | |
| } | |
| return { | |
| - version: 1, | |
| + version: organizer.version, | |
| views: resolvedViews, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In api/src/unraid-api/organizer/organizer.ts around lines 139 to 150, the
function resolveOrganizer currently sets the output version to a hardcoded value
of 1. To fix this, modify the return statement to use the version property from
the input organizer object instead of the fixed number, ensuring the output
preserves the input organizer's version for consistency.
|
Claude finished @pujitm's task —— View job Code Review Complete ✅I've completed a thorough review of this PR which adds docker organizer query functionality to the Unraid API. Here's my analysis: Todo List:
SummaryThis PR introduces a well-architected organizer system for Docker containers that allows users to organize containers into custom views with folders and references. The implementation follows NestJS best practices and includes comprehensive validation and testing. ✅ StrengthsArchitecture & Design
Code Quality
Test Coverage
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
api/src/unraid-api/graph/resolvers/docker/docker-organizer.service.ts (3)
23-31: Add safety check for empty names array.The code assumes
container.namesalways has at least one element. If the array is empty,container.names[0]will be undefined.- const stableRef = container.names[0] || container.image; + const stableRef = container.names?.[0] || container.image || container.id;This ensures we always have a valid name, falling back to image and then to id if necessary.
55-70: Avoid mutating input parameters and fix async/await usage.The method has two issues:
- It mutates the input
organizerobject, which can lead to unexpected side effects- It's marked as
asyncbut only usesawaitconditionallyasync syncDefaultView( organizer: OrganizerV1, resources?: OrganizerV1['resources'] ): Promise<OrganizerView> { const view = organizer.views.default ?? { id: 'default', name: 'Default', root: 'root', entries: {}, }; - resources ??= await this.getResources(); + const resolvedResources = resources ?? await this.getResources(); - const updatedView = addMissingResourcesToView(resources, view); - organizer.views.default = updatedView; + const updatedView = addMissingResourcesToView(resolvedResources, view); return updatedView; }If you need to update the organizer, consider returning a new object or making the mutation explicit in the method name (e.g.,
syncAndUpdateDefaultView).
72-77: Fix incorrect usage of syncDefaultView.The
getOrganizermethod callssyncDefaultViewbut doesn't use its return value or await it properly. Also, it relies on the side effect ofsyncDefaultViewmutating the organizer.async getOrganizer(): Promise<OrganizerV1> { const organizer = this.dockerConfigService.getConfig(); organizer.resources = await this.getResources(); - this.syncDefaultView(organizer, organizer.resources); + organizer.views.default = await this.syncDefaultView(organizer, organizer.resources); return organizer; }
🧹 Nitpick comments (2)
api/src/unraid-api/organizer/organizer.dto.ts (2)
128-152: Consider improving type safety for entries validation.The view class is well-structured, but the
@Type(() => Object)decorator on line 145 is too generic. Consider creating a custom transformer or validator for better type safety.- @Type(() => Object) // we'll validate the values below + // Consider implementing a custom transformer that properly validates each entry entries!: { [id: string]: OrganizerFolder | OrganizerResourceRef };You could implement a custom decorator that validates each entry is either an OrganizerFolder or OrganizerResourceRef.
155-173: Improve type safety for resources field.The root organizer class has good version validation, but the resources field uses a generic
Objecttype decorator which reduces type safety.Consider implementing proper type validation for the resources map to ensure each value conforms to the AnyOrganizerResource union type.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**
📒 Files selected for processing (7)
api/.gitignore(1 hunks)api/generated-schema.graphql(6 hunks)api/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/docker/docker-organizer.service.ts(1 hunks)api/src/unraid-api/organizer/organizer.dto.ts(4 hunks)api/src/unraid-api/organizer/organizer.resolution.test.ts(1 hunks)api/src/unraid-api/organizer/organizer.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- api/.gitignore
- api/src/unraid-api/organizer/organizer.resolution.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- api/src/unraid-api/graph/resolvers/docker/docker-organizer.service.spec.ts
- api/src/unraid-api/organizer/organizer.ts
🧰 Additional context used
📓 Path-based instructions (2)
api/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*: Use pnpm as the only package manager
Always run scripts from api/package.json unless specifically requested otherwise
Files:
api/src/unraid-api/graph/resolvers/docker/docker-organizer.service.tsapi/src/unraid-api/organizer/organizer.dto.tsapi/generated-schema.graphql
api/src/unraid-api/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the NestJS repository located at api/src/unraid-api/ instead of the legacy code
Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
Files:
api/src/unraid-api/graph/resolvers/docker/docker-organizer.service.tsapi/src/unraid-api/organizer/organizer.dto.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: pujitm
PR: unraid/api#1352
File: packages/unraid-api-plugin-generator/src/create-plugin.ts:91-112
Timestamp: 2025-04-21T18:27:36.482Z
Learning: For utility functions like file operations in api plugin generators, pujitm prefers minimal error handling rather than verbose try/catch blocks for each operation, as errors will propagate properly to higher level error handlers.
📚 Learning: when reviewing configuration files in refactoring prs, avoid suggesting field name changes that coul...
Learnt from: elibosley
PR: unraid/api#1425
File: api/dev/configs/connect.json:2-3
Timestamp: 2025-06-20T21:05:24.582Z
Learning: When reviewing configuration files in refactoring PRs, avoid suggesting field name changes that could break compatibility with existing code that references those fields, even if the naming doesn't follow ideal conventions.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker-organizer.service.ts
📚 Learning: in the unraid api project, the duplicate graphql query and mutation strings in gql.ts files are inte...
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:
api/src/unraid-api/organizer/organizer.dto.ts
📚 Learning: the project prefers using class-validator decorators directly on entity classes rather than separate...
Learnt from: elibosley
PR: unraid/api#1352
File: packages/unraid-api-plugin-connect/src/config.entity.ts:37-39
Timestamp: 2025-04-21T18:44:24.383Z
Learning: The project prefers using class-validator decorators directly on entity classes rather than separate Zod schemas to maintain type consistency between GraphQL entities and validation rules.
Applied to files:
api/src/unraid-api/organizer/organizer.dto.ts
📚 Learning: the json scalar type in web/codegen.ts was temporarily changed from 'string' to 'any' for compatibil...
Learnt from: pujitm
PR: unraid/api#1211
File: web/codegen.ts:14-14
Timestamp: 2025-03-12T13:35:43.900Z
Learning: The JSON scalar type in web/codegen.ts was temporarily changed from 'string' to 'any' for compatibility with JsonForms integration. This change facilitates the implementation of the Connect settings web component.
Applied to files:
api/src/unraid-api/organizer/organizer.dto.ts
📚 Learning: applies to web/**/*.{query,mutation}.ts : all graphql under `web/` must follow this naming conventio...
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,mutation}.ts : All GraphQL under `web/` must follow this naming convention
Applied to files:
api/src/unraid-api/organizer/organizer.dto.ts
📚 Learning: applies to web/**/*.{query,mutation}.ts : all graphql under web/ must follow this naming convention ...
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:34:24.715Z
Learning: Applies to web/**/*.{query,mutation}.ts : All GraphQL under web/ must follow this naming convention (.query.ts for queries, .mutation.ts for mutations)
Applied to files:
api/src/unraid-api/organizer/organizer.dto.ts
📚 Learning: in the unraid api project, error handling for mutations is handled at the service level rather than ...
Learnt from: mdatelle
PR: unraid/api#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/organizer/organizer.dto.ts
📚 Learning: input types for graphql mutations can be pulled from the graphql codegen types, as they are generate...
Learnt from: elibosley
PR: unraid/api#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/organizer/organizer.dto.ts
📚 Learning: for the unraid api project, class-validator should be used for validation to avoid mismatches betwee...
Learnt from: elibosley
PR: unraid/api#1352
File: packages/unraid-api-plugin-connect/src/config.entity.ts:16-26
Timestamp: 2025-04-21T18:44:15.414Z
Learning: For the Unraid API project, class-validator should be used for validation to avoid mismatches between different validation schemas (like Zod).
Applied to files:
api/src/unraid-api/organizer/organizer.dto.ts
📚 Learning: in the unraid api project's theme system, exact typescript type definitions are preferred over index...
Learnt from: elibosley
PR: unraid/api#1181
File: web/store/theme.ts:0-0
Timestamp: 2025-02-24T14:51:21.328Z
Learning: In the Unraid API project's theme system, exact TypeScript type definitions are preferred over index signatures for theme variables to ensure better type safety.
Applied to files:
api/src/unraid-api/organizer/organizer.dto.ts
🧬 Code Graph Analysis (1)
api/src/unraid-api/organizer/organizer.dto.ts (1)
api/src/unraid-api/cli/generated/graphql.ts (7)
OrganizerResource(1140-1146)OrganizerContainerResource(1132-1138)DockerContainer(685-703)ResolvedOrganizerFolder(1477-1483)ResolvedOrganizerEntry(1475-1475)ResolvedOrganizerView(1491-1497)ResolvedOrganizerV1(1485-1489)
🔇 Additional comments (13)
api/src/unraid-api/graph/resolvers/docker/docker-organizer.service.ts (3)
33-41: LGTM!The function correctly transforms the container list into a resources object keyed by container ID.
50-53: LGTM!The method correctly retrieves containers and transforms them into the resources object format.
79-82: LGTM!The method correctly retrieves the organizer and applies the resolution logic.
api/src/unraid-api/organizer/organizer.dto.ts (7)
23-28: Well-structured base interface.Good use of a base interface to define common fields for all resource types. The
unknowntype formetaallows for proper specialization in implementations.
31-49: LGTM!The generic resource class is well-implemented with proper GraphQL field decorators and validation rules. Good use of GraphQLJSON for the untyped meta field.
52-71: Excellent type-safe implementation for container resources.The specialized container resource class properly extends the base interface with:
- Strict type validation using
@IsIn(['container'])- Type-safe meta field with
DockerContainertype- Proper class transformation decorators
74-86: Well-implemented union type for resources.Good implementation of both TypeScript and GraphQL union types. The
resolveTypefunction correctly discriminates between container-specific and generic resources based on the type field.
89-107: LGTM!The folder class is well-structured with proper validation. Correctly uses string array for children IDs in the non-resolved version.
109-122: LGTM!The resource reference class is well-implemented with proper validation for referencing resources by ID.
175-252: Excellent implementation of resolved DTOs.The resolved types provide a clean architectural separation between:
- Storage format (ID-based references)
- API response format (nested object structures)
This pattern makes it easier for frontend consumption while maintaining efficient storage.
api/generated-schema.graphql (3)
1228-1264: Schema correctly reflects the organizer DTOs.The GraphQL schema additions for the organizer feature properly match the DTO definitions:
- Non-nullable
organizerfield onDockertype ensures consistent API behavior- Union types and resolved structures align with the TypeScript implementations
- Proper use of GraphQL types (String!, Float!, JSON, etc.)
1434-1562: Excellent UPS type definitions with comprehensive documentation.The UPS schema additions are well-structured with:
- Detailed field descriptions explaining units and examples
- Proper nesting of related data (battery, power)
- Appropriate use of nullable/non-nullable fields
1837-1839: Well-designed UPS API operations.The UPS operations provide a complete API surface:
- Query operations for listing and fetching UPS devices
- Mutation for configuration with strongly-typed input
- Subscription for real-time updates
- Enums provide excellent type safety for configuration values
Also applies to: 1879-1879, 1921-1995, 2082-2082
elibosley
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't leave any feedback because I didn't see anything that struck out, this seems good to me.
|
Claude finished @pujitm's task —— View job Code Review Complete ✅I've completed a thorough review of this PR which adds docker organizer query functionality to the Unraid API. Here's my analysis: Todo List:
SummaryThis PR introduces a well-architected organizer system for Docker containers that allows users to organize containers into custom views with folders and references. The implementation follows NestJS best practices and includes comprehensive validation and testing. ✅ StrengthsArchitecture & Design
Code Quality
Test Coverage
|
|
Claude finished @pujitm's task —— View job Code Review Complete ✅I've completed a thorough review of this PR which adds docker organizer query functionality to the Unraid API. Here's my analysis: Todo List:
SummaryThis PR introduces a well-architected organizer system for Docker containers that allows users to organize containers into custom views with folders and references. The implementation follows NestJS best practices and includes comprehensive validation and testing. ✅ StrengthsArchitecture & Design
Code Quality
Test Coverage
|
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores