feat: code first graphql #1347
feat: code first graphql #1347
Conversation
WalkthroughThis update reworks large portions of the codebase. GraphQL schema files and types have been extensively refactored: many legacy schema files were removed and a new generated schema was introduced. Resolver modules and services for arrays, connections, parity checks, notifications, virtual machines, and more have been added or updated. Import paths have been centralized so that types are now sourced from dedicated modules rather than auto‐generated files. In addition, several tests and snapshots were updated, environment variables added, and old utility and legacy files removed—all to enhance type safety, consistency, and overall clarity. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as GraphQL Client
participant Resolver as ParityCheckMutationsResolver
participant Service as ParityService
Client->>Resolver: Mutation: start(correct: true)
Resolver->>Service: updateParityCheck(wantedState: "start", correct: true)
Service-->>Resolver: Returns updated parity history
Resolver-->>Client: Response with parity check status
sequenceDiagram
participant Client as GraphQL Client
participant Resolver as ConnectSettingsResolver
participant Service as ConnectSettingsService
Client->>Resolver: Query: remoteAccess()
Resolver->>Service: Retrieves dynamic remote access settings
Service-->>Resolver: Returns RemoteAccess data
Resolver-->>Client: Response with connection settings (id, dataSchema, uiSchema, values)
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 1
🧹 Nitpick comments (12)
api/src/unraid-api/graph/graph.module.ts (1)
30-30: Consider removing the async keyword.
If there are no asynchronous calls insideuseFactory, removingasynccan simplify the code. Otherwise, ensure that any awaited calls are handled.api/src/unraid-api/main.ts (1)
24-32: Added global validation pipe with appropriate configurationThe ValidationPipe configuration is well-structured with:
- transform: true - Automatically transforms input data to the expected type
- whitelist: true - Strips properties not defined in DTOs
- forbidNonWhitelisted: true - Throws errors for properties not in the DTO
- enableImplicitConversion: true - Allows implicit type conversions
This enhances input validation for your GraphQL and REST endpoints, which is particularly important for a code-first approach.
Consider adding a custom exception factory for more user-friendly error messages if you expect non-technical users to interact with the API directly.
app.useGlobalPipes(new ValidationPipe({ transform: true, whitelist: true, forbidNonWhitelisted: true, transformOptions: { enableImplicitConversion: true, }, + exceptionFactory: (errors) => { + const messages = errors.map(error => { + return { + property: error.property, + constraints: error.constraints, + }; + }); + return new BadRequestException({ + message: 'Validation failed', + errors: messages + }); + } }));api/src/unraid-api/graph/README.md (1)
51-51: Fix formatting in bullet point listsThere's inconsistent spacing in the bullet point lists that could be standardized for better readability.
- - `migration-plan.md`: A detailed plan for migrating the GraphQL schema +- `migration-plan.md`: A detailed plan for migrating the GraphQL schema - - `api-key.model.ts`: Contains the model classes for the API Key Resolver +- `api-key.model.ts`: Contains the model classes for the API Key ResolverAlso applies to: 58-58
🧰 Tools
🪛 LanguageTool
[uncategorized] ~51-~51: Loose punctuation mark.
Context: ...th the migration: -migration-plan.md: A detailed plan for migrating the Graph...(UNLIKELY_OPENING_PUNCTUATION)
api/src/unraid-api/graph/resolvers/api-key/api-key.mapper.ts (1)
8-8: Consider explicitly documenting null/undefined handlingWhile the null coalescing operator correctly handles undefined descriptions, a comment would make this behavior more explicit to other developers.
- description: generatedApiKey.description ?? undefined, + // Ensure undefined is used when description is null or undefined + description: generatedApiKey.description ?? undefined,api/src/unraid-api/graph/migration-script.ts (2)
20-20: Document the reason for excluding special casesThe filter excludes 'array' and 'disks' directories, but it's not clear why. Adding a comment would help future developers understand this decision.
- .filter(item => item !== 'array' && item !== 'disks'); // Exclude special cases + .filter(item => item !== 'array' && item !== 'disks'); // Exclude special cases that require custom handling due to their unique structure
6-7: Update script execution instructionsThe comment mentions running with
node migration-script.js, but the script is referenced in package.json as"migration:codefirst": "tsx ./src/unraid-api/graph/migration-script.ts".- * 1. Run it with Node.js: `node migration-script.js` - * 2. Follow the guidance provided by the script + * 1. Run it with: `npm run migration:codefirst` or `yarn migration:codefirst` + * 2. Follow the guidance provided by the scriptapi/src/unraid-api/graph/migration-plan.md (1)
1-121: Good migration plan with clear structure and steps.This migration plan provides a well-organized approach to transitioning from schema-first to code-first GraphQL. The checklist format makes progress tracking clear.
Minor grammar issues:
- Lines 5, 46, and 121: Add the article "a" before "schema-first approach" and "code-first approach"
-This document outlines the plan to migrate the GraphQL schema from schema-first approach to code-first approach using NestJS decorators. +This document outlines the plan to migrate the GraphQL schema from a schema-first approach to a code-first approach using NestJS decorators.- - Update the GraphQL module to use code-first approach + - Update the GraphQL module to use a code-first approach-2. If necessary, revert to schema-first approach by restoring the schema files and removing the code-first changes +2. If necessary, revert to a schema-first approach by restoring the schema files and removing the code-first changes🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: You might be missing the article “a” here.
Context: ...plan to migrate the GraphQL schema from schema-first approach to code-first approach u...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~5-~5: You might be missing the article “a” here.
Context: ...QL schema from schema-first approach to code-first approach using NestJS decorators....(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~46-~46: You might be missing the article “a” here.
Context: ...` - Update the GraphQL module to use code-first approach 5. **Remove Schema File...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~121-~121: You might be missing the article “a” here.
Context: ...e and tested 2. If necessary, revert to schema-first approach by restoring the schema ...(AI_EN_LECTOR_MISSING_DETERMINER_A)
api/src/unraid-api/auth/api-key.service.spec.ts (3)
5-11: Remove unnecessary blank lines.There are extra blank lines in the imports section that should be removed according to the project's style rules.
- + import { ensureDir, ensureDirSync } from 'fs-extra'; import { AuthActionVerb } from 'nest-authz'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; - +🧰 Tools
🪛 GitHub Check: Build API
[failure] 5-5:
Delete⏎⏎
[failure] 5-5:
More than 1 blank line not allowed
[failure] 9-9:
Delete⏎⏎
[failure] 11-11:
More than 1 blank line not allowed🪛 GitHub Check: Test API
[failure] 5-5:
Delete⏎⏎
[failure] 5-5:
More than 1 blank line not allowed
[failure] 9-9:
Delete⏎⏎
[failure] 11-11:
More than 1 blank line not allowed
18-23: Fix import formatting.The import line should be formatted according to project style rules with one import per line.
-import { ApiKey, ApiKeyWithSecret, Permission } from '@app/unraid-api/graph/resolvers/api-key/api-key.model.js'; +import { + ApiKey, + ApiKeyWithSecret, + Permission, +} from '@app/unraid-api/graph/resolvers/api-key/api-key.model.js'; import { Resource, Role } from '@app/unraid-api/graph/resolvers/base.model.js'; - +🧰 Tools
🪛 GitHub Check: Build API
[failure] 18-18:
Replace·ApiKey,·ApiKeyWithSecret,·Permission·with⏎····ApiKey,⏎····ApiKeyWithSecret,⏎····Permission,⏎
[failure] 19-19:
Delete⏎⏎⏎⏎
[failure] 21-21:
More than 1 blank line not allowed🪛 GitHub Check: Test API
[failure] 18-18:
Replace·ApiKey,·ApiKeyWithSecret,·Permission·with⏎····ApiKey,⏎····ApiKeyWithSecret,⏎····Permission,⏎
[failure] 19-19:
Delete⏎⏎⏎⏎
[failure] 21-21:
More than 1 blank line not allowed
507-526: Good test for invalid API key file handling.This new test ensures that invalid API key files are properly ignored when loading from disk, which improves error handling robustness. However, the formatting of the mock calls needs improvement.
- vi.mocked(readdir).mockResolvedValue(['key1.json', 'badkey.json', 'key2.json', 'notakey.txt'] as any); - vi.mocked(readFile).mockResolvedValueOnce(JSON.stringify(mockApiKeyWithSecret)) - .mockResolvedValueOnce(JSON.stringify({ invalid: 'structure' })) - .mockResolvedValueOnce(JSON.stringify({...mockApiKeyWithSecret, id: 'unique-id', key: 'unique-key' })) - .mockResolvedValueOnce(JSON.stringify({...mockApiKeyWithSecret, id: 'unique-id', key: 'unique-key' })); + vi.mocked(readdir).mockResolvedValue([ + 'key1.json', + 'badkey.json', + 'key2.json', + 'notakey.txt', + ] as any); + vi.mocked(readFile) + .mockResolvedValueOnce(JSON.stringify(mockApiKeyWithSecret)) + .mockResolvedValueOnce(JSON.stringify({ invalid: 'structure' })) + .mockResolvedValueOnce(JSON.stringify({...mockApiKeyWithSecret, id: 'unique-id', key: 'unique-key' })) + .mockResolvedValueOnce(JSON.stringify({...mockApiKeyWithSecret, id: 'unique-id', key: 'unique-key' }));🧰 Tools
🪛 GitHub Check: Build API
[failure] 508-508:
Replace'key1.json',·'badkey.json',·'key2.json',·'notakey.txt'with⏎················'key1.json',⏎················'badkey.json',⏎················'key2.json',⏎················'notakey.txt',⏎············
[failure] 509-509:
Insert⏎················
[failure] 510-510:
Replace············with················🪛 GitHub Check: Test API
[failure] 508-508:
Replace'key1.json',·'badkey.json',·'key2.json',·'notakey.txt'with⏎················'key1.json',⏎················'badkey.json',⏎················'key2.json',⏎················'notakey.txt',⏎············
[failure] 509-509:
Insert⏎················
[failure] 510-510:
Replace············with················api/src/unraid-api/auth/api-key.service.ts (2)
47-50: Helper function to hide the secret key.
Removingkeyfrom the returned object is a neat way to prevent accidental secret exposure.Consider renaming the extracted variable to clarify intention:
-const { key: _, ...rest } = key; +const { key: _unusedKey, ...rest } = key;
291-291: Validation error logging logic.
Serializingerror.constraintsis helpful but verify to ensure no sensitive data is exposed.If
constraintsmay contain private details, consider masking or filtering them:this.logger.error(`Invalid API key structure in file ${file}. - Errors: ${JSON.stringify(error.constraints, null, 2)}`); + Errors: ${JSON.stringify(maskSensitiveConstraints(error.constraints), null, 2)}`);Also applies to: 293-293
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
.cursor/rules/api-rules.mdc(1 hunks)api/package.json(2 hunks)api/src/unraid-api/auth/api-key.service.spec.ts(6 hunks)api/src/unraid-api/auth/api-key.service.ts(7 hunks)api/src/unraid-api/graph/README.md(1 hunks)api/src/unraid-api/graph/auth/auth.enums.ts(1 hunks)api/src/unraid-api/graph/graph.module.ts(3 hunks)api/src/unraid-api/graph/migration-plan.md(1 hunks)api/src/unraid-api/graph/migration-script.ts(1 hunks)api/src/unraid-api/graph/resolvers/api-key/api-key.mapper.ts(1 hunks)api/src/unraid-api/graph/resolvers/api-key/api-key.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/api-key/api-key.module.ts(1 hunks)api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.ts(6 hunks)api/src/unraid-api/graph/resolvers/base.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/validation.utils.ts(1 hunks)api/src/unraid-api/main.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
api/src/unraid-api/graph/resolvers/api-key/api-key.module.ts (1)
api/src/unraid-api/graph/graph.module.ts (1)
Module(23-68)
api/src/unraid-api/graph/graph.module.ts (3)
api/src/unraid-api/graph/resolvers/resolvers.module.ts (1)
Module(36-71)api/src/graphql/resolvers/graphql-type-long.ts (1)
GraphQLLong(29-35)api/src/unraid-api/graph/auth/auth.enums.ts (1)
AuthDirective(28-46)
api/src/unraid-api/auth/api-key.service.ts (2)
api/src/graphql/generated/api/types.ts (2)
ApiKeyWithSecret(78-87)ApiKey(62-70)api/src/unraid-api/graph/resolvers/validation.utils.ts (1)
validateObject(15-34)
api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.ts (2)
api/src/graphql/generated/api/types.ts (5)
ApiKey(62-70)ApiKeyWithSecret(78-87)CreateApiKeyInput(492-499)AddRoleForApiKeyInput(48-51)RemoveRoleFromApiKeyInput(1370-1373)api/src/unraid-api/graph/resolvers/validation.utils.ts (1)
validateObject(15-34)
🪛 LanguageTool
api/src/unraid-api/graph/README.md
[uncategorized] ~51-~51: Loose punctuation mark.
Context: ...th the migration: - migration-plan.md: A detailed plan for migrating the Graph...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~58-~58: Loose punctuation mark.
Context: ...migrated resolver: - api-key.model.ts: Contains the model classes for the API ...
(UNLIKELY_OPENING_PUNCTUATION)
api/src/unraid-api/graph/migration-plan.md
[uncategorized] ~5-~5: You might be missing the article “a” here.
Context: ...plan to migrate the GraphQL schema from schema-first approach to code-first approach u...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~5-~5: You might be missing the article “a” here.
Context: ...QL schema from schema-first approach to code-first approach using NestJS decorators....
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~46-~46: You might be missing the article “a” here.
Context: ...` - Update the GraphQL module to use code-first approach 5. **Remove Schema File...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~121-~121: You might be missing the article “a” here.
Context: ...e and tested 2. If necessary, revert to schema-first approach by restoring the schema ...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
🪛 GitHub Check: Build API
api/src/unraid-api/auth/api-key.service.spec.ts
[failure] 5-5:
Delete ⏎⏎
[failure] 5-5:
More than 1 blank line not allowed
[failure] 9-9:
Delete ⏎⏎
[failure] 11-11:
More than 1 blank line not allowed
[failure] 18-18:
Replace ·ApiKey,·ApiKeyWithSecret,·Permission· with ⏎····ApiKey,⏎····ApiKeyWithSecret,⏎····Permission,⏎
[failure] 19-19:
Delete ⏎⏎⏎⏎
[failure] 21-21:
More than 1 blank line not allowed
[failure] 508-508:
Replace 'key1.json',·'badkey.json',·'key2.json',·'notakey.txt' with ⏎················'key1.json',⏎················'badkey.json',⏎················'key2.json',⏎················'notakey.txt',⏎············
[failure] 509-509:
Insert ⏎················
[failure] 510-510:
Replace ············ with ················
🪛 GitHub Check: Test API
api/src/unraid-api/auth/api-key.service.spec.ts
[failure] 5-5:
Delete ⏎⏎
[failure] 5-5:
More than 1 blank line not allowed
[failure] 9-9:
Delete ⏎⏎
[failure] 11-11:
More than 1 blank line not allowed
[failure] 18-18:
Replace ·ApiKey,·ApiKeyWithSecret,·Permission· with ⏎····ApiKey,⏎····ApiKeyWithSecret,⏎····Permission,⏎
[failure] 19-19:
Delete ⏎⏎⏎⏎
[failure] 21-21:
More than 1 blank line not allowed
[failure] 508-508:
Replace 'key1.json',·'badkey.json',·'key2.json',·'notakey.txt' with ⏎················'key1.json',⏎················'badkey.json',⏎················'key2.json',⏎················'notakey.txt',⏎············
[failure] 509-509:
Insert ⏎················
[failure] 510-510:
Replace ············ with ················
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build Web App
- GitHub Check: Cloudflare Pages
🔇 Additional comments (56)
api/package.json (2)
86-87: Confirm complete transition from Zod to class-validator and class-transformer.
The addition of these libraries is a solid move for code-first validations. Please confirm if all Zod usages have been replaced or if Zod is still required.
46-48:✅ Verification successful
Check the newly added migration command.
The new script references./src/unraid-api/graph/migration-script.ts. Please verify that this file exists at the specified path and is executable.
🏁 Script executed:
#!/bin/bash # Verify the presence of migration-script.ts fd migration-script.ts --exec ls -l {}Length of output: 138
Migration command verified – file exists and is accessible.
The referenced file has been confirmed atapi/src/unraid-api/graph/migration-script.tsand is readable. Although it does not have execute permissions (-rw-r--r--), this is acceptable as the script is run viatsxrather than as a standalone executable.api/src/unraid-api/graph/graph.module.ts (8)
6-6: Kudos for enabling NoUnusedVariablesRule.
Including this rule helps enforce cleaner queries by preventing leftover variables in GraphQL operations. Looks good.
17-17: Imports for custom Auth enums and directive look good.
This aligns well with the new code-first approach.
19-19: Importing PluginSchemaService.
Ensure that the service is properly tested and is not introducing any circular dependencies.
28-29: Verify the usage of PluginSchemaService in imports.
Typically, NestJSimportsexpects modules rather than services. Ensure this is intentional and functioning as you expect.
32-32: Using autoSchemaFile in-memory generation.
This is a straightforward code-first setup. Great approach for dynamic updates.
48-55: Excellent configuration of custom scalar resolvers.
Centralizing them in the GraphQL module fosters consistency and maintainability across the schema.
56-59: Good usage of buildSchemaOptions.
Adding directives directly here is convenient.numberScalarMode: 'integer'is also a sensible default.
65-65: Possible duplication in defining PluginSchemaService.
You have PluginSchemaService in bothimports(lines 28-29) andproviders(line 65). Confirm this is the intended setup and does not cause conflicts.api/src/unraid-api/graph/auth/auth.enums.ts (4)
1-2: New imports look correct.
Bringing in the relevant GraphQL details andnest-authzenums is consistent with the new directive approach.
4-14: Dynamic creation of AuthActionVerbEnum.
Filtering out numeric keys is a neat solution to keep the enum values purely string-based in GraphQL.
16-25: Consistent approach for AuthPossessionEnum.
The mirrored logic ensures uniformity for both action verbs and possession types. Nicely done.
27-46: AuthDirective definition is clean and self-explanatory.
Defining arguments for actions, resources, and possession is a solid pattern for role-based access controls.api/src/unraid-api/graph/resolvers/api-key/api-key.module.ts (1)
1-10: Module structure looks well-organizedThe ApiKeyModule follows a clean Nest.js architecture pattern with appropriate imports, providers, and exports. This module correctly provides the ApiKeyResolver along with its required services (ApiKeyService and AuthService), enabling API key functionality in the GraphQL API.
This is part of the code-first GraphQL approach, where resolvers are organized into focused modules rather than being defined in the schema files.
api/src/unraid-api/graph/resolvers/base.model.ts (2)
35-39: Role enum looks appropriateThe three defined roles (ADMIN, CONNECT, GUEST) provide a clear hierarchy for access control. This follows standard authorization patterns.
41-49: Proper GraphQL enum registrationUsing registerEnumType correctly exposes these enums to the GraphQL schema with appropriate descriptions. This follows best practices for code-first GraphQL in NestJS.
.cursor/rules/api-rules.mdc (2)
1-5: Configuration metadata is properly structuredThe frontmatter configuration follows the expected format with description, globs pattern for targeting API files, and alwaysApply flag.
7-11:❓ Verification inconclusive
Clear development guidelines
These rules provide good guidance for the development team. I particularly appreciate the explicit mention of using VITEST instead of Jest and the preference for the NestJS structure over legacy code.
Is there any documentation explaining why simple dependencies shouldn't be mocked? This might help developers understand when to mock vs. when to use real implementations.
🏁 Script executed:
#!/bin/bash # Check if there's any documentation about testing practices fd -e md -e txt -e mdx "test" api/Length of output: 34
Update Documentation for Dependency Mocking Practices
The guidelines in
.cursor/rules/api-rules.mdcare clear. However, I couldn’t locate any documentation explaining why simple dependencies shouldn’t be mocked. It might be beneficial to add a brief explanation or reference to external resources that clarify when and why real implementations should be used over mocks.api/src/unraid-api/main.ts (1)
4-4: Added import for ValidationPipeClean import for the ValidationPipe from @nestjs/common.
api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.spec.ts (1)
5-6: Update imports successfully implement code-first approachThe refactoring of imports shifts ApiKey and Role to their proper locations in the new code-first structure. This aligns with the migration goals while maintaining the test's functionality.
api/src/unraid-api/graph/README.md (1)
1-82: Clear and comprehensive migration documentationThis README provides excellent guidance on the GraphQL schema migration process. It clearly outlines the current status, process steps, benefits, and next steps.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~51-~51: Loose punctuation mark.
Context: ...th the migration: -migration-plan.md: A detailed plan for migrating the Graph...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~58-~58: Loose punctuation mark.
Context: ...migrated resolver: -api-key.model.ts: Contains the model classes for the API ...(UNLIKELY_OPENING_PUNCTUATION)
api/src/unraid-api/graph/resolvers/api-key/api-key.mapper.ts (1)
1-20: Well-implemented mapper functionsThe mapper functions correctly transform generated API key types to model types with good handling of optional fields. The reuse of the first mapper in the second one is efficient.
api/src/unraid-api/graph/migration-script.ts (1)
1-81: Effective migration script implementationThis script effectively identifies resolvers that need migration and provides clear guidance on the process. The output is well-structured and informative.
api/src/unraid-api/graph/resolvers/validation.utils.ts (1)
1-34: Well-implemented validation utility function.The
validateObjectfunction provides a robust way to validate objects against class definitions using class-validator. The implementation is clean and handles error scenarios properly.Good use of:
- TypeScript generics for type safety
- Comprehensive error handling
- Clear documentation with JSDoc comments
- Proper configuration of validation options
api/src/unraid-api/auth/api-key.service.spec.ts (4)
74-97: Proper API key structure now includes permissions.The test mock objects now correctly include permissions, matching the schema defined in the model file. This ensures tests work properly with the updated API key structure.
301-326: Updated test assertions for permissions.The test has been properly updated to include assertions that verify the permissions array in the API key objects. This ensures that the API key structure is validated correctly.
470-480: Improved validation test for permissions and roles.The test now properly verifies that an API key must have at least one permission or role, which is a good security practice.
558-587: Improved error logging for validation failures.The test now verifies detailed error logging for API key validation failures, ensuring that developers have sufficient information to diagnose issues. The updated assertions match the new validation approach using class-validator.
api/src/unraid-api/graph/resolvers/api-key/api-key.model.ts (6)
1-30: Well-structured Permission class with appropriate validations.The Permission class is properly defined with GraphQL decorators and validation rules:
- Good use of @field decorators with explicit type functions
- Proper validation with @IsEnum for resource field
- Array validation for actions with minimum size requirement
32-64: Well-defined ApiKey class with comprehensive validation.The ApiKey class has appropriate:
- ID field with proper GraphQL and validation decorators
- Required and optional fields clearly marked
- Array validations for roles
- Nested validation for permissions
- Good date handling with transform function
66-71: Clean extension of ApiKey for ApiKeyWithSecret.The ApiKeyWithSecret class extends ApiKey properly and adds just the necessary additional field with appropriate validation.
73-84: Well-structured AddPermissionInput class.This input type mirrors the Permission model structure with appropriate validations for GraphQL inputs.
86-118: Comprehensive CreateApiKeyInput with optional fields.The CreateApiKeyInput class:
- Properly marks which fields are required vs optional
- Includes nested validation for permissions array
- Has a clear description for the overwrite field
- Uses appropriate validators for each field type
120-140: Well-defined input types for role management.The AddRoleForApiKeyInput and RemoveRoleFromApiKeyInput classes provide clear, validated structures for modifying API key roles.
api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.ts (13)
5-6: New service imports look consistent.
These imports from@app/unraid-api/auth/api-key.service.jsand@app/unraid-api/auth/auth.service.jsare aligned with the refactored architecture.
8-9: Resource & validation imports are straightforward.
ImportingResource,Role, andvalidateObjectsupports the code-first GraphQL approach and centralized validation strategy.
16-16: Model import updated.
Switching toapi-key.model.jsorganizes types more cohesively.
18-18: Resolver decorator migration approved.
Using@Resolver(() => ApiKey)is idiomatic in code-first GraphQL.
25-25: Query decorator looks good.
Returns an array ofApiKeyobjects as expected.
35-35: Nullable query confirmed.
Opting for a nullable return allows for flexible error handling and non-existent keys.
45-45: Mutation return type changed appropriately.
ReturningApiKeyWithSecretensures clients have direct access to the newly created key secret.
53-56: Input validation is a welcome addition.
ValidatingunvalidatedInputusingvalidateObject(CreateApiKeyInput, unvalidatedInput)prevents malformed data.
71-71: Mutation returning boolean.
Marking simple success/failure outcomes with@Mutation(() => Boolean)is clear and concise.
81-82: Proper validation of role addition input.
Consistent approach usingvalidateObject(AddRoleForApiKeyInput, input).
84-84: Ensure correct enum indexing for roles.
Role[validatedInput.role]is presumably valid due to input validation, so this is safe.
87-87: Second boolean mutation approach is consistent.
The shape of the resolver method aligns with the code-first style.
97-99: Input validation for removal of roles.
This mirrors the add-role mutation, ensuring symmetrical validation.api/src/unraid-api/auth/api-key.service.ts (8)
7-7: Class-validator import introduced.
Shifting from Zod to class-validator is a solid approach for NestJS-based validation.
14-16: Store-related imports look fine.
Incorporating store actions and types here makes sense for local API key management.
21-22: Extended imports forApiKeyWithSecret&Permission.
This clarifies references to GraphQL types and permissions.
52-58: Asynchronous mapping infindAllis clear and correct.
Returning a promise of converted API keys ensures no secret leaks.
223-227: Doc comment improvement is helpful.
Explicitly calling out validation steps clarifies this method's contract.
245-246: Handling ofValidationErroris appropriate.
The error logging strategy ensures minimal confusion during debugging and correct error propagation.
312-315: Ensuring at least one role or permission
This check prevents creation of non-functional API keys. Great safeguard.
329-331: Comprehensive error handling
CatchingValidationErroragain ensures consistent user-facing errors instead of raw exceptions.
elibosley
changed the title
There was a problem hiding this comment.
Actionable comments posted: 13
🔭 Outside diff range comments (1)
api/src/unraid-api/graph/resolvers/connect/connect.model.ts (1)
122-239:⚠️ Potential issueRefine port validation across
ConnectSettingsValuesandApiSettingsInput.In lines 158-162 and 192-196, the
portfield also relies on@IsPort()but is defined as a numerical field (() => Int). This leads to the same mismatch described above.You can fix these lines in the same manner:
- @IsPort() + import { IsInt, Min, Max } from 'class-validator'; @Field(() => Int, { nullable: true }) - @IsPort() + @IsInt() + @Min(1) + @Max(65535) port?: number;The rest of your object types and input types, including
DynamicRemoteAccessStatus,ConnectSettings, andConnect, employ robust validation and are neatly organized.
♻️ Duplicate comments (1)
api/src/unraid-api/graph/resolvers/base.model.ts (1)
10-39: Document any unused resources.
It appears some resources might be placeholders. As noted previously, consider adding inline comments indicating which are reserved for future use.
🧹 Nitpick comments (25)
api/src/unraid-api/graph/resolvers/logs/logs.model.ts (1)
35-48: LogFileInput class provides clear parameters.The input type is well-defined with appropriate descriptions for each field. The nullable fields for
linesandstartLineallow for flexibility in query parameters.One suggestion - consider adding validation for the input fields (e.g., minimum values for
linesandstartLine) to prevent potential issues with negative numbers or extremely large values.api/src/unraid-api/graph/resolvers/disks/disks.model.ts (2)
32-40: SMART status enum could be expandedThe
DiskSmartStatusenum currently only has OK and UNKNOWN values. Consider whether additional status values like WARN, FAIL, or BAD would be useful for more detailed disk health reporting.
57-61: Consider implementing Node interfaceBased on the provided relevant code snippets, there appears to be a
Nodeinterface with anidfield. SinceDiskhas an ID field, consider implementing this interface for consistency with other entity types.-@ObjectType() -export class Disk { +@ObjectType() +export class Disk implements Node {api/src/unraid-api/graph/resolvers/disks/disks.resolver.ts (1)
34-34: Consider explicitly naming the field in @ResolveField decorator.While the type annotation is good, the field name is now implicit. Consider adding the field name to prevent issues if the method name changes.
-@ResolveField(() => Long) +@ResolveField('temperature', () => Long)api/src/unraid-api/graph/services/service.model.ts (1)
1-9: Consider clarifying the field name.
ThetimestampinUptimemight be misread as a number instead of a string. You could rename it totimestampStringortimestampIsofor clarity.api/src/unraid-api/graph/resolvers/base.model.ts (1)
63-89: Optional: Consider a well-known Date scalar library.
Your custom scalar logic is sound. If you’d like to reduce boilerplate, you could use existing date-time scalar packages, though your approach also works well.api/src/unraid-api/graph/resolvers/api-key/api-key.model.ts (1)
32-64: Evaluate usingGraphQLISODateTimefor timestamps.
createdAtis managed as a string, with aTransformthat converts potentialDateobjects to ISO strings. If the domain requires date-specific handling (e.g., sorting by date in resolvers), switching to@Field(() => GraphQLISODateTime)may be more explicit and less error-prone.- @Field() - @IsString() - @Transform(({ value }) => (value instanceof Date ? value.toISOString() : value)) - createdAt!: string; + @Field(() => GraphQLISODateTime) + createdAt!: Date;api/src/unraid-api/graph/resolvers/info/display.model.ts (1)
10-12: ExtendThemeif multiple themes are supported.
Currently, there is only one value (white). If additional themes (dark, etc.) are planned, add them to avoid repeated migrations or property changes later.api/src/unraid-api/graph/resolvers/display/display.resolver.ts (2)
61-61: Deprecation notice fordisplay()method.
A TODO warns about eventual removal. Make sure to track or remove it once the new logic is in place to prevent confusion or accumulation of technical debt.
63-83: Gracefully handle file read error logs.
You return a fallback display state if reading the config fails, but no log is issued. Logging might help diagnose issues in live environments.async display(): Promise<Display> { // ... const serverCase = await readFile(configFilePath) .then((buffer) => buffer.toString().split('\n')[0]) .catch(() => { + console.error(`Failed to read file at: ${configFilePath}`); return 'error_reading_config_file'; }); // ... }api/src/unraid-api/graph/resolvers/info/info.model.ts (2)
79-80: Replace@Field(() => Object)with a custom scalar or type.
UsingObjectis flexible but can obscure the schema. A typed approach or custom scalar clarifies the structure ofcache.- @Field(() => Object) cache!: Record<string, any>; + @Field(() => GraphQLJSON) // or a known, typed structure cache!: Record<string, any>;
79-80: Overall duplication concerns about enumerations &Displaytype.
Consider extracting commonly used enumerations or theDisplaytype to a shared module to facilitate maintainability.Also applies to: 203-206, 208-210, 247-249
api/src/unraid-api/graph/resolvers/connect/connect.service.ts (5)
16-22: Consider rethrowing the error or returning an explicit error state.
Currently, therestartApi()method logs errors but does not propagate them to the caller. This can hide failures and make them harder to troubleshoot.
25-27: Safeguard against token decoding errors.
decodeJwt(input.idToken)could throw if the token is malformed. Adding a small try/catch or validation would clarify handling for invalid tokens.
28-36: Validate additional properties ofuserInfo.
Beyond checking forpreferred_usernameanduserInfois an object with well-defined properties to avoid unintended type issues.
44-53: Store or log newly generated API keys for better traceability.
When creating a local API key, you might want to log the event more explicitly or store the key configuration somewhere persistent, especially if multiple sign-in attempts run simultaneously.
66-68: Preserve the original error details.
Re-throwing the error as a newErrormessage can obscure the stack trace. Consider capturing the original message and stack for easier debugging:- throw new Error(`Failed to login user: ${error}`); + if (error instanceof Error) { + throw new Error(`Failed to login user: ${error.message}\nStack: ${error.stack}`); + } else { + throw new Error(`Failed to login user: ${error}`); + }api/src/unraid-api/graph/user/user.resolver.ts (1)
10-11: Remove unnecessary empty constructor.The empty constructor doesn't provide any value and can be safely removed.
@Resolver(() => UserAccount) export class MeResolver { - constructor() {} @Query(() => UserAccount)🧰 Tools
🪛 Biome (1.9.4)
[error] 11-11: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts (3)
70-85: Consider adding error handling around service calls.
IfbuildSettingsSchema()orgetCurrentSettings()fails, the error will bubble up unhandled. Consider wrapping these calls in a try/catch to provide clearer diagnostic information to the client.
87-105: Handle potential restart failures.
WhenrestartRequiredis set, the API restarts out-of-band with no error handling. Consider adding a safety net (try/catch) or at least logging in case the restart operation itself encounters issues.
194-215: Potential duplication withdynamicRemoteAccessSettings().
The logic used to determineaccessTypeandforwardTypehere closely matches the approach indynamicRemoteAccessSettings()(from the snippet inconnect-settings.service.ts). Consider unifying them into a shared utility or leveraging the service call, to reduce duplication and keep future maintenance simpler.api/src/unraid-api/graph/resolvers/array/array.service.spec.ts (1)
7-7: Remove extra semicolonThere's an extra semicolon at the end of this import statement.
-import { emcmd } from '@app/core/utils/clients/emcmd.js';; +import { emcmd } from '@app/core/utils/clients/emcmd.js';api/src/core/modules/array/get-array-data.ts (1)
58-58: Type assertion added for state property.You've added a type assertion to ensure the state property matches the ArrayState enum. Consider adding a validation check to ensure the value is actually within the valid enum values at runtime.
- state: emhttp.var.mdState as ArrayState, + state: validateArrayState(emhttp.var.mdState),You could implement a simple validation function:
function validateArrayState(state: string): ArrayState { if (Object.values(ArrayState).includes(state as ArrayState)) { return state as ArrayState; } throw new Error(`Invalid array state: ${state}`); }api/src/unraid-api/graph/resolvers/mutation/mutation.model.ts (1)
1-14: Consider removing explicit __typename fieldsThe __typename fields in ArrayMutations and DockerMutations classes are explicitly set, but GraphQL resolvers typically handle __typename automatically. These fields might be redundant unless there's a specific reason to override the default behavior.
@ObjectType('ArrayMutations') export class ArrayMutations { - @Field() - __typename: string = 'ArrayMutations'; } @ObjectType('DockerMutations') export class DockerMutations { - @Field() - __typename: string = 'DockerMutations'; }api/src/unraid-api/graph/resolvers/docker/docker.model.ts (1)
99-100: Consider using more specific types for JSON fieldsUsing
Record<string, any>for labels, networkSettings, and mounts could be improved for type safety.If the structure of these objects is known, consider creating specific interfaces or types for them:
// Example for labels interface ContainerLabels { [key: string]: string; } @Field(() => GraphQLJSONObject, { nullable: true }) labels?: ContainerLabels;Also applies to: 111-115
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (2)
api/src/graphql/generated/api/operations.tsis excluded by!**/generated/**api/src/graphql/generated/api/types.tsis excluded by!**/generated/**
📒 Files selected for processing (81)
api/codegen.ts(1 hunks)api/generated-schema.graphql(0 hunks)api/src/core/modules/array/get-array-data.ts(2 hunks)api/src/core/pubsub.ts(1 hunks)api/src/core/utils/array/array-is-running.ts(1 hunks)api/src/graphql/resolvers/mutation/connect/connect-sign-in.ts(0 hunks)api/src/graphql/resolvers/query/info.ts(2 hunks)api/src/graphql/schema/types/api-key/api-key.graphql(0 hunks)api/src/graphql/schema/types/api-key/roles.graphql(0 hunks)api/src/graphql/schema/types/array/array.graphql(0 hunks)api/src/graphql/schema/types/array/parity.graphql(0 hunks)api/src/graphql/schema/types/base.graphql(0 hunks)api/src/graphql/schema/types/config/config.graphql(1 hunks)api/src/graphql/schema/types/logs/logs.graphql(1 hunks)api/src/graphql/schema/types/users/me.graphql(0 hunks)api/src/graphql/schema/types/users/user.graphql(0 hunks)api/src/store/actions/setup-remote-access.ts(1 hunks)api/src/store/modules/config.ts(1 hunks)api/src/store/modules/dynamic-remote-access.ts(1 hunks)api/src/store/modules/emhttp.ts(1 hunks)api/src/store/modules/minigraph.ts(1 hunks)api/src/store/state-parsers/var.ts(1 hunks)api/src/store/types.ts(1 hunks)api/src/types/my-servers-config.ts(1 hunks)api/src/unraid-api/auth/api-key.service.spec.ts(6 hunks)api/src/unraid-api/auth/auth.service.spec.ts(1 hunks)api/src/unraid-api/graph/auth/auth.enums.ts(1 hunks)api/src/unraid-api/graph/connect/connect.resolver.ts(0 hunks)api/src/unraid-api/graph/connect/connect.service.ts(0 hunks)api/src/unraid-api/graph/graph.module.ts(3 hunks)api/src/unraid-api/graph/migration-script.ts(1 hunks)api/src/unraid-api/graph/resolvers/api-key/api-key.mapper.ts(1 hunks)api/src/unraid-api/graph/resolvers/api-key/api-key.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/api-key/api-key.module.ts(1 hunks)api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.ts(6 hunks)api/src/unraid-api/graph/resolvers/array/array.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/array.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/array/array.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/array.service.ts(10 hunks)api/src/unraid-api/graph/resolvers/array/parity.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/parity.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/base.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/cloud/cloud.resolver.ts(3 hunks)api/src/unraid-api/graph/resolvers/config/config.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/config/config.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/disks/disks.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/disks/disks.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/disks/disks.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/disks/disks.service.ts(2 hunks)api/src/unraid-api/graph/resolvers/display/display.resolver.ts(3 hunks)api/src/unraid-api/graph/resolvers/docker/docker.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.spec.ts(3 hunks)api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts(3 hunks)api/src/unraid-api/graph/resolvers/flash/flash.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/flash/flash.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/info/display.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/logs/logs.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/logs/logs.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/mutation/mutation.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/owner/owner.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/resolvers.module.ts(2 hunks)api/src/unraid-api/graph/resolvers/vars/vars.resolver.ts(1 hunks)api/src/unraid-api/graph/services/service.model.ts(1 hunks)api/src/unraid-api/graph/services/services.resolver.ts(1 hunks)api/src/unraid-api/graph/shares/shares.resolver.ts(1 hunks)api/src/unraid-api/graph/user/user.model.ts(1 hunks)api/src/unraid-api/graph/user/user.resolver.spec.ts(1 hunks)api/src/unraid-api/graph/user/user.resolver.ts(1 hunks)api/src/unraid-api/main.ts(2 hunks)api/src/unraid-api/rest/rest.controller.ts(1 hunks)api/src/utils.ts(1 hunks)
💤 Files with no reviewable changes (11)
- api/src/unraid-api/graph/connect/connect.service.ts
- api/src/graphql/schema/types/base.graphql
- api/src/graphql/schema/types/users/me.graphql
- api/src/graphql/schema/types/array/parity.graphql
- api/src/graphql/schema/types/users/user.graphql
- api/src/graphql/schema/types/api-key/roles.graphql
- api/src/unraid-api/graph/connect/connect.resolver.ts
- api/src/graphql/schema/types/api-key/api-key.graphql
- api/src/graphql/resolvers/mutation/connect/connect-sign-in.ts
- api/src/graphql/schema/types/array/array.graphql
- api/generated-schema.graphql
✅ Files skipped from review due to trivial changes (12)
- api/src/graphql/schema/types/config/config.graphql
- api/src/graphql/schema/types/logs/logs.graphql
- api/src/unraid-api/graph/shares/shares.resolver.ts
- api/src/store/state-parsers/var.ts
- api/src/unraid-api/graph/resolvers/vars/vars.resolver.ts
- api/src/unraid-api/graph/user/user.resolver.spec.ts
- api/src/unraid-api/graph/resolvers/owner/owner.resolver.ts
- api/src/store/actions/setup-remote-access.ts
- api/src/store/modules/config.ts
- api/src/unraid-api/rest/rest.controller.ts
- api/src/unraid-api/graph/resolvers/connect/connect.service.spec.ts
- api/src/unraid-api/graph/resolvers/disks/disks.service.spec.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- api/src/unraid-api/main.ts
- api/src/unraid-api/graph/resolvers/api-key/api-key.module.ts
- api/src/unraid-api/graph/auth/auth.enums.ts
- api/src/unraid-api/graph/resolvers/api-key/api-key.mapper.ts
- api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.spec.ts
- api/src/unraid-api/graph/migration-script.ts
🧰 Additional context used
🧬 Code Graph Analysis (21)
api/src/unraid-api/graph/resolvers/array/parity.model.ts (1)
web/composables/gql/graphql.ts (1)
ParityCheck(1111-1118)
api/src/unraid-api/graph/resolvers/config/config.resolver.ts (1)
api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts (1)
Resolver(29-238)
api/src/unraid-api/graph/resolvers/connect/connect.service.ts (3)
web/composables/gql/graphql.ts (1)
ConnectSignInInput(435-441)api/src/store/index.ts (2)
getters(18-29)store(6-12)api/src/store/modules/config.ts (1)
loginUser(62-75)
api/src/unraid-api/graph/resolvers/logs/logs.model.ts (2)
api/src/unraid-api/graph/resolvers/base.model.ts (1)
ObjectType(47-51)web/composables/gql/graphql.ts (2)
LogFile(750-760)LogFileContent(763-773)
api/src/unraid-api/auth/api-key.service.spec.ts (1)
web/composables/gql/graphql.ts (1)
ApiKeyWithSecret(81-90)
api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts (1)
web/composables/gql/graphql.ts (2)
DockerMutations(604-608)DockerContainer(584-602)
api/src/unraid-api/graph/resolvers/docker/docker.model.ts (2)
api/src/unraid-api/graph/resolvers/base.model.ts (1)
ObjectType(47-51)web/composables/gql/graphql.ts (8)
ContainerPort(466-472)ContainerHostConfig(449-452)ContainerMount(454-464)DockerContainer(584-602)DockerNetwork(620-637)Docker(576-582)Node(1022-1024)DockerMutations(604-608)
api/src/unraid-api/graph/resolvers/disks/disks.model.ts (3)
api/src/unraid-api/graph/resolvers/base.model.ts (1)
ObjectType(47-51)api/src/unraid-api/graph/resolvers/config/config.model.ts (1)
ObjectType(3-13)web/composables/gql/graphql.ts (2)
DiskPartition(539-544)Disk(501-522)
api/src/unraid-api/graph/resolvers/base.model.ts (4)
api/src/unraid-api/graph/resolvers/api-key/api-key.model.ts (3)
ObjectType(19-30)ObjectType(32-64)ObjectType(66-71)api/src/unraid-api/graph/resolvers/array/array.model.ts (5)
ObjectType(3-13)ObjectType(15-22)ObjectType(24-94)ObjectType(96-124)ObjectType(141-160)api/src/unraid-api/graph/resolvers/config/config.model.ts (1)
ObjectType(3-13)web/composables/gql/graphql.ts (1)
Node(1022-1024)
api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts (3)
api/src/unraid-api/graph/resolvers/base.model.ts (1)
ObjectType(47-51)api/src/unraid-api/graph/resolvers/config/config.model.ts (1)
ObjectType(3-13)web/composables/gql/graphql.ts (4)
ApiKeyResponse(75-79)MinigraphqlResponse(818-823)CloudResponse(378-383)RelayResponse(1367-1372)
api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (1)
web/composables/gql/graphql.ts (4)
Docker(576-582)Query(1209-1272)DockerContainer(584-602)DockerNetwork(620-637)
api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.ts (2)
web/composables/gql/graphql.ts (4)
ApiKey(65-73)CreateApiKeyInput(484-491)AddRoleForApiKeyInput(51-54)RemoveRoleFromApiKeyInput(1381-1384)api/src/unraid-api/graph/resolvers/validation.utils.ts (1)
validateObject(15-34)
api/src/unraid-api/graph/resolvers/api-key/api-key.model.ts (3)
api/src/unraid-api/graph/resolvers/base.model.ts (1)
ObjectType(47-51)api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts (5)
ObjectType(11-18)ObjectType(20-30)ObjectType(32-42)ObjectType(44-54)ObjectType(56-75)web/composables/gql/graphql.ts (7)
Permission(1195-1199)ApiKey(65-73)ApiKeyWithSecret(81-90)AddPermissionInput(46-49)CreateApiKeyInput(484-491)AddRoleForApiKeyInput(51-54)RemoveRoleFromApiKeyInput(1381-1384)
api/src/unraid-api/graph/resolvers/resolvers.module.ts (5)
api/src/unraid-api/graph/graph.module.ts (1)
Module(27-72)api/src/unraid-api/graph/resolvers/api-key/api-key.module.ts (1)
Module(7-11)api/src/unraid-api/graph/resolvers/disks/disks.module.ts (1)
Module(6-10)api/src/unraid-api/graph/resolvers/logs/logs.module.ts (1)
Module(6-10)api/src/unraid-api/auth/auth.module.ts (1)
Module(16-71)
api/src/unraid-api/graph/resolvers/disks/disks.resolver.ts (1)
web/composables/gql/graphql.ts (1)
Disk(501-522)
api/src/unraid-api/graph/resolvers/flash/flash.resolver.ts (1)
web/composables/gql/graphql.ts (1)
Flash(657-662)
api/src/unraid-api/graph/resolvers/cloud/cloud.resolver.ts (16)
api/src/unraid-api/graph/resolvers/array/array.resolver.ts (1)
Resolver(12-35)api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.ts (1)
Resolver(17-100)api/src/unraid-api/graph/resolvers/array/parity.resolver.ts (1)
Resolver(11-42)api/src/unraid-api/graph/resolvers/config/config.resolver.ts (1)
Resolver(9-25)api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts (1)
Resolver(29-238)api/src/unraid-api/graph/resolvers/disks/disks.resolver.ts (1)
Resolver(10-38)api/src/unraid-api/graph/resolvers/display/display.resolver.ts (1)
Resolver(61-122)api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts (1)
Resolver(9-32)api/src/unraid-api/graph/resolvers/flash/flash.resolver.ts (1)
Resolver(9-27)api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (1)
Resolver(13-48)api/src/unraid-api/graph/resolvers/info/info.resolver.ts (1)
Resolver(31-120)api/src/unraid-api/graph/resolvers/owner/owner.resolver.ts (1)
Resolver(9-43)api/src/unraid-api/graph/resolvers/vars/vars.resolver.ts (1)
Resolver(8-22)api/src/unraid-api/graph/services/services.resolver.ts (1)
Resolver(13-54)api/src/unraid-api/graph/shares/shares.resolver.ts (1)
Resolver(8-26)api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (1)
Resolver(16-146)
api/src/unraid-api/graph/resolvers/array/array.model.ts (4)
api/src/unraid-api/graph/resolvers/api-key/api-key.model.ts (3)
ObjectType(19-30)ObjectType(32-64)ObjectType(66-71)api/src/unraid-api/graph/resolvers/array/parity.model.ts (2)
ObjectType(3-19)ObjectType(21-25)api/src/unraid-api/graph/resolvers/base.model.ts (1)
ObjectType(47-51)web/composables/gql/graphql.ts (6)
Capacity(353-358)ArrayDisk(144-186)Node(1022-1024)ArrayDiskInput(199-204)ArrayStateInput(317-320)ArrayMutations(238-249)
api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts (6)
api/src/store/index.ts (2)
store(6-12)getters(18-29)api/src/remoteAccess/remote-access-controller.ts (1)
RemoteAccessController(17-137)api/src/store/modules/config.ts (2)
logoutUser(77-100)updateAllowedOrigins(189-191)api/src/store/actions/setup-remote-access.ts (1)
setupRemoteAccessThunk(26-50)api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts (2)
dynamicRemoteAccessSettings(155-169)updateAllowedOrigins(104-107)api/src/common/allowed-origins.ts (2)
getExtraOrigins(64-74)getAllowedOrigins(91-99)
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (1)
api/src/graphql/resolvers/query/info.ts (7)
generateApps(39-49)generateCpu(61-77)generateDevices(176-422)generateDisplay(79-116)generateMemory(128-174)generateOs(51-59)generateVersions(118-126)
api/src/graphql/resolvers/query/info.ts (2)
api/src/unraid-api/graph/resolvers/display/display.resolver.ts (1)
display(69-111)api/src/unraid-api/graph/resolvers/info/info.resolver.ts (1)
display(82-84)
🪛 GitHub Check: Test API
api/src/types/my-servers-config.ts
[failure] 1-1:
Insert import·{·z·}·from·'zod';⏎⏎
[failure] 2-2:
Delete ⏎import·{·z·}·from·'zod';⏎
api/src/unraid-api/auth/auth.service.spec.ts
[failure] 7-7:
Delete Resource,·Role·}·from·'@app/unraid-api/graph/resolvers/base.model.js';⏎import·{·
[failure] 11-11:
Replace FastifyRequest·}·from·'@app/unraid-api/types/fastify with ApiKey,·ApiKeyWithSecret·}·from·'@app/unraid-api/graph/resolvers/api-key/api-key.model
[failure] 12-12:
Replace ApiKey,·ApiKeyWithSecret·}·from·'@app/unraid-api/graph/resolvers/api-key/api-key with Resource,·Role·}·from·'@app/unraid-api/graph/resolvers/base
[failure] 13-13:
Insert .js';⏎import·{·FastifyRequest·}·from·'@app/unraid-api/types/fastify
api/src/graphql/resolvers/query/info.ts
[failure] 36-36:
Delete .js';⏎import·{·getCasePathIfPresent·}·from·'@app/core/utils/images/image-file-helpers
[failure] 114-114:
Insert ,
🪛 GitHub Check: Build API
api/src/types/my-servers-config.ts
[failure] 1-1:
Insert import·{·z·}·from·'zod';⏎⏎
[failure] 2-2:
Delete ⏎import·{·z·}·from·'zod';⏎
api/src/unraid-api/auth/auth.service.spec.ts
[failure] 7-7:
Delete Resource,·Role·}·from·'@app/unraid-api/graph/resolvers/base.model.js';⏎import·{·
[failure] 11-11:
Replace FastifyRequest·}·from·'@app/unraid-api/types/fastify with ApiKey,·ApiKeyWithSecret·}·from·'@app/unraid-api/graph/resolvers/api-key/api-key.model
[failure] 12-12:
Replace ApiKey,·ApiKeyWithSecret·}·from·'@app/unraid-api/graph/resolvers/api-key/api-key with Resource,·Role·}·from·'@app/unraid-api/graph/resolvers/base
[failure] 13-13:
Insert .js';⏎import·{·FastifyRequest·}·from·'@app/unraid-api/types/fastify
api/src/graphql/resolvers/query/info.ts
[failure] 36-36:
Delete .js';⏎import·{·getCasePathIfPresent·}·from·'@app/core/utils/images/image-file-helpers
[failure] 114-114:
Insert ,
🪛 Biome (1.9.4)
api/src/unraid-api/graph/user/user.resolver.ts
[error] 11-11: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
api/src/unraid-api/graph/resolvers/array/parity.resolver.ts
[error] 19-19: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
[error] 24-24: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
[error] 29-29: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
[error] 34-34: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
🪛 GitHub Actions: CI - Main (API)
api/src/graphql/resolvers/query/info.ts
[error] 15-15: Insert getCasePathIfPresent·}·from·'@app/core/utils/images/image-file-helpers.js';⏎import·{· prettier/prettier
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (145)
api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.spec.ts (3)
9-10: Good module restructuring.The updated imports correctly reference the model definitions from their new location in
docker.model.jsinstead of the generated types file. This change aligns with the project's move toward a code-first GraphQL approach.
49-49: Mock container updated properly with names property.The addition of the
namesproperty to the mock container object ensures the test data matches the updatedDockerContainermodel requirements.
69-69: Mock container correctly updated for stop test.Similar to the start test, the mock container in the stop test is properly updated with the names property to match the model requirements.
api/src/unraid-api/graph/resolvers/logs/logs.model.ts (3)
1-4: Imports look good.The appropriate GraphQL decorators and types are imported from NestJS, along with the custom DateTimeScalar.
5-18: LogFile class properly models log file metadata.The class is correctly defined as a GraphQL ObjectType with appropriate field types and descriptive comments. The fields cover all essential log file metadata (name, path, size, and modification time).
20-33: LogFileContent class is well-structured.This class appropriately captures the content data along with metadata like total lines and starting line. Making the starting line nullable is the right approach since it's optional.
api/src/unraid-api/graph/resolvers/config/config.model.ts (1)
1-13: Implement clear type definition for ConfigThe implementation of the Config class correctly follows the code-first GraphQL approach with appropriate field decorators and types. The class properly implements the Node interface with required fields.
api/src/unraid-api/graph/resolvers/config/config.resolver.ts (2)
1-1: Updated imports and resolver approachThe changes correctly implement the code-first approach by updating the resolver decorator to use a type reference (
@Resolver(() => Config)) rather than a string name. The imports have been properly updated to reflect the new structure.Also applies to: 5-7, 9-9
17-23: Verify config query return structureThe config query implementation returns the expected structure with id, valid, and error fields. However, there's a potential issue with the error field when configValid is true - it explicitly sets the value to null. Ensure this is the intended behavior as the type definition shows error as an optional field, which could be undefined rather than null.
api/src/unraid-api/graph/resolvers/resolvers.module.ts (1)
4-6: Import path updates look goodThese import path changes correctly reflect the code reorganization where Connect-related services are moved to the resolvers directory, and MeResolver is now imported from the user directory. This supports the code-first GraphQL approach mentioned in the PR.
Also applies to: 34-34
api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts (6)
3-9: Well-defined enum for minigraph statusThe
MinigraphStatusenum provides clear, descriptive states that follow a logical progression from initialization to connection states, including error handling states.
11-18: Properly structured API key response modelThe
ApiKeyResponseclass follows GraphQL object type best practices with appropriate field decorators and nullability settings. The non-nullablevalidfield ensures clients always receive this critical validation status.
20-30: Well-defined minigraph response modelThe
MinigraphqlResponseclass properly uses the enum type for status field and includes optional fields for timeout and error information.
32-42: Standard cloud response modelThe
CloudResponseclass follows the established pattern with a required status and optional fields for additional information.
44-54: Consistent relay response modelThe
RelayResponseclass maintains the same structure as other response types, providing a consistent API.
56-75: Comprehensive cloud aggregate modelThe
Cloudclass effectively aggregates all the response types into a cohesive data structure. This model provides a complete view of the cloud status with all necessary components.api/src/unraid-api/graph/resolvers/cloud/cloud.resolver.ts (4)
1-8: Clean import organizationThe imports have been streamlined to include only what's needed for the current functionality. The transition to code-first GraphQL is reflected in the imports.
10-10: Improved resolver type safetyThe resolver now uses a function that returns the
Cloudtype instead of a string identifier, enhancing type safety in the GraphQL schema.
12-12: Explicit query return typeThe query decorator now explicitly defines the return type as
Cloud, improving schema accuracy and developer experience.
18-38: Enhanced type safety and consistency in cloud resolverThe resolver method now has an explicit return type of
Promise<Cloud>and usesundefinedconsistently for null/optional values instead of mixingnullandundefined.Two specific improvements:
- Changed
timeout: nulltotimeout: undefinedfor consistency- Modified error concatenation to return
undefinedinstead of empty stringapi/src/unraid-api/graph/graph.module.ts (8)
6-6: Good addition for validation.Adding
NoUnusedVariablesRuleis a good practice for eliminating unused variables in GraphQL operations.
17-21: Well-structured auth enum imports.Restructuring the auth-related imports improves code organization and readability. These enums will be important for the code-first GraphQL approach.
23-23: Good addition of the PluginSchemaService.This service will be responsible for schema generation in the code-first approach.
32-34: Proper dependency injection setup.The module is correctly configured to import and inject the
PluginSchemaServicefor use in the factory function.
36-36: Successful transition to code-first approach.Setting
autoSchemaFile: trueenables in-memory schema generation based on code decorators rather than SDL files. This is a key part of the code-first GraphQL approach.
52-59: Appropriate scalar resolver configuration.Explicitly defining the scalar resolvers is necessary for the code-first approach. All common scalar types are properly configured.
60-63: Good schema build options configuration.The
buildSchemaOptionsobject correctly sets up:
- The
AuthDirectivefor authorization- Integer-based number scalar mode
These settings will ensure proper schema generation for the code-first approach.
69-69: Proper service registration.Adding
PluginSchemaServiceto the providers array correctly makes it available for dependency injection.api/src/unraid-api/graph/resolvers/disks/disks.service.ts (4)
1-1: Good update to the importsAdding
NotFoundExceptionis appropriate for the new error handling in thegetDiskmethod.
7-12: Import path changes align with code-first GraphQL approachChanged import paths now reference the local model file instead of the generated types, which is consistent with the code-first GraphQL approach being implemented.
43-50: Well-implemented disk retrieval methodThe new
getDiskmethod is implemented correctly:
- Reuses the existing
getDisks()method to avoid code duplication- Includes appropriate error handling with
NotFoundException- Has a clear and descriptive error message
This addition improves the service's API by allowing retrieval of individual disks by ID.
55-55: Updated return type to match implementation realityThe return type change from
Promise<Disk>toPromise<Omit<Disk, 'temperature'>>accurately reflects that thetemperatureproperty is populated later, not in this method.api/src/unraid-api/graph/resolvers/disks/disks.model.ts (6)
5-17: Well-structured filesystem type enumThe
DiskFsTypeenum is properly defined with good GraphQL registration. The description adds clarity to the schema documentation.
19-30: Clear interface type definitionsThe
DiskInterfaceTypeenum covers all necessary interface types and includes a sensible UNKNOWN fallback option. The GraphQL registration and description are helpful.
42-55: Well-structured DiskPartition classThe
DiskPartitionclass definition includes:
- Appropriate validation decorators
- Descriptive GraphQL field definitions
- Clear property types
This looks good for representing partition information.
62-126: Well-defined disk properties with validationThe disk properties are well-structured with:
- Appropriate validation decorators for each field type
- Descriptive GraphQL field annotations
- Clear field descriptions
This provides a solid foundation for the GraphQL schema and ensures proper validation of incoming data.
127-130: Good handling of optional temperature fieldThe temperature field is properly marked as:
- Optional with @IsOptional()
- Nullable in the GraphQL schema
- Has appropriate type validation
This correctly handles cases where temperature data might not be available.
132-136: Proper array validation for partitionsThe partitions field uses:
- @isarray() to validate it's an array
- @ValidateNested() to validate each item
- @type() to ensure proper deserialization
- Appropriate GraphQL field definition
This ensures proper validation of the nested partition objects.
api/src/unraid-api/graph/resolvers/logs/logs.resolver.ts (6)
6-7: Imports properly updated for the code-first approach.The import statements have been appropriately updated to import the newly defined model classes (
LogFileandLogFileContent). This supports the transition to a code-first GraphQL approach, where TypeScript classes with decorators define the schema.
10-10: Resolver decorator properly updated.The
@Resolverdecorator now uses a type function returningLogFileinstead of a string identifier. This is consistent with the code-first approach and provides better type safety and IDE support.
14-14: Query return type appropriately defined.The
@Querydecorator and method signature have been updated to explicitly returnPromise<LogFile[]>. This improves type safety and ensures the GraphQL schema accurately represents the resolver's return type.Also applies to: 20-20
24-24: Query parameters and return type properly defined.The
logFilequery has been updated with:
- Explicit return type of
LogFileContent- Nullable arguments properly annotated with both TypeScript optional syntax and GraphQL nullable configuration
This ensures type consistency between the GraphQL schema and implementation.
Also applies to: 32-34
38-38: Subscription return type properly updated.The
@Subscriptiondecorator now returnsLogFileContentinstead of using a string identifier, consistent with the code-first approach.
1-67:❓ Verification inconclusive
Verify the model definitions match expected usage.
The implementation looks solid. One suggestion is to verify that the nullable fields in GraphQL match the TypeScript optionality to prevent runtime type errors.
🏁 Script executed:
#!/bin/bash # Checking model definitions to ensure they match resolver usage cat api/src/unraid-api/graph/resolvers/logs/logs.model.jsLength of output: 145
Action Required: Verify GraphQL Model Definitions and Their File Location
It appears that the expected file for model definitions (
api/src/unraid-api/graph/resolvers/logs/logs.model.js) could not be found. Please verify:
- Whether the model definitions (e.g.,
LogFileandLogFileContent) exist in a different file (perhaps as a TypeScript file, such aslogs.model.ts) or have been renamed/moved.- That the nullable fields defined in your GraphQL schema properly correspond to the TypeScript optionality in the model definitions. This check is crucial to prevent runtime type errors.
Once you confirm the correct file location and ensure that the GraphQL field nullability aligns with the TypeScript definitions, you can update the import path in the resolver accordingly.
api/src/unraid-api/graph/resolvers/disks/disks.resolver.ts (4)
10-10: Great move to code-first GraphQL approach.Changing from string-based
@Resolver('Disk')to type-based@Resolver(() => Disk)improves type safety and IDE support.
14-14: Good type-safety enhancement.Changing from
@Query('disks')to@Query(() => [Disk])provides better type information and makes the return type explicit.
24-32: Well-structured single disk query implementation.The new query is properly implemented with:
- Appropriate return type annotation
- Permission checks consistent with the existing
disksquery- Clear parameter handling with
@ArgsThis enhances the API by allowing clients to fetch a specific disk by ID.
24-37:✅ Verification successful
Verify consistent error handling across resolver methods.
Ensure that
disksService.getDisk(id)handles the case when a disk with the given ID is not found. According to the AI summary, it should throw aNotFoundException, which is appropriate.
🏁 Script executed:
#!/bin/bash # Check if the getDisk method in DisksService handles not found cases grep -A 10 "getDisk" api/src/unraid-api/graph/resolvers/disks/disks.service.tsLength of output: 852
Verified Error Handling in disksService.getDisk
The
disksService.getDiskmethod correctly throws aNotFoundExceptionwhen a disk with the provided ID is not found. This confirms that consistent error handling is in place across the resolver methods.
- Verified that
getDisk()checks for a missing disk and throwsNotFoundException.- No further changes required regarding error handling consistency.
api/src/unraid-api/graph/services/service.model.ts (1)
11-27: Looks good!
TheServiceclass structure is straightforward and aligns well with theNodeinterface. No issues found.api/src/unraid-api/graph/resolvers/base.model.ts (2)
41-45: Enum usage is clear.
TheRoleenum is concise and effectively captures possible roles. No issues found.
47-51: Node implementation is good.
TheNodeclass meets the code-first approach requirements. Looks fine.api/src/unraid-api/graph/resolvers/api-key/api-key.model.ts (3)
19-30: Consider validating or restricting empty lists foractions.
The current constraints (@IsArray(),@ArrayMinSize(1)) appropriately ensure a non-empty array of actions. If you foresee scenarios where permissions might not have any actions, consider making it optional rather than requiring at least one action.
66-71: Key inApiKeyWithSecretis publicly exposed.
Ensure that you only return the secret key where appropriate (e.g., upon creation) and avoid exposing it in queries that fetch existing API keys. Consider restricting it in resolvers or adding an access guard.
86-118: Properly validate nestedpermissions.
Nested permissions are optional but validated with@ValidateNested. Ensure the rest of your resolvers handle the absence ofpermissionsgracefully if it’s not provided.api/src/unraid-api/graph/resolvers/info/display.model.ts (1)
24-37: Optional validation forCasefields.
All fields inCaseare optional strings. If there's any requirement around formatting or length, you might include appropriate class-validator decorators. Otherwise, this looks fine.api/src/unraid-api/graph/resolvers/display/display.resolver.ts (1)
113-113: Validate subscription topic usage.
Make sure your subscription consumer code checks message types increateSubscription(PUBSUB_CHANNEL.DISPLAY). Otherwise, unexpected messages might be relayed to clients.api/src/unraid-api/graph/resolvers/info/info.model.ts (1)
208-210: Handle potential expansions forTheme.
ThisThemeis in all caps, while the other file uses lowercase for the same'white'theme. Consider unifying naming conventions and definitions.Do you plan to unify or do these represent different enumerations conceptually? If identical, let's unify them to maintain code clarity.
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (7)
17-29: Good job updating imports for code-first GraphQL approachThe new imports from dedicated model files align well with the code-first GraphQL approach, providing better type safety and structure than string-based type definitions.
31-31: Appropriate use of type function in @resolver decoratorChanging from a string identifier to a function that returns the
Infotype follows best practices for code-first GraphQL implementations.
33-33: Query decorator correctly uses type functionGood update to the
@Querydecorator using a type function instead of a string identifier.
39-53: Well-structured Info object with proper typingThe
infomethod now returns a properly typedPromise<Info>object with all relevant fields. Each field is appropriately awaited and has consistent structure.
56-59: Good implementation of time field resolverThe
timemethod replaces the previousnowmethod with a more descriptive name and proper return type annotation.
61-109: Well-implemented field resolvers with proper type annotationsAll resolver field methods have appropriate
@ResolveFielddecorators with type functions and explicit return type annotations, which enhances type safety and code clarity.
111-111: Subscription decorator properly updated to type functionGood update to the
@Subscriptiondecorator using a type function instead of a string identifier.api/src/store/modules/minigraph.ts (1)
8-8: Import path updated correctly for MinigraphStatusThe import path for MinigraphStatus has been updated from the generated types to a specific model file, aligning with the code-first GraphQL approach.
api/src/utils.ts (1)
7-7: Import path updated correctly for UserAccountThe import path for UserAccount has been updated from the generated types to the new model file, aligning with the code-first GraphQL approach.
api/src/store/modules/emhttp.ts (1)
20-20: Import path updated correctly for ArrayDiskThe import path for ArrayDisk has been updated from the generated types to a specific model file, aligning with the code-first GraphQL approach.
api/src/unraid-api/graph/user/user.model.ts (1)
1-22: UserAccount model looks well-structuredThis new UserAccount model is well-defined with proper field descriptions and types. The class uses appropriate GraphQL decorators and follows NestJS conventions.
api/src/types/my-servers-config.ts (1)
1-2: Updated import paths align with code-first GraphQL approach.The import paths for
MinigraphStatusandDynamicRemoteAccessTypehave been updated to use the new model files instead of the generated types. This change is consistent with the PR's objective of adopting a code-first GraphQL approach.🧰 Tools
🪛 GitHub Check: Test API
[failure] 1-1:
Insertimport·{·z·}·from·'zod';⏎⏎
[failure] 2-2:
Delete⏎import·{·z·}·from·'zod';⏎🪛 GitHub Check: Build API
[failure] 1-1:
Insertimport·{·z·}·from·'zod';⏎⏎
[failure] 2-2:
Delete⏎import·{·z·}·from·'zod';⏎api/src/unraid-api/graph/user/user.resolver.ts (4)
6-7: Updated imports to use code-first GraphQL models.The imports have been updated to use the new model files, which is consistent with the code-first GraphQL approach.
9-9: Resolver now properly typed with UserAccount.Changed from string-based type name ('Me') to the actual UserAccount class, improving type safety.
13-13: Query return type updated to UserAccount.This change improves type consistency between the resolver and the query.
19-19: Method signature now uses UserAccount type.Both parameter and return types now use UserAccount, enhancing type safety.
api/src/store/types.ts (2)
20-21: Updated import paths for model types.The import paths have been updated to use the code-first model files instead of the generated types. This change is consistent with the PR's approach and improves code organization.
79-83: Type safety preserved for MinigraphStatus enum usage.After changing the import source for MinigraphStatus, its usage in the MOTHERSHIP_CRITICAL_STATUSES constant remains correct, preserving type safety.
api/src/store/modules/dynamic-remote-access.ts (1)
5-6: Updated import paths with mixed approach.The imports have been updated, with:
AccessUrlInputandURL_TYPEstill coming from a generated file but with an updated pathDynamicRemoteAccessTypemoved to the code-first model approachThis hybrid approach maintains consistency with the PR's objective while ensuring all necessary types are available.
api/codegen.ts (1)
68-68: All good on the default export.No issues found with this export statement. The configuration object is neatly packaged for code generation usage.
api/src/unraid-api/graph/services/services.resolver.ts (1)
5-11: Imports are correctly updated for modular clarity.These revised imports align with the new organizational structure of your GraphQL schema and models. This helps maintain a clean separation of concerns for each module.
api/src/unraid-api/graph/resolvers/connect/connect.model.ts (2)
1-20: Enums structure looks solid.The enumerations
WAN_ACCESS_TYPE,WAN_FORWARD_TYPE, andDynamicRemoteAccessTypeare well-defined, offering clear distinctions among access and forwarding methods.
21-74: Concise input classes with appropriate validation.Your
ConnectUserInfoInput,ConnectSignInInput, andAllowedOriginInputmaintain clarity by specifying required fields and properly using class-validator decorators (@IsString,@IsEmail, etc.). This ensures reliable validation at runtime.api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts (4)
1-28: No immediate issues with imports.
All required dependencies are imported correctly, and there is no redundancy or obvious security vulnerability in how they are loaded.
37-63: Confirm that returning a static object is intended.
Theconnect()query method always returns a static object withdynamicRemoteAccessset toDISABLED. If this is an intentional placeholder (e.g., for an uninitialized state), it may be fine. Otherwise, ensure the returned response reflects real-time data from the store or a service.
217-226: Looks good for retrieving extra allowed origins.
Returning a filtered list of extra origins aligns well with the pattern ingetExtraOrigins. No concerns noted.
228-237: No major concerns in updating allowed origins.
This mutation updates the store with the new origins and returns the updated list. The approach seems correct, particularly as it leverages existing store logic.api/src/core/pubsub.ts (1)
22-22: Added new PARITY channel to PUBSUB_CHANNEL enumThis addition looks good - it extends the available channels for PubSub subscriptions to include parity-related events.
api/src/unraid-api/graph/resolvers/flash/flash.model.ts (1)
1-18: Well-structured Flash GraphQL model classThis new model correctly implements the Node interface and provides appropriate field definitions with proper GraphQL type decorators. The class follows the code-first GraphQL approach mentioned in the PR.
api/src/core/utils/array/array-is-running.ts (1)
2-2: Updated import path for ArrayStateThe import path has been updated to reflect the new location of the ArrayState enum as part of the code-first GraphQL approach. This change ensures the utility function continues to work with the reorganized type definitions.
api/src/unraid-api/graph/resolvers/array/array.service.spec.ts (1)
10-10: Updated import path for array modelsThe import has been correctly updated to use the new location of array-related types from the code-first GraphQL model definitions.
api/src/unraid-api/auth/auth.service.spec.ts (2)
7-7: Imports look consistent with usage.🧰 Tools
🪛 GitHub Check: Test API
[failure] 7-7:
DeleteResource,·Role·}·from·'@app/unraid-api/graph/resolvers/base.model.js';⏎import·{·🪛 GitHub Check: Build API
[failure] 7-7:
DeleteResource,·Role·}·from·'@app/unraid-api/graph/resolvers/base.model.js';⏎import·{·
12-13: No issues found with the new imports.🧰 Tools
🪛 GitHub Check: Test API
[failure] 12-12:
ReplaceApiKey,·ApiKeyWithSecret·}·from·'@app/unraid-api/graph/resolvers/api-key/api-keywithResource,·Role·}·from·'@app/unraid-api/graph/resolvers/base
[failure] 13-13:
Insert.js';⏎import·{·FastifyRequest·}·from·'@app/unraid-api/types/fastify🪛 GitHub Check: Build API
[failure] 12-12:
ReplaceApiKey,·ApiKeyWithSecret·}·from·'@app/unraid-api/graph/resolvers/api-key/api-keywithResource,·Role·}·from·'@app/unraid-api/graph/resolvers/base
[failure] 13-13:
Insert.js';⏎import·{·FastifyRequest·}·from·'@app/unraid-api/types/fastifyapi/src/unraid-api/auth/api-key.service.spec.ts (7)
6-6: Import usage is valid.
14-19: The new imports align properly with the tested model classes.
70-75: Defining a permissions array here ensures the test object is valid per the model constraints.
297-322: Validates the transformation logic and ensures the correct fields are carried over.
466-475: Good negative test case for empty roles and permissions.
503-531: Robust approach to ignore and skip invalid API key files without halting execution.
566-585: Comprehensive checks for invalid structures bolster your error handling.api/src/unraid-api/graph/resolvers/array/array.model.ts (5)
1-23:CapacityandArrayCapacitylook well-defined for representing free, used, and total storage fields.
24-95:ArrayDiskclass provides thorough coverage of disk attributes, with clear and descriptive GraphQL fields.
96-125:UnraidArraycorrectly composes disks, parity, caches, and capacity within a single model, adhering to theNodeinterface.
126-161: Input classes andArrayMutationsare well-structured and should simplify resolver logic for array operations.
162-217: Enums are clearly named and match typical array management states, aiding readability and maintainability.api/src/unraid-api/graph/resolvers/array/parity.model.ts (1)
1-25: GraphQL model classes look good with appropriate field decorators.This is a clean implementation of the GraphQL object types for parity checks. The model classes correctly use the
@ObjectType()and@Field()decorators from NestJS GraphQL.A couple of suggestions:
- Consider adding field descriptions using the
descriptionproperty in the@Field()decorator to improve documentation in the GraphQL schema- Consider adding validation decorators (e.g., from class-validator) for stronger runtime validation
api/src/core/modules/array/get-array-data.ts (2)
6-11: Updated imports for code-first approach.The imports have been correctly updated to use the new model classes from the code-first GraphQL implementation instead of generated types.
13-13: Updated return type to UnraidArray.The function signature now correctly reflects the code-first model type.
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts (2)
1-2: Updated imports for code-first approach.The imports have been correctly updated to use the
Mutationdecorator and to import the model class.
4-9:❓ Verification inconclusive
Refactored resolver to use code-first approach.
The resolver has been successfully refactored from a string-based approach to a code-first approach with proper type definitions. The mutation now returns a new instance of the
MutationTypeclass, which presumably contains nested fields for the various mutation categories.Make sure to verify that existing GraphQL clients can still make the same mutations as before with this new structure.
🌐 Web query:
What are the best practices for migrating from schema-first to code-first GraphQL in NestJS?💡 Result:
Migrating from a schema-first to a code-first GraphQL approach in NestJS requires careful planning to maintain type safety and minimize disruptions. Here are the key best practices:
1. Initial Setup Configuration
Configure
GraphQLModuleto generate schema automatically usingautoSchemaFile:GraphQLModule.forRoot<ApolloDriverConfig>({ driver: ApolloDriver, autoSchemaFile: join(process.cwd(), 'src/schema.gql'), sortSchema: true })This generates SDL from TypeScript decorators while preserving schema consistency[3][6].
2. Incremental Migration Strategy
- Convert types gradually: Start with simple types (e.g., scalar fields) before tackling complex relationships
- Use hybrid mode: Temporarily combine both approaches using
typePathswith code-first decorators[3][9]- Leverage code generation: Use
@nestjs/graphql'sdefinitionsoption to generate TS interfaces from existing SDL during transition[3]3. Type Safety Enforcement
@ObjectType() export class Note { @Field(() => Int) id: number; @Field() title: string; }
- Annotate all fields with
@Field()decorators- Use explicit type signatures for non-string scalar types[6][9]
4. Resolver Alignment
@Resolver(() => Note) export class NoteResolver { @Query(() => [Note]) async notes() { return this.notesService.findAll(); } }
- Match resolver method return types to GraphQL type definitions
- Use NestJS resource generator for boilerplate (
nest generate resource)[6][9]5. Schema Validation
- Compare auto-generated
schema.gqlwith original SDL- Use schema diff tools to catch discrepancies
- Implement end-to-end tests for critical queries/mutations[6][9]
6. Modularization Techniques
// users.module.ts @Module({ imports: [GraphQLModule.forFeature({ autoSchemaFile: true })] }) export class UsersModule {}
- Break schema into domain-specific modules
- Use
GraphQLModule.forFeature()for federated services[9]7. Tooling Integration
- Prisma ORM: Combine with code-first for end-to-end type safety
model Note { id Int @id @default(autoincrement()) title String }
- Apollo Studio: Monitor schema changes during migration
- GraphQL Code Generator: Maintain client-side type parity[7][9]
Pitfalls to Avoid
- Mixed approach conflicts: Remove all
typePathsreferences once migration completes[3][8]- Directive mismatches: Reimplement custom directives using
@Directive()decorators- Nullable vs Non-nullable: Audit existing schema nullability rules before annotating[10]
By following these practices, teams at companies like Cato Networks have successfully transitioned while maintaining 100% backward compatibility during migration periods[4][9]. The code-first approach ultimately reduces schema-resolver mismatch errors by 63% according to Apollo's 2024 State of GraphQL report[1][2].
Citations:
- 1: https://www.apollographql.com/blog/schema-first-vs-code-only-graphql
- 2: https://graphql.org/conf/2023/sessions/118f99976647d953d6554bac33dbf3bf/
- 3: https://docs.nestjs.com/graphql/quick-start
- 4: https://www.catonetworks.com/blog/how-schema-first-approach-creates-better-api-reduces-boilerplate-and-eliminates-human-error/
- 5: https://the-guild.dev/graphql/hive/docs/other-integrations/code-first
- 6: https://dev.to/andisiambuku/working-with-graphql-in-nest-js-137
- 7: https://fullstackdatasolutions.com/blog/graphql/code-first-graphql
- 8: nestjs/graphql#937
- 9: https://www.restack.io/p/nestjs-graphql-best-practices
- 10: https://dgraph.io/docs/graphql/schema/migration/
GraphQL Code-First Migration Verified – Ensure Backward Compatibility
The resolver has been successfully refactored to the code-first approach by returning a new instance of
MutationTypewith proper type definitions. In line with best practices for migrating from schema-first to code-first GraphQL in NestJS, please verify the following:
- Confirm that your
GraphQLModuleconfiguration (usingautoSchemaFileor equivalent) is maintaining an accurate and consistent schema.- Ensure that the mutation’s schema generated from
MutationTyperemains fully backward compatible with existing GraphQL clients.- Run an end-to-end test for the mutation to verify that no unexpected behavior is introduced after the refactor.
api/src/unraid-api/graph/resolvers/flash/flash.resolver.ts (3)
6-7: Updated imports for code-first approach.The imports have been correctly updated to use the model classes from the code-first GraphQL implementation.
9-9: Updated resolver decorator for code-first approach.The resolver is now correctly defined using the code-first approach with proper type reference.
11-11: Updated query decorator for code-first approach.The query is now correctly defined using the code-first approach with proper return type specification.
api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts (3)
5-5: Import changes improve type organization and maintainabilityThe import changes move from using generated types to explicitly defined models in the codebase, which aligns with the code-first GraphQL approach. This makes the code more maintainable as it directly references source code rather than generated code.
Also applies to: 7-7
9-9: Improved type safety with function-based Resolver annotationChanging from a string-based resolver annotation to a function-based one provides better type safety and enables IDE support for type checking and navigation.
13-13: Enhanced field documentation and type safetyAdding explicit return types and descriptions to the ResolveField decorators improves API documentation and ensures type safety. This change makes the API more self-documenting and provides better developer experience for API consumers.
Also applies to: 23-23
api/src/unraid-api/graph/resolvers/mutation/mutation.model.ts (1)
15-22: Review singleton/instance management in Mutation classThe Mutation class initializes array and docker fields with new instances directly in the property declarations. If these instances need to maintain state or be shared across the application, consider using dependency injection instead of direct instantiation.
Also, verify if there's potential duplication between these model definitions and the types already generated in the web/composables/gql/graphql.ts file (as seen in the relevant code snippets).
api/src/unraid-api/graph/resolvers/array/array.resolver.ts (3)
9-10: Import changes improve maintainabilityThe import updates are consistent with the code-first GraphQL approach, importing from defined models rather than generated types.
23-23: Improved service usage with dependency injectionUpdating the array method to use the service method instead of directly calling a function improves dependency injection and separation of concerns. This makes the code more testable and maintainable.
26-26: Enhanced type safety with function-based Subscription decoratorChanging to a function-based type definition in the Subscription decorator improves type safety and documentation, consistent with the code-first GraphQL approach.
api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (3)
5-10: Import changes align with code-first approachThe updated imports for Resource and Docker-related models are consistent with the code-first GraphQL approach, improving maintainability by referencing defined models rather than generated types.
13-13: Improved type safety with function-based decorator annotationsConverting string-based decorators to function-based ones provides better type safety, enables IDE support, and improves documentation. This is a good practice for GraphQL resolvers.
Also applies to: 22-22, 34-34, 44-44
39-43: Added permission checks for networks endpointAdding the UsePermissions decorator to the networks method ensures consistent security controls across all resolver methods. This improves the overall security posture of the API.
api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.ts (10)
5-15: Improved import structure with explicit JS extensionsThe imports now use explicit
.jsextensions, which is a good practice for module resolution in TypeScript with ES modules. The addition of thevalidateObjectutility also prepares for the validation enhancements in this file.
17-17: Enhanced type safety with function-based resolver declarationChanged from string-based
@Resolver('ApiKey')to function-based@Resolver(() => ApiKey), which provides better type safety and enables IDE tooling like autocompletion and static analysis.
24-24: Explicit return type specification for QueryAdding an explicit return type
@Query(() => [ApiKey])improves the GraphQL schema documentation and type safety.
34-34: Added nullable option for apiKey queryThe Query decorator now specifies that this endpoint can return null with
{ nullable: true }, which accurately represents the behavior of thefindByIdmethod that may return null.
44-44: Explicit return type for createApiKey mutationSpecifying the return type
@Mutation(() => ApiKeyWithSecret)improves documentation and type safety in the GraphQL schema.
51-56: Added input validation for createApiKeyNow validates input data before processing using class-validator, which adds a layer of security and ensures data integrity. The parameter name change from
inputtounvalidatedInputalso makes the validation need more explicit.
70-70: Added explicit Boolean return type for addRoleForApiKey mutationSpecifying
@Mutation(() => Boolean)clarifies the expected return type in the GraphQL schema.
80-83: Added input validation for addRoleForApiKeyNow properly validates the input using class-validator before processing, enhancing security and data integrity.
86-86: Added explicit Boolean return type for removeRoleFromApiKey mutationSpecifying
@Mutation(() => Boolean)clarifies the expected return type in the GraphQL schema.
96-98: Added input validation for removeRoleFromApiKeyNow validates input data before processing using class-validator, improving security and data integrity.
api/src/unraid-api/graph/resolvers/array/parity.resolver.ts (3)
1-11: Initial setup for parity resolver with subscription supportThis section properly sets up the necessary imports and creates a PubSub instance for handling real-time updates on parity checks.
12-16: Placeholder implementation for parityHistory queryThe method is properly typed to return an array of ParityCheck objects but currently throws a "Not implemented" error.
Since this is a placeholder implementation, consider adding a TODO comment with an estimated implementation timeline or tracking issue number.
38-41: Subscription implementation for parity updatesThis correctly sets up a GraphQL subscription that returns ParityCheck objects through the PubSub mechanism.
api/src/unraid-api/graph/resolvers/docker/docker.model.ts (7)
7-14: Well-defined ContainerPortType enumThe enum is properly defined and registered for GraphQL, following best practices for GraphQL type definitions.
16-29: Properly structured ContainerPort classThe ContainerPort class has appropriate field decorators and type definitions. The nullable fields are correctly marked.
31-38: Well-defined ContainerState enumThe enum is properly defined and registered for GraphQL, following best practices.
40-71: Well-structured container configuration classesBoth ContainerHostConfig and ContainerMount classes have appropriate field decorators and type definitions.
73-119: Comprehensive DockerContainer class definitionThe class includes all necessary fields with appropriate types and decorators. The nullable fields and descriptions are correctly specified.
121-167: Well-structured DockerNetwork classAll fields are properly defined with appropriate types and decorators.
169-179: Correctly implemented Node interfaceThe Docker class correctly implements the Node interface from base.model.js.
api/src/unraid-api/graph/resolvers/array/array.service.ts (5)
8-8: Renamed import to avoid naming conflictGood practice to rename the imported function to avoid collision with the new method name.
12-18: Updated imports for new model typesImports now reference the new model types, which aligns with the type changes throughout the file.
32-34: Added getter method for array dataAdding this method improves encapsulation and will make it easier to extend functionality in the future.
36-36: Updated return types to UnraidArrayConsistently updated all method return types from ArrayType to UnraidArray, ensuring type safety throughout the service.
Also applies to: 77-77, 94-94, 111-111, 125-125, 139-139, 165-165
72-72: Consistent use of the new getArrayData methodAll direct calls to the utility function have been replaced with calls to the new encapsulated method, which improves consistency and maintainability.
Also applies to: 91-91, 108-108, 122-122, 136-136, 150-150, 211-211
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
api/src/__test__/store/modules/config.test.ts (1)
149-154: 💡 Verification agent🧩 Analysis chain
Fix failing test in CI pipeline.
There's a test failure in the pipeline related to this assertion. The expected arguments for the pubsub.publish call don't match the actual arguments.
🏁 Script executed:
#!/bin/bash # Examine the test failure in more detail # Let's find the failing test and check how 'owner' is being constructed cat api/src/__test__/store/modules/config.test.ts | grep -A 10 "expect(pubsub.publish).toHaveBeenCalledWith(PUBSUB_CHANNEL.OWNER" # Check how loginUser is defined to understand what it's publishing rg -A 5 "loginUser = " api/src/store/modules/config.tsLength of output: 1205
Action Required: Align pubsub.publish assertions with actual behavior
The CI test fails because the expected arguments for
pubsub.publishare inconsistent. One assertion expects the owner to be built fromuserInfo(usinguserInfo.usernameanduserInfo.avatar), while a later assertion expects a hardcoded object ({ username: 'root', url: '', avatar: '' }). Please verify whether the component should publish the dynamic user info or the default static values. Then, update the test assertions inapi/src/__test__/store/modules/config.test.tsaccordingly, and ensure that theloginUserfunction inapi/src/store/modules/config.tscomplies with the intended behavior.🧰 Tools
🪛 GitHub Check: Test API
[failure] 149-149: src/test/store/modules/config.test.ts > loginUser updates state and publishes to pubsub
AssertionError: expected "spy" to be called with arguments: [ 'OWNER', { owner: { …(2) } } ]Received:
1st spy call:
[
"OWNER",
{
"owner": {
"avatar": "https://via.placeholder.com/200",
"url": "", "username": "testuser", },
]Number of calls: 1
❯ src/test/store/modules/config.test.ts:149:28
🪛 GitHub Actions: CI - Main (API)
[error] 149-149: AssertionError: expected "spy" to be called with arguments: [ 'OWNER', { owner: { …(2) } } ]
♻️ Duplicate comments (6)
api/src/unraid-api/graph/resolvers/info/display.model.ts (1)
39-103: Possible overlap withDisplayininfo.model.ts.
Consider removing one to avoid confusion or unify into a single definition if they must share the same structure.api/src/unraid-api/graph/resolvers/info/info.model.ts (1)
216-277:Displayduplication withdisplay.model.ts.
Evaluate whether both definitions are necessary; consider unifying.api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts (1)
142-146: Await asynchronous remote access operations.
This is identical to a past comment highlighting the need toawaitthese asynchronous methods. Omittingawaitmay cause the mutation to resolve before remote access completes, leaving the system in an inconsistent state.- controller.stopRemoteAccess({ + await controller.stopRemoteAccess({ - controller.beginRemoteAccess({ + await controller.beginRemoteAccess({ - controller.extendRemoteAccess({ + await controller.extendRemoteAccess({Also applies to: 158-162, 163-167
api/src/unraid-api/graph/resolvers/connect/connect.model.ts (1)
110-112: Revisit usage of@IsPort()with numeric fields.This is identical to a past comment pointing out a validation mismatch.
@IsPort()expects a string, while these fields are typed asnumber. Using numeric validators (e.g.,@IsInt(),@Min(1),@Max(65535)) aligns better with integer-based port usage.Also applies to: 134-136, 200-203, 251-253
api/src/unraid-api/graph/resolvers/array/parity.resolver.ts (1)
17-20: Replace 'Object' with a more specific return typeThis is identical to a previously suggested improvement:
Also applies to: 22-25, 27-30, 32-35
api/src/unraid-api/graph/resolvers/array/array.model.ts (1)
194-196: Replace 'Object' type with a more specific return typeThis matches a previously recommended refactor to use an explicit return type.
🧹 Nitpick comments (29)
api/src/remoteAccess/handlers/upnp-remote-access.ts (1)
22-24: Consider removing redundant null check.Line 22 already ensures
urlis either anAccessUrlornull, making the second null check on line 24 redundant.- const url = urlsForServer.urls.find((url) => url.type === URL_TYPE.WAN) ?? null; - - return url ?? null; + const url = urlsForServer.urls.find((url) => url.type === URL_TYPE.WAN) ?? null; + + return url;api/src/unraid-api/cli/apikey/api-key.command.ts (1)
123-123: Consider removing type assertion toanyLine 123 uses
error as anywhich bypasses TypeScript's type checking. Consider using a more specific type for better type safety.- this.logger.error(error as any); + this.logger.error(error instanceof Error ? error.message : String(error));api/src/unraid-api/auth/user.decorator.ts (1)
6-16: Return type decorator parameter is inconsistent with implementation.The decorator is parameterized to return UserAccount (
createParamDecorator<null, UserAccount>), but there's no type checking to ensure the user object actually conforms to the UserAccount type at runtime.Consider adding a runtime type check or assertion to ensure the returned object meets the UserAccount interface expectations.
export const GraphqlUser = createParamDecorator<null, UserAccount>( (data: null, context: ExecutionContext): UserAccount => { if (context.getType<GqlContextType>() === 'graphql') { const ctx = GqlExecutionContext.create(context); const user = ctx.getContext().req.user; - return user; + // Ensure user conforms to UserAccount type + if (!user) { + throw new Error('User not found in GraphQL context'); + } + return user as UserAccount; } else { return context.switchToHttp().getRequest().user; } } );api/src/unraid-api/graph/resolvers/registration/registration.model.ts (3)
69-71: Inconsistent enum name registration convention.The
RegistrationTypeenum is registered with a camelCase name (registrationType), which is inconsistent with the PascalCase used forRegistrationState. Consider using PascalCase for both to maintain consistency.- name: 'registrationType', + name: 'RegistrationType',
44-58: Redundant documentation comments for ENOFLASH states.All ENOFLASH enum values (ENOFLASH1 through ENOFLASH7) have identical comments "No Flash". Consider using a more specific comment for each value or consolidating these enum values if they represent the same state.
86-105: Consider implementing the Node interface.Other model classes like
Configimplement theNodeinterface with anidfield. TheRegistrationclass has aguidfield that could serve as an ID, but it doesn't explicitly implement theNodeinterface. Consider implementing the interface for consistency with other models.@ObjectType() -export class Registration { +export class Registration implements Node { + @Field(() => ID) + id!: string; + @Field(() => ID, { nullable: true }) guid?: string; // Rest of the class... }Then you could set the
idvalue to theguidin your resolver.api/src/unraid-api/graph/resolvers/registration/registration.resolver.ts (1)
35-38: Consider extracting the timestamp conversion logic.The expression for the
expirationfield includes a complex calculation with multiplication by 1000. Consider extracting this logic to a helper function or constant for improved readability.+ const convertToMilliseconds = (seconds: number) => (seconds * 1_000).toString(); + registration: Registration = { guid: emhttp.var.regGuid, type: emhttp.var.regTy, state: emhttp.var.regState, - expiration: (1_000 * (isTrial || isExpired ? Number(emhttp.var.regTm2) : 0)).toString(), + expiration: convertToMilliseconds(isTrial || isExpired ? Number(emhttp.var.regTm2) : 0),api/src/unraid-api/graph/resolvers/docker/docker.service.ts (2)
84-117: Consider making container name extraction more robustThe transformation looks good overall with proper handling of nullish values and type safety. However, there's a potential issue with the
autoStartproperty.The current implementation assumes that
container.Namesalways has at least one element and that it follows a specific format:- autoStart: this.autoStarts.includes(container.Names[0].split('/')[1]), + autoStart: container.Names.length > 0 ? + this.autoStarts.includes(container.Names[0].split('/')[1] || '') : + false,This change adds a check for empty arrays and handles cases where the split operation might not yield the expected result.
159-180: Consider using nullish coalescing for boolean propertiesThe network transformation looks good, but could be improved for boolean properties.
For boolean properties, using
||might not correctly handle explicitfalsevalues. Consider using the nullish coalescing operator (??) instead:- enableIPv6: network.EnableIPv6 || false, + enableIPv6: network.EnableIPv6 ?? false, - internal: network.Internal || false, + internal: network.Internal ?? false, - attachable: network.Attachable || false, + attachable: network.Attachable ?? false, - ingress: network.Ingress || false, + ingress: network.Ingress ?? false, - configOnly: network.ConfigOnly || false, + configOnly: network.ConfigOnly ?? false,This ensures that explicit
falsevalues are preserved rather than being replaced with the default.api/src/unraid-api/graph/resolvers/owner/owner.model.ts (2)
1-16: New Owner model looks good with room for improvementThe Owner model is well structured with proper GraphQL decorators and type definitions. The TODO comment about deprecating this in favor of UserAccount is valuable for future planning.
Consider adding more details to the TODO comment about when and how this deprecation will happen, perhaps with a ticket/issue reference.
7-16: Consider inheriting from the Node base classThe model doesn't extend the Node base class which provides an 'id' field. If Owner objects need to be uniquely identifiable in the GraphQL API, inheriting from Node would be beneficial.
import { Field, ObjectType } from '@nestjs/graphql'; +import { Node } from '@app/unraid-api/graph/resolvers/base.model.js'; /** * @todo Deprecate this type in favor of the UserAccount type */ @ObjectType() -export class Owner { +export class Owner extends Node { @Field(() => String) username!: string; @Field(() => String) url!: string; @Field(() => String) avatar!: string; }api/src/unraid-api/graph/resolvers/vars/vars.model.ts (1)
29-463: Consider refactoring for maintainability, consistency, and type-safety
This class has numerous fields, including repeated patterns for NTP servers, booleans typed as strings in some fields, and an unutilized enumMdStateformdState.Possible improvements:
- Use an array of strings for NTP servers (
ntpServer1,ntpServer2, etc.) to simplify loops and reduce boilerplate fields.- Replace string-typed booleans (e.g.,
enableFruit) with proper boolean or enumerated types for consistency with fields likehideDotFilesoruseNtp.- Update the
mdStatefield at lines 412–413 to reference theMdStateenum for type-safety:- @Field({ nullable: true }) - mdState?: string; + @Field(() => MdState, { nullable: true }) + mdState?: MdState;
- Consider grouping related settings into smaller classes or input types to improve readability and maintainability.
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (4)
4-4: Consider handling errors fromsysteminformationcalls.
IfgetBaseboardorgetSystemthrows, the resolver might fail without producing a descriptive error for the client.
39-53: Optimize parallel calls withPromise.all.
Consecutiveawaitcalls for apps, baseboard, cpu, etc. can be parallelized for better performance.Apply a diff like this:
public async info(): Promise<Info> { - return { - id: 'info', - time: new Date(), - apps: await this.apps(), - baseboard: await this.baseboard(), - cpu: await this.cpu(), - devices: await this.devices(), - display: await this.display(), - machineId: await this.machineId(), - memory: await this.memory(), - os: await this.os(), - system: await this.system(), - versions: await this.versions(), - }; + const [ + apps, + baseboard, + cpu, + devices, + display, + machineId, + memory, + os, + system, + versions + ] = await Promise.all([ + this.apps(), + this.baseboard(), + this.cpu(), + this.devices(), + this.display(), + this.machineId(), + this.memory(), + this.os(), + this.system(), + this.versions(), + ]); + + return { + id: 'info', + time: new Date(), + apps, + baseboard, + cpu, + devices, + display, + machineId, + memory, + os, + system, + versions, + }; }
66-72: Baseboard resolver might benefit from error handling.
IfgetBaseboard()fails, consider catching errors or returning partial data.
105-111: System info extraction.
Likebaseboard(), callinggetSystem()might fail in rare cases. Adding a try/catch block could improve stability.api/src/unraid-api/graph/resolvers/info/display.model.ts (1)
10-12: Consider adding more values or verifying the need for a single-value enum.
A single enum value (e.g.,white) might be limiting if more themes are anticipated.api/src/unraid-api/graph/resolvers/info/info.model.ts (1)
285-288:MemoryFormFactorenum is appropriate.
Possible extension for SODIMM or other.api/src/unraid-api/graph/resolvers/servers/server.model.ts (1)
24-52: Consider implementing the Node interface for consistency.The Server class is well-structured with proper field decorators, but unlike other model classes in your codebase (as seen in the provided snippets), it doesn't implement the Node interface with an id field.
@ObjectType() -export class Server { +export class Server implements Node { + @Field(() => ID) + id!: string; @Field(() => ProfileModel) owner!: ProfileModel;api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts (1)
20-30: Inconsistent usage oferrortyping (string vs. string | null).
Elsewhere in this file,erroris sometimes typed asstring, sometimes asstring | null. Consider unifying usage for clarity.-@Field(() => String, { nullable: true }) -error?: string | null; +@Field(() => String, { nullable: true }) +error?: string;api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts (1)
1-122: Add JSDoc comments for better documentationConsider adding JSDoc comments to classes and fields to improve documentation and provide more context for developers.
+/** + * Enumeration of notification types. + */ export enum NotificationType { UNREAD = 'UNREAD', ARCHIVE = 'ARCHIVE', } +/** + * Enumeration of notification importance levels. + */ export enum NotificationImportance { ALERT = 'ALERT', INFO = 'INFO', WARNING = 'WARNING', }api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts (3)
40-66: Consider externalizing default values.The
connect()query returns hardcoded default values for itsConnectobject. Centralizing these defaults in a configuration or constants file can improve maintainability and consistency, especially as new default fields are added.
90-108: Review concurrency for API restart.The
setTimeoutapproach defers thethis.connectService.restartApi()call by 300ms. If the restart fails or takes longer, the mutation still returns success prematurely. Consider employing error handling or returning an operation status to the client.
204-225: Return early for clarity.The
remoteAccess()query reads the store and returns an object with conditional logic spanning multiple lines. Consider extracting the WAN access logic into a helper method for clarity and testability.api/src/core/modules/disks/id/get-disk.ts (1)
14-14: Use lower-case parameter namingConsider renaming the function parameter
Diskstodisksto align with typical naming conventions.-export const getDisk = async (context: Context, Disks: ArrayDisk[]): Promise<CoreResult> => { +export const getDisk = async (context: Context, disks: ArrayDisk[]): Promise<CoreResult> => {api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts (1)
5-9: Consider a more descriptive namingReturning
new MutationType()is valid, but consider giving the method a more descriptive name (for example,rootMutation()) to clarify its purpose. Also verify you do not require injection or constructors forMutationType.api/src/unraid-api/graph/resolvers/array/parity.resolver.ts (1)
12-15: Method not yet implementedYou might consider implementing the logic for fetching the parity check history or removing this placeholder until ready.
api/src/unraid-api/graph/resolvers/array/array.model.ts (1)
167-171: Consider defaulting to 'STOPPED'
STOPis set as the default desired state. Confirm this aligns with how your domain logic typically initializes the array.api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts (1)
59-70: Consider extracting disk lookup logic to a helper methodThe disk lookup logic is duplicated in both
mountArrayDiskandunmountArrayDisk. Consider extracting this to a private helper method to reduce duplication.+ private findDiskInArray(array: UnraidArray, id: string): ArrayDisk | undefined { + return array.disks.find((disk) => disk.id === id) || + array.parities.find((disk) => disk.id === id) || + array.caches.find((disk) => disk.id === id); + } public async mountArrayDisk(@Args('id') id: string): Promise<ArrayDisk> { const array = await this.arrayService.mountArrayDisk(id); - const disk = - array.disks.find((disk) => disk.id === id) || - array.parities.find((disk) => disk.id === id) || - array.caches.find((disk) => disk.id === id); + const disk = this.findDiskInArray(array, id); if (!disk) { throw new BadRequestException(`Disk with id ${id} not found in array`); } return disk; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (94)
api/src/__test__/store/modules/config.test.ts(1 hunks)api/src/core/modules/disks/id/get-disk.ts(2 hunks)api/src/core/types/states/var.ts(2 hunks)api/src/core/utils/shares/process-share.ts(1 hunks)api/src/core/utils/vms/index.ts(0 hunks)api/src/core/utils/vms/system-network-interfaces.ts(0 hunks)api/src/graphql/resolvers/query/cloud/check-api.ts(1 hunks)api/src/graphql/resolvers/query/cloud/check-cloud.ts(1 hunks)api/src/graphql/resolvers/query/cloud/check-minigraphql.ts(1 hunks)api/src/graphql/resolvers/query/info.ts(8 hunks)api/src/graphql/resolvers/subscription/network.ts(3 hunks)api/src/graphql/schema/utils.ts(1 hunks)api/src/mothership/graphql-client.ts(1 hunks)api/src/mothership/jobs/ping-timeout-jobs.ts(1 hunks)api/src/remoteAccess/handlers/remote-access-interface.ts(1 hunks)api/src/remoteAccess/handlers/static-remote-access.ts(1 hunks)api/src/remoteAccess/handlers/upnp-remote-access.ts(2 hunks)api/src/remoteAccess/remote-access-controller.ts(1 hunks)api/src/store/actions/set-minigraph-status.ts(1 hunks)api/src/store/actions/shutdown-api-event.ts(1 hunks)api/src/store/getters/index.ts(1 hunks)api/src/store/listeners/dynamic-remote-access-listener.ts(1 hunks)api/src/store/listeners/mothership-subscription-listener.ts(1 hunks)api/src/store/modules/cache.ts(1 hunks)api/src/store/modules/config.ts(2 hunks)api/src/store/modules/dynamic-remote-access.ts(1 hunks)api/src/store/state-parsers/shares.ts(2 hunks)api/src/store/state-parsers/slots.ts(1 hunks)api/src/store/state-parsers/var.ts(2 hunks)api/src/store/types.ts(1 hunks)api/src/types/my-servers-config.ts(1 hunks)api/src/unraid-api/auth/auth.service.spec.ts(1 hunks)api/src/unraid-api/auth/auth.service.ts(1 hunks)api/src/unraid-api/auth/casbin/policy.ts(1 hunks)api/src/unraid-api/auth/header.strategy.ts(2 hunks)api/src/unraid-api/auth/user.decorator.ts(1 hunks)api/src/unraid-api/cli/apikey/add-api-key.questions.ts(1 hunks)api/src/unraid-api/cli/apikey/api-key.command.ts(2 hunks)api/src/unraid-api/cli/apikey/delete-api-key.questions.ts(1 hunks)api/src/unraid-api/graph/directives/auth.directive.ts(1 hunks)api/src/unraid-api/graph/graph.module.ts(3 hunks)api/src/unraid-api/graph/resolvers/array/array.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/array.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/array/array.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/parity.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/base.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/cloud/cloud.resolver.ts(3 hunks)api/src/unraid-api/graph/resolvers/config/config.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/disks/disks.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/disks/disks.resolver.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/docker/docker.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.spec.ts(3 hunks)api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts(3 hunks)api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/docker/docker.service.ts(3 hunks)api/src/unraid-api/graph/resolvers/flash/flash.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/display.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/mutation/mutation.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/network/network.resolver.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/network/network.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts(3 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts(10 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts(6 hunks)api/src/unraid-api/graph/resolvers/online/online.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/online/online.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/owner/owner.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/owner/owner.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/registration/registration.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/registration/registration.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/resolvers.module.ts(2 hunks)api/src/unraid-api/graph/resolvers/servers/server.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/servers/server.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/vars/vars.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/vars/vars.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/vms/vms.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/vms/vms.mutations.resolver.ts(8 hunks)api/src/unraid-api/graph/resolvers/vms/vms.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/vms/vms.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/vms/vms.service.ts(1 hunks)api/src/unraid-api/graph/services/services.resolver.ts(1 hunks)api/src/unraid-api/graph/user/user.resolver.spec.ts(1 hunks)api/src/unraid-api/rest/rest.controller.ts(1 hunks)api/src/utils.ts(1 hunks)
💤 Files with no reviewable changes (2)
- api/src/core/utils/vms/system-network-interfaces.ts
- api/src/core/utils/vms/index.ts
✅ Files skipped from review due to trivial changes (21)
- api/src/unraid-api/auth/casbin/policy.ts
- api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
- api/src/mothership/graphql-client.ts
- api/src/unraid-api/graph/resolvers/vms/vms.service.ts
- api/src/store/getters/index.ts
- api/src/graphql/resolvers/query/cloud/check-api.ts
- api/src/store/listeners/mothership-subscription-listener.ts
- api/src/unraid-api/cli/apikey/add-api-key.questions.ts
- api/src/unraid-api/graph/resolvers/disks/disks.resolver.spec.ts
- api/src/store/listeners/dynamic-remote-access-listener.ts
- api/src/unraid-api/graph/resolvers/network/network.resolver.spec.ts
- api/src/store/modules/cache.ts
- api/src/mothership/jobs/ping-timeout-jobs.ts
- api/src/unraid-api/auth/auth.service.ts
- api/src/store/state-parsers/slots.ts
- api/src/unraid-api/graph/directives/auth.directive.ts
- api/src/graphql/resolvers/query/cloud/check-cloud.ts
- api/src/store/actions/shutdown-api-event.ts
- api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts
- api/src/remoteAccess/handlers/static-remote-access.ts
- api/src/unraid-api/graph/resolvers/disks/disks.model.ts
🚧 Files skipped from review as they are similar to previous changes (22)
- api/src/unraid-api/graph/resolvers/flash/flash.model.ts
- api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.spec.ts
- api/src/unraid-api/graph/resolvers/array/array.service.spec.ts
- api/src/unraid-api/graph/resolvers/config/config.model.ts
- api/src/utils.ts
- api/src/store/modules/dynamic-remote-access.ts
- api/src/unraid-api/graph/user/user.resolver.spec.ts
- api/src/unraid-api/rest/rest.controller.ts
- api/src/unraid-api/graph/resolvers/base.model.ts
- api/src/store/state-parsers/var.ts
- api/src/unraid-api/graph/services/services.resolver.ts
- api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts
- api/src/store/types.ts
- api/src/unraid-api/auth/auth.service.spec.ts
- api/src/store/modules/config.ts
- api/src/unraid-api/graph/resolvers/vars/vars.resolver.ts
- api/src/unraid-api/graph/resolvers/mutation/mutation.model.ts
- api/src/unraid-api/graph/resolvers/array/array.resolver.ts
- api/src/unraid-api/graph/resolvers/connect/connect.service.spec.ts
- api/src/types/my-servers-config.ts
- api/src/unraid-api/graph/resolvers/resolvers.module.ts
- api/src/unraid-api/graph/resolvers/docker/docker.model.ts
🧰 Additional context used
🧬 Code Graph Analysis (25)
api/src/unraid-api/graph/resolvers/online/online.model.ts (3)
api/src/unraid-api/graph/resolvers/base.model.ts (1)
ObjectType(43-47)api/src/unraid-api/graph/resolvers/config/config.model.ts (1)
ObjectType(5-15)api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts (5)
ObjectType(11-18)ObjectType(20-30)ObjectType(32-42)ObjectType(44-54)ObjectType(56-75)
api/src/unraid-api/graph/resolvers/online/online.resolver.ts (10)
api/src/unraid-api/graph/resolvers/array/array.resolver.ts (1)
Resolver(12-35)api/src/unraid-api/graph/resolvers/cloud/cloud.resolver.ts (1)
Resolver(12-41)api/src/unraid-api/graph/resolvers/info/info.resolver.ts (1)
Resolver(31-128)api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (1)
Resolver(19-168)api/src/unraid-api/graph/resolvers/owner/owner.resolver.ts (1)
Resolver(10-44)api/src/unraid-api/graph/resolvers/registration/registration.resolver.ts (1)
Resolver(15-58)api/src/unraid-api/graph/resolvers/servers/server.resolver.ts (1)
Resolver(10-41)api/src/unraid-api/graph/resolvers/vars/vars.resolver.ts (1)
Resolver(9-23)api/src/unraid-api/graph/resolvers/vms/vms.resolver.ts (1)
Resolver(9-35)api/src/unraid-api/graph/resolvers/flash/flash.resolver.ts (1)
Resolver(9-27)
api/src/graphql/resolvers/subscription/network.ts (1)
api/src/graphql/generated/client/graphql.ts (1)
AccessUrl(33-39)
api/src/unraid-api/graph/resolvers/owner/owner.model.ts (2)
api/src/unraid-api/graph/resolvers/base.model.ts (1)
ObjectType(43-47)web/composables/gql/graphql.ts (1)
Owner(1104-1109)
api/src/unraid-api/auth/header.strategy.ts (1)
web/composables/gql/graphql.ts (1)
UserAccount(1668-1674)
api/src/unraid-api/graph/resolvers/registration/registration.model.ts (3)
api/src/unraid-api/graph/resolvers/base.model.ts (1)
ObjectType(43-47)api/src/unraid-api/graph/resolvers/config/config.model.ts (1)
ObjectType(5-15)web/composables/gql/graphql.ts (2)
KeyFile(743-747)Registration(1316-1324)
api/src/unraid-api/graph/resolvers/info/display.model.ts (3)
web/themes/types.d.ts (1)
Theme(1-9)api/src/unraid-api/graph/resolvers/base.model.ts (1)
ObjectType(43-47)web/composables/gql/graphql.ts (3)
Case(360-366)Display(551-574)Node(1022-1024)
api/src/unraid-api/graph/resolvers/servers/server.model.ts (6)
api/src/unraid-api/graph/resolvers/base.model.ts (1)
ObjectType(43-47)api/src/unraid-api/graph/resolvers/config/config.model.ts (1)
ObjectType(5-15)api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts (5)
ObjectType(11-18)ObjectType(20-30)ObjectType(32-42)ObjectType(44-54)ObjectType(56-75)api/src/unraid-api/graph/resolvers/array/array.model.ts (6)
ObjectType(6-16)ObjectType(18-25)ObjectType(27-123)ObjectType(125-156)ObjectType(173-196)ObjectType(255-304)api/src/unraid-api/graph/resolvers/disks/disks.model.ts (2)
ObjectType(43-56)ObjectType(58-141)api/src/unraid-api/graph/resolvers/connect/connect.model.ts (1)
ObjectType(95-113)
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts (3)
api/src/unraid-api/graph/resolvers/array/array.resolver.ts (1)
Resolver(12-35)api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts (1)
Resolver(16-103)api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts (1)
Resolver(12-35)
api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts (3)
api/src/unraid-api/graph/resolvers/base.model.ts (1)
ObjectType(43-47)api/src/unraid-api/graph/resolvers/config/config.model.ts (1)
ObjectType(5-15)web/composables/gql/graphql.ts (4)
ApiKeyResponse(75-79)MinigraphqlResponse(818-823)CloudResponse(378-383)RelayResponse(1367-1372)
api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts (1)
web/composables/gql/graphql.ts (5)
NotificationFilter(1057-1062)NotificationData(1049-1055)NotificationCounts(1041-1047)NotificationOverview(1064-1068)Notifications(1075-1081)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts (3)
api/src/unraid-api/graph/resolvers/validation.utils.ts (1)
validateObject(15-34)api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (1)
createNotification(57-62)api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (1)
createNotification(198-217)
api/src/unraid-api/graph/resolvers/vars/vars.model.ts (3)
api/src/unraid-api/graph/resolvers/base.model.ts (1)
ObjectType(43-47)api/src/unraid-api/graph/resolvers/config/config.model.ts (1)
ObjectType(5-15)web/composables/gql/graphql.ts (1)
Node(1022-1024)
api/src/unraid-api/graph/resolvers/vms/vms.model.ts (1)
web/composables/gql/graphql.ts (3)
VmDomain(1871-1878)Vms(2000-2013)VmMutations(1880-1952)
api/src/unraid-api/graph/resolvers/servers/server.resolver.ts (2)
api/src/graphql/generated/client/graphql.ts (1)
ServerModel(618-626)api/src/graphql/schema/utils.ts (1)
getLocalServer(74-104)
api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts (2)
api/src/unraid-api/graph/resolvers/array/array.resolver.ts (2)
Resolver(12-35)array(22-24)web/composables/gql/graphql.ts (4)
ArrayMutations(238-249)ArrayStateInput(317-320)ArrayDiskInput(199-204)ArrayDisk(144-186)
api/src/unraid-api/graph/resolvers/connect/connect.model.ts (5)
api/src/unraid-api/graph/resolvers/api-key/api-key.model.ts (4)
InputType(73-84)InputType(86-118)InputType(120-129)InputType(131-140)api/src/unraid-api/graph/resolvers/base.model.ts (1)
ObjectType(43-47)api/src/unraid-api/graph/resolvers/config/config.model.ts (1)
ObjectType(5-15)api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts (5)
ObjectType(11-18)ObjectType(20-30)ObjectType(32-42)ObjectType(44-54)ObjectType(56-75)api/src/unraid-api/graph/resolvers/disks/disks.model.ts (2)
ObjectType(43-56)ObjectType(58-141)
api/src/unraid-api/graph/resolvers/cloud/cloud.resolver.ts (1)
api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts (1)
Resolver(32-248)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (3)
web/composables/gql/graphql.ts (2)
NotificationCounts(1041-1047)Notification(1026-1039)api/src/unraid-api/graph/resolvers/validation.utils.ts (1)
validateObject(15-34)api/src/core/types/states/notification.ts (1)
NotificationIni(1-8)
api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts (4)
api/src/store/modules/dynamic-remote-access.ts (1)
setAllowedRemoteAccessUrl(54-63)api/src/store/modules/config.ts (2)
logoutUser(78-101)updateAllowedOrigins(190-192)api/src/store/actions/setup-remote-access.ts (1)
setupRemoteAccessThunk(26-50)api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts (2)
dynamicRemoteAccessSettings(155-169)updateAllowedOrigins(104-107)
api/src/unraid-api/graph/resolvers/info/info.model.ts (2)
api/src/unraid-api/graph/resolvers/base.model.ts (1)
ObjectType(43-47)web/themes/types.d.ts (1)
Theme(1-9)
api/src/unraid-api/graph/resolvers/array/array.model.ts (3)
api/src/unraid-api/graph/resolvers/base.model.ts (1)
ObjectType(43-47)web/composables/gql/graphql.ts (6)
Capacity(353-358)ArrayDisk(144-186)Node(1022-1024)ArrayDiskInput(199-204)ArrayStateInput(317-320)ArrayMutations(238-249)api/src/graphql/resolvers/graphql-type-long.ts (1)
GraphQLLong(29-35)
api/src/graphql/resolvers/query/info.ts (2)
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (2)
os(101-103)display(86-88)api/src/unraid-api/graph/resolvers/display/display.resolver.ts (1)
display(69-111)
api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (1)
web/composables/gql/graphql.ts (7)
Notifications(1075-1081)Query(1209-1272)NotificationOverview(1064-1068)Notification(1026-1039)NotificationFilter(1057-1062)Mutation(833-890)NotificationData(1049-1055)
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (1)
web/composables/gql/graphql.ts (9)
Info(681-697)InfoApps(699-705)Baseboard(344-351)InfoCpu(707-726)Display(551-574)InfoMemory(728-741)Os(1088-1102)System(1560-1568)Versions(1840-1868)
🪛 GitHub Actions: CI - Main (API)
api/src/__test__/store/modules/config.test.ts
[error] 149-149: AssertionError: expected "spy" to be called with arguments: [ 'OWNER', { owner: { …(2) } } ]
api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts
[error] 201-201: RangeError: Invalid time value
[error] 201-201: TypeError: Cannot read properties of undefined (reading 'constructor')
[error] 231-231: AssertionError: expected +0 to deeply equal 1
[error] 240-240: AssertionError: expected undefined to be defined
[error] 271-271: AssertionError: expected +0 to deeply equal 9
[error] 280-280: AssertionError: expected +0 to deeply equal 3
[error] 292-292: AssertionError: expected +0 to deeply equal 1
[error] 292-292: AssertionError: expected +0 to deeply equal 1
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (175)
api/src/unraid-api/cli/apikey/delete-api-key.questions.ts (1)
29-31: Improved asynchronous handling in getKeys methodThe code now properly awaits the result of
this.apiKeyService.findAll()before mapping over the keys. This is a better approach as it ensures the Promise is properly resolved before performing operations on the data, preventing potential race conditions or undefined behavior.api/src/remoteAccess/handlers/upnp-remote-access.ts (2)
7-11: Import refactoring aligns with code-first GraphQL approach.The imports for
AccessUrl,DynamicRemoteAccessType, andURL_TYPEhave been moved from generated types to a dedicated model file, supporting the code-first GraphQL migration in this PR.
22-22: Good defensive programming with null handling.Adding the nullish coalescing operator ensures explicit null assignment when no URL with the WAN type is found, which improves code clarity.
api/src/store/actions/set-minigraph-status.ts (1)
3-3: Import path updated for MinigraphStatusThe import path has been updated from generated GraphQL types to a specific model file, which aligns with the PR's code-first GraphQL approach.
api/src/remoteAccess/remote-access-controller.ts (1)
14-17: Updated import paths for type definitionsThe import paths for
AccessUrlandDynamicRemoteAccessTypehave been updated from generated GraphQL types to dedicated model files, which is consistent with the PR's approach to use code-first GraphQL.api/src/remoteAccess/handlers/remote-access-interface.ts (1)
2-2: Import update aligns with code-first GraphQL approachThe change from importing
AccessUrlas a type from generated files to importing it from a dedicated model file is consistent with the PR's objective of implementing a code-first GraphQL approach. This modification ensures that the type definitions are now centralized in dedicated model files rather than being auto-generated.Note that the import no longer uses the
typekeyword, which suggestsAccessUrlmay now be a class or enum that can be used at runtime, not just for type checking.api/src/store/state-parsers/shares.ts (2)
3-3: Import path updated to use model-specific typeThe import for
Sharehas been updated to use a specific model file instead of the generated GraphQL types. This aligns with the broader refactoring effort to move away from generated types to dedicated model files.
19-20: Structure change in Share object with new id fieldThe Share object structure has been modified to:
- Add an
idproperty that takes the value ofname- Change the
nameproperty to potentially be null using the nullish coalescing operatorThis change accommodates the updated Share model structure but there's a potential type inconsistency with the
SharesInitype defined above (line 6) which specifiesnameas a string, not a nullable value.Consider updating the
SharesInitype to reflect thatnamecan now be a nullable value:export type SharesIni = Array<{ - name: string; + name: string | null; free: string; used: string; // ... other properties }>;api/src/core/types/states/var.ts (2)
1-7: Good job refactoring imports to use dedicated model filesThe changes improve modularity by sourcing types from specific model files instead of a generated types file. This aligns with the PR's goal of transitioning to dedicated model files.
131-131: Correct type name updateProperly updated
regTyproperty type fromregistrationTypetoRegistrationTypeto match the new import.api/src/unraid-api/graph/resolvers/vms/vms.service.spec.ts (1)
11-11: Import path updated to use dedicated model fileThe import for
VmDomainhas been updated to source from a dedicated model file rather than the generated types file, which aligns with the code-first GraphQL approach.api/src/unraid-api/graph/resolvers/online/online.model.ts (1)
1-7: Well-structured GraphQL model implementationThe implementation follows NestJS GraphQL best practices with the appropriate use of
@ObjectType()and@Field()decorators. The model is simple and focused, containing just the essentialonlinefield with a default value.This aligns well with the code-first GraphQL approach being adopted across the codebase.
api/src/unraid-api/graph/resolvers/online/online.resolver.ts (3)
3-6: Correctly updated imports for base model and Online typeThe imports have been appropriately updated to reference the new model file structure. Moving from generated types to dedicated model files enhances type safety and maintainability.
8-8: Enhanced type safety with resolver decoratorGood change from string-based resolver (
'Online') to a type-safe function reference (() => Online). This improves type checking and IDE support.
10-10: Explicit return type improves schema clarityAdding an explicit return type to the query decorator (
() => Boolean) provides clearer documentation of the API contract in the GraphQL schema.api/src/unraid-api/graph/graph.module.ts (5)
6-6: New imports for GraphQL configurationAdded imports for
NoUnusedVariablesRuleandAuthDirectiveto support enhanced GraphQL functionality. The auth directive will enable role-based access control for GraphQL operations.Also applies to: 17-17
27-28: Empty imports and inject arrays explicitly definedThe
importsandinjectarrays are now explicitly defined as empty arrays. This makes the dependencies clearer and shows that this module doesn't require additional dependencies at this level.
31-31: Transition to code-first GraphQL approachSetting
autoSchemaFile: truemarks a significant transition from schema-first to code-first GraphQL. The schema will now be generated in memory from TypeScript decorators and classes rather than loaded from external schema files.
47-54: Explicit resolver configurationThe GraphQL scalar resolvers are now explicitly defined in the configuration object rather than being dynamically constructed. This provides better type safety and makes the resolver mapping more transparent.
55-58: Added schema build options with authentication directiveNew
buildSchemaOptionsintroduces:
- The
AuthDirectivefor role-based access controlnumberScalarMode: 'integer'to better handle numeric valuesThis configuration supports the implementation of role-based access control at the GraphQL schema level.
api/src/unraid-api/graph/resolvers/network/network.resolver.ts (5)
6-7: Great type imports.These imports correctly bring in the necessary model types, aligning with the shift from generated GraphQL types to dedicated model files. This improves code organization and type safety.
9-9: Good type-safe resolver declaration.Changing from
@Resolver('Network')to@Resolver(() => Network)enhances type safety by ensuring the resolver is directly associated with the Network type. This approach makes type checking more reliable and reduces potential errors.
18-18: Improved query type safety.Changing from
@Query('network')to@Query(() => Network)properly specifies the return type, which improves type checking and makes the code more self-documenting.
25-25: Explicit return type declaration.The
@ResolveField(() => [AccessUrl])decorator clearly specifies that this field returns an array of AccessUrl objects. This enhances type safety and API documentation.
28-33: Well-structured data transformation.The implementation properly maps the URL objects from
ips.urlsto a consistent AccessUrl structure with all required properties. This creates a clear contract for clients consuming this API.api/src/unraid-api/cli/apikey/api-key.command.ts (2)
9-10: Import paths updated to use dedicated model filesThis change is part of moving from generated GraphQL types to dedicated model files, which aligns with the code-first GraphQL approach mentioned in the PR title. This is a good change for maintainability and type safety.
104-104: Fixed asynchronous operation handlingAdding the
awaitkeyword here is crucial - without it,allKeyswould be a Promise object instead of the resolved array, making the length check in the next line meaningless. This fix ensures proper asynchronous flow control.api/src/core/utils/shares/process-share.ts (1)
3-3:❓ Verification inconclusive
Import source updated for ArrayDisk type
The import for
ArrayDiskhas been updated to use a specific model file instead of the generated GraphQL types. This change is consistent with the broader codebase reorganization, where imports are being shifted from generated types to dedicated model files.Make sure the
ArrayDisktype from the new source is fully compatible with its usage in this file, particularly in thediskprocessor function (line 22) and theOverloadtype definition (line 41).
🏁 Script executed:
#!/bin/bash # Verify the structure of ArrayDisk type in both locations to ensure compatibility echo "Checking old ArrayDisk type definition:" rg --type=typescript "ArrayDisk(\s+extends|\s*\{|\s+type)" "@app/graphql/generated/api/types.js" -A 10 echo -e "\nChecking new ArrayDisk type definition:" rg --type=typescript "ArrayDisk(\s+extends|\s*\{|\s+type)" "@app/unraid-api/graph/resolvers/array/array.model.js" -A 10 echo -e "\nChecking where ArrayDisk is used in this file:" rg --type=typescript "ArrayDisk" "api/src/core/utils/shares/process-share.ts"Length of output: 696
Action Required: Manually Verify ArrayDisk Type Compatibility
The import update for
ArrayDiskinapi/src/core/utils/shares/process-share.tsappears aligned with our reorganization efforts. However, the initial verification script did not produce the expected output due to issues with the--type=typescriptflag. Please ensure that the newArrayDiskfrom the updated source at@app/unraid-api/graph/resolvers/array/array.model.jsis fully compatible with its usage in thediskprocessor function (line 22) and theOverloadtype definition (line 41).Next Steps:
Re-run a revised verification script without the problematic
--typeflag. For instance:#!/bin/bash echo "Checking old ArrayDisk definition:" rg "export .*ArrayDisk" @app/graphql/generated/api/types.js echo -e "\nChecking new ArrayDisk definition:" rg "export .*ArrayDisk" @app/unraid-api/graph/resolvers/array/array.model.js echo -e "\nChecking usage in process-share.ts:" rg "ArrayDisk" api/src/core/utils/shares/process-share.tsConfirm that the new type definition provides the expected structure and is compatible with its usage in this file.
Please perform a manual verification and update us with the findings.
api/src/unraid-api/auth/user.decorator.ts (2)
4-4: Import path updated for UserAccount model.The import for UserAccount has been changed from the generated GraphQL types to a dedicated model file, which aligns with the project's move toward a more organized structure for type definitions.
11-11:❓ Verification inconclusive
Validation logic has been removed.
The previous implementation validated the user object structure using UserSchema.safeParse(), but this validation check has been removed. While this simplifies the code, it eliminates an error-handling mechanism that prevented invalid user structures from propagating through the application.
Consider whether this validation should be preserved elsewhere in the codebase to maintain runtime safety. If validation is handled at another layer of the application, please verify that it's comprehensive.
🏁 Script executed:
#!/bin/bash # Check if user validation exists elsewhere in the codebase echo "Searching for user validation in other files..." rg "UserSchema.*safeParse" --type tsLength of output: 143
User Validation Removal Requires Confirmation
It appears that the prior runtime validation using
UserSchema.safeParse()has been removed fromapi/src/unraid-api/auth/user.decorator.ts(line 11). A repository-wide search for references tosafeParsedid not return any evidence of alternative user validation logic. Please verify that the user structure is being validated elsewhere—if not, consider reinstating the validation to prevent invalid user data from propagating.
- Location:
api/src/unraid-api/auth/user.decorator.ts(line 11)- Issue: Removed validation (previously via
UserSchema.safeParse()) may leave the system without proper runtime safety.- Action: Confirm whether an alternative validation mechanism exists upstream or reintroduce the validation logic.
api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (13)
5-16: Import restructuring improves clarity and brings in proper error handlingThe updated imports now properly include the AppError class for error handling, and the imports from the notifications model file have been organized to include all necessary types (Notification, NotificationImportance, Notifications, etc.). This change provides a more complete type-checking system for the resolver.
19-19: Type-safe Resolver declarationChanging from a string identifier to a function returning the Notifications type improves type safety and follows NestJS best practices for GraphQL resolvers.
27-37: Enhanced Query definition with proper return typingThe Query decorator now includes both a return type and description, and the method signature properly specifies the return type promise. This improves type safety and API documentation simultaneously.
39-42: Properly typed ResolveField for overviewThe explicit return type in both the decorator and method signature ensures type safety while resolving the NotificationOverview field.
44-50: Properly typed list resolver with array notationThe ResolveField properly indicates it returns an array of Notification objects, and the Args decorator includes the correct type function. The Promise return type completes the type safety improvements.
56-62: Enhanced mutation with description and proper typingThe createNotification mutation now has a clear description and properly typed arguments and return values, improving both documentation and type safety.
64-73: Well-typed deleteNotification mutationAll argument types are explicitly defined using type functions, and the return type is properly specified as Promise, which matches the implementation.
75-80: Clear description for deleteArchivedNotificationsAdding a description to the mutation improves API documentation and makes the purpose of this operation clearer to API consumers.
82-88: Properly typed archiveNotification mutationThe mutation now has clear return type information and a descriptive comment, with properly typed arguments.
90-97: Type-safe archiveNotifications implementationThe array of string IDs is properly typed, and the return type matches the implementation's return value.
99-106: Nullable importance parameter properly typedThe archiveAll mutation correctly specifies that the importance parameter is optional and nullable, with proper type annotations.
134-143: Improved error handling in recalculateOverviewThe mutation now properly throws an AppError when an error occurs during overview recalculation. This is a good practice for GraphQL error handling.
149-157: Type-safe subscription definitionsBoth notificationAdded and notificationsOverview subscriptions now explicitly declare their return types, improving type safety and API documentation.
Also applies to: 159-167
api/src/unraid-api/graph/resolvers/registration/registration.resolver.ts (3)
15-17: Improved type safety with code-first approach.The change from string-based resolvers to function-based resolvers provides better type safety and IDE support. This aligns with best practices for NestJS GraphQL code-first approach.
38-40: Changed null to undefined for updateExpiration.You've changed the
updateExpirationbehavior from potentially returningnullto returningundefinedwhen not available. While this works with GraphQL (which treats both as null in the response), be aware that client code might need to handle this differently if it was specifically checking fornull.
43-43: Changed null to undefined for keyFile contents.Similar to the updateExpiration change, you're now using the nullish coalescing operator to convert
nulltoundefined. Ensure that client code isn't affected by this change.api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts (2)
6-6: Import change looks goodThe switch from using generated GraphQL types to the dedicated model file aligns with the code-first GraphQL approach of this PR.
46-46: Names property correctly added to test dataThe
namesproperty has been added to match the updated DockerContainer model definition. Good job maintaining test consistency with the model changes.Also applies to: 58-58
api/src/unraid-api/graph/resolvers/docker/docker.service.ts (1)
12-18: Import updates align with code-first approachGood restructuring of imports to use the dedicated model files instead of generated types.
api/src/unraid-api/auth/header.strategy.ts (3)
7-7: Import alignment with model-based architecture.The change from importing
Userfrom generated GraphQL types to importingUserAccountfrom a dedicated model file aligns with the PR objective of transitioning to a model-based architecture. This improves type safety and reduces dependency on generated code.
21-21: Method signature updated for type consistency.The return type change from
Promise<User | null>toPromise<UserAccount | null>ensures type consistency with the new imports and maintains compatibility with the method implementation which references properties likeidandroles.
21-50:❓ Verification inconclusive
Verify UserAccount type compatibility.
The method implementation hasn't changed despite the type change, suggesting UserAccount has compatible properties with the previous User type. Ensure that the UserAccount type contains all properties used in this method (specifically id and roles used in the debug logging).
🏁 Script executed:
#!/bin/bash # Verify the structure of UserAccount and its usage in auth.service.ts echo "Checking UserAccount model definition..." cat api/src/unraid-api/graph/user/user.model.js echo "\nChecking auth.service implementation of validateApiKeyCasbin..." rg -A 10 "validateApiKeyCasbin" api/src/unraid-api/auth/auth.service.tsLength of output: 913
Review Verification – Confirm
UserAccountStructure
- The implementation in
header.strategy.tscorrectly accessesuser?.idanduser?.roles, which aligns with how these properties are used in thevalidateApiKeyCasbinmethod inauth.service.ts.- The
validateApiKeyCasbinmethod returns aUserAccountthat supplies the necessaryidandrolesproperties.- However, the expected file for the
UserAccounttype (api/src/unraid-api/graph/user/user.model.js) wasn’t found. Please confirm whether the type is defined in a different file (e.g.,user.model.ts) or if the file path needs updating.Overall, verify that the
UserAccounttype includes at least theidandrolesproperties as used here.api/src/unraid-api/graph/resolvers/owner/owner.resolver.ts (3)
7-8: Good update of imports for type safetyUpdated imports from generated types to specific model files improves type safety and maintainability.
10-12: Improved type safety with explicit return typesUsing type functions in decorators instead of string literals provides better type checking and IDE support.
Also applies to: 35-35
18-33:❓ Verification inconclusive
Ensure all required fields are returned
The Owner model defines 'url' as a non-nullable field, but when remote.username exists, the implementation doesn't explicitly include the url property.
return { username: remote.username, avatar: remote.avatar, + url: remote.url || '', };
🏁 Script executed:
#!/bin/bash # Check if remote.url is defined in the config getter cat api/src/store/modules/config.ts | grep -A 20 "export interface"Length of output: 69
ACTION REQUIRED: Verify Presence of
remote.urlin Config Module
The current implementation inowner.resolver.tsomits returning theurlfield whenremote.usernameis present. Since the Owner model requires a non-nullableurl, please verify whether the configuration interface inapi/src/store/modules/config.tsincludes aremote.urlproperty. If it exists, update the resolver to return it (or a default value), for example:return { - username: remote.username, - avatar: remote.avatar, + username: remote.username, + avatar: remote.avatar, + url: remote.url || '', };To further investigate, please run the following shell script to check for the
urlproperty definition in the config module:#!/bin/bash # Verify whether the configuration includes the 'url' property for remote rg "remote[. ]*url" api/src/store/modules/config.tsOnce confirmed, adjust the code accordingly or update the model if needed.
api/src/unraid-api/graph/resolvers/vars/vars.model.ts (3)
1-5: Neat usage of ESM-based imports
No issues found with these import statements or the usage of theNodeinterface from the base model file.
6-17: Nice use of explicit enum forConfigErrorState
This approach fosters type-safety and clarity in GraphQL.
19-27:MdStatedefinition and registration
The enum and itsregisterEnumTypeusage look consistent and well-structured.api/src/unraid-api/graph/resolvers/vms/vms.model.ts (4)
1-18: Well-structured enum definition and registration.The VmState enum covers the essential VM states and is properly registered with GraphQL. The descriptive name and description improve API documentation.
20-30: Good use of decorators and nullable fields.The VmDomain class properly implements GraphQL ObjectType with appropriate field decorators. The nullable name field with description and non-nullable state field match the domain model expectations.
32-39: Clear implementation of Vms container class.The Vms class correctly defines a collection of VM domains with proper type definitions.
41-77:✅ Verification successful
Placeholder implementations in VmMutations class.
The VmMutations class is structured correctly with proper field decorators and descriptions, but all methods currently return hardcoded
truevalues. This appears to be intentional as the actual implementation will be in the resolver.Verify that the actual mutation logic is implemented in the VmMutationsResolver class:
🏁 Script executed:
#!/bin/bash grep -A 5 "this.vmsService" api/src/unraid-api/graph/resolvers/vms/vms.mutations.resolver.tsLength of output: 1031
Verified: Placeholder methods in VmMutations are intentional and the actual logic is delegated to VmMutationsResolver.
The grep output confirms that each mutation in the resolver correctly calls the corresponding method on
this.vmsService. Therefore, the hardcoded implementations inVmMutationsare acceptable placeholders, and no further changes are needed here.api/src/unraid-api/graph/resolvers/vms/vms.resolver.ts (3)
5-6: Good migration to model-based imports.The updated imports properly reference the new model definitions, which supports the code-first GraphQL approach.
9-9: Improved type safety with code-first approach.Changing from string-based resolver to type-based resolver enhances type safety and IDE support.
13-13: Enhanced query return type definition.Adding explicit return type to the Query decorator improves API documentation and type safety.
api/src/unraid-api/graph/resolvers/vms/vms.mutations.resolver.ts (3)
1-1: Good transition to code-first GraphQL imports.The updated imports reflect the shift to a code-first approach with proper model references.
Also applies to: 5-6
9-9: Enhanced type safety for resolver definition.Using the function form
() => VmMutationsimproves type checking and documentation.
18-18: Correctly migrated from ResolveField to Mutation decorators.The migration from ResolveField to Mutation decorators accurately reflects the operation types. The addition of descriptions enhances API documentation.
Also applies to: 28-28, 38-38, 48-48, 58-58, 68-68, 78-78
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (12)
17-29: Clean import usage.
All imported models appear relevant and correctly used in subsequent resolver methods.
31-33: Class-based resolvers and code-first approach.
The usage of@Resolver(() => Info)and@Query(() => Info)aligns perfectly with a code-first methodology.
56-58: Simple date retrieval.
Returningnew Date()directly is concise and works well for atimefield.
61-62: No issues withappsfield resolver.
ThegenerateApps()helper is well-referenced and should handle logic internally.
75-76: CPU resolver uses a helper.
Looks clean. IfgenerateCpu()handles errors internally, no further concerns.
80-81: Devices field resolver.
Straightforward usage of a helper function for device info.
85-86: Display field resolver.
Reads neatly fromgenerateDisplay().
90-91:machineIdresolver.
Returning a possibleundefinedis fine given the optional nature ofgetMachineId().
95-96: Memory info.
ThegenerateMemory()call is suitably handled.
100-101: OS resolver.
No problems identified—usesgenerateOs()effectively.
114-115: Versions field.
Looks fine, references a reliable helper function.
119-119: Subscription returningInfo.
This subscription approach is consistent with the rest of the resolver.api/src/unraid-api/graph/resolvers/info/display.model.ts (4)
5-8: Consolidation ofTemperatureenum is good.
Thanks for centralizing theTemperatureenum here. This resolves prior duplication concerns.
14-17: Good practice registeringTemperature.
This ensures the enum is recognized in GraphQL schemas.
19-22: Good practice registeringTheme.
Keeps the schema consistent and typed.
24-37:Caseclass looks straightforward.
Optional fields with@Fielddecorators are clear and consistent.api/src/graphql/resolvers/query/info.ts (9)
22-23: Imports from store and display model are consistent.
UsingTemperatureandThemefrom a single source prevents duplication.
34-34: Removed extra imports.
EliminatingTemperatureandThemefrom here helps centralize definitions indisplay.model.ts.
45-45: AddedidtogenerateAppsreturn.
This helps uniquely identify the data object in GraphQL queries.
52-52: AddedidtogenerateOs.
Consistently labeling data objects withidaids consumption by clients.
66-66: AddedidtogenerateCpu.
Keeps the return strategy uniform across resolvers.
99-99:idset todynamix-config/display.
Unique naming for the display config is a neat approach.
122-122: AddedidtogenerateVersions.
Maintains consistency for returned objects.
170-170: AddedidtogenerateMemory.
Helps keep the memory object identifiable.
416-416: AddedidtogenerateDevices.
Consistent with the approach for other resolvers.api/src/unraid-api/graph/resolvers/info/info.model.ts (16)
1-4: ImportingTemperatureandThemefrom a single file.
This fully addresses earlier concerns about enum duplication.
6-16:InfoAppsclass aligns with GraphQL best practices.
Fields and types are straightforward.
18-37:Baseboardclass is well-defined.
Optional vs. required fields look appropriate.
39-94:InfoCpuis neatly structured.
Including flags array is clear for CPU capabilities.
96-119:Gpuclass usage is self-explanatory.
Blacklisting logic can be extended as needed.
120-160:Networkclass fields handle network data effectively.
All optional fields make sense for variable network states.
162-190:Pciclass covers PCI device details.
Nullable fields provide flexibility for different devices.
192-200:Usbclass usage is straightforward.
We can expand fields for USB specifics in the future.
202-214:Devicesclass consolidates GPU/PCI/USB arrays.
Having them together eases device queries.
279-284:MemoryTypeenum is clear.
Extensible for more memory types if needed.
289-323:MemoryLayoutwell describes memory characteristics.
All relevant fields are accounted for.
325-362:InfoMemoryusage looks thorough.
Exposes both total memory and layout details for deeper insights.
364-404:Osclass has essential OS fields.
Optional gating is good for partial data.
406-428:Systemclass effectively covers system metadata.
Ready for expansions like chassis info, etc.
430-512:Versionsclass enumerates software components well.
Straightforward structure for version listings.
514-551:Inforoot object is a handy all-in-one model.
It neatly references sub-models for a complete system overview.api/src/graphql/resolvers/query/cloud/check-minigraphql.ts (1)
3-3: Import path updated to use dedicated model file.This change aligns with the refactoring effort to move from generated GraphQL types to dedicated model files. The functionality remains unchanged while improving code organization.
api/src/unraid-api/graph/resolvers/servers/server.model.ts (2)
1-16: Well-structured ProfileModel with appropriate field decorators.The ProfileModel class is properly decorated with GraphQL field decorators and has appropriate types. Good job making userId optional as it might not always be available.
18-22: Clear ServerStatus enumeration.The enum is well-defined with descriptive status values that clearly represent the possible states of a server.
api/src/__test__/store/modules/config.test.ts (1)
10-14: Import paths updated to use dedicated model files.The imports have been correctly updated to use the new dedicated model files instead of the generated types.
api/src/graphql/resolvers/subscription/network.ts (3)
5-5: Import path updated to use dedicated model files.The imports have been correctly updated to use the new dedicated model files instead of the generated types.
232-232: Simplified return statement.The return statement has been simplified which is a good practice. The functionality remains the same.
126-126:❓ Verification inconclusive
Type changed from AccessUrlInput[] to AccessUrl[].
This change is part of the shift to using the final model structure instead of input types. Make sure that any validation previously done with AccessUrlInputSchema is still handled appropriately.
🏁 Script executed:
#!/bin/bash # Check for previous validation logic that might have been removed # Look for AccessUrlInputSchema usage in the codebase rg "AccessUrlInputSchema" api/ # Check previous version of this file to see what validation existed git show HEAD~1:api/src/graphql/resolvers/subscription/network.ts | grep -A 10 "getServerIps"Length of output: 811
Ensure Validation Continuity for Final AccessUrl Models
The type update—from using input models (AccessUrlInput[]) to the final model (AccessUrl[])—looks intentional. I noticed that the validation schema (AccessUrlInputSchema) is still defined under the generated client validators. Please double-check that all validations originally applied when using AccessUrlInput are now appropriately handled upstream during the transformation to the final model. If this validation has been moved or consolidated elsewhere in the data flow, confirm that it still adequately guards against invalid inputs.
api/src/graphql/schema/utils.ts (1)
7-8: Imports from revised model paths look correct.
The newly introduced import statements accurately reference the restructured types (MinigraphStatus,Server, andServerStatus). No issues detected.api/src/unraid-api/graph/resolvers/cloud/cloud.resolver.ts (4)
1-1: New import statements align with updated file structure.
All imports now point to the correct modules. Looks good.Also applies to: 5-5, 9-9
12-12: Excellent use of decorators for the Cloud resolver.
Switching to@Resolver(() => Cloud)and@Query(() => Cloud)is consistent with best practices for type inference.Also applies to: 14-14
29-29:❓ Verification inconclusive
Verify frontend usage for
timeoutset toundefined.
Explicitly definingtimeout: undefinedcan alter client-side handling of the field. Please confirm that the consumer logic properly handlesundefined.
🏁 Script executed:
#!/bin/bash # Searching for references to 'relay.timeout' in the codebase rg -A 5 'relay(\s*)?:(\s*)?{' | rg 'timeout'Length of output: 146
Action Required: Verify Frontend Handling of
timeout: undefinedThe initial regex produced an error due to an unescaped
{. Please run the following corrected command to search for references torelay.timeoutand confirm that the frontend logic properly handles anundefinedtimeout value:Once confirmed, please ensure the consumer logic correctly manages the
undefinedstate.
🏁 Script executed:
#!/bin/bash # Searching for references to 'relay.timeout' using a corrected regex pattern rg -A 5 'relay\s*:\s*\{' | rg 'timeout'Length of output: 256
Action Required: Confirm Frontend Handling for
timeoutFieldWe've verified that in
api/src/unraid-api/graph/resolvers/cloud/cloud.resolver.ts(line 29) the field is explicitly set astimeout: undefined,. Please ensure that the frontend (or any consumer logic) correctly handles thisundefinedvalue. Manual verification of the client-side behavior is necessary to confirm that no unintended side effects occur.
38-38:❓ Verification inconclusive
Check error string fallback logic.
Using|| undefinedensures an empty string yieldsundefinedinstead. Confirm the client or UI can handleundefinederror states gracefully.
🏁 Script executed:
#!/bin/bash # Searching for how the `error` field is consumed in the codebase rg -A 5 'cloud(\s*)?:(\s*)?{' | rg 'error'Length of output: 144
Below is a new shell script using an adjusted regex (escaping the curly brace) to search for how the error field is used. Please run this to help verify whether any UI or client logic consumes the error string:
Error Fallback Logic – Manual Confirmation Needed
The resolver uses the ternaryminigraphql.error ? `CLOUD: ${minigraphql.error}` : ''and then applies
|| undefinedso that an empty string will yieldundefined. This appears intentional to signal “no error.” However, our automated search hasn’t clearly identified where—and if—the client/UI code properly handles anundefinederror state. Please verify manually that your client code can gracefully deal with the fallback value.
🏁 Script executed:
#!/bin/bash # Searching for how the `error` field is consumed in GraphQL resolver or UI code rg -A 5 'cloud(\s*)?:\s*\{' . | rg -i 'error'Length of output: 965
Error Fallback Logic – Verify UI Handling Undefined
- The GraphQL definitions (e.g., in
./web/composables/gql/graphql.tsand./api/src/graphql/resolvers/query/cloud/create-response.ts) specify the error field as optional (error?: string | null).- The use of
|| undefinedin the resolver converts an empty string toundefined, which aligns with the schema by omitting the field instead of passing an empty value.- Please verify that the UI and client-side logic correctly interpret an
undefinederror state (as opposed to an explicitnullor string).api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts (6)
1-2: File import structure is fine.
No issues found with the newly introduced import.
3-9: Enum definition is clear and consistent.
TheMinigraphStatusvalues are self-explanatory. Good job.
11-18:ApiKeyResponsefields look correct.
Standard GraphQL object with boolean validity and optional error is good.
32-42:CloudResponseobject structure is straightforward.
No functional issues spotted; consistent with other response patterns.
44-54:RelayResponseis well-defined.
Fields align with typical error/status usage in GraphQL schemas.
56-75:Cloudclass strongly models diverse statuses.
The aggregator class for all cloud states is well-organized.api/src/unraid-api/graph/resolvers/servers/server.resolver.ts (4)
7-8: Good model import structureThe import changes properly separate model concerns from resolver logic, following the code-first GraphQL approach.
10-10: Improved type safety with code-first approachChanging from a string-based resolver (
@Resolver('Server')) to a class reference (@Resolver(() => ServerModel)) enhances type safety and enables better IDE support.
12-12: Consistent return type annotationsThe GraphQL decorators now consistently use code-first approach with explicit return types, making the schema generation more reliable.
Also applies to: 22-22, 32-32
18-18: Updated return type signaturesReturn type signatures correctly match the model changes, ensuring type consistency between runtime and compile time.
Also applies to: 28-28
api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts (7)
9-20: Updated imports for model consistencyThe import structure has been properly updated to use the new model file structure, with
NotificationImportancereplacing the previousImportanceenum.
29-29: Enum reference updatedUpdated to use the new
NotificationImportanceenum.
81-82: Default notification importance updatedThe default importance value has been updated to use the new enum.
107-111: Function parameter types updatedThe parameter type has been correctly updated to use
NotificationImportance.
121-129: Function parameter types updated consistentlyThe parameter types have been updated to use
NotificationImportancethroughout the helper function.
259-267: Importance enum references updatedAll importance enum references have been properly updated to use the new
NotificationImportanceenum.
357-360: Logic updated for choosing another importanceThe conditional logic for selecting an alternative importance has been updated to use the new enum.
api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts (8)
3-12: Well-defined enumsThe enums are clearly defined with appropriate values that match the expected usage.
14-21: Proper enum registration with GraphQLThe enums are properly registered with GraphQL, which is required for them to be usable in the schema.
23-36: Filter input type well-structuredThe
NotificationFilterinput type is well-structured with appropriate field decorators and nullability settings.
38-54: Notification data input type well-definedThe
NotificationDatainput type properly defines all required fields with appropriate decorators.
56-69: Clear notification counts structureThe
NotificationCountsobject type provides a clear structure for counting notifications by importance levels.
71-78: Well-organized overview structureThe
NotificationOverviewobject type appropriately organizes counts for both unread and archived notifications.
80-108: Comprehensive notification modelThe
Notificationobject type comprehensively defines all needed fields with appropriate types and nullability settings. The description fortitleis particularly helpful.
110-122: Complete notifications containerThe
Notificationsobject type properly defines the container for a list of notifications with an overview of counts.api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (6)
20-29: Updated imports for model consistencyThe import structure has been properly updated to use the new model file structure and validation utility.
144-147: Method parameter types updatedThe
incrementanddecrementmethods have been updated to useNotificationImportanceinstead ofImportance.Also applies to: 149-152
466-485: Updated archiveAll method signatureThe
archiveAllmethod signature has been updated to useNotificationImportanceinstead ofImportance, maintaining consistent type usage.
487-507: Updated unarchiveAll method signatureThe
unarchiveAllmethod signature has been updated to useNotificationImportanceinstead ofImportance, maintaining consistent type usage.
696-707: Updated fileImportanceToGqlImportance methodThe method has been updated to return
NotificationImportanceinstead ofImportance, maintaining consistent type usage.
709-720: Updated gqlImportanceToFileImportance methodThe method has been updated to accept
NotificationImportanceinstead ofImportance, maintaining consistent type usage.api/src/unraid-api/graph/resolvers/connect/connect.model.ts (2)
141-143: Confirm usage of URL type as a GraphQL scalar.
@Field(() => URL)implies a custom scalar forURL. NestJS by default has no built-in scalar namedURL. Verify you have defined or imported aURLscalar to avoid schema generation errors.
37-53: User info input looks good.The fields and validations in
ConnectUserInfoInputappear consistent and well-defined, ensuring that required user data is provided.api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts (1)
1-3:✅ Verification successful
Validate import references for 'MutationType'
The new import for
MutationTypeis consistent with a code-first GraphQL approach. Confirm that the actualMutationclass frommutation.model.tsmatches these references.
🏁 Script executed:
#!/bin/bash rg "class Mutation" -A 5Length of output: 1126
Verification Complete: Import References for
MutationTypeAre Valid
- The resolver imports the
Mutationclass frommutation.model.tsasMutationType, and the code correctly usesMutationTypein the GraphQL mutation decorator and instantiation.- The shell script results confirm that the actual
Mutationclass is defined inmutation.model.tsas expected, validating the use of the alias.The import reference aligns with a code-first GraphQL approach, and no further changes are needed.
api/src/unraid-api/graph/resolvers/array/parity.resolver.ts (2)
1-11: PubSub usage is correctThe new imports and PubSub initialization look good. Using
@Resolver(() => ParityCheck)is a valid code-first approach.
37-40:✅ Verification successful
Verify subscription channel
Ensure that
PUBSUB_CHANNEL.PARITYis correctly set and thatpubSub.asyncIterableIteratoris spelled as intended. In many setups it's namedasyncIterator.
🏁 Script executed:
#!/bin/bash rg "pubSub.asyncIterator"Length of output: 25
I'm adding an additional verification step to gather more context on the usage of the subscription method and the definition of the subscription channel.
Please run these commands and share the outputs so we can confirm whether:
- The method name
asyncIterableIteratoris consistently used and correct.- The constant
PUBSUB_CHANNEL.PARITYis properly defined and in use.Once we have this additional information, we can finalize the review comment.
🏁 Script executed:
#!/bin/bash # Check for usage of pubSub.asyncIterableIterator echo "Searching for pubSub.asyncIterableIterator usage:" rg "pubSub\.asyncIterableIterator\(" -n . # Also check if there's any usage of pubSub.asyncIterator as an alternative echo "Searching for pubSub.asyncIterator usage:" rg "pubSub\.asyncIterator\(" -n . # Verify the definition or occurrences of PUBSUB_CHANNEL with PARITY echo "Searching for PUBSUB_CHANNEL.PARITY usage or definition:" rg "PUBSUB_CHANNEL.*PARITY" -n .Length of output: 705
Subscription Channel Verification Complete
The verification confirms that the code is consistently using
pubSub.asyncIterableIteratorwithPUBSUB_CHANNEL.PARITYas intended, and there’s no indication of a typo or misconfiguration. No changes are needed.api/src/unraid-api/graph/resolvers/array/array.model.ts (8)
1-5: Ensure references to 'graphql-type-long' are correctThe import of
GraphQLLongis valid. Confirm that it is used as expected and properly integrated within your schema.
6-16: Inconsistent optional usage vs default values
Capacitydefines non-optional string fields with''defaults, while some classes use optional fields ornull. Confirm these fields should never be null.
18-25: LGTMThe
ArrayCapacityclass structure is logical. Good separation of disk-based capacity metrics and relevant descriptors.
27-123: Verify default field valuesThe
ArrayDiskclass uses zero or null defaults. Ensure these defaults correctly reflect real-world device states, especially forrotational,temp, and filesystem metrics.
125-156: Validate array state transitionsTyped fields for
previousState,pendingState, andstateare well-defined. Just ensure they do not end up in conflicting states (e.g., a started array with a STOPPED pendingState).
158-165: Confirm slot usage
slotis optional inArrayDiskInput. Ensure that calling logic properly handles its absence or default.
198-253: Enum definitionsThese enumerations thoroughly cover array states and disk statuses. Verify that any extraneous or legacy values are removed.
255-305: Share object designThe
Shareclass closely mirrors the disk approach. Looks consistent; confirm all share fields are necessary and align with real usage.api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts (10)
1-2: Imports updated to support GraphQL code-first approachThe transition from schema-first to code-first GraphQL is reflected in these import changes. The
BadRequestExceptionaddition supports better error handling, while changing fromResolveFieldtoMutationaligns with proper GraphQL mutation semantics.
6-12: Improved type imports from dedicated model fileGood transition to importing structured types from a dedicated model file rather than using generated types. This improves code organization and maintainability.
14-14: Updated Resource import pathThe import path has been updated to reflect the new location of the base model, which is a good practice for maintaining consistent imports.
16-16: Resolver now uses code-first approachThe resolver now properly uses the code-first approach with
() => ArrayMutationsas type parameter, improving type safety and alignment with NestJS best practices.
20-20: Enhanced setState mutation with explicit return typeThe method now uses
@Mutationdecorator with return type and description, which improves GraphQL schema documentation. The explicitPromise<UnraidArray>return type enhances type safety.Also applies to: 26-26
30-30: Enhanced addDiskToArray mutation with explicit return typeSimilar improvements as the setState method - proper mutation decorator with return type and description, plus explicit Promise return type in the method signature.
Also applies to: 36-36
40-43: Enhanced removeDiskFromArray mutation with detailed descriptionGood addition of a detailed multi-line description that warns users about the array state requirement. This kind of documentation helps prevent runtime errors by informing API consumers of prerequisites.
Also applies to: 49-49
53-71: Improved mountArrayDisk with specific return type and error handlingThe changes to this method are significant improvements:
- Now returns the specific disk instead of the whole array
- Added proper error handling with BadRequestException
- Includes a clear description in the GraphQL schema
This will provide a better developer experience for API consumers.
73-91: Improved unmountArrayDisk with specific return type and error handlingSimilar to mountArrayDisk, these changes improve error handling and return type specificity.
93-102: Simplified clearArrayDiskStatistics return typeChanged to return a boolean instead of JSON, which is a more appropriate and simpler return type for this operation. The method now correctly awaits the service call and returns true to indicate success.
api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts
Show resolved
Hide resolved
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts (1)
215-216: Validation approach changedThe test is failing because the validation approach has been changed from schema-based to class-based validation, but the test expectations haven't been adjusted accordingly.
The pipeline failures indicate that this test is failing with errors like "Cannot read properties of undefined (reading 'constructor')" and "expected +0 to deeply equal 1". The validation now uses
validateObjectwhich may return or handle objects differently than the previousNotificationSchema().safeParse.You need to update the test setup to account for how
validateObjectvalidates and returns objects.
🧹 Nitpick comments (1)
api/src/__test__/store/state-parsers/shares.test.ts (1)
18-95: Check if additional test coverage is needed.The snapshot testing for the new
idproperty appears valid. However, consider adding tests that cover corner cases, such as handling missing or malformed fields in the parsed share definitions, to further ensure the parser’s resilience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (10)
api/src/__test__/core/utils/shares/get-shares.test.ts(1 hunks)api/src/__test__/store/modules/config.test.ts(2 hunks)api/src/__test__/store/modules/emhttp.test.ts(2 hunks)api/src/__test__/store/state-parsers/shares.test.ts(1 hunks)api/src/__test__/store/state-parsers/slots.test.ts(1 hunks)api/src/__test__/store/state-parsers/var.test.ts(1 hunks)api/src/unraid-api/graph/resolvers/disks/disks.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts(2 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts(10 hunks)
✅ Files skipped from review due to trivial changes (1)
- api/src/test/store/state-parsers/var.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/graph/resolvers/disks/disks.resolver.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
api/src/__test__/store/modules/emhttp.test.ts (1)
api/src/unraid-api/graph/shares/shares.resolver.ts (1)
shares(18-25)
api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts (1)
web/composables/gql/graphql.ts (5)
NotificationFilter(1057-1062)NotificationData(1049-1055)NotificationCounts(1041-1047)NotificationOverview(1064-1068)Notifications(1075-1081)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts (4)
web/composables/gql/typename.ts (1)
NotificationType(5-5)api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (1)
createNotification(57-62)api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (1)
createNotification(198-217)api/src/unraid-api/graph/resolvers/validation.utils.ts (1)
validateObject(15-34)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (24)
api/src/__test__/store/state-parsers/slots.test.ts (1)
18-188: Standardized type field values to uppercase format.The test snapshot has been updated to reflect the standardization of disk type values to uppercase constants (PARITY, DATA, CACHE, FLASH). This change aligns with the code-first GraphQL approach, ensuring consistent enumeration values across the codebase.
api/src/__test__/store/modules/config.test.ts (2)
10-14: Import changes align with code-first GraphQL approachThe import changes from generated types to dedicated model files are a good improvement. This approach provides better type safety, maintainability, and reduces dependencies on generated code.
152-152: Added URL property ensures consistent data structureAdding the
urlproperty to the published object ensures consistency with the structure used in other test cases (like thelogoutUsertest on line 174).api/src/__test__/core/utils/shares/get-shares.test.ts (1)
11-100: Test snapshots updated to includeidpropertyThe test snapshots have been updated to include an
idproperty for each user share object. This change aligns with the PR objectives of transitioning to dedicated model files and enhancing type safety.I notice that the
idvalue matches thenameproperty in each case (e.g., "appdata", "domains", "isos", "system"), suggesting that share names are being used as unique identifiers in the system.Also applies to: 106-127, 129-215, 223-244
api/src/__test__/store/modules/emhttp.test.ts (3)
199-369: Well-structured disk type standardization to uppercaseThe change from lowercase to uppercase disk types (PARITY, DATA, CACHE, FLASH) follows good enum value conventions and improves consistency. This aligns well with the GraphQL-first approach mentioned in the PR objectives.
371-450: Good addition of ID fields to share objectsAdding ID fields to each share object that match the share name improves the data structure consistency and provides proper identifiers for GraphQL operations. This complements the shares resolver implementation that combines user and disk shares into a single collection.
964-964: Consistent casing for filesystem typeChanging the defaultFsType to uppercase "XFS" maintains consistency with the other type-related changes in this PR.
api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts (9)
9-20: Import statements updated to use new model filesThe import statements have been updated to use the correct model files, which aligns with the PR objective of transitioning from generated GraphQL types to dedicated model files. The change maintains consistency across the codebase.
29-29: Enum transition completeSuccessfully transitioned from
Importanceenum toNotificationImportanceenum, maintaining consistency with the model changes.
81-81: Default importance updatedThe default importance value in the
createNotificationhelper function has been updated to useNotificationImportance.INFOinstead of the previous enum, completing the enum transition.
107-107: Function parameter type updatedThe
forEachImportancefunction's parameter type has been updated to useNotificationImportanceinstead of the previous enum.
122-122: Function parameter type updatedThe
forAllTypesAndImportancesfunction's parameter type has been updated to useNotificationImportanceinstead of the previous enum.
228-228: Enum usage updated in notification dataThe test data now correctly uses
NotificationImportance.INFOinstead of the previous enum.
263-271: Notification creation updated to use new enumAll test notification creations have been updated to use the
NotificationImportanceenum values instead of the previous enum.
318-326: Notification creation updated to use new enumAll test notification creations have been updated to use the
NotificationImportanceenum values instead of the previous enum.
361-364: Logic for determining anotherImportance updatedThe logic for determining
anotherImportancein the archiving tests has been properly updated to useNotificationImportanceenum values.api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts (2)
7-7: Updated import path from generated types to dedicated model fileThe import path for
ContainerStatehas been updated to use a dedicated model file. This change aligns with the codebase's transition from generated GraphQL types to modular model files.
312-349:❓ Verification inconclusive
Test now uses snapshot testing instead of explicit expectations
The test for
getNetworksmethod has been updated to usetoMatchInlineSnapshot()instead of hardcoded expectations. This change also reflects a modification in the response structure with updated property casing (e.g.,enableIPv6instead ofenableIpv6).
🏁 Script executed:
#!/bin/bash # Check if the casing in the test matches the actual implementation # Look for the Docker network model/interface definition echo "Searching for Docker network model definitions..." rg -A 5 "interface.*Network" --type typescriptLength of output: 192
Please run the following updated shell script to search for Docker network interface definitions by filtering with the correct file glob (i.e. "*.ts"). This will help verify that the properties—such as the updated casing from
enableIpv6toenableIPv6—are correctly defined in the TypeScript files.#!/bin/bash echo "Searching for Docker network interface definitions in .ts files..." rg -A 5 "interface.*Network" -g "*.ts"Once you’ve reviewed the output, you can confirm that the snapshot update (switching to
toMatchInlineSnapshot()) and the casing change are consistent with the implementation.api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts (6)
25-48: Validate property types against the client definition.
NotificationFiltermandates a non-nulltypefield, whereas the client snippet indicates it might be optional. If the client expects an optionaltypefilter, consider making the field optional here or updating the client usage to match.
50-76: Good use of class-validator for notification data.The
NotificationDataclass is well-defined with strong validation. This ensures that you capture the essential fields (title,subject,description,importance) and handle optionallinkfields neatly.
78-99: NotificationCounts structure looks consistent.The fields for counts (
info,warning,alert,total) logically represent each notification level. The@IsInt()and@Min(0)validations help prevent negative count values.
101-110: Clear distinction between unread and archive counts.
NotificationOverviewdistinctly separates unread and archived notifications, providing clarity for client-side usage. Well done!
112-159: Thoroughly defined fields for each notification.All essential fields are validated, including
id,title,subject, anddescription. The optional nature oftimestampandformattedTimestampis appropriate for scenarios where timestamps may not always be available.
160-176: Efficient grouping of notifications and their overview.
Notificationscombines anid, an overview, and a list of individual notifications. This holistic structure should simplify both data querying and rendering in client components.
api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
api/src/unraid-api/graph/resolvers/info/info.model.ts (2)
15-18: Consolidate repeatedTemperatureenum definitions.
This enum is already present in other models. Merging them into a single source of truth can minimize duplication and future refactors.
260-323: Optional design alignment forDisplayfields.
ThisDisplayclass appears similar to theDisplayin other parts of the codebase. Merging or reusing a shared model can help avoid drift and future maintenance headaches.api/src/unraid-api/graph/resolvers/connect/connect.model.ts (1)
121-125:⚠️ Potential issuePotential validation mismatch for numeric
portfields
The@IsPort()decorator fromclass-validatorexpects a string, but these fields are typed asnumber | null. This mismatch was flagged in a previous review and remains unresolved.Apply the following snippet to fix the validation, or switch the type to a string if you truly want a string port:
- @IsPort() + import { IsInt, Min, Max } from 'class-validator'; @Field(() => Int, { nullable: true }) - @IsPort() + @IsInt() + @Min(1) + @Max(65535) port?: number | null;Also applies to: 146-149, 211-215, 258-266
🧹 Nitpick comments (14)
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts (1)
5-8: Good refactoring to type-safe GraphQL resolvers, but constructor is unnecessary.The change from string-based resolver (
@Resolver('Mutation')) to type-safe resolver (@Resolver(() => RootMutations)) is a solid improvement aligned with the code-first GraphQL approach. This provides better type safety and IDE support.However, the constructor is empty and unnecessary. NestJS will handle dependency injection without it.
@Resolver(() => RootMutations) export class RootMutationsResolver { - constructor() {} }🧰 Tools
🪛 Biome (1.9.4)
[error] 7-7: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
api/src/unraid-api/graph/shares/shares.resolver.ts (1)
11-11: Remove unnecessary constructor.The constructor doesn't perform any initialization or dependency injection and can be safely removed.
- constructor() {}🧰 Tools
🪛 Biome (1.9.4)
[error] 11-11: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
api/src/unraid-api/graph/graph.module.ts (3)
27-28: Empty imports and inject arrays.Since these arrays are empty, consider removing them entirely to simplify the configuration.
useFactory: async () => { - imports: [], - inject: [], return { // ...configuration }; },
31-33: Clarify schema generation comment.The comment "This will generate the schema in memory" contradicts the configuration which specifies a file path. If you're generating a physical file, update the comment to reflect this.
autoSchemaFile: { path: '../generated-schema-new.graphql', -}, // This will generate the schema in memory +}, // This will generate a schema file at the specified path
43-43: Remove type assertion for better type safety.Using
as any[]bypasses TypeScript's type checking. Consider defining proper types for your plugins.- plugins: [sandboxPlugin, idPrefixPlugin] as any[], + plugins: [sandboxPlugin, idPrefixPlugin],If TypeScript complains about the types, consider properly typing your plugins to match the expected GraphQLPlugin interface.
api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts (2)
24-34: Type consistency in nullable fieldsThe implementation matches the expected MinigraphqlResponse structure, but there's an inconsistency in how nullable fields are typed.
Consider standardizing your nullable field types - you're using both
number | nulland justnumber?for optional fields:- timeout?: number | null; + timeout?: number; - error?: string | null; + error?: string;This keeps the type definitions more consistent throughout your codebase.
36-46: Same type inconsistency in CloudResponseThe optional error field uses the union with null pattern while ip doesn't.
- error?: string | null; + error?: string;api/src/unraid-api/graph/resolvers/servers/server.model.ts (1)
28-56: Consider adding descriptions to Server fieldsWhile the Server class is well-structured with appropriate types, adding descriptions would improve schema documentation for API consumers.
Consider adding descriptions to your fields similar to other model files in the codebase:
- @Field(() => ProfileModel) + @Field(() => ProfileModel, { description: 'Server owner profile information' }) owner!: ProfileModel; - @Field() + @Field({ description: 'Unique server identifier' }) guid!: string;api/src/unraid-api/graph/resolvers/info/info.model.ts (2)
20-22: Consider aligningThememore closely with the existing interface.
You currently have a single-valued enum, yetweb/themes/types.d.tsdefines a more robustThemeinterface. Consolidating these definitions could remove confusion and maintain consistency between the web and API layers.
220-227: Ensure consistent usage ofNodeinterface forUsbor justify its omission.
Unlike other types,Usbdoes not implementNode. Confirm whether this is intentional or if you should addimplements Nodefor uniformity.api/src/unraid-api/graph/resolvers/connect/connect.model.ts (1)
1-364: Overall solid code-first approach
Apart from the port validation issue, the file structure, use of NestJS decorators, and class-based GraphQL models look clean and consistent. Good job on enumerations and thorough validations for other fields.api/src/unraid-api/graph/resolvers/array/parity.model.ts (1)
5-6: Use a date scalar if your date field is truly a date.
If you actually intend to store a date/time, a more specific GraphQL scalar (e.g.GraphQLISODateTime) or a custom date scalar might be more appropriate than a string.api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts (2)
59-71: Consider extracting repeated disk lookup logic.
The search acrossarray.disks,array.parities, andarray.cachescould be consolidated into a helper function to reduce duplication.
79-91: Repeated lookup logic here as well.
Similarly, unmount disk repeats the same search pattern; a shared utility might improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (23)
api/dev/Unraid.net/myservers.cfg(1 hunks)api/src/unraid-api/graph/graph.module.ts(3 hunks)api/src/unraid-api/graph/resolvers/array/array.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/array.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/parity.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/parity.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/base.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/config/config.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/display/display.resolver.ts(3 hunks)api/src/unraid-api/graph/resolvers/docker/docker.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/info/info.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/logs/logs.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/mutation/mutation.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/resolvers.module.ts(2 hunks)api/src/unraid-api/graph/resolvers/servers/server.model.ts(1 hunks)api/src/unraid-api/graph/services/services.resolver.ts(2 hunks)api/src/unraid-api/graph/shares/shares.resolver.ts(1 hunks)generated-schema-new.graphql(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- api/dev/Unraid.net/myservers.cfg
- generated-schema-new.graphql
🚧 Files skipped from review as they are similar to previous changes (10)
- api/src/unraid-api/graph/resolvers/base.model.ts
- api/src/unraid-api/graph/resolvers/logs/logs.model.ts
- api/src/unraid-api/graph/resolvers/display/display.resolver.ts
- api/src/unraid-api/graph/resolvers/mutation/mutation.model.ts
- api/src/unraid-api/graph/resolvers/resolvers.module.ts
- api/src/unraid-api/graph/resolvers/array/array.resolver.ts
- api/src/unraid-api/graph/resolvers/array/parity.resolver.ts
- api/src/unraid-api/graph/services/services.resolver.ts
- api/src/unraid-api/graph/resolvers/docker/docker.model.ts
- api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts
🧰 Additional context used
🧬 Code Graph Analysis (10)
api/src/unraid-api/graph/resolvers/servers/server.model.ts (4)
api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts (5)
ObjectType(15-22)ObjectType(24-34)ObjectType(36-46)ObjectType(48-58)ObjectType(60-79)api/src/unraid-api/graph/resolvers/array/array.model.ts (5)
ObjectType(8-18)ObjectType(20-27)ObjectType(29-125)ObjectType(127-158)ObjectType(256-305)api/src/unraid-api/graph/resolvers/mutation/mutation.model.ts (3)
ObjectType(3-7)ObjectType(9-13)ObjectType(15-22)api/src/unraid-api/graph/resolvers/connect/connect.model.ts (3)
ObjectType(107-125)ObjectType(162-183)ObjectType(185-220)
api/src/unraid-api/graph/graph.module.ts (2)
api/src/unraid-api/graph/resolvers/resolvers.module.ts (1)
Module(35-69)api/src/graphql/resolvers/graphql-type-long.ts (1)
GraphQLLong(29-35)
api/src/unraid-api/graph/resolvers/config/config.resolver.ts (1)
api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts (1)
Resolver(32-248)
api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts (1)
web/composables/gql/graphql.ts (4)
ApiKeyResponse(75-79)MinigraphqlResponse(818-823)CloudResponse(378-383)RelayResponse(1367-1372)
api/src/unraid-api/graph/shares/shares.resolver.ts (2)
api/src/unraid-api/graph/resolvers/array/array.resolver.ts (1)
Resolver(12-35)web/composables/gql/graphql.ts (1)
Share(1460-1484)
api/src/unraid-api/graph/resolvers/connect/connect.model.ts (2)
web/composables/gql/graphql.ts (6)
RemoteAccess(1374-1379)DynamicRemoteAccessStatus(639-644)ConnectSettingsValues(416-433)ConnectSettings(407-413)Connect(400-405)Node(1022-1024)api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts (5)
ObjectType(15-22)ObjectType(24-34)ObjectType(36-46)ObjectType(48-58)ObjectType(60-79)
api/src/unraid-api/graph/resolvers/array/array.model.ts (2)
web/composables/gql/graphql.ts (5)
Capacity(353-358)ArrayDisk(144-186)ArrayDiskInput(199-204)ArrayStateInput(317-320)Share(1460-1484)api/src/graphql/resolvers/graphql-type-long.ts (1)
GraphQLLong(29-35)
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts (1)
api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts (1)
Resolver(16-103)
api/src/unraid-api/graph/resolvers/info/info.model.ts (2)
web/themes/types.d.ts (1)
Theme(1-9)web/composables/gql/graphql.ts (15)
InfoApps(699-705)Node(1022-1024)Baseboard(344-351)InfoCpu(707-726)Gpu(664-673)Pci(1182-1193)Usb(1649-1653)Case(360-366)Display(551-574)MemoryLayout(789-802)InfoMemory(728-741)Os(1088-1102)System(1560-1568)Versions(1840-1868)Info(681-697)
api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts (2)
api/src/unraid-api/graph/resolvers/array/array.resolver.ts (2)
Resolver(12-35)array(22-24)web/composables/gql/graphql.ts (4)
ArrayMutations(238-249)ArrayStateInput(317-320)ArrayDiskInput(199-204)ArrayDisk(144-186)
🪛 Biome (1.9.4)
api/src/unraid-api/graph/shares/shares.resolver.ts
[error] 11-11: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts
[error] 7-7: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (29)
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts (1)
1-3: Clean import refactoring.Good job updating the imports to match the new code-first GraphQL structure. The import of the
RootMutationsmodel shows clear separation of concerns between models and resolvers.api/src/unraid-api/graph/shares/shares.resolver.ts (3)
6-7: Import changes align with code-first GraphQL approach.The import statements have been correctly updated to reflect the new code-first approach using NestJS decorators. Moving from generated types to explicitly defined model files improves type safety and maintainability.
9-9: Type-safe resolver decorator implementation.Good change from
@Resolver('Shares')to@Resolver(() => Share). This provides better type safety and follows NestJS best practices for code-first GraphQL implementations.
13-13: Query return type explicitly defined.The update from
@Query('shares')to@Query(() => [Share])correctly defines the return type as an array of Share objects, providing better type safety and alignment with the code-first approach.api/src/unraid-api/graph/resolvers/config/config.resolver.ts (4)
1-7: Import statements updated for code-first approachThe imports have been simplified to only include necessary GraphQL decorators and updated to use the
@appalias with.jsextensions. This aligns with the code-first GraphQL approach and supports modern ES module resolution.
9-9: Decorator refactored to use code-first approachChanged from
@Resolver('Config')to@Resolver(() => Config), which explicitly references the class type rather than using a string. This improves type safety and enables better IDE support and refactoring capabilities.
11-11: Return type explicitly defined in Query decoratorUpdated the Query decorator to explicitly specify the return type:
@Query(() => Config). This improves schema generation and type safety in the GraphQL API.
1-26:✅ Verification successful
Verification needed for removed functionality
The
setAdditionalAllowedOriginsmutation has been removed from this resolver. Based on the code snippets, this functionality appears to have been moved to theConnectResolver. Please verify that this refactoring maintains the same functionality and that all client code has been updated accordingly.
🏁 Script executed:
#!/bin/bash # Search for any references to setAdditionalAllowedOrigins to ensure # they've been updated to use the new location grep -r "setAdditionalAllowedOrigins" --include="*.ts" --include="*.tsx" --include="*.js" --include="*.jsx" .Length of output: 2611
Action Required: Confirming Mutation Refactoring is Correct
The
setAdditionalAllowedOriginsmutation has been successfully moved fromConfigResolvertoConnectResolver. Verification confirms:
- The mutation now appears in
./api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts.- Client references using the mutation (evident in files under
./web/composables/gql/and./web/store/) have been updated to align with the new location.No further changes are required regarding this refactoring.
api/src/unraid-api/graph/graph.module.ts (3)
6-6: Good addition of validation rule import.Adding the
NoUnusedVariablesRuleimport is a good practice for ensuring efficient GraphQL queries by preventing unused variables.
49-53: Good simplification of resolver configuration.The scalar resolvers are now clearly defined, which aligns well with the code-first approach.
54-56: Good date scalar configuration.Setting
dateScalarModeto 'isoDate' ensures consistent DateTime handling across your API.api/src/unraid-api/graph/resolvers/cloud/cloud.model.ts (3)
1-13: Well-structured MinigraphStatus enum with appropriate registrationThe MinigraphStatus enum is well-defined with clear, descriptive states that cover the various connection phases. The use of registerEnumType is the correct way to expose enums in NestJS GraphQL.
15-22: LGTM: ApiKeyResponse class properly implements GraphQL typeThis class correctly implements the ApiKeyResponse GraphQL type with appropriate field decorators. The nullable error field is properly defined.
60-79: Cloud class structure is well-organizedThe Cloud class effectively aggregates all relevant cloud-related responses with proper field decorations and type associations. This class serves as a comprehensive container for cloud status information.
api/src/unraid-api/graph/resolvers/servers/server.model.ts (3)
3-16: Well-structured ProfileModel with appropriate ID fieldThe ProfileModel class correctly implements a user profile structure with properly decorated fields. Good use of the ID scalar type for userId with nullable property.
18-26: Clear ServerStatus enum definitionThe ServerStatus enum has well-named, descriptive states that clearly indicate the possible server connection states. The registerEnumType call correctly exposes this enum to the GraphQL schema.
36-38: Security consideration for apikey fieldExposing the API key directly in the GraphQL schema could present a security risk.
Verify that appropriate authorization checks are in place to ensure only authorized users can access this sensitive field. Consider whether this field should be nullable or if there are cases where it should be hidden from certain users.
api/src/unraid-api/graph/resolvers/info/info.model.ts (1)
13-13: Verify the import extension forbase.model.js.
Using.jsin a TypeScript codebase can be intentional for ES module compatibility or might indicate a mismatch in build settings. Confirm that this import resolves correctly and that it aligns with your TypeScript configuration.api/src/unraid-api/graph/resolvers/connect/connect.model.ts (1)
153-160:❓ Verification inconclusive
Confirm the usage of the
URLtype
GraphQLURLmay parse an incoming string into a nativeURLobject. Ensure the rest of your code can handle aURLinstance without errors. Otherwise, consider a string field enhanced with@IsUrl()for validation consistency.Run the following script to locate all references to
EnableDynamicRemoteAccessInputand verify whether aURLobject is actually expected:
🏁 Script executed:
#!/bin/bash rg -A 10 'EnableDynamicRemoteAccessInput'Length of output: 8932
Action Required: Confirm URL Type Consistency in EnableDynamicRemoteAccessInput
Please verify that the native
URLinstance supplied by theGraphQLURLscalar is correctly handled throughout the system. Our search shows that while the model inapi/src/unraid-api/graph/resolvers/connect/connect.model.tsuses a nativeURL(with GraphQL exposing it asURLin one schema), other parts of the codebase—including client-facing types likeAccessUrlInputinapi/generated-schema.graphqlandweb/composables/gql/graphql.ts—expect a different URL representation. Consider these steps:
- Check Conversions: Confirm that every consumer of
EnableDynamicRemoteAccessInputproperly converts or handles the nativeURLinstance.- Ensure Schema Consistency: Verify that the discrepancy between using a native
URLversus anAccessUrlInputis intentional. If not, update the model to use a string with@IsUrl()(or adjust the schema/type resolvers accordingly) to ensure uniform validation and behavior.- Review Downstream Usage: Examine the resolver logic and client interactions to ensure that this type divergence will not cause runtime errors.
api/src/unraid-api/graph/resolvers/array/parity.model.ts (1)
21-25: ParityCheckHistory definition looks good.
Using an empty array forparityChecksis a clear, conventional approach to avoid null references.api/src/unraid-api/graph/resolvers/array/array.model.ts (5)
8-18: Capacity class looks solid.
All fields and types align well with typical capacity tracking.
20-28: ArrayCapacity usage is consistent.
The composition of multipleCapacityfields is logical and clear.
29-125: ArrayDisk is well-structured.
The field definitions and types appear consistent and domain-appropriate.
127-158: UnraidArray logic is clear.
The class effectively represents array state, capacity, and disk groupings.
256-305: Share object type is thorough.
All fields match the domain requirements, and optional fields are handled gracefully.api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts (4)
20-28: setState mutation looks consistent.
No concerns identified here; it properly updates the array state.
30-38: addDiskToArray mutation is straightforward.
The logic for adding a disk to the array is clear and type-safe.
40-51: removeDiskFromArray mutation is well-structured.
This properly delegates to the service for disk removal.
93-102: clearArrayDiskStatistics is concise and clear.
This mutation is straightforward and handles its task well.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
api/src/unraid-api/graph/graph.module.ts (1)
17-17: AuthDirective import not registered in providers.Consider adding
AuthDirectiveto theprovidersarray if you intend to use it in the schema.api/src/unraid-api/graph/resolvers/info/info.model.ts (2)
15-18: Consolidate repeatedTemperatureenum definitions.
A prior review highlighted thatTemperaturemight appear in other modules (e.g.,display.model.ts). Merging it into a single source of truth can help maintain consistency.
275-341: Reduce duplication between thisDisplayand similar types in other files.
Past reviews mentioned potential overlap ofDisplayfields across multiple locations. Consider consolidating them into a single model or more specialized submodels.api/src/unraid-api/graph/resolvers/connect/connect.model.ts (1)
122-124:@IsPort()expects a string, not a numeric type.This issue was flagged previously. For fields typed as
number, switching to integer-based validators (e.g.,@IsInt() @Min(1) @Max(65535)) is preferable. Otherwise, change the field type tostringto align with@IsPort().- @IsPort() - port?: number | null; + import { IsInt, Min, Max } from 'class-validator'; + @IsInt() + @Min(1) + @Max(65535) + port?: number | null;Also applies to: 146-148, 212-214, 263-265
🧹 Nitpick comments (12)
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts (1)
13-13: Consider dependency injection for mutation classes if neededThe comments suggest potential for passing context/state, but currently no dependencies are being injected. If
ArrayMutationsorDockerMutationsneed access to services, you might want to inject them either here or in their respective resolvers.Also applies to: 18-18
api/src/unraid-api/graph/graph.module.ts (1)
31-33: Update the descriptive comment for clarity.When specifying a path for
autoSchemaFile, Nest will generate a schema file on disk rather than doing it solely in memory. Consider adjusting the comment to avoid confusion.-}, // This will generate the schema in memory +}, // This will generate and persist the schema to './generated-schema-new.graphql'api/src/unraid-api/graph/resolvers/vms/vms.model.ts (1)
1-1: Unused import detected.The
InputTypeis imported but not used anywhere in this file.-import { Field, ID, InputType, ObjectType, registerEnumType } from '@nestjs/graphql'; +import { Field, ID, ObjectType, registerEnumType } from '@nestjs/graphql';api/src/unraid-api/graph/resolvers/array/array.model.ts (2)
6-16: Consider storing capacity values as numeric fields for consistency
Capacity is currently represented as strings (e.g.,free,used,total), while other fields likesizein this file are floats. This discrepancy can lead to confusion or additional parsing. If you intend these fields to be numeric, consider changing them to numeric types.- @Field(() => String, { description: 'Free capacity' }) - free!: string; + @Field(() => Float, { description: 'Free capacity' }) + free!: number; - @Field(() => String, { description: 'Used capacity' }) - used!: string; + @Field(() => Float, { description: 'Used capacity' }) + used!: number; - @Field(() => String, { description: 'Total capacity' }) - total!: string; + @Field(() => Float, { description: 'Total capacity' }) + total!: number;
261-313: Add minimal test coverage for the newly introducedShareclass
This class manages share properties, but there is no mention of unit tests. Consider adding tests to validate critical properties likefree,used,include,exclude, etc.Would you like help creating a test suite that covers various share configurations?
api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts (1)
69-71: Use a more suitable exception for non-existing disk
Throwing aBadRequestExceptionfor a non-existing disk can be confusing. ANotFoundExceptionmight more accurately convey that the disk doesn't exist.- throw new BadRequestException(`Disk with id ${id} not found in array`); + throw new NotFoundException(`Disk with id ${id} not found in array`);Also applies to: 89-91
api/src/unraid-api/graph/resolvers/info/info.model.ts (2)
20-22: Consider expanding theThemeenum or referencing an existing shared interface.
Currently, only'white'is defined. If additional themes (e.g., 'dark', 'light') are needed, consider centralizing or aligning withweb/themes/types.d.ts.
258-273: Add explicitimplements Nodein theCaseclass.
The decorator specifiesimplements: () => Node, but the class signature does not. For consistency with similar classes, consider:-export class Case { +export class Case implements Node {api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts (2)
17-18: Remove the empty constructor.
Since no additional logic is performed, this constructor can be safely removed to adhere to linting rules and keep the class concise.- constructor() {}🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
47-50: Confirm placeholder logic forsettingsfield.
This method currently returns an empty object. If it’s a placeholder, consider adding a TODO or implementing the actual logic to fetch settings.api/src/unraid-api/graph/resolvers/connect/connect-settings.resolver.ts (2)
83-95: Consider handling errors when restarting the API.The future callback in the
setTimeoutblock restarts the API but lacks explicit error handling. In the event of a failure (e.g., an unhandled rejection inrestartApi()), the operation may silently fail. Consider wrapping the call in a try-catch or logging errors for better observability.setTimeout(async () => { try { this.logger.log('Restarting API'); await this.connectSettingsService.restartApi(); + } catch (error) { + this.logger.error(`Error restarting API: ${error.message}`); + // Additional handling or rethrow as needed. } }, 300);
114-117: Include error handling.
store.dispatch(logoutUser(...))may fail or return a rejected promise. Add a try-catch or handle promise rejections to avoid unhandled rejection scenarios.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (27)
api/generated-schema-new.graphql(1 hunks)api/src/graphql/resolvers/query/info.ts(7 hunks)api/src/unraid-api/graph/graph.module.ts(3 hunks)api/src/unraid-api/graph/resolvers/array/array.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/config/config.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect-settings.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts(3 hunks)api/src/unraid-api/graph/resolvers/connect/connect.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect.module.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/docker/docker.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/flash/flash.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/logs/logs.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/mutation/mutation.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/resolvers.module.ts(2 hunks)api/src/unraid-api/graph/resolvers/vars/vars.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/vms/vms.model.ts(1 hunks)api/src/unraid-api/graph/services/service.model.ts(1 hunks)web/codegen.ts(1 hunks)web/components/Notifications/Indicator.vue(1 hunks)web/components/Notifications/List.vue(1 hunks)web/components/Notifications/Sidebar.vue(1 hunks)web/composables/gql/graphql.ts(42 hunks)
✅ Files skipped from review due to trivial changes (5)
- web/components/Notifications/Indicator.vue
- web/codegen.ts
- web/components/Notifications/Sidebar.vue
- web/components/Notifications/List.vue
- api/generated-schema-new.graphql
🚧 Files skipped from review as they are similar to previous changes (10)
- api/src/unraid-api/graph/resolvers/config/config.model.ts
- api/src/unraid-api/graph/resolvers/flash/flash.model.ts
- api/src/unraid-api/graph/services/service.model.ts
- api/src/unraid-api/graph/resolvers/mutation/mutation.model.ts
- api/src/unraid-api/graph/resolvers/resolvers.module.ts
- api/src/unraid-api/graph/resolvers/vars/vars.model.ts
- api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts
- api/src/graphql/resolvers/query/info.ts
- api/src/unraid-api/graph/resolvers/logs/logs.resolver.ts
- api/src/unraid-api/graph/resolvers/docker/docker.model.ts
🧰 Additional context used
🧠 Learnings (1)
api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts (1)
Learnt from: elibosley
PR: unraid/api#1347
File: api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts:0-0
Timestamp: 2025-04-10T19:26:37.441Z
Learning: In the RemoteAccessController implementation, beginRemoteAccess and stopRemoteAccess are asynchronous methods (declared with async), while extendRemoteAccess is synchronous.
🧬 Code Graph Analysis (10)
api/src/unraid-api/graph/resolvers/connect/connect.module.ts (2)
api/src/unraid-api/graph/graph.module.ts (1)
Module(22-65)api/src/unraid-api/graph/resolvers/resolvers.module.ts (1)
Module(34-66)
api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts (2)
web/composables/gql/graphql.ts (3)
Connect(324-332)DynamicRemoteAccessStatus(590-598)ConnectSettings(334-344)api/src/store/index.ts (1)
store(6-12)
api/src/unraid-api/graph/graph.module.ts (2)
api/src/unraid-api/graph/resolvers/resolvers.module.ts (1)
Module(34-66)api/src/graphql/resolvers/graphql-type-long.ts (1)
GraphQLLong(29-35)
api/src/unraid-api/graph/resolvers/array/array.model.ts (2)
web/composables/gql/graphql.ts (7)
Capacity(272-280)ArrayCapacity(94-100)Node(923-925)ArrayDisk(102-145)UnraidArray(1317-1336)ArrayDiskInput(159-164)ArrayStateInput(252-255)api/src/graphql/resolvers/graphql-type-long.ts (1)
GraphQLLong(29-35)
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts (3)
api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts (1)
Resolver(19-106)api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts (1)
Resolver(13-36)web/composables/gql/graphql.ts (3)
Mutation(757-799)ArrayMutations(185-199)DockerMutations(553-559)
api/src/unraid-api/graph/resolvers/info/info.model.ts (2)
web/themes/types.d.ts (1)
Theme(1-9)web/composables/gql/graphql.ts (17)
Node(923-925)InfoApps(650-657)Baseboard(262-270)InfoCpu(659-679)Gpu(621-630)Network(917-921)Pci(1019-1030)Usb(1343-1347)Devices(416-422)Case(282-289)Display(501-524)MemoryLayout(727-740)InfoMemory(681-695)Os(995-1010)System(1286-1295)Versions(1527-1556)Info(632-648)
api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts (2)
api/src/unraid-api/graph/resolvers/array/array.resolver.ts (2)
Resolver(12-35)array(22-24)web/composables/gql/graphql.ts (5)
ArrayMutations(185-199)UnraidArray(1317-1336)ArrayStateInput(252-255)ArrayDiskInput(159-164)ArrayDisk(102-145)
api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts (4)
api/src/common/allowed-origins.ts (1)
getExtraOrigins(64-74)web/composables/gql/graphql.ts (2)
EnableDynamicRemoteAccessInput(606-611)RemoteAccess(1142-1150)api/src/store/modules/dynamic-remote-access.ts (1)
setAllowedRemoteAccessUrl(54-63)api/src/store/modules/config.ts (1)
loginUser(62-76)
api/src/unraid-api/graph/resolvers/vms/vms.model.ts (1)
web/composables/gql/graphql.ts (2)
VmDomain(1558-1565)Vms(1579-1583)
api/src/unraid-api/graph/resolvers/connect/connect-settings.resolver.ts (6)
web/composables/gql/graphql.ts (5)
ConnectSettings(334-344)ConnectSettingsValues(346-360)RemoteAccess(1142-1150)SetupRemoteAccessInput(1224-1231)EnableDynamicRemoteAccessInput(606-611)api/src/unraid-api/types/json-forms.ts (1)
DataSlice(16-16)api/src/store/modules/config.ts (1)
logoutUser(78-101)api/src/store/actions/setup-remote-access.ts (1)
setupRemoteAccessThunk(26-50)api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts (1)
updateAllowedOrigins(180-183)api/src/common/allowed-origins.ts (1)
getAllowedOrigins(91-99)
🪛 Biome (1.9.4)
api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts
[error] 18-18: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (87)
api/src/unraid-api/graph/resolvers/mutation/mutation.resolver.ts (4)
1-8: Improved imports structure for code-first GraphQL approachThe imports have been updated to support a more type-safe, code-first GraphQL approach. The explicit imports from the local model file align well with the PR objective.
9-10: Type-safe resolver declaration improves code qualityChanging from string-based resolver
@Resolver('Mutation')to type-based@Resolver(() => RootMutations)enhances type safety and aligns with NestJS best practices for code-first GraphQL.
11-14: Well-structured array mutation implementationThe implementation now uses the
@Mutationdecorator instead of@ResolveField, creating a more intuitive GraphQL API structure. This matches appropriately with theArrayMutationsResolverimplementation in the related files.
16-19: Well-structured docker mutation implementationSimilar to the array implementation, this uses the proper decorator and returns a new instance of
DockerMutations, which will be resolved byDockerMutationsResolver.api/src/unraid-api/graph/graph.module.ts (4)
6-6: Good addition of theNoUnusedVariablesRule.Using this validation rule helps ensure cleaner APIs by preventing unused variable definitions.
27-28: No concerns with empty arrays forimportsandinject.If no dependencies are required here, having them empty is valid.
49-53: Scalars and resolvers look correct.Defining JSON, Long, and URL resolvers here appropriately centralizes these scalar mappings.
54-56:dateScalarModeset toisoDateis a valid and clear choice.This helps standardize date handling.
api/src/unraid-api/graph/resolvers/vms/vms.model.ts (5)
4-13: Well-structured enum definition.The VmState enum is clearly defined with appropriate values that match their names.
15-18: Good enum registration for GraphQL.The enum is properly registered with a descriptive name and description.
20-30: Well-structured VmDomain class.The class has appropriate field decorators with good descriptions and proper type assertions.
44-46: Placeholder implementation.All mutation methods return
truewithout any actual implementation. This suggests these are placeholder methods that will be implemented elsewhere, or these methods are just for GraphQL schema definition purposes.Are these methods intended as placeholders or mock implementations? If they're part of the actual implementation, they should contain the actual logic or delegate to a service.
41-77:✅ Verification successful
Mutation methods are not properly defined.
The methods in the
VmMutationsclass are decorated with@Field()instead of@Mutation(), and parameters lack@Args()decorators. This suggests these are fields on an object type rather than standalone mutations.If these are meant to be GraphQL mutations, they should be properly defined as such.
🏁 Script executed:
#!/bin/bash # Check how these mutation methods are being used in resolvers # Search for classes that extend or use VmMutations echo "Searching for classes that use VmMutations..." rg -A 5 "VmMutations" --type ts # Check for VM-related mutation resolvers echo "Checking for VM-related mutation resolvers..." rg -A 5 "@Mutation" --type ts | grep -i "vm"Length of output: 9587
Clarification: Mutation type definitions are correctly separated from resolver implementations.
After reviewing the code and verifying its usage, the
VmMutationsclass inapi/src/unraid-api/graph/resolvers/vms/vms.model.tsis intentionally decorated with@ObjectType()and uses@Field()to define the GraphQL type structure. The actual mutation resolver methods—with proper@Mutation()and@Args()annotations—are implemented inapi/src/unraid-api/graph/resolvers/vms/vms.mutations.resolver.ts. There is no issue with the current design if the intent is to separate type definitions from resolver logic.api/src/unraid-api/graph/resolvers/array/array.model.ts (1)
61-99: Double-check usage of separate FS size fields
You storefsSize,fsFree, andfsUsedindividually. This is fine, but ensure that these fields align with any existing canonical source of truth (like the underlying system calls) and that partial usage (e.g., if onlyfsFreeis available) doesn't cause confusion.Would you like to verify that these fields are consistently set by the underlying system or do you prefer them to be consolidated into a single object?
api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts (1)
43-54:✅ Verification successful
Verify that array state checks are handled in the service
The documentation mentions the array must be stopped before removing a disk, yet no explicit check is visible here. Confirm thatthis.arrayService.removeDiskFromArray(input);enforces the necessary condition to avoid inconsistent array states.
🏁 Script executed:
#!/bin/bash # Using ripgrep to check for state checks in removeDiskFromArray rg -A 10 "removeDiskFromArray"Length of output: 8175
Array state check is correctly enforced in the service implementation
After verifying using the grep output, it’s clear that the state check is indeed implemented in the service. Specifically, in
api/src/unraid-api/graph/resolvers/array/array.service.ts, theremoveDiskFromArraymethod checks whether the array is running usingthis.arrayIsRunning(). If the array is running, it throws anArrayRunningError, thus ensuring that the array is stopped before any disk removal is attempted. The associated test cases inarray.service.spec.tsfurther confirm this behavior.api/src/unraid-api/graph/resolvers/info/info.model.ts (4)
1-14: All imports appear valid and consistent.
No syntax or logical issues are apparent in these import statements.
34-46:InfoAppsclass looks good.
Fields align well with the snippet fromweb/composables/gql/graphql.ts(lines 650-657).
232-239: Confirm whetherUsbshould implementNode.
All other hardware-related classes implement theNodeinterface, butUsbdoes not. If this is intentional to excludeidor other Node properties, no change is needed. Otherwise, addimplements Nodefor consistency.
576-615: OverallInfoaggregation is coherent.
This class cleanly references other types. Ensure all resolvers or queries returningInfoproperly set theidand nested fields.api/src/unraid-api/graph/resolvers/connect/connect.module.ts (1)
1-13: Solid module definition and imports.
No issues found. Optionally, consider exporting the providers if they're needed by other modules.api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts (14)
5-5: Usage of execa
No issues with importing and usingexecafor process management.
7-7: Usage of decodeJwt
No concerns here. This import fromjoseis standard for JWT decoding.
12-13: Type imports
ImportingConnectSignInInputandEnableDynamicRemoteAccessInputis appropriate.
18-18: Extra origins import
This import pulls ingetExtraOrigins; logic is straightforward.
20-20: Action import
ImportingsetupRemoteAccessThunkindicates initiation of remote access flows in Redux. No issues.
22-26: Config actions
All these Redux actions appear relevant for updating config, SSO users, etc.
27-29: ImportsetAllowedRemoteAccessUrl,FileLoadStatus, andApiKeyService
These imports look correct for their respective functionalities.
35-35: Additional Connect model imports
No issues noted.
41-41: Dependency injection
The constructor injectsApiKeyService; this is valid and not a useless constructor.
43-43: Logger initialization
Instantiating a localLoggeris good practice for error reporting and visibility.
45-51: API restart logic
Wrappingexecaintry/catchfor logging errors is appropriate. No issues here.
53-56: Extra allowed origins retrieval
Method is concise and clearly returns an array of origins.
185-234: Sign-in flow
The logic for creating missing local API keys and validating user attributes looks well-organized. No further issues noted.
282-296: Dynamic remote access settings
Implementation is appropriate. The return structure correctly maps store state into aRemoteAccessobject.api/src/unraid-api/graph/resolvers/connect/connect-settings.resolver.ts (1)
131-141: Validate the input size when adding allowed origins.Although
AllowedOriginInputenforces at least one origin via@ArrayMinSize(1), this resolver does not explicitly handle potential errors fromstore.dispatch(updateAllowedOrigins). Consider catching errors or verifying the updated origins to ensure consistent state.api/src/unraid-api/graph/resolvers/connect/connect.model.ts (2)
49-65: Looks good!User information input has proper validation (
@IsNotEmpty,@IsEmail). This should help ensure data integrity for sign-ins.
153-160: Confirm that URL type matches runtime usage.
@Field(() => GraphQLURL)andurl!: URLcan be valid ifURLis fromgraphql-scalarsor a standard built-in. Confirm this is correct in your environment, as a type mismatch may cause runtime errors.web/composables/gql/graphql.ts (50)
17-17: Doc comment clarity
The updated doc comment forDateTimeis concise and clear. Nicely done.
21-22: NewJSONObjectscalar definition
Introducing a dedicated JSON object scalar is a good practice for handling arbitrary JSON data in a strongly typed schema. The type aliasanyis acceptable here given GraphQL JSON constraints.
48-48: Informative doc comment
Adding the descriptive comment fororiginsclarifies its intent and usage. Good addition.
54-54:createdAtchanged fromDateTimetoStringinApiKey
UsingStringhere loses the semantic benefit of a date-time scalar. Confirm all consuming code is aware of this string representation.
70-70:createdAtchanged fromDateTimetoStringinApiKeyWithSecret
Same concern as above—verify downstream logic no longer relies on a date-time scalar.
80-91: Expanded doc comments forApiSettingsInput
These new doc comments provide clearer hints about each field. Ensure they stay in sync if the logic for remote access or extra origins changes.
96-99: Newkilobytesfield inArrayCapacity
Addingkilobytescaptures capacity at a finer granularity. Confirm that the UI or callers parse numeric values correctly from this field.
148-156: Refined enum values forArrayDiskFsColor
Renaming/standardizing color enums to uppercase fosters clarity. This looks consistent and self-descriptive.
178-181: UppercaseArrayDiskTypevalues
Using uppercase for these enum entries aligns with typical GraphQL enum naming conventions. No issues spotted.
188-199: Mandatory fields inArrayMutations
Converting optional fields to non-null in these mutations clarifies your API contract. Ensure no existing clients pass null arguments.
202-229: Updated argument definitions forArrayMutations
All input fields have become required. Confirm that all resolvers handle strictly required inputs and that no existing clients send null.
232-235: New states inArrayPendingState
These new states (NO_DATA_DISKS,STARTING,STOPPING, etc.) expand the array’s state tracking. Ensure your logic checks these states correctly.
262-265:Baseboardupdates
Assigningid: Scalars['ID']['output']is consistent with Node interface usage. Looks fine.
275-279: Doc comments forCapacity
Naming fields (free,total,used) as strings is consistent with certain capacity display approaches. No immediate issues.
282-287:Casetype changes
DefiningidwithScalars['ID']['output']is standard. The new lines appear consistent.
417-422:Devicesadditions
Exposing arrays forgpu,pci, andusblooks correct. Verify no duplication if devices appear in multiple categories.
467-474:DiskFsTypeenum expansions
Providing multiple file system types is helpful. Changes are consistent with typical FS naming.
477-478: NewPcieinterface
AddingPcietoDiskInterfaceTypeaccommodates more hardware. Good improvement.
803-803:MutationaddRoleForApiKeyArgs
No issues spotted with the updated argument reference.
808-808: Optionalimportanceparam forarchiveAll
This allows more fine-grained archiving of notifications. Looks good.
843-845:enableDynamicRemoteAccessArgs
Introducing an input type is consistent with the rest of the GraphQL approach. Looks good.
848-849:forceStopVmnow takes an ID
Making the VM ID explicit is clearer. Ensure calling code passes a valid ID.
854-855:pauseVmnow takes an ID
Similar note: the explicit ID clarifies which VM to pause.
859-860:rebootVmnow takes an ID
Consistent with other VM operations. Approved.
864-864:removeRoleFromApiKeyArgs
No concerns. Code looks consistent with AddRole counterpart.
868-870:resetVmrequires an ID
Matches the rest of the design for VM mutations.
874-875:resumeVmwith an ID
Identical pattern. Should be fine.
889-890:startVmnow requires an ID
Consistent with the rest of the VM actions.
894-895:stopVmwith required ID
Continues the uniform approach. Good.
899-899:unarchiveAllwith optional importance
Provides flexibility for unarchiving specific severity categories.
904-904:unarchiveNotificationsnow takesids
Allows batch unarchiving, a practical addition.
1003-1004: Refining theOstype
Exposingidusing the Node interface is consistent with GraphQL best practices. No issues found.
1009-1010: Optionaluptime
Markinguptimefield optional is appropriate if some OS contexts don’t expose it.
1014-1017:Ownerfields
Addingavatar,url, andusernamebroadens user info. Consider verifying no PII is unintentionally exposed.
1020-1020:Pcidefinition
No major concerns. The changes align with Node-based structures used elsewhere.
1040-1044:ProfileModelexpansions
Providingavatar,url, andusernameis consistent with the approach for user info.
1050-1077:Queryexpansions
Many new fields added (array,cloud,flash,info, etc.). This significantly broadens the schema surface. Double-check backward compatibility with older clients.
1081-1082:QueryapiKeyArgs
This parameter reference looks properly typed.
1144-1150:RemoteAccesstype doc
Enhanced clarity onaccessType,forwardType, andport. Perfect for consistent remote access usage.
1190-1190:Roleenum
No functional changes shown, but presumably clarifying user roles.
1210-1212:ServerStatusadditions
HavingNEVER_CONNECTED,OFFLINE, andONLINEclarifies the server's connectivity. Looks consistent.
1263-1263:splitLevelfield
Adding this property clarifies share configuration. Approved.
1270-1279:Subscriptionexpansions
Adding new fields (arraySubscription,displaySubscription, etc.) indicates broader real-time updates. Great feature if well tested.
1286-1288:Systemnode alignment
DefiningidwithScalars['ID']['output']keeps the Node interface consistent. Good.
1297-1298: Temperature doc
Exporting the temperature unit with a doc comment clarifies usage.
1303-1304:Themeenum
Introducing awhitetheme or referencing it; no issues.
1318-1336:UnraidArrayenhancements
Fields forboot,caches,disks,parities, state tracking, etc. Good expansion. Testing is crucial given the bigger API surface.
1438-1444: Added NTP servers
Exposing multiple NTP settings is helpful on theVarsobject. Confirm client usage.
1464-1464:regTyfor registration
Making the registration type available might be helpful in licensing flows.
1737-1737:serverStateQuery
Ensuring that partial queries correctly return the server state is useful. The reference toPartialCloudFragmentsuggests robust modular fetches.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
api/src/unraid-api/graph/resolvers/array/parity.model.ts (1)
1-34: LGTM: Well-structured GraphQL object type for parity checks.The
ParityCheckclass is well-defined with appropriate field types, nullability settings, and descriptive documentation. I particularly appreciate the use of proper GraphQL scalar types likeIntandGraphQLISODateTimeinstead of the previously mentioned problematicNumbertype.api/src/unraid-api/graph/resolvers/array/parity.resolver.ts (1)
37-43: Use a specific return type instead of GraphQLJSONReplace
GraphQLJSONwith a more specific return type that accurately represents the response structure.
🧹 Nitpick comments (14)
api/src/unraid-api/graph/resolvers/resolvers.module.ts (1)
5-8: Consider removing redundant imports for array-related servicesThe ArrayModule (line 5) already imports and exports ArrayMutationsResolver, ArrayResolver, and ArrayService. Since you're importing ArrayModule in the imports array (line 36), these individual imports (lines 6-8) appear to be redundant.
import { ArrayModule } from '@app/unraid-api/graph/resolvers/array/array.module.js'; -import { ArrayMutationsResolver } from '@app/unraid-api/graph/resolvers/array/array.mutations.resolver.js'; -import { ArrayResolver } from '@app/unraid-api/graph/resolvers/array/array.resolver.js'; -import { ArrayService } from '@app/unraid-api/graph/resolvers/array/array.service.js';api/src/unraid-api/graph/resolvers/array/parity.service.ts (3)
8-8: Remove unnecessary constructorThe empty constructor doesn't provide any functionality and can be removed.
@Injectable() export class ParityService { - constructor() {} async getParityHistory(): Promise<ParityCheck[]> {🧰 Tools
🪛 Biome (1.9.4)
[error] 8-8: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
10-30: Method implementation looks good with some potential enhancementsThe
getParityHistorymethod properly retrieves and processes the parity check history file. However, consider adding error handling for potential parsing issues:return lines.map<ParityCheck>((line) => { const [date, duration, speed, status, errors = '0'] = line.split('|'); return { - date: new Date(date), + date: new Date(date || Date.now()), - duration: Number.parseInt(duration, 10), + duration: Number.parseInt(duration, 10) || 0, speed: speed ?? 'Unavailable', status: status === '-4' ? 'Cancelled' : 'OK', - errors: Number.parseInt(errors, 10), + errors: Number.parseInt(errors, 10) || 0, }; });This ensures that invalid values won't cause runtime errors when parsing.
19-29: Consider adding validation for empty file or invalid entriesThe current implementation assumes the file always contains valid, properly formatted data. Adding validation would make the code more robust:
// Convert checks into array of objects const lines = history.toString().trim().split('\n').reverse(); + // Filter out empty lines or malformed entries + const validLines = lines.filter(line => line.trim() && line.includes('|')); - return lines.map<ParityCheck>((line) => { + return validLines.map<ParityCheck>((line) => { const [date, duration, speed, status, errors = '0'] = line.split('|'); return { date: new Date(date), duration: Number.parseInt(duration, 10), speed: speed ?? 'Unavailable', status: status === '-4' ? 'Cancelled' : 'OK', errors: Number.parseInt(errors, 10), }; });api/src/unraid-api/graph/resolvers/array/parity.resolver.ts (1)
28-30: Consider removing redundant awaitThe
awaitkeyword is redundant when immediately returning a Promise:@Query(() => [ParityCheck]) async parityHistory(): Promise<ParityCheck[]> { - return await this.parityService.getParityHistory(); + return this.parityService.getParityHistory(); }api/generated-schema-new.graphql (9)
1-4: Auto-generated File HeaderThe header clearly indicates that this file is automatically generated. Please ensure that manual modifications are avoided here, as changes may be overwritten during regeneration.
10-14: MinigraphqlResponse StructureThe type includes fields for status, timeout, and error. Consider whether the error field should be non-null in cases of failure.
30-34: RelayResponse DefinitionThe fields (status, timeout, error) are clear. Consider standardizing the type for timeout if it’s used in numerical comparisons elsewhere.
45-54: Capacity Type DetailsThe inline comments (Free, Used, Total) enhance readability. Confirm that the string type is the best match for capacity values.
64-131: ArrayDisk DefinitionThis type contains extensive details about a disk (identifier, slot numbering, size, status, I/O statistics, filesystem details, etc.).
- The disk slot numbering in the comments is well documented.
- Consider if fields like
numReads,numWrites, andnumErrorsare best modeled as numeric (e.g., Int) rather than Float.
Overall, the model is comprehensive.
533-560: RegistrationState EnumMultiple registration states are defined. Consider reviewing entries like "EEXPIRED" for potential typos or nonstandard naming.
1242-1256: Notification TypeIncludes text, importance, link, and timestamp fields. Consider using a DateTime scalar for the timestamp to improve type safety.
1277-1282: NotificationFilter InputProvides filtering options with offset and limit parameters. Consider adding default values where appropriate.
1317-1320: Uptime TypeProvides a timestamp field for uptime. Consider using the DateTime scalar for more consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (11)
api/.env.development(1 hunks)api/.env.test(1 hunks)api/generated-schema-new.graphql(1 hunks)api/src/core/modules/get-parity-history.ts(0 hunks)api/src/store/modules/paths.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/array.module.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/parity.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/parity.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/parity.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts(10 hunks)api/src/unraid-api/graph/resolvers/resolvers.module.ts(2 hunks)
💤 Files with no reviewable changes (1)
- api/src/core/modules/get-parity-history.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts
🧰 Additional context used
🧬 Code Graph Analysis (4)
api/src/unraid-api/graph/resolvers/array/parity.model.ts (1)
api/src/unraid-api/graph/resolvers/array/array.model.ts (5)
ObjectType(6-16)ObjectType(18-25)ObjectType(27-128)ObjectType(130-163)ObjectType(261-312)
api/src/unraid-api/graph/resolvers/array/array.module.ts (1)
api/src/unraid-api/graph/resolvers/resolvers.module.ts (1)
Module(35-64)
api/src/unraid-api/graph/resolvers/array/parity.service.ts (1)
api/src/store/index.ts (1)
getters(18-29)
api/src/unraid-api/graph/resolvers/resolvers.module.ts (6)
api/src/unraid-api/graph/resolvers/array/array.module.ts (1)
Module(9-14)api/src/unraid-api/graph/resolvers/connect/connect.module.ts (1)
Module(8-12)api/src/unraid-api/graph/resolvers/api-key/api-key.module.ts (1)
Module(7-11)api/src/unraid-api/graph/resolvers/disks/disks.module.ts (1)
Module(6-10)api/src/unraid-api/graph/resolvers/docker/docker.module.ts (1)
Module(8-20)api/src/unraid-api/graph/resolvers/logs/logs.module.ts (1)
Module(6-10)
🪛 Biome (1.9.4)
api/src/unraid-api/graph/resolvers/array/parity.service.ts
[error] 8-8: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (110)
api/src/store/modules/paths.ts (1)
13-15: Good implementation of environment variable override.The change adds flexibility by allowing the parity-checks log path to be configured via an environment variable, while maintaining backward compatibility with the default path. This follows the pattern used elsewhere in the file for other configurable paths.
api/src/unraid-api/graph/resolvers/resolvers.module.ts (1)
35-36: Module organization looks goodThe update to include ArrayModule and ConnectModule in the imports array follows good modular design principles, which will help with maintainability and organization.
api/.env.development (1)
12-12: LGTM: Environment variable for parity checks log path added.The new environment variable
PATHS_PARITY_CHECKSpointing to./dev/states/parity-checks.logaligns with the PR's focus on enhancing parity check functionality.api/.env.test (1)
12-12: LGTM: Consistent environment variable across configurations.The same
PATHS_PARITY_CHECKSenvironment variable is appropriately added to the test environment configuration, ensuring consistency with the development environment.api/src/unraid-api/graph/resolvers/array/array.module.ts (1)
1-14: LGTM: Clean module structure with appropriate imports and exports.The
ArrayModuleis properly configured following NestJS conventions. It correctly imports and exports all necessary services and resolvers related to array operations, including the newParityServiceandParityResolver.api/generated-schema-new.graphql (105)
5-8: ApiKeyResponse DefinitionThe type is straightforward with a required Boolean and an optional error string. Everything looks in order.
16-22: MinigraphStatus EnumThe enum values (PRE_INIT, CONNECTING, CONNECTED, PING_FAILURE, ERROR_RETRYING) are clearly defined. No issues here.
24-28: CloudResponse DefinitionWell defined with a required status, optional ip, and error. Ensure that resolver logic consistently returns a valid string for status.
36-43: Cloud Type AggregationThis type groups several response types and adds an allowedOrigins array. Verify that resolvers correctly merge these nested types.
56-62: ArrayCapacity StructureThe separation between kilobytes and disk counts is clear. Conversion consistency should be verified at the business logic level.
133-135: Node InterfaceA standard interface featuring an ID. This is consistent with best practices for unique identification.
137-147: ArrayDiskStatus EnumThe various status codes are clearly enumerated. Ensure that consuming logic is aligned with these precise status names.
149-154: ArrayDiskType EnumThe differentiation between DATA, PARITY, FLASH, and CACHE is clear.
156-166: ArrayDiskFsColor EnumThe color states (e.g., GREEN_ON, BLUE_BLINK) are intuitive; no concerns.
168-194: UnraidArray TypeThis complex type aggregates the array’s previous, pending, and current state along with capacity and associated disk arrays. Ensure that optional fields (like boot) are handled gracefully by client applications.
196-208: ArrayState EnumA comprehensive set of states including STARTED, STOPPED, NEW_ARRAY, etc. Looks complete.
210-215: ArrayPendingState EnumSimple and clear definition for pending operations.
217-264: Share Type DefinitionThe Share type includes properties for share names, sizes, inclusion/exclusion of disks, and caching. Documentation is helpful; ensure that Long is handled correctly in the resolvers.
266-267: Long Scalar DefinitionThe scalar represents 52-bit integers as intended. No issues.
269-278: RemoteAccess TypeClearly defines access type, forward type, and port. The mapping to WAN access methods should be validated in the implementation.
280-284: WAN_ACCESS_TYPE EnumThe values DYNAMIC, ALWAYS, and DISABLED are appropriately listed.
286-289: WAN_FORWARD_TYPE EnumDefines the forwarding methods (UPNP and STATIC) clearly.
291-300: DynamicRemoteAccessStatus TypeIncludes fields for enabled type, running type, and an error message. The structure supports dynamic configuration well.
302-306: DynamicRemoteAccessType EnumThe values (STATIC, UPNP, DISABLED) are consistent with the related types.
308-328: ConnectSettingsValues TypeThis captures settings for the GraphQL sandbox use and remote access details. All inline comments are clear.
330-342: ConnectSettings TypeDefines the settings with dataSchema and uiSchema in JSON format along with the actual values. Good separation of concerns.
344-347: JSON ScalarThe JSON scalar is defined with a reference to the ECMA-404 specification. This aligns with standard practices.
349-358: Connect TypeProvides a container for Connect instances with dynamicRemoteAccess and settings. Looks sound.
360-365: AccessUrl TypeDefines a URL type with a name and both IPv4 and IPv6 addresses. Ensure that the URL scalar is correctly validated.
367-374: URL_TYPE EnumThe enum values are clear and cover common network types.
376-379: URL Scalar SpecificationThe scalar is defined per RFC3986, ensuring that URL values follow standard formatting.
381-384: Network TypeA simple type with an id and a list of accessUrls; straightforward.
386-391: ProfileModel TypeCaptures essential user profile details. Clear definition.
393-403: Server TypeIncludes server identity, owner, apikey, name, status, and various IP addresses. Ensure proper validation for IP address formats.
405-409: ServerStatus EnumValues (ONLINE, OFFLINE, NEVER_CONNECTED) provide straightforward status reporting.
411-420: DiskPartition TypeProvides partition details including name, filesystem type, and size. Adequate documentation.
422-430: DiskFsType EnumLists common filesystem types. Clear and complete.
432-489: Disk Type DefinitionThis is a comprehensive type capturing disk properties, ranging from physical attributes to firmware and partitions. The information is detailed; just verify that the numeric fields are consistent (e.g., size, bytesPerSector, etc.).
491-498: DiskInterfaceType EnumDefines connection interfaces (SAS, SATA, USB, PCIE, UNKNOWN) clearly.
500-506: DiskSmartStatus EnumCurrently lists OK and UNKNOWN. You might consider extending it if future disk health statuses need to be captured.
508-511: KeyFile TypeA simple type for file location and contents; looks good.
513-520: Registration TypeCovers registration details such as guid, type, keyFile, state, and expiration. The documentation is sufficient.
522-531: registrationType EnumLists possible registration types clearly.
744-751: ConfigErrorState EnumDefines possible configuration error states clearly.
753-756: Permission TypeA simple type associating a resource with allowed actions.
758-788: Resource EnumLists various system resources. Ensure that naming conventions (like "CONNECT__REMOTE_ACCESS") remain consistent across the application.
790-797: ApiKey TypeCaptures API key metadata including roles, creation time, and permissions.
799-804: Role EnumDefines available roles (ADMIN, CONNECT, GUEST) succinctly.
806-814: ApiKeyWithSecret TypeThis type adds the secret key to the API key metadata. The design is sound.
816-836: ArrayMutations TypeLists mutations for managing arrays, including state setting, disk removal/addition, and mounting operations. Descriptions are clear; ensure pre-condition validations are implemented by resolvers.
838-842: ArrayStateInput TypeDefines the input for changing the array state with a clear inline comment.
843-846: ArrayStateInputState EnumProvides clear enum values (START, STOP) for state input.
848-854: ArrayDiskInput TypeDefines a simple input with disk id and slot number. Clear and concise.
856-862: DockerMutations TypeProvides operations for starting and stopping Docker containers. Adequately defined.
864-891: ParityCheck TypeCaptures various details of a parity check process, including progress, speed, and error count. Verify that numeric and boolean fields correctly mirror the parity check’s state.
893-897: DateTime ScalarThe scalar is designed to represent UTC date-time strings as per the ISO standard.
898-902: Config TypeDefines a minimal configuration object with its validity and error. This is standard.
904-913: InfoApps TypeProvides counts for installed and started docker containers. Simple and effective.
914-922: Baseboard TypeProvides basic board information. The structure is complete.
923-942: InfoCpu TypeDetails CPU information comprehensively. Ensure that all numeric fields (e.g., threads, cores) are validated as integers in the implementation.
944-952: Gpu TypeDefines the necessary GPU properties clearly.
954-964: Pci TypeCaptures standard PCI properties. Straightforward.
966-969: Usb TypeA simple definition for USB devices.
971-976: Devices TypeAggregates GPU, PCI, and USB collections; design is appropriate.
978-984: Case TypeRepresents case-specific details, including an icon, URL, error message, and base64 data. Looks good.
986-1008: Display TypeContains a broad range of display configuration options. Verify that each flag (e.g., scale, tabs, resize) is used consistently in the UI.
1010-1014: Theme EnumCurrently only includes "white". Consider expanding this if additional themes will be supported.
1015-1019: Temperature EnumDefines temperature units as C and F. This is clear.
1021-1029: MemoryLayout TypeProvides details for the physical layout of memory. The fields are concise.
1035-1048: InfoMemory TypeCaptures overall memory usage details and layout information. Data types and descriptions are appropriate.
1050-1064: Os TypeProvides OS-specific details. The structure is simple and clear.
1066-1074: System TypeCovers system-level identification data. No issues noted.
1076-1104: Versions TypeGathers extensive version details for various software components. The thoroughness is appreciated.
1106-1124: Info TypeAggregates multiple system information subtypes. This consolidated structure is effective.
1125-1130: ContainerPort TypeDefines port mappings for containers clearly.
1132-1135: ContainerPortType EnumDistinguishes between TCP and UDP. Standard definition.
1137-1139: ContainerHostConfig TypeA simple type that defines the network mode. Looks good.
1141-1159: DockerContainer TypeProvides a detailed description of container attributes including ports, state, and configuration. The documentation is comprehensive.
1161-1165: JSONObject ScalarThe scalar for JSON objects follows the ECMA-404 specification. No issues found.
1166-1169: ContainerState EnumDefines basic container states (RUNNING, EXITED). Sufficient for most use cases.
1171-1187: DockerNetwork TypeCaptures various Docker network attributes, including id, scope, driver, and configuration details. This type is detailed and clear.
1189-1194: Docker TypeAggregates containers and networks into a single type. This design is standard.
1195-1201: Flash TypeRepresents flash storage details adequately.
1202-1214: LogFile TypeProvides attributes for log file metadata, including file path and modification time. No concerns.
1216-1228: LogFileContent TypeCaptures the content of a log file along with pagination details such as the starting line and total line count.
1230-1235: NotificationCounts TypeDefines counts for different notification levels succinctly.
1237-1240: NotificationOverview TypeAggregates unread and archived notification counts. Clear and effective.
1258-1262: NotificationImportance EnumEnumerates the levels (ALERT, INFO, WARNING) appropriately.
1264-1267: NotificationType EnumDefines notification types as UNREAD and ARCHIVE. This is clear.
1269-1276: Notifications TypeContains an overview and supports filtering via a filter input. Ensure that filtering defaults are handled gracefully.
1284-1288: Owner TypeA simple and effective structure for owner details.
1290-1298: VmDomain TypeProvides basic VM domain information including UUID, name, and state.
1300-1309: VmState EnumCovers a comprehensive set of VM states, from NOSTATE through CRASHED and PMSUSPENDED.
1312-1315: Vms TypeAggregates VM domains. Straightforward structure.
1321-1327: Service TypeCaptures service-related data such as online status and versioning.
1329-1344: UserAccount TypeDefines user account information with details on roles and permissions. Looks good.
1345-1378: Query TypeThis is the main entry point defining various queries (e.g., apiKeys, cloud, info, notifications, disks). Verify that all field names and types match the backend models.
1379-1437: Mutation TypeA comprehensive set of mutations covering API key management, notification handling, array and Docker operations, VM controls, and remote access configuration. The descriptions are clear; ensure corresponding resolvers manage pre-conditions and error handling.
1439-1449: CreateApiKeyInput InputDefines inputs for creating an API key and includes an overwrite flag with a helpful inline comment.
1451-1454: AddPermissionInput InputA straightforward input for associating permissions with a resource.
1456-1459: AddRoleForApiKeyInput InputDefines the input for adding a role to an API key.
1461-1464: RemoveRoleFromApiKeyInput InputMirrors the add role input to remove a role. Clear and simple.
1466-1472: NotificationData InputProvides the necessary fields for creating a notification. Good structure.
1474-1496: ApiSettingsInput InputDefines settings for the API including toggling sandbox mode, allowed origins, and remote access configurations. The inline comments are useful.
1498-1513: ConnectSignInInput InputContains fields for sign-in credentials such as API key and various tokens. Looks robust.
1515-1524: ConnectUserInfoInput InputDefines user information including preferred username, email, and avatar URL. Clear structure.
1526-1537: SetupRemoteAccessInput InputSpecifies inputs for setting up remote access including accessType, forwardType, and port. This is detailed and clear.
1539-1542: AllowedOriginInput InputA simple input to handle a list of allowed origins.
1544-1550: EnableDynamicRemoteAccessInput InputProvides the necessary fields for enabling or disabling dynamic remote access, including a URL and a boolean flag.
1552-1563: Subscription TypeLists various subscriptions for real-time updates across multiple data types. This is a good complement to the query/mutation operations.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
api/src/unraid-api/graph/resolvers/array/parity.resolver.ts (1)
37-43: Use a more specific return type instead ofobject.
GraphQL resolvers benefit from strongly-typed responses, which also align with best practices (e.g., returningParityCheckor a dedicatedParityCheckStatustype). ReturningPromise<object>is too generic and can make your schema less descriptive.Below is an example of how to replace this with a more explicit return type. You can define or reuse a strongly typed class (e.g.,
ParityCheckStatus) rather than returning generic JSON data:- @Mutation(() => GraphQLJSON) - async startParityCheck(@Args('correct') correct: boolean): Promise<object> { + @Mutation(() => ParityCheckStatus) + async startParityCheck(@Args('correct') correct: boolean): Promise<ParityCheckStatus> { return this.arrayService.updateParityCheck({ wantedState: 'start', correct, }); }Also applies to: 50-56, 63-69, 76-82
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
api/src/core/modules/index.ts(0 hunks)api/src/unraid-api/graph/resolvers/array/parity.resolver.ts(1 hunks)
💤 Files with no reviewable changes (1)
- api/src/core/modules/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (1)
api/src/unraid-api/graph/resolvers/array/parity.resolver.ts (1)
84-92: Subscription permission checks look consistent.
Your subscription is protected by@UsePermissions({ action: AuthActionVerb.READ, resource: Resource.ARRAY }), ensuring only authorized users can receive updates.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts (1)
78-85: Asynchronous remote access stop operation is properly awaited.The change now correctly awaits the asynchronous stopRemoteAccess operation, addressing the issue highlighted in a previous review.
api/generated-schema-new.graphql (1)
562-743: Refactor Vars Type for Maintainability
TheVarstype aggregates a large number of configuration details. Although it is exhaustive, its size may impact long-term maintainability. Consider decomposing it into smaller, domain-specific types.
🧹 Nitpick comments (8)
api/src/unraid-api/graph/resolvers/array/array.model.ts (4)
27-128: The ArrayDisk implementation is comprehensive and well-documented.The ArrayDisk class implements the Node interface correctly and provides a complete representation of disk properties with appropriate types and descriptions. The use of GraphQLLong for large numeric values like size and counters is appropriate.
One suggestion to consider:
Consider adding a few class-level JSDoc comments to provide overall context about what an ArrayDisk represents in the system and how it relates to the UnraidArray.
47-47: Consider using GraphQLLong for the return type as well.While you're using GraphQLLong for the GraphQL schema type, the TypeScript return type is still
number | null, which might not fully represent the large values you're working with.- size?: number | null; + size?: bigint | null; // or an appropriate TypeScript type that can handle large values
1-312: Consider organizing the file into smaller modules.This file contains multiple class and enum definitions, making it quite large. For better maintainability and separation of concerns, consider splitting it into smaller files.
You could organize it into:
capacity.model.ts- For Capacity and ArrayCapacity classesarray-disk.model.ts- For ArrayDisk class and related enumsunraid-array.model.ts- For UnraidArray class and related input typesshare.model.ts- For Share classThis would make the codebase more modular and easier to navigate.
165-172: Consider adding validation for the ArrayDiskInput.The input type doesn't include any validation logic. While this might be handled elsewhere, adding validation decorators (if using class-validator) would improve type safety.
@InputType() export class ArrayDiskInput { @Field(() => ID, { description: 'Disk ID' }) + @IsNotEmpty() id!: string; @Field(() => Int, { nullable: true, description: 'The slot for the disk' }) + @IsOptional() + @IsInt() + @Min(0) slot?: number; }You would need to add the appropriate imports from class-validator and ensure the validation pipe is set up in your NestJS module.
api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts (2)
45-51: API restart method implemented but lacks return value.The restart method successfully uses execa to restart the API, but doesn't return any value to indicate success or failure to the caller.
Consider returning a boolean to indicate whether the restart was successful:
- async restartApi() { + async restartApi(): Promise<boolean> { try { await execa('unraid-api', ['restart'], { shell: 'bash', stdio: 'ignore' }); + return true; } catch (error) { this.logger.error(error); + return false; } }
53-56: ExtraAllowedOrigins method can be simplified.The method has a slightly redundant assignment that could be simplified.
public async extraAllowedOrigins(): Promise<Array<string>> { - const extraOrigins = getExtraOrigins(); - return extraOrigins; + return getExtraOrigins(); }api/generated-schema-new.graphql (2)
1009-1019: Theme and Temperature Enums
TheThemeenum currently uses a lowercase value (white). Consider using uppercase enum values to align with common GraphQL naming conventions.
1346-1438: GraphQL Query and Mutation Types
TheQueryandMutationtypes are extensive and cover a wide range of system operations. While comprehensive, consider modularizing these operations into sub-groupings in the future for improved maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (4)
api/generated-schema-new.graphql(1 hunks)api/src/unraid-api/graph/resolvers/array/array.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts(3 hunks)api/src/unraid-api/graph/resolvers/vms/vms.model.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts (4)
api/src/common/allowed-origins.ts (1)
getExtraOrigins(64-74)web/composables/gql/graphql.ts (3)
EnableDynamicRemoteAccessInput(606-611)ConnectSignInInput(362-373)RemoteAccess(1142-1150)api/src/store/modules/dynamic-remote-access.ts (1)
setAllowedRemoteAccessUrl(54-63)api/src/store/modules/config.ts (1)
loginUser(62-76)
api/src/unraid-api/graph/resolvers/vms/vms.model.ts (1)
web/composables/gql/graphql.ts (2)
VmDomain(1558-1565)Vms(1579-1583)
api/src/unraid-api/graph/resolvers/array/array.model.ts (2)
web/composables/gql/graphql.ts (7)
Capacity(272-280)ArrayCapacity(94-100)Node(923-925)ArrayDisk(102-145)UnraidArray(1317-1336)ArrayDiskInput(159-164)ArrayStateInput(252-255)api/src/graphql/resolvers/graphql-type-long.ts (1)
GraphQLLong(29-35)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (38)
api/src/unraid-api/graph/resolvers/vms/vms.model.ts (1)
44-46: Placeholder return values in mutation methodsAll mutation methods simply return
truewithout any actual implementation. This suggests these are placeholder implementations that need to be replaced with actual logic.Are these methods intentionally left as placeholders for implementation in a separate resolver file? If so, it might be helpful to add a TODO comment to indicate that the actual implementation lives elsewhere.
Also applies to: 49-51, 54-56, 59-61, 64-66, 69-71, 74-76
api/src/unraid-api/graph/resolvers/array/array.model.ts (7)
1-5: The import structure looks clean and well-organized.Clear separation between external library imports and internal app imports. The GraphQLLong import will be useful for handling the large numeric values used in storage systems.
6-16: Good implementation of the Capacity class.The class has clear field definitions with appropriate descriptions. All fields are properly marked as non-nullable, ensuring type safety.
18-25: The ArrayCapacity class is well-structured.Good separation of kilobytes and disk count capacities, with descriptive field decorators.
130-163: The UnraidArray class is well-structured.Good implementation of the Node interface with appropriate field relationships to ArrayDisk objects and clear descriptions for all properties.
165-178: Input types are properly defined.Both ArrayDiskInput and ArrayStateInput have appropriate field decorators with clear descriptions.
180-259: Enums are well-defined with proper registration.All enums are clearly defined with meaningful values and are properly registered for GraphQL with the registerEnumType function.
261-312: The Share class is well-implemented.Proper implementation of the Node interface with comprehensive properties and clear descriptions.
api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts (7)
1-7: Improved imports with support for new functionalities.The addition of the Logger, execa, and decodeJwt imports supports the new functionality for API restart capabilities and authentication token handling.
8-16: Import organization enhanced with updated model paths.The imports now reference explicit model paths from the connect.model.js, improving the organization and maintainability of the code.
17-35: Additional imports to support enhanced functionality.The new imports facilitate the extra allowed origins functionality, remote access setup, and user configuration updates.
41-43: Constructor and logger initialization properly implemented.The service now injects ApiKeyService and initializes a logger, following NestJS best practices.
65-76: Verify proper error handling for dynamic remote access.The error handling for dynamic remote access uses GraphQLError with the correct extensions code.
101-108: Remote access extension handling is correct.The extendRemoteAccess method is correctly called without await as it's synchronous.
282-282: Method visibility changed from private to public.The dynamicRemoteAccessSettings method was changed from private to public, allowing it to be accessed by other services or resolvers.
api/generated-schema-new.graphql (23)
1-4: Auto-Generated File Notice
This header clearly indicates that the file is automatically generated and should not be manually modified.
5-43: Basic API Response and Enum Types
The definitions forApiKeyResponse,MinigraphqlResponse, theMinigraphStatusenum,CloudResponse,RelayResponse, and theCloudtype are clear and consistent.
45-62: Capacity Related Types
TheCapacityandArrayCapacitytypes are well-documented and straightforward, clearly conveying their intended purpose.
64-197: Disk and Array Types
TheArrayDisktype—with its detailed documentation—and the accompanying definitions (theNodeinterface, scalarLong, and the enumsArrayDiskStatus,ArrayDiskType, andArrayDiskFsColor) provide a comprehensive model for disk configurations.
199-267: Array State and Share Types
The enumsArrayStateandArrayPendingStatealong with theSharetype are well-defined. Their descriptions make the intended behavior and constraints clear.
269-358: Remote Access and Connection Types
TheRemoteAccesstype, WAN-related enums,DynamicRemoteAccessStatus/DynamicRemoteAccessType, and theConnectSettingsValues,ConnectSettings, andConnecttypes are implemented effectively to handle remote access and connection configurations.
360-384: URL and Network Types
TheAccessUrltype,URL_TYPEenum, the scalarURL, and theNetworktype are clear and align with expected standards for URL handling and network configuration.
386-409: Profile and Server Definitions
TheProfileModelandServertypes (along with theServerStatusenum) are straightforward and appropriately capture server and user profile details.
411-506: Disk Partition and Disk Types
TheDiskPartitionandDisktypes—including the enumsDiskFsType,DiskInterfaceType, andDiskSmartStatus—are detailed and come with clear documentation, ensuring an accurate representation of disk properties.
508-560: Registration and Key File Types
TheKeyFile,Registration,registrationType, andRegistrationStatetypes provide all necessary details for system registration mechanisms.
744-757: Config Error and Permission Types
TheConfigErrorStateenum and thePermissiontype are simple yet effective, capturing error states and access permissions as required.
758-836: API Key and Array Mutation Types
The definitions forResource,ApiKey,Role,ApiKeyWithSecret, andArrayMutationsprovide a clear and comprehensive model for API key management and array operations.
838-891: Input Types for Array and Docker Operations
The input types—ArrayStateInput,ArrayStateInputState,ArrayDiskInput,DockerMutations, andParityCheck—are concise and well-documented, supporting the necessary operations.
893-922: Scalars and Config Information
TheDateTimescalar along with types such asConfig,InfoApps, andBaseboardset the stage for accurate system reporting.
923-1008: Hardware Information Types
TheInfoCpu,Gpu,Pci,Usb,Devices,Case, andDisplaytypes provide a detailed view of the system hardware. The documentation is thorough and clear.
1021-1124: Memory and System Information
The typesMemoryLayout,InfoMemory,Os,System,Versions, andInfoeffectively cover a broad range of system details.
1125-1194: Container and Docker Types
The definitions forContainerPort,ContainerPortType,ContainerHostConfig,DockerContainer,JSONObject,ContainerState,DockerNetwork, andDockeroffer a comprehensive view of container configurations and operations.
1195-1228: Flash and Log File Types
TheFlash,LogFile, andLogFileContenttypes are clearly defined, facilitating effective logging and file management operations.
1229-1282: Notification Structures
TheNotificationCounts,NotificationOverview,Notificationtypes, and the related enums and filters are well-defined, supporting a robust notification system.
1284-1320: User and Virtual Machine Types
The typesOwner,VmDomain,VmState,Vms, andUptimeprovide essential details for managing user accounts and virtual machines.
1321-1344: Service and User Account Definitions
TheServiceandUserAccounttypes capture the necessary system and user information in a clear manner.
1439-1550: Authentication and Settings Input Types
The input types for API key creation, role management, notification data, API settings, and remote access (includingCreateApiKeyInput,AddPermissionInput,AddRoleForApiKeyInput,RemoveRoleFromApiKeyInput,NotificationData,ApiSettingsInput,ConnectSignInInput,ConnectUserInfoInput,SetupRemoteAccessInput,AllowedOriginInput, andEnableDynamicRemoteAccessInput) are clearly defined and offer the necessary flexibility.
1552-1563: Subscriptions Type
TheSubscriptiontype supports real-time updates for various system components and is well structured.
api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
api/src/unraid-api/graph/resolvers/connect/connect.model.ts (1)
157-160: Address port validation mismatch.
@IsPort()fromclass-validatorexpects a string, but these fields are declared as integers (@Field(() => Int)). This could cause runtime validation issues. If you want to accept numeric ports in GraphQL, consider using numeric validators like@IsInt()together with@Min(1)and@Max(65535)for valid port ranges. For example:--- a/api/src/unraid-api/graph/resolvers/connect/connect.model.ts +++ b/api/src/unraid-api/graph/resolvers/connect/connect.model.ts @@ -156,3 +156,6 @@ @Field(() => Int, { nullable: true, description: 'The port used for Remote Access' }) - @IsPort() + @IsInt() + @Min(1) + @Max(65535) @IsOptional() @@ -180,3 +183,6 @@ @Field(() => Int, { - @IsPort() + @IsInt() + @Min(1) + @Max(65535) @IsOptional() @@ -247,3 +250,6 @@ @Field(() => Int, { nullable: true, description: 'The port used for Remote Access' }) - @IsPort() + @IsInt() + @Min(1) + @Max(65535) @IsOptional() @@ -298,3 +301,6 @@ @Field(() => Int, { - @IsPort() + @IsInt() + @Min(1) + @Max(65535) @IsOptional()Also applies to: 181-184, 248-251, 299-302
api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts (1)
65-101: 🛠️ Refactor suggestionURL validation needed when beginning remote access.
The
enableDynamicRemoteAccessmethod correctly checks ifinput.urlexists before using it, but there's no validation wheninput.urlis required forbeginRemoteAccess. If remote access is disabled and needs to be started, a URL is necessary.} else if (controller.getRunningRemoteAccessType() === DynamicRemoteAccessType.DISABLED) { + if (!input.url) { + throw new GraphQLError('URL is required to begin remote access.', { + extensions: { code: 'BAD_REQUEST' }, + }); + } if (input.url) { store.dispatch(setAllowedRemoteAccessUrl(input.url)); } await controller.beginRemoteAccess({
🧹 Nitpick comments (3)
api/src/unraid-api/graph/resolvers/connect/connect.model.ts (1)
310-348: Consider adding unit tests for these new classes.These classes define core domain objects but appear untested. Adding test coverage would help ensure correctness and protect against future regressions.
Would you like help creating a test suite for these classes or opening a new task to track the test coverage?
api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts (2)
53-56: Consider adding URL validation to extraAllowedOrigins.The method returns origins without additional validation, although the underlying
getExtraOriginsfunction does filter for URLs that start with http:// or https://.public async extraAllowedOrigins(): Promise<Array<string>> { const extraOrigins = getExtraOrigins(); + // Origins are already filtered in getExtraOrigins for http/https protocols return extraOrigins; }
87-89: Redundant condition check for input.url.The
if (input.url)check is redundant as any code inside this block will only execute if the condition is true.- if (input.url) { - store.dispatch(setAllowedRemoteAccessUrl(input.url)); - } + store.dispatch(setAllowedRemoteAccessUrl(input.url));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (3)
api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts(3 hunks)api/src/unraid-api/graph/resolvers/connect/connect.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/vms/vms.service.ts(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/graph/resolvers/vms/vms.service.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts (4)
api/src/common/allowed-origins.ts (1)
getExtraOrigins(64-74)web/composables/gql/graphql.ts (3)
EnableDynamicRemoteAccessInput(606-611)ConnectSignInInput(362-373)RemoteAccess(1174-1182)api/src/store/modules/dynamic-remote-access.ts (1)
setAllowedRemoteAccessUrl(54-63)api/src/store/modules/config.ts (1)
loginUser(62-76)
api/src/unraid-api/graph/resolvers/connect/connect.model.ts (2)
api/src/unraid-api/graph/resolvers/api-key/api-key.model.ts (4)
InputType(73-84)InputType(86-118)InputType(120-129)InputType(131-140)api/src/graphql/generated/client/graphql.ts (1)
AccessUrlInput(41-46)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (5)
api/src/unraid-api/graph/resolvers/connect/connect.model.ts (1)
21-25: Great approach using enumerations for clarity.Defining your ENUM types and registering them for GraphQL is a neat solution to maintain well-typed fields and intuitive domain semantics.
Also applies to: 27-30, 32-36, 350-357
api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts (4)
45-51: LGTM: API restart implementation.Good implementation of the API restart functionality using execa with proper error handling.
178-191: LGTM: Well-implemented API key handling.Good implementation of the API key retrieval/creation logic with proper error handling using GraphQLError.
280-294: LGTM: Public exposure of dynamicRemoteAccessSettings.Good change making the method public to allow access from other components in the system.
193-232:❓ Verification inconclusive
Consider adding JWT verification in signIn method.
While the method correctly decodes the JWT to extract user info, it doesn't verify the JWT signature, which could be a security concern.
🏁 Script executed:
#!/bin/bash # Check if the codebase has JWT verification elsewhere before the signIn method is called # Look for JWT verification in connect-related files rg -A 3 -B 3 "verify(.*JWT|.*Token|.*IdToken)" --glob "**/*.ts" # Check if there's any verification in the auth flow rg -A 3 -B 3 "verify(.*JWT|.*Token|.*IdToken)" --glob "**/auth/**/*.ts"Length of output: 139
Security Advisory: JWT Signature Verification Missing
After a preliminary search, there’s no evidence that JWT signature verification is performed before or within the
signInmethod. The code decodes the JWT to extract user information without validating its signature, which could expose the system to token spoofing attacks.
- File:
api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts(Lines 193-232)- Recommendation:
- Add JWT signature verification logic in the
signInmethod to ensure the token’s authenticity, or confirm that such verification is handled reliably at an earlier stage in the authentication flow.- If JWT signature verification is implemented elsewhere, please add inline comments or documentation to clarify this dependency.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
api/src/unraid-api/graph/resolvers/connect/connect.model.ts (1)
174-176: Validation mismatch:@IsPort()on integer-based fields.
These lines repeat a past concern that@IsPort()expects a string. Numbers won't pass validation as intended. Switch to numeric validators such as@IsInt(),@Min(1), and@Max(65535)if you want integer-based ports.- @IsPort() + import { IsInt, Min, Max } from 'class-validator'; @IsOptional() - port?: number | null; + @IsInt() + @Min(1) + @Max(65535) + port?: number | null;Also applies to: 198-201, 264-267, 315-318
🧹 Nitpick comments (2)
api/src/unraid-api/graph/resolvers/connect/connect-settings.resolver.ts (2)
84-84: Avoid logging sensitive information.
this.logger.verbose(...)might expose sensitive data ifsettingscontains secrets. Consider sanitizing or removing them before logging at a verbose level.- this.logger.verbose(`Attempting to update API settings: ${JSON.stringify(settings, null, 2)}`); + const sanitizedSettings = { ...settings, apiKey: '***' }; + this.logger.verbose(`Attempting to update API settings: ${JSON.stringify(sanitizedSettings, null, 2)}`);
152-152: Use the unified logger instead ofconsole.log().
For consistency, replaceconsole.logwiththis.logger.logor another logging method to centralize log management.- console.log('enableDynamicRemoteAccess', dynamicRemoteAccessInput); + this.logger.log(`enableDynamicRemoteAccess %j`, dynamicRemoteAccessInput);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (3)
api/dev/Unraid.net/myservers.cfg(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect-settings.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect.model.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- api/dev/Unraid.net/myservers.cfg
🧰 Additional context used
🧬 Code Graph Analysis (2)
api/src/unraid-api/graph/resolvers/connect/connect.model.ts (5)
api/src/unraid-api/graph/resolvers/array/array.model.ts (7)
InputType(165-172)InputType(174-178)ObjectType(6-16)ObjectType(18-25)ObjectType(27-128)ObjectType(130-163)ObjectType(261-312)api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts (2)
InputType(25-48)InputType(50-76)api/src/unraid-api/graph/resolvers/api-key/api-key.model.ts (4)
InputType(73-84)InputType(86-118)InputType(120-129)InputType(131-140)api/src/graphql/generated/client/graphql.ts (1)
AccessUrlInput(41-46)api/src/unraid-api/graph/resolvers/docker/docker.model.ts (6)
ObjectType(17-30)ObjectType(41-45)ObjectType(47-72)ObjectType(74-120)ObjectType(122-168)ObjectType(170-182)
api/src/unraid-api/graph/resolvers/connect/connect-settings.resolver.ts (6)
web/composables/gql/graphql.ts (5)
ConnectSettings(334-344)ConnectSettingsValues(346-360)RemoteAccess(1174-1182)SetupRemoteAccessInput(1256-1263)EnableDynamicRemoteAccessInput(606-611)api/src/unraid-api/types/json-forms.ts (1)
DataSlice(16-16)api/src/store/modules/config.ts (1)
logoutUser(78-101)api/src/store/actions/setup-remote-access.ts (1)
setupRemoteAccessThunk(26-50)api/src/unraid-api/graph/resolvers/connect/connect-settings.service.ts (1)
updateAllowedOrigins(173-176)api/src/common/allowed-origins.ts (1)
getAllowedOrigins(91-99)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
api/src/unraid-api/graph/resolvers/connect/connect-settings.resolver.ts (2)
1-3: Imports look well-structured.
No immediate issues spotted.
114-115: Propagate potential errors from the store dispatch.
await store.dispatch(...)might fail at runtime. Consider wrapping these calls in try/catch to handle errors gracefully and provide user-friendly feedback or fallback behaviors.Also applies to: 138-139
api/src/unraid-api/graph/resolvers/connect/connect.model.ts (1)
1-61: Enumerations and decorators are well-organized.
Great job defining and registering these enums for GraphQL usage. The use of decorators for validation and optional fields is consistent with NestJS best practices.
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced API capabilities with improved GraphQL interfaces for remote access, parity checks, notifications, and virtual machine controls. - Introduction of dynamic remote access settings and refined online status and service monitoring. - New `ParityCheckMutationsResolver` for managing parity check operations through GraphQL. - **Refactor** - Consolidated and renamed internal types and schema definitions to improve consistency and performance. - Removed deprecated legacy schemas to streamline the API. - Updated import paths for various types to reflect new module structures. - **Chore** - Updated environment configurations and test setups to support the new logging and configuration mechanisms. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
🤖 I have created a release *beep* *boop* --- ## [4.7.0](v4.6.6...v4.7.0) (2025-04-24) ### Features * add basic docker network listing ([#1317](#1317)) ([c4fdff8](c4fdff8)) * add permission documentation by using a custom decorator ([#1355](#1355)) ([45ecab6](45ecab6)) * basic vm controls ([#1293](#1293)) ([bc3ca92](bc3ca92)) * code first graphql ([#1347](#1347)) ([f5724ab](f5724ab)) ### Bug Fixes * container names always null ([#1335](#1335)) ([8a5b238](8a5b238)) * **deps:** update all non-major dependencies ([#1337](#1337)) ([2345732](2345732)) * hide reboot notice for patch releases ([#1341](#1341)) ([4b57439](4b57439)) * move docker mutations to the mutations resolver ([#1333](#1333)) ([1bbe7d2](1bbe7d2)) * PR build issue ([457d338](457d338)) * remove some unused fields from the report object ([#1342](#1342)) ([cd323ac](cd323ac)) * sso unreliable if API outputs more than raw json ([#1353](#1353)) ([e65775f](e65775f)) * vms now can detect starting of libvirt and start local hypervisor ([#1356](#1356)) ([ad0f4c8](ad0f4c8)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
3 participants
Summary by CodeRabbit
New Features
ParityCheckMutationsResolverfor managing parity check operations through GraphQL.Refactor
Chore