chore: add organizer data structure for docker folders
#1540
Conversation
WalkthroughThis update introduces a type-safe, modular validation framework for organizer data structures, along with comprehensive DTOs and validation logic for organizer views and resources. It adds new services for configuration management, extensive validation utilities, and robust test suites covering structural and referential integrity, including edge cases and error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DockerConfigService
participant OrganizerV1 DTO
participant Validator
Client->>DockerConfigService: validate(config)
DockerConfigService->>OrganizerV1 DTO: Validate structure
OrganizerV1 DTO-->>DockerConfigService: Structured result or errors
DockerConfigService->>Validator: Run integrity checks
Validator-->>DockerConfigService: Error(s) or validated config
DockerConfigService-->>Client: Throw errors or return validated config
sequenceDiagram
participant Client
participant ValidationProcessor
participant Step1
participant Step2
Client->>ValidationProcessor: process(input, config)
ValidationProcessor->>Step1: Run validator
Step1-->>ValidationProcessor: Result
alt Error and failFast
ValidationProcessor-->>Client: Return errors
else No error or not failFast
ValidationProcessor->>Step2: Run validator
Step2-->>ValidationProcessor: Result
ValidationProcessor-->>Client: Return aggregated result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
api/src/unraid-api/organizer/organizer.dto.ts (1)
79-82: Consider implementing proper type discrimination for entries validationThe current setup with
@Type(() => Object)bypasses deep validation of the entries dictionary values. Since entries can contain eitherOrganizerFolderorOrganizerResourceRef, consider implementing a custom validator or using a discriminator pattern to ensure proper validation of each entry type.api/src/unraid-api/organizer/organizer.validation.test.ts (1)
18-37: Consider extracting shared test helpersThe helper functions
createRef,createFolder, andcreateVieware duplicated fromorganizer-view.validation.test.ts. Consider extracting these to a shared test utilities file to maintain DRY principles.Create a new file
api/src/unraid-api/organizer/__tests__/test-helpers.ts:import { OrganizerFolder, OrganizerResourceRef, OrganizerView, } from '@app/unraid-api/organizer/organizer.dto.js'; export const createRef = (id: string, target: string): OrganizerResourceRef => ({ id, type: 'ref' as const, target, }); export const createFolder = (id: string, name: string, children: string[]): OrganizerFolder => ({ id, type: 'folder' as const, name, children, }); export const createView = ( partial: Partial<OrganizerView> & Pick<OrganizerView, 'root' | 'entries'> ): OrganizerView => ({ id: 'view1', name: 'Test View', ...partial, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
api/src/core/utils/validation/validation-processor.test.ts(1 hunks)api/src/core/utils/validation/validation-processor.ts(1 hunks)api/src/unraid-api/graph/resolvers/docker/docker-config.service.ts(1 hunks)api/src/unraid-api/organizer/organizer-view.validation.test.ts(1 hunks)api/src/unraid-api/organizer/organizer-view.validation.ts(1 hunks)api/src/unraid-api/organizer/organizer.dto.ts(1 hunks)api/src/unraid-api/organizer/organizer.validation.test.ts(1 hunks)api/src/unraid-api/organizer/organizer.validation.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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/core/utils/validation/validation-processor.test.tsapi/src/unraid-api/graph/resolvers/docker/docker-config.service.tsapi/src/unraid-api/organizer/organizer-view.validation.tsapi/src/unraid-api/organizer/organizer.validation.tsapi/src/unraid-api/organizer/organizer-view.validation.test.tsapi/src/unraid-api/organizer/organizer.dto.tsapi/src/unraid-api/organizer/organizer.validation.test.tsapi/src/core/utils/validation/validation-processor.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/core/utils/validation/validation-processor.test.tsapi/src/unraid-api/organizer/organizer-view.validation.test.tsapi/src/unraid-api/organizer/organizer.validation.test.ts
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
Files:
api/src/unraid-api/graph/resolvers/docker/docker-config.service.tsapi/src/unraid-api/organizer/organizer-view.validation.tsapi/src/unraid-api/organizer/organizer.validation.tsapi/src/unraid-api/organizer/organizer-view.validation.test.tsapi/src/unraid-api/organizer/organizer.dto.tsapi/src/unraid-api/organizer/organizer.validation.test.ts
🧠 Learnings (9)
📓 Common learnings
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.
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.
api/src/core/utils/validation/validation-processor.test.ts (11)
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T13:59:55.481Z
Learning: Test suite is VITEST, do not use jest in the API codebase
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
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
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
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
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 interactions (clicks, inputs, etc.)
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
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
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 : Check for expected prop handling and event emissions in component 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
Learnt from: elibosley
PR: #1120
File: plugin/scripts/build-plugin-and-txz.ts:132-147
Timestamp: 2025-02-06T15:32:09.488Z
Learning: For validation steps in build scripts where the goal is to fail fast on invalid state, simple error handling (console.error + process.exit) is appropriate since we want to abort the build process immediately.
api/src/unraid-api/graph/resolvers/docker/docker-config.service.ts (5)
Learnt from: mdatelle
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.
Learnt from: pujitm
PR: #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.
Learnt from: pujitm
PR: #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.
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
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T13:59:55.481Z
Learning: Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
api/src/unraid-api/organizer/organizer-view.validation.ts (1)
Learnt from: elibosley
PR: #1120
File: plugin/scripts/build-plugin-and-txz.ts:132-147
Timestamp: 2025-02-06T15:32:09.488Z
Learning: For validation steps in build scripts where the goal is to fail fast on invalid state, simple error handling (console.error + process.exit) is appropriate since we want to abort the build process immediately.
api/src/unraid-api/organizer/organizer.validation.ts (1)
Learnt from: elibosley
PR: #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).
api/src/unraid-api/organizer/organizer-view.validation.test.ts (16)
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
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
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
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
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
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
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
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
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 interactions (clicks, inputs, etc.)
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
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 findAll to check for multiple elements
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T13:59:55.481Z
Learning: Test suite is VITEST, do not use jest in the API codebase
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)
Learnt from: elibosley
PR: #1120
File: plugin/scripts/build-plugin-and-txz.ts:132-147
Timestamp: 2025-02-06T15:32:09.488Z
Learning: For validation steps in build scripts where the goal is to fail fast on invalid state, simple error handling (console.error + process.exit) is appropriate since we want to abort the build process immediately.
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 : Stub complex child components that aren't the focus of the test
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 : Check for expected prop handling and event emissions in component tests
api/src/unraid-api/organizer/organizer.dto.ts (4)
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
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T13:59:55.481Z
Learning: Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
Learnt from: elibosley
PR: #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.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T13:59:55.481Z
Learning: All GraphQL under web/ must follow this naming convention (.query.ts for queries, .mutation.ts for mutations)
api/src/unraid-api/organizer/organizer.validation.test.ts (16)
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
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
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
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
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
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
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
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
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 the right parameters 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
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T13:59:55.481Z
Learning: Test suite is VITEST, do not use jest in the API codebase
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()
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
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
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
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()
api/src/core/utils/validation/validation-processor.ts (1)
Learnt from: elibosley
PR: #1120
File: plugin/scripts/build-plugin-and-txz.ts:132-147
Timestamp: 2025-02-06T15:32:09.488Z
Learning: For validation steps in build scripts where the goal is to fail fast on invalid state, simple error handling (console.error + process.exit) is appropriate since we want to abort the build process immediately.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Build Web App
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
api/src/core/utils/validation/validation-processor.ts (3)
39-75: Well-designed type system for validation frameworkThe type definitions provide excellent type safety with proper inference of input types and step results. The use of conditional types and mapped types ensures compile-time safety across the validation pipeline.
178-181: Type assertion is acceptable hereThe
(errors as any)type assertion is necessary because TypeScript cannot statically verify the dynamic assignment to the errors object keyed by step names. This is a reasonable trade-off for the flexibility provided by the validation framework.
196-215: Comprehensive set of result interpretersThe
ResultInterpretersobject provides a well-thought-out collection of helper functions for common validation patterns. The naming is intuitive and the implementations are correct.api/src/core/utils/validation/validation-processor.test.ts (1)
1-155: Comprehensive test coverage for validation processorThe test suite thoroughly covers all major functionality of the validation processor including fail-fast behavior, different result interpreters, and edge cases. Good use of Vitest as per project standards.
api/src/unraid-api/organizer/organizer-view.validation.ts (1)
1-201: Excellent implementation of view structure validationThe module provides comprehensive validation with well-separated concerns. The cycle detection with path tracking, depth validation with configurable limits, and orphan detection are all correctly implemented. The documentation clearly explains each validation step.
api/src/unraid-api/organizer/organizer.validation.test.ts (1)
514-530: Verify error handling behavior in testThe test simulates an error on the second
validateViewStructurecall, but the comment at line 525 suggests view2 wasn't processed. However, the test expects only 2 views in the results. Please verify if this correctly tests the error handling behavior - should failed validations be included in the results or skipped entirely?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
api/src/core/utils/validation/validation-processor.ts (1)
195-195: Consider avoiding theanytype assertion.The type assertion to
anyworks but bypasses TypeScript's type checking.You could maintain type safety by using a type-safe assignment:
- (errors as any)[step.name] = result; + const typedErrors = errors as Record<string, unknown>; + typedErrors[step.name] = result;Or leverage TypeScript's ability to handle the assignment without assertion since the types should align correctly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
api/src/core/utils/validation/validation-processor.test.ts(1 hunks)api/src/core/utils/validation/validation-processor.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/core/utils/validation/validation-processor.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
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/core/utils/validation/validation-processor.ts
🧠 Learnings (2)
📓 Common learnings
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.
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.
api/src/core/utils/validation/validation-processor.ts (1)
Learnt from: elibosley
PR: #1120
File: plugin/scripts/build-plugin-and-txz.ts:132-147
Timestamp: 2025-02-06T15:32:09.488Z
Learning: For validation steps in build scripts where the goal is to fail fast on invalid state, simple error handling (console.error + process.exit) is appropriate since we want to abort the build process immediately.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build API
- GitHub Check: Build Web App
- GitHub Check: Test API
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
api/src/core/utils/validation/validation-processor.ts (2)
39-84: Type definitions are well-structured and type-safe.The advanced TypeScript types correctly handle the validation pipeline's type inference needs.
211-230: ResultInterpreters provides comprehensive helper functions.The helper functions cover common validation patterns effectively and are correctly implemented.
pujitm
changed the title
organizer data structure for docker folders
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
api/src/unraid-api/auth/auth.service.ts (1)
121-121: Consider fixing the variable name throughout instead of renaming during destructuring.While this maintains functionality, it would be cleaner to fix the misspelled variable name on line 127 rather than renaming the correctly spelled property during destructuring.
Apply this diff to use consistent spelling throughout:
- const { errors, errorOccurred: errorOccured } = await batchProcess( + const { errors, errorOccurred } = await batchProcess( permissionActions, ({ resource, action }) => this.authzService.addPermissionForUser(apiKeyId, resource, action) ); - if (errorOccured) { + if (errorOccurred) { this.logger.warn(`Some permissions failed to sync for API key ${apiKeyId}:`, errors); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
api/src/unraid-api/auth/auth.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/docker/docker-config.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts(1 hunks)api/src/unraid-api/organizer/organizer-view.validation.test.ts(1 hunks)api/src/unraid-api/organizer/organizer.validation.ts(1 hunks)api/src/unraid-api/plugin/plugin.service.ts(1 hunks)api/src/utils.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- api/src/utils.ts
- api/src/unraid-api/plugin/plugin.service.ts
- api/src/unraid-api/graph/resolvers/docker/docker-config.service.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- api/src/unraid-api/organizer/organizer-view.validation.test.ts
- api/src/unraid-api/organizer/organizer.validation.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/auth/auth.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
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
Files:
api/src/unraid-api/auth/auth.service.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Web App
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (1)
185-185: LGTM! Typo correction for property name consistency.The spelling correction from
errorOccuredtoerrorOccurredaligns with proper English spelling and ensures consistency with thebatchProcessutility's return value.Also applies to: 191-191
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
| const steps = [ | ||
| { | ||
| name: 'emailFormat', | ||
| validator: (input: ComplexInput) => |
There was a problem hiding this comment.
Is this kind of thing something we should be using class-validator for? Or is this more specialized?
| @@ -0,0 +1,230 @@ | |||
| /** | |||
| * @fileoverview Type-safe sequential validation processor | |||
There was a problem hiding this comment.
Yeah, my only question here is should we have utilized a library to provide the type-safe validations, and then add the chaining logic ourselves? Seems like it could be a case of not-built-here but it may be 100% necessary
There was a problem hiding this comment.
Here's what AI said: 🛠️ TL;DR Recommendation
Use Zod or Valibot if you want standard schema-based validation.
Stick with your custom createValidationProcessor if:
You want step-named error reporting
You need typed per-step errors
You want fail-fast customization
You want full flexibility over validator logic
There was a problem hiding this comment.
ik it's kinda crazy that the zod's out there aren't great for validating trees/graphs. the more convincing benefit is the fact that we can unit test validation steps + pipelines individually as opposed to the zod refine style.
| ); | ||
|
|
||
| const { errors, errorOccured } = await batchProcess( | ||
| const { errors, errorOccurred: errorOccured } = await batchProcess( |
There was a problem hiding this comment.
Suggestion for later - should we move to https://www.npmjs.com/package/p-map instead of the custom batchProcess - just thinking about developer maintainability and documentation as that library is essentially the same functionality but with a bit more control and options.
| }); | ||
|
|
||
| describe('findCycleInView', () => { | ||
| it('should return null when no cycle exists', () => { |
| expect(result).toEqual(['root1', 'root1']); | ||
| }); | ||
|
|
||
| it('should detect simple two-node cycle', () => { |
There was a problem hiding this comment.
This too, you've literally built a file system you weren't kidding!
| /** | ||
| * Finds duplicate children in folder entries. | ||
| */ | ||
| export function findDuplicateChildren( |
There was a problem hiding this comment.
Would we be better suited with a drop-in lib like: https://github.com/dagrejs/graphlib/wiki instead of building these data structures by hand?
Alternatively, I see a world where we publish this as a separate library on our org, this is very cool code.
| IsOptional, | ||
| IsString, | ||
| ValidateNested, | ||
| } from 'class-validator'; |
There was a problem hiding this comment.
Oh ! You are using class validator for this...
pujitm
changed the title
organizer data structure for docker foldersorganizer data structure for docker folders
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Introduced a comprehensive validation system for organizer data, including structural and referential integrity checks for views and organizers. * Added new data models for resources, folders, references, and views, with strong typing and validation. * Implemented a sequential validation processor with configurable fail-fast behavior and detailed error reporting. * Added a dedicated service for managing and validating Docker organizer configuration files. * **Bug Fixes** * Corrected spelling of error-related properties from "errorOccured" to "errorOccurred" in multiple services to ensure consistent error handling. * **Tests** * Added extensive unit tests for organizer validation logic, view structure validation, and the validation processor to ensure correctness across various edge cases and scenarios. * Added comprehensive tests verifying validation processor behavior under diverse conditions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests