fix(docker): sync template mappings in organizer to prevent false orphan warnings #1866
Conversation
…han warnings When a new container is installed, the GraphQL query fetches both containers and organizer data. The containers resolver correctly syncs template mappings before returning data, but the organizer service was fetching containers independently without syncing. This caused a race condition where the organizer showed isOrphaned=true for newly installed containers until page refresh. Added template sync to DockerOrganizerService.getResources() to ensure containers have correct template mappings before being returned. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughIntroduces RawDockerContainer handling and an enrichWithOrphanStatus flow, switches callers from getContainers to getRawContainers and adds DockerTemplateScannerService sync calls; updates tests, GraphQL deprecations for skipCache, a noRetry guard in Apollo retry logic, and frontend error UI for orphan removal. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Resolver as DockerResolver / OrganizerService
participant Docker as DockerService
participant Scanner as DockerTemplateScannerService
Client->>Resolver: request containers/organizer
Resolver->>Docker: getRawContainers(...)
Docker-->>Resolver: rawContainers[]
Resolver->>Scanner: syncMissingContainers(rawContainers)
Scanner-->>Resolver: synced? (boolean)
Resolver->>Docker: enrichWithOrphanStatus(rawContainers)
Docker-->>Resolver: enrichedContainers[] (with isOrphaned, templatePath)
Resolver-->>Client: enrichedContainers[]
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1866 +/- ##
==========================================
- Coverage 46.43% 46.42% -0.01%
==========================================
Files 954 954
Lines 59755 59791 +36
Branches 5544 5542 -2
==========================================
+ Hits 27746 27759 +13
- Misses 31890 31913 +23
Partials 119 119 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (1)
89-122: Preserve size parameter on container refetch after template sync.Line 121 refetches containers without the
sizeparameter that was determined in lines 98-102. WhenwasSyncedis true, the refetched containers will lack the size data ifsizeRootFsorsizeRwfields were requested. Thedocker-organizer.service.tsfile demonstrates the correct pattern: preserve the options object and reuse it in the refetch call.Suggested fix
const requestsRootFsSize = GraphQLFieldHelper.isFieldRequested(info, 'sizeRootFs'); const requestsRwSize = GraphQLFieldHelper.isFieldRequested(info, 'sizeRw'); const requestsLogSize = GraphQLFieldHelper.isFieldRequested(info, 'sizeLog'); + const shouldFetchSize = requestsRootFsSize || requestsRwSize; const containers = await this.dockerService.getContainers({ - size: requestsRootFsSize || requestsRwSize, + size: shouldFetchSize, }); if (requestsLogSize) { const names = Array.from( new Set( containers .map((container) => container.names?.[0]?.replace(/^\//, '') || null) .filter((name): name is string => Boolean(name)) ) ); const logSizes = await this.dockerService.getContainerLogSizes(names); containers.forEach((container) => { const normalized = container.names?.[0]?.replace(/^\//, '') || ''; container.sizeLog = normalized ? (logSizes.get(normalized) ?? 0) : 0; }); } const wasSynced = await this.dockerTemplateScannerService.syncMissingContainers(containers); - return wasSynced ? await this.dockerService.getContainers() : containers; + return wasSynced ? await this.dockerService.getContainers({ size: shouldFetchSize }) : containers;api/src/unraid-api/graph/resolvers/docker/docker.service.ts (1)
239-251: Fix useless initial assignment.The initial
getContainers()call on line 239 is never used since the loop unconditionally overwrites it on the first iteration. This was flagged by static analysis (CodeQL).🔧 Proposed fix
- let containers = await this.getContainers(); let updatedContainer: DockerContainer | undefined; for (let i = 0; i < 5; i++) { await sleep(500); - containers = await this.getContainers(); + const containers = await this.getContainers(); updatedContainer = containers.find((c) => c.id === id);
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
api/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.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.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts
💤 Files with no reviewable changes (1)
- api/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use TypeScript imports with.jsextensions for ESM compatibility
Never add comments unless they are needed for clarity of function
Never add comments for obvious things, and avoid commenting when starting and ending code blocks
Files:
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts
api/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer adding new files to the NestJS repo located at
api/src/unraid-api/instead of the legacy code
Files:
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use theanytype. Always prefer proper typing
Avoid using casting whenever possible, prefer proper typing from the start
Files:
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 expects TTL values in milliseconds, not seconds (e.g., 600000 for 10 minutes, not 600)
Files:
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-event.service.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts
🧠 Learnings (16)
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Applies to api/**/*.test.{ts,tsx} : Prefer to not mock simple dependencies
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Mock external services and API calls
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Mock external dependencies appropriately in Pinia store tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Test Pinia store getter dependencies are properly mocked
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Use factory functions for module mocks in Pinia store tests to avoid hoisting issues
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Mock external dependencies and services in Vue component tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Use `createPinia()` instead of `createTestingPinia()` for most Pinia store tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.ts
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to web/__test__/**/*.test.{ts,tsx} : Use `createPinia()` and `setActivePinia` when testing Store files, only use `createTestingPinia` if you specifically need its testing features
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Use `vi.mock()` for module-level mocks
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Reset mocks between tests with `vi.clearAllMocks()`
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.ts
📚 Learning: 2025-08-09T01:03:29.676Z
Learnt from: elibosley
Repo: unraid/api PR: 1575
File: packages/unraid-shared/src/services/socket-config.service.spec.ts:10-13
Timestamp: 2025-08-09T01:03:29.676Z
Learning: Vitest is used for all testing across all repositories in the unraid organization, not Jest. Always use `vi` for mocking utilities, not `jest`.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Applies to api/**/*.test.{ts,tsx} : Use Vitest for the test suite, not Jest
Applied to files:
api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: This is a Nuxt.js app but tests are run with vitest outside of the Nuxt environment
Applied to files:
api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use VITEST for test suite, not jest
Applied to files:
api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to web/__test__/**/*.test.{ts,tsx} : Use `mount` from Vue Test Utils for component testing
Applied to files:
api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Specify return values for component methods with `vi.spyOn()`
Applied to files:
api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
🧬 Code graph analysis (4)
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts (1)
api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (1)
containers(89-122)
api/src/unraid-api/graph/resolvers/docker/docker-network.service.ts (3)
api/src/unraid-api/graph/resolvers/docker/utils/docker-client.ts (1)
getDockerClient(5-12)api/src/unraid-api/cli/generated/graphql.ts (1)
DockerNetwork(900-917)web/src/composables/gql/graphql.ts (1)
DockerNetwork(924-941)
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts (3)
api/src/unraid-api/cli/generated/graphql.ts (1)
DockerContainer(734-785)web/src/composables/gql/graphql.ts (1)
DockerContainer(758-809)api/src/unraid-api/utils/graphql-field-helper.ts (1)
GraphQLFieldHelper(13-63)
api/src/unraid-api/graph/resolvers/docker/docker.service.ts (2)
api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (2)
containers(89-122)container(79-82)web/src/composables/gql/graphql.ts (4)
Docker(704-715)DockerContainer(758-809)DockerPortConflicts(949-953)DockerNetwork(924-941)
🪛 GitHub Check: CodeQL
api/src/unraid-api/graph/resolvers/docker/docker.service.ts
[warning] 239-239: Useless assignment to local variable
The initial value of containers is unused, since it is always overwritten.
🔇 Additional comments (28)
api/src/unraid-api/graph/resolvers/docker/docker-network.service.ts (1)
1-41: Inconsistency: Cache removal unrelated to stated PR objectives.The PR objectives state this fixes a race condition by syncing template mappings in
DockerOrganizerService.getResources(), but this file removes caching infrastructure fromDockerNetworkService. These changes appear to be part of a broader cache removal refactor that may have been bundled with the template sync fix.Verify that cache removal across Docker services was an intended part of this PR or if these changes should be in a separate PR focused on architectural refactoring.
api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts (1)
231-236: LGTM!The mock provider for
DockerTemplateScannerServiceis correctly structured and aligns with the PR objective of adding template mapping sync to the organizer service. The default mock behavior (returningfalsefromsyncMissingContainers) is appropriate for most test scenarios.api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts (1)
90-90: LGTM!Removing the
skipCacheparameter is consistent with the broader cache removal across Docker services.api/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts (1)
76-76: LGTM!Simplifying the
DockerServicemock to an empty object is correct given thatclearContainerCache()has been removed from the service interface.api/src/unraid-api/graph/resolvers/docker/docker-event.service.ts (1)
127-136: Verified:getAppInfo()returns fresh data without caching.The method directly calls
getContainers(), which queries the Docker API viathis.client.listContainers()on every invocation with no caching layer applied. The change from cache clearing to fetch+publish is correct.api/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.ts (1)
26-60: LGTM! Tests properly updated for cache removal.The test suite correctly reflects the simplified service API without caching. Mock setup and assertions are appropriate.
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts (2)
276-290: LGTM! Container fetching test correctly updated.The test properly verifies direct Docker API interaction without cache layer involvement.
318-345: LGTM! AppInfo test properly refactored.The test now correctly mocks direct container listing instead of relying on cache behavior. The assertions verify both the returned data structure and that the Docker API was called.
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.ts (2)
70-101: LGTM! Integration tests properly simplified.The removal of
CACHE_MANAGERmock andskipCacheparameters from test calls correctly reflects the production behavior. Integration tests now test real Docker daemon interactions without cache layer interference.
112-142: LGTM! Autostart configuration test properly updated.The test correctly calls
getContainers()without cache parameters and verifies real behavior with temp files.api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts (3)
124-164: LGTM! Container resolver test properly updated.The test correctly verifies that the resolver now passes a
{ size: boolean }parameter object togetContainers()instead of the removedskipCacheparameter. The assertion properly checks that size isfalsewhen no size-related fields are requested.
166-194: LGTM! Size field tests correctly verify conditional behavior.These tests properly verify that the resolver conditionally requests container size data based on GraphQL field selections (
sizeRootFsandsizeRw).
271-281: LGTM! Deprecation test correctly verifies backward compatibility.The test properly verifies that the deprecated
skipCacheparameter is now ignored, ensuring backward compatibility while moving away from caching.api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (5)
79-82: LGTM! Container lookup simplified correctly.The method now fetches containers directly without cache interaction, maintaining the same lookup logic.
145-154: LGTM! Networks resolver properly updated.The
skipCacheparameter is correctly deprecated and the service method call is simplified.
161-170: LGTM! Port conflicts resolver properly updated.The
skipCacheparameter is correctly deprecated and the service method call is simplified.
178-187: LGTM! Organizer resolver properly updated.The
skipCacheparameter is correctly deprecated and the service method call is simplified.
363-374: LGTM! Template reset mutation properly simplified.The mutation correctly resets template mappings without attempting to clear a cache that no longer exists.
api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts (3)
55-57: LGTM: Constructor properly injects the new dependency.The addition of
DockerTemplateScannerServicefollows NestJS DI patterns and enables the sync functionality needed to fix the race condition.
59-66: LGTM: Sync logic aligns with the containers resolver pattern.The conditional re-fetch after
syncMissingContainersmatches the pattern indocker.resolver.tsand correctly avoids unnecessary API calls when no sync occurred.
86-98: LGTM: Simplified method signatures after caching removal.The removal of
skipCacheparameters aligns with the broader PR goal of eliminating caching. The methods retain their core sync and resolve functionality.api/src/unraid-api/graph/resolvers/docker/docker.service.ts (7)
1-1: LGTM: Simplified imports after caching removal.The imports are now cleaner without the cache-manager dependency.
128-133: LGTM: Clean signature simplification.The destructuring with sensible defaults (
all = true,size = false) and rest spread for additional options is idiomatic TypeScript.
263-287: LGTM: Correct variable declaration pattern.Unlike
stop(), this method correctly declarescontainerswithout an initial assignment, avoiding the unnecessary fetch.
43-54: LGTM: Consistent use of fresh data fetch.
getAppInfo()now uses the simplifiedgetContainers()call without caching parameters.
162-184: LGTM: Simplified method signatures.Both
getPortConflicts()andgetNetworks()now delegate to their respective services with simplified interfaces, consistent with the caching removal.
186-197: LGTM: start() method correctly fetches fresh data.The method properly fetches containers after starting to return the updated state.
315-351: LGTM: updateContainer() fetches fresh data correctly.The method fetches containers before and after the update operation, ensuring accurate state is returned.
Replace text-warning color with text-foreground and text-muted-foreground for proper accessibility in both light and dark modes. The warning icon retains the warning color as visual indicator. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Skip Apollo retry link for orphan container removal mutations so errors display instantly in the UI instead of waiting through 20 retries. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web/src/helpers/create-apollo-client.ts (1)
88-91: LGTM! Clean implementation of conditional retry opt-out.The logic correctly allows operations to skip retries by setting
noRetry: truein their context, which aligns with the PR objective of surfacing orphan container removal errors immediately.Optional: Consider adding a type definition for better type safety
While Apollo Client contexts are typically untyped, you could add a type augmentation for better IDE support and to prevent typos:
// Add after the imports interface ApolloOperationContext { noRetry?: boolean; serverState?: { unraidApi?: { status?: string; }; }; }However, this is a minor enhancement since the current implementation follows standard Apollo Client patterns.
web/src/components/Docker/DockerOrphanedAlert.vue (1)
71-85: Error display works well; consider addingaria-labelfor accessibility.The inline error alert with dismissal is well-implemented and provides clear user feedback. The destructive color scheme appropriately signals the error state.
♿ Optional: Enhance accessibility of the dismiss button
Add an
aria-labelattribute to the dismiss button for better screen reader support:<button type="button" - class="hover:text-destructive/80 flex-shrink-0" + class="hover:text-destructive/80 flex-shrink-0" + aria-label="Dismiss error message" title="Dismiss" @click="dismissError" >
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/components/Docker/DockerOrphanedAlert.vueweb/src/helpers/create-apollo-client.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use TypeScript imports with.jsextensions for ESM compatibility
Never add comments unless they are needed for clarity of function
Never add comments for obvious things, and avoid commenting when starting and ending code blocks
Files:
web/src/helpers/create-apollo-client.ts
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/src/helpers/create-apollo-client.tsweb/src/components/Docker/DockerOrphanedAlert.vue
web/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Files:
web/src/helpers/create-apollo-client.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use theanytype. Always prefer proper typing
Avoid using casting whenever possible, prefer proper typing from the start
Files:
web/src/helpers/create-apollo-client.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
web/src/helpers/create-apollo-client.tsweb/src/components/Docker/DockerOrphanedAlert.vue
🧠 Learnings (6)
📚 Learning: 2024-11-01T17:52:55.311Z
Learnt from: pujitm
Repo: unraid/api PR: 941
File: web/components/Notifications/Item.vue:0-0
Timestamp: 2024-11-01T17:52:55.311Z
Learning: In the `web/components/Notifications/Item.vue` component, ensure that when delete or archive mutations fail, the notification remains displayed, and an error message is shown below the action buttons to inform the user.
Applied to files:
web/src/components/Docker/DockerOrphanedAlert.vue
📚 Learning: 2025-07-24T18:48:44.035Z
Learnt from: mdatelle
Repo: unraid/api PR: 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:
web/src/components/Docker/DockerOrphanedAlert.vue
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to web/__test__/**/*.test.{ts,tsx} : Use `mount` from Vue Test Utils for component testing
Applied to files:
web/src/components/Docker/DockerOrphanedAlert.vue
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Use `mount` from Vue Test Utils for component testing
Applied to files:
web/src/components/Docker/DockerOrphanedAlert.vue
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Nuxt is set to auto import so some Vue files may need `computed` or `ref` imported explicitly in store files
Applied to files:
web/src/components/Docker/DockerOrphanedAlert.vue
📚 Learning: 2024-12-13T17:28:15.384Z
Learnt from: pujitm
Repo: unraid/api PR: 982
File: web/helpers/apollo-cache/index.ts:142-142
Timestamp: 2024-12-13T17:28:15.384Z
Learning: In `web/helpers/apollo-cache/index.ts`, it's acceptable to use `structuredClone` since the project targets Node.js v20 and browsers where `structuredClone` is supported.
Applied to files:
web/src/components/Docker/DockerOrphanedAlert.vue
🔇 Additional comments (4)
web/src/components/Docker/DockerOrphanedAlert.vue (4)
2-2: LGTM! Error state properly initialized.The addition of the
refimport and theerrorMessagestate ref are clean and correctly typed.Also applies to: 27-27
33-33: Good error handling pattern.Clearing the error message before the operation and capturing detailed error information aligns well with user-focused error handling. The error message includes the container name for context, which improves the user experience.
Based on learnings, this properly ensures that mutation failures display an error message to inform the user.
Also applies to: 38-41
44-46: LGTM! Clean error dismissal.The
dismissErrorfunction is simple and appropriate for clearing the error state.
54-54: LGTM! Consistent styling updates.The changes to use
text-foregroundandtext-muted-foregroundclasses improve theme consistency across the component.Also applies to: 63-63, 66-66, 94-94
Reduces Docker API calls from 3 to 1 when syncing template mappings by separating isOrphaned computation from container fetching: - Add RawDockerContainer type (without isOrphaned/templatePath) - Add enrichWithOrphanStatus() to compute orphan status from config - Add getRawContainers() to fetch containers without enrichment - Update callers to use raw -> sync -> enrich pattern Previously, getContainers() computed isOrphaned inline, requiring a refetch after sync to get updated values. Now enrichment is a separate step that reads current config, eliminating redundant Docker API calls. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (1)
114-119: Consider extracting the sizeLog type augmentation.The type cast
(container as { sizeLog?: number })works but mutates the raw container objects directly. This is acceptable here since the objects are short-lived, but for clarity you could consider:
- Extending
RawDockerContainerwith an optionalsizeLogfield, or- Creating a mapped type during enrichment
This is minor since the current approach is functional and contained within this resolver method.
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.ts (1)
99-103: Test data type could be updated to RawDockerContainer for accuracy.The test containers are cast as
DockerContainerbut the service now expectsRawDockerContainer. SinceRawDockerContaineris a strict subset (omitsisOrphanedandtemplatePath), the current casts work, but updating toas RawDockerContainerwould be more precise.This is optional since the tests pass and the behavior is correct.
Also applies to: 115-120, 132-137, 148-153, 163-167
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts (1)
203-215: Test data types could be updated for consistency.Several tests declare
mockContainers: DockerContainer[]but pass them togetRawContainerswhich should returnRawDockerContainer[]. While this works due to TypeScript structural typing, updating to the correct type would improve clarity:-const mockContainers: DockerContainer[] = []; +const mockRawContainers: RawDockerContainer[] = [];This is optional as the tests function correctly.
Also applies to: 217-250, 252-276, 278-288
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.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.service.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use TypeScript imports with.jsextensions for ESM compatibility
Never add comments unless they are needed for clarity of function
Never add comments for obvious things, and avoid commenting when starting and ending code blocks
Files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts
api/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer adding new files to the NestJS repo located at
api/src/unraid-api/instead of the legacy code
Files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use theanytype. Always prefer proper typing
Avoid using casting whenever possible, prefer proper typing from the start
Files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 expects TTL values in milliseconds, not seconds (e.g., 600000 for 10 minutes, not 600)
Files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts
🧠 Learnings (16)
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Mock external services and API calls
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Applies to api/**/*.test.{ts,tsx} : Prefer to not mock simple dependencies
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Use `vi.mock()` for module-level mocks
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-08-09T01:03:29.676Z
Learnt from: elibosley
Repo: unraid/api PR: 1575
File: packages/unraid-shared/src/services/socket-config.service.spec.ts:10-13
Timestamp: 2025-08-09T01:03:29.676Z
Learning: Vitest is used for all testing across all repositories in the unraid organization, not Jest. Always use `vi` for mocking utilities, not `jest`.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Reset mocks between tests with `vi.clearAllMocks()`
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-07-24T18:48:44.035Z
Learnt from: mdatelle
Repo: unraid/api PR: 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-template-scanner.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Mock external dependencies and services in Vue component tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Specify return values for component methods with `vi.spyOn()`
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Mock external dependencies appropriately in Pinia store tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Test Pinia store getter dependencies are properly mocked
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Use factory functions for module mocks in Pinia store tests to avoid hoisting issues
Applied to files:
api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Place all mock declarations at the top level in Pinia store tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Applies to api/**/*.test.{ts,tsx} : Use Vitest for the test suite, not Jest
Applied to files:
api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: This is a Nuxt.js app but tests are run with vitest outside of the Nuxt environment
Applied to files:
api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use VITEST for test suite, not jest
Applied to files:
api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to web/__test__/**/*.test.{ts,tsx} : Use `mount` from Vue Test Utils for component testing
Applied to files:
api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts
🧬 Code graph analysis (4)
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts (3)
api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (1)
containers(89-124)api/src/unraid-api/utils/graphql-field-helper.ts (1)
GraphQLFieldHelper(13-63)web/src/composables/gql/graphql.ts (1)
DockerContainer(758-809)
api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts (1)
api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (1)
containers(89-124)
api/src/unraid-api/graph/resolvers/docker/docker.service.ts (2)
web/composables/gql/graphql.ts (2)
DockerContainer(693-711)Docker(675-681)api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (2)
container(79-82)containers(89-124)
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts (2)
api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (2)
containers(89-124)container(79-82)api/src/unraid-api/graph/resolvers/docker/docker.service.ts (1)
RawDockerContainer(27-27)
🔇 Additional comments (15)
api/src/unraid-api/graph/resolvers/docker/docker.service.ts (3)
27-27: LGTM! Clean type definition for separating raw and enriched container data.The
RawDockerContainertype correctly omits the fields that are added during the enrichment step, establishing a clear data flow from Docker API → raw container → enriched container.
130-141: LGTM! Proper enrichment logic.The method correctly:
- Normalizes container names (removes leading slash, lowercases)
- Looks up template mappings from config
- Sets
isOrphanedbased on absence oftemplatePathOne minor observation: the
|| undefinedon line 134 convertsnulltemplate mappings toundefined, which aligns with the GraphQL schema wheretemplatePathis nullable.
143-169: Well-structured refactor maintaining backward compatibility.The separation of
getRawContainersandgetContainersallows:
- Template scanner to work with raw data without triggering enrichment
- Resolver to control exactly when enrichment happens
getContainersremains a simple composition for internal callers that need the full dataapi/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts (3)
11-14: LGTM! Import correctly updated for new type.The import now includes
RawDockerContaineralongsideDockerService, aligning with the updated method signatures.
61-61: Correct type update for syncMissingContainers.Using
RawDockerContainer[]is appropriate here since:
- This method is called before enrichment
- It only needs
namesfor container identification, which exists on raw containers- It avoids requiring callers to pre-enrich containers unnecessarily
92-92: Consistent use of getRawContainers in scanTemplates.This aligns with the PR's goal of synchronizing template mappings before orphan status is computed.
api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (1)
101-123: Core fix for the race condition - looks good!The updated flow ensures template mappings are synchronized before computing orphan status:
- Fetch raw containers
- Optionally add log sizes
- Sync missing container templates
- Enrich with orphan status and return
This fixes the parallel query race condition described in the PR objectives.
api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.ts (2)
27-27: LGTM! Mock correctly updated to getRawContainers.The mock setup aligns with the updated service method.
199-199: LGTM! All mock calls consistently updated.All
vi.mocked(dockerService.getRawContainers)calls are correctly updated throughout the test file.Also applies to: 239-239, 257-257, 271-271, 293-293, 328-328
api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts (4)
6-6: LGTM! Required import added for new dependency.The
DockerTemplateScannerServiceimport is needed since the organizer service now depends on it for template sync.
197-229: LGTM! DockerService mock properly updated with new methods.The mock correctly includes:
getRawContainersreturning container data without orphan statusenrichWithOrphanStatusthat addsisOrphaned: falseandtemplatePathto each containerThis accurately simulates the new two-step container retrieval flow.
238-243: LGTM! DockerTemplateScannerService mock added.The mock provides
syncMissingContainersreturningfalse(indicating no sync was needed), which is appropriate for most test scenarios.
568-568: LGTM! Test scenarios correctly updated.Both error handling and ordering tests now use
getRawContainersmocks, consistent with the service implementation changes.Also applies to: 702-715
api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts (2)
39-46: LGTM! Mock setup correctly reflects new API.The DockerService mock now includes:
getRawContainersfor fetching unenriched container dataenrichWithOrphanStatusthat adds orphan status fieldsThe enrichment mock correctly adds
isOrphaned: falseandtemplatePathto each container.
132-170: LGTM! Test correctly validates the enrichment flow.The test:
- Provides raw containers without
isOrphaned- Mocks
getRawContainersto return them- Verifies the result contains
isOrphaned: false(added by enrichment)- Confirms
getRawContainerswas called with correct optionsThis accurately tests the new container resolution flow.
Summary
Changes
RawDockerContainertype (containers withoutisOrphaned/templatePath)enrichWithOrphanStatus()to compute orphan status from current configgetRawContainers()to fetch containers without orphan enrichmentContext
When a new container is installed, the GraphQL query fetches both
docker.containersanddocker.organizerdata in parallel. Previously,isOrphanedwas computed insidegetContainers()based onconfig.templateMappings, requiring a refetch after sync to get updated values. This caused:Now orphan status enrichment is a separate step that reads the current config, eliminating redundant API calls while ensuring newly installed containers are correctly identified.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Deprecations
skipCacheparameter on Docker queries; it is now ignored.✏️ Tip: You can customize this high-level summary in your review settings.