feat: info resolver cleanup #1425
Conversation
WalkthroughThis update introduces a modular service-based architecture for system information and device queries in the API. It adds dedicated service and resolver classes for both general system info and device info, removes the old monolithic resolver, and provides comprehensive unit tests for the new services and resolvers. A new static configuration file is also included. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GraphQL API
participant InfoResolver
participant InfoService
participant DevicesResolver
participant DevicesService
Client->>GraphQL API: Query system info
GraphQL API->>InfoResolver: Resolve info fields
InfoResolver->>InfoService: generateApps/generateCpu/etc.
InfoService-->>InfoResolver: Info data
InfoResolver-->>GraphQL API: Info response
Client->>GraphQL API: Query device info (gpu/pci/usb)
GraphQL API->>DevicesResolver: Resolve device fields
DevicesResolver->>DevicesService: generateGpu/generatePci/generateUsb
DevicesService-->>DevicesResolver: Device data
DevicesResolver-->>GraphQL API: Device response
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
elibosley
force-pushed
the
feat/info-resolver-cleanup
branch
from
5616120 to
d1a3c59
Compare
elibosley
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
api/dev/configs/connect.json (1)
5-15: Load secrets via environment or secure vault.Storing API keys and tokens in a committed JSON can expose secrets. Consider loading these values from environment variables or a secure vault, and reserve this file for non-sensitive defaults only.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
api/dev/configs/connect.json(1 hunks)api/src/graphql/resolvers/query/info.ts(0 hunks)api/src/unraid-api/graph/resolvers/info/devices.resolver.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/devices.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/devices.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/devices.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.resolver.ts(5 hunks)api/src/unraid-api/graph/resolvers/info/info.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/resolvers.module.ts(2 hunks)
💤 Files with no reviewable changes (1)
- api/src/graphql/resolvers/query/info.ts
🧰 Additional context used
🪛 Biome (1.9.4)
api/src/unraid-api/graph/resolvers/info/info.service.ts
[error] 78-78: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build Web App
- GitHub Check: Test API
- GitHub Check: Build API
🔇 Additional comments (11)
api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts (1)
7-7: Test setup correctly updated for dependency injection.The addition of
InfoServiceimport and provider aligns with the refactoring ofInfoResolverto use dependency injection. The test setup follows NestJS patterns correctly.Also applies to: 14-14
api/src/unraid-api/graph/resolvers/resolvers.module.ts (1)
14-17: Module configuration properly updated for new architecture.The addition of
DevicesResolver,DevicesService, andInfoServiceto both imports and providers correctly enables dependency injection for the new modular architecture.Also applies to: 51-52, 56-56
api/src/unraid-api/graph/resolvers/info/devices.resolver.ts (1)
1-25: Clean resolver implementation following NestJS patterns.The
DevicesResolverproperly delegates toDevicesServiceusing dependency injection, maintaining clean separation of concerns. The GraphQL decorators and typing are correctly applied.api/src/unraid-api/graph/resolvers/info/devices.resolver.spec.ts (1)
1-102: Comprehensive test suite with proper mocking strategy.The test suite effectively verifies resolver behavior by mocking
DevicesServicemethods and ensuring proper delegation. The mock data structures are realistic and the test coverage is thorough.api/src/unraid-api/graph/resolvers/info/devices.service.spec.ts (3)
1-187: Excellent test suite with comprehensive mocking and error handling.The test suite thoroughly covers all service methods with proper mocking of external dependencies. The error handling tests ensure graceful degradation, and the mock data structures are realistic.
1-5: Flag inconsistency between PR title and actual changes.The PR title "fix: theme issues when sent from graph" doesn't match the actual changes, which introduce a modular service-based architecture for device and system information resolvers. Consider updating the PR title to accurately reflect the refactoring work.
Likely an incorrect or invalid review comment.
143-143: Let’s inspect both the spec and service implementation around theblacklistedmapping:#!/bin/bash # Show spec file (lines 1–200) sed -n '1,200p' api/src/unraid-api/graph/resolvers/info/devices.service.spec.ts echo "----- service implementation (lines 1–200) -----" sed -n '1,200p' api/src/unraid-api/graph/resolvers/info/devices.service.tsapi/src/unraid-api/graph/resolvers/info/info.resolver.ts (2)
25-30: LGTM! Clean dependency injection pattern.The refactoring to use InfoService via constructor injection follows NestJS best practices and improves testability.
49-104: Consistent delegation pattern implemented correctly.All resolver methods properly delegate to their corresponding InfoService methods, maintaining a clean separation between the GraphQL layer and business logic.
api/src/unraid-api/graph/resolvers/info/info.service.spec.ts (1)
1-372: Excellent test coverage with comprehensive scenarios.The test suite thoroughly covers all InfoService methods including error cases, with proper mocking of external dependencies.
api/src/unraid-api/graph/resolvers/info/info.service.ts (1)
160-168: Clarify the architectural separation for device data.The method returns empty arrays with a comment about DevicesResolver handling the actual data. This creates implicit coupling between services.
Consider either:
- Remove this method if DevicesResolver handles devices independently
- Have this method delegate to DevicesService for consistency with other methods
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
api/src/unraid-api/graph/resolvers/info/devices.service.ts (3)
99-99: Address the hardcoded path as suggested in previous review.The hardcoded path should be extracted to a constant as previously suggested.
133-133: Address the hardcoded path as suggested in previous review.The hardcoded path should be extracted to a constant as previously suggested.
132-204: The complex method still needs refactoring as suggested in previous review.The
getSystemUSBDevicesmethod continues to handle multiple responsibilities and would benefit from the previously suggested refactoring into smaller, focused methods.
🧹 Nitpick comments (1)
api/src/unraid-api/graph/resolvers/info/devices.service.ts (1)
101-108: Optimize async operations in map.The Promise.all with individual async operations in map could be inefficient for large device lists and might benefit from batching or limiting concurrent operations.
Consider using a utility like
p-limitfor controlling concurrency:import pLimit from 'p-limit'; const limit = pLimit(10); // Limit to 10 concurrent operations const filteredDevices = await Promise.all( devices.map(device => limit(async () => { const exists = await access(`${basePath}${device.id}/iommu_group/`) .then(() => true) .catch(() => false); return exists ? device : null; }) ) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
api/src/unraid-api/graph/resolvers/info/devices.service.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Build Web App
- GitHub Check: Build Unraid UI Library (Webcomponent Version)
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
api/src/unraid-api/graph/resolvers/info/devices.service.ts (2)
167-167: Good implementation of proper UUID generation.The use of
crypto.randomUUID()properly addresses the previous concern about usingMath.random()for GUID generation.
48-48: Potential string operation error.The substring operation assumes the
typeidhas at least 4 characters, which could cause runtime errors if the assumption is violated.Add length validation:
- vendorid: device.typeid.substring(0, 4), // Extract vendor ID from type ID + vendorid: device.typeid.length >= 4 ? device.typeid.substring(0, 4) : device.typeid,Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
api/src/unraid-api/graph/resolvers/display/display.resolver.spec.ts (1)
72-79: Consider adding error handling test.The test covers the happy path well, but consider adding a test case for when
displayService.generateDisplay()throws an error to ensure proper error propagation.+ it('should handle service errors gracefully', async () => { + mockDisplayService.generateDisplay.mockRejectedValueOnce(new Error('Service error')); + + await expect(resolver.display()).rejects.toThrow('Service error'); + });api/src/unraid-api/graph/resolvers/info/info.service.spec.ts (1)
64-71: Consider making the bytes mock more flexible.The hardcoded byte conversions could become brittle if different memory sizes are tested. Consider using a more dynamic approach or documenting the specific test cases these values support.
-vi.mock('bytes', () => ({ - default: vi.fn((value) => { - if (value === '32 GB') return 34359738368; - if (value === '16 GB') return 17179869184; - if (value === '4 GB') return 4294967296; - return 0; - }), -})); +vi.mock('bytes', () => ({ + default: vi.fn((value) => { + const conversions = { + '32 GB': 34359738368, + '16 GB': 17179869184, + '4 GB': 4294967296, + }; + return conversions[value] ?? 0; + }), +}));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
api/src/unraid-api/graph/resolvers/display/display.resolver.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/display/display.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/display/display.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/display/display.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.resolver.ts(6 hunks)api/src/unraid-api/graph/resolvers/info/info.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- api/src/unraid-api/graph/resolvers/info/info.service.ts
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts
🧰 Additional context used
🪛 Biome (1.9.4)
api/src/unraid-api/graph/resolvers/display/display.service.ts
[error] 114-114: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Build Web App
- GitHub Check: Build API
- GitHub Check: Build Unraid UI Library (Webcomponent Version)
- GitHub Check: Test API
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (19)
api/src/unraid-api/graph/resolvers/display/display.resolver.spec.ts (2)
4-15: Well-structured test setup with comprehensive mocking.The test setup properly mocks external dependencies and provides good isolation. The pubsub module mocking ensures tests don't rely on external services.
21-43: Comprehensive mock data structure.The mock display data covers all the required fields and provides realistic test values. This ensures tests validate the complete data structure.
api/src/unraid-api/graph/resolvers/display/display.resolver.ts (2)
16-16: Excellent dependency injection implementation.The constructor properly injects the DisplayService, following NestJS best practices for dependency injection.
24-26: Clean separation of concerns.The resolver method now properly delegates to the service, keeping the resolver focused on GraphQL orchestration rather than business logic.
api/src/unraid-api/graph/resolvers/display/display.service.spec.ts (3)
8-15: Solid mocking strategy for filesystem operations.The module-level mock with selective implementation replacement allows for flexible testing of different file scenarios.
52-83: Thorough error handling test.This test properly simulates ENOENT errors and verifies the service handles missing files gracefully. The mock restoration ensures test isolation.
98-123: Comprehensive configuration merging test.The test verifies that configuration values are properly merged from multiple sources and that type conversions work correctly. The specific assertions for boolean conversions and numeric parsing are valuable.
api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts (3)
12-71: Comprehensive mocking setup addresses previous concerns.The extensive mocking of all external dependencies (fs, pubsub, dockerode, systeminformation, etc.) and the mock cache manager provide excellent test isolation. This addresses the previous review comment about missing tests.
175-189: Well-structured service mocks.The mock services with predefined return values enable testing of the resolver's delegation pattern without testing the service internals.
361-381: Excellent error handling test coverage.The error handling tests verify that the resolver properly propagates errors from both external libraries and injected services, ensuring robust error handling.
api/src/unraid-api/graph/resolvers/display/display.service.ts (4)
13-59: Well-defined state management.The predefined states provide clear error conditions and success states, making the service behavior predictable and testable.
63-75: Clean service architecture.The public method properly orchestrates the private methods and returns a well-structured Display object.
77-107: Robust file handling logic.The getCaseInfo method properly handles various file states: missing files, read errors, and empty content. The error handling is comprehensive.
121-139: Comprehensive type casting and validation.The configuration parsing properly handles type conversions for booleans, numbers, and provides sensible defaults. The explicit type assertions for theme and unit are appropriate.
api/src/unraid-api/graph/resolvers/info/info.service.spec.ts (5)
323-334: Good coverage of dmidecode fallback behavior.The test correctly verifies that when dmidecode fails, the service falls back to using
mem.total. This demonstrates good error handling testing practices.
248-255: Excellent error handling test for CPU flags.The test properly verifies that when CPU flags retrieval fails, the service gracefully returns an empty array instead of crashing. This is good defensive programming.
107-133: Well-structured test setup with proper mock management.The beforeEach setup correctly initializes the NestJS testing module and properly resets mocks between tests. The pattern of getting mock references after module compilation is also correct.
337-348: ```shell
#!/bin/bashLocate and inspect the implementation of generateDevices
file=$(fd "info.service.ts" | head -n1)
if [ -z "$file" ]; then
echo "info.service.ts not found"
exit 1
fi
rg -n "generateDevices" -C5 "$file"--- `221-222`: ```shell #!/bin/bash # Extract generateCpu() method body for cores/threads mapping verification sed -n '49,100p' api/src/unraid-api/graph/resolvers/info/info.service.ts
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
api/src/unraid-api/graph/resolvers/info/info.service.spec.ts (2)
64-71: Consider making the bytes mock more flexible.The hardcoded return values in the bytes mock could make tests brittle if different memory sizes are needed in future tests.
-vi.mock('bytes', () => ({ - default: vi.fn((value) => { - if (value === '32 GB') return 34359738368; - if (value === '16 GB') return 17179869184; - if (value === '4 GB') return 4294967296; - return 0; - }), -})); +vi.mock('bytes', () => ({ + default: vi.fn((value) => { + const sizeMap = { + '32 GB': 34359738368, + '16 GB': 17179869184, + '4 GB': 4294967296, + }; + return sizeMap[value as keyof typeof sizeMap] || 0; + }), +}));
158-168: Good error handling test, but consider adding more edge cases.The test handles empty container arrays well, but could benefit from testing other error scenarios like cache misses or invalid container data.
Consider adding a test case for when the cache returns null or undefined:
+ it('should handle cache miss gracefully', async () => { + mockCacheManager.get.mockResolvedValue(null); + + const result = await service.generateApps(); + + expect(result).toEqual({ + id: 'info/apps', + installed: 0, + started: 0, + }); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
api/src/unraid-api/graph/resolvers/info/info.service.spec.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Build Unraid UI Library (Webcomponent Version)
- GitHub Check: Build API
- GitHub Check: Build Web App
- GitHub Check: Test API
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
api/src/unraid-api/graph/resolvers/info/info.service.spec.ts (6)
141-156: Test coverage looks good for the happy path.The test properly validates container state counting logic with appropriate mock data.
194-229: Excellent CPU information mapping test.The test comprehensively validates the complex mapping from systeminformation format to the expected output format, including proper handling of cores vs threads distinction.
248-255: Good error handling for CPU flags.Properly tests the graceful degradation when CPU flags cannot be retrieved.
323-334: Solid test for dmidecode fallback behavior.This test properly validates the fallback logic when dmidecode is not available, which is a realistic scenario.
107-133: Well-structured test setup with proper dependency injection.The beforeEach setup properly initializes the testing module and handles mock cleanup correctly.
337-345: ```shell
#!/bin/bashShow implementation of generateDevices method
rg -n -C 10 "async generateDevices" api/src/unraid-api/graph/resolvers/info/info.service.ts
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
elibosley
force-pushed
the
feat/info-resolver-cleanup
branch
from
c5b63e4 to
99e7d74
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
api/src/unraid-api/graph/resolvers/info/devices.service.ts (1)
153-224: Consider refactoring this complex method for better maintainability.The method handles multiple responsibilities: USB hub detection, device parsing, filtering, and sanitization. Breaking it into smaller focused methods would improve readability and testability.
🧹 Nitpick comments (1)
api/src/unraid-api/graph/resolvers/info/devices.service.ts (1)
136-140: Remove empty if block.The empty if block adds no value. Either implement the driver detection logic or remove this code.
- await isSymlink(`${basePath}${device.id}/driver`).then((symlink) => { - if (symlink) { - // Future: Add driver detection logic here - } - }); + // TODO: Add driver detection logic when needed
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
api/dev/configs/connect.json(1 hunks)api/src/graphql/resolvers/query/info.ts(0 hunks)api/src/unraid-api/graph/resolvers/display/display.resolver.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/display/display.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/display/display.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/display/display.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/devices.resolver.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/devices.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/devices.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/devices.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.model.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.resolver.ts(6 hunks)api/src/unraid-api/graph/resolvers/info/info.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/resolvers.module.ts(2 hunks)
💤 Files with no reviewable changes (1)
- api/src/graphql/resolvers/query/info.ts
✅ Files skipped from review due to trivial changes (2)
- api/src/unraid-api/graph/resolvers/info/devices.resolver.ts
- api/src/unraid-api/graph/resolvers/info/devices.resolver.spec.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- api/dev/configs/connect.json
- api/src/unraid-api/graph/resolvers/resolvers.module.ts
- api/src/unraid-api/graph/resolvers/info/info.model.ts
- api/src/unraid-api/graph/resolvers/info/devices.service.spec.ts
- api/src/unraid-api/graph/resolvers/display/display.resolver.ts
- api/src/unraid-api/graph/resolvers/display/display.service.spec.ts
- api/src/unraid-api/graph/resolvers/info/info.service.ts
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts
- api/src/unraid-api/graph/resolvers/display/display.service.ts
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Build Web App
- GitHub Check: Test API
- GitHub Check: Build Unraid UI Library (Webcomponent Version)
- GitHub Check: Build API
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (19)
api/src/unraid-api/graph/resolvers/display/display.resolver.spec.ts (10)
4-4: LGTM: Proper vitest import addition.The addition of
viimport is appropriate for the mocking functionality used throughout the test file.
7-7: LGTM: Service import aligns with architectural changes.The DisplayService import is consistent with the service-based refactoring mentioned in the PR objectives.
9-15: LGTM: Well-structured pubsub module mock.The mock correctly stubs both
createSubscriptionandPUBSUB_CHANNELwith appropriate return values for testing the subscription functionality.
21-43: LGTM: Comprehensive mock display data.The mock data structure covers all expected display configuration fields with appropriate default values for thorough testing.
45-47: LGTM: Proper service mock implementation.The mock service with stubbed
generateDisplaymethod follows good testing practices by returning the predefined mock data.
51-57: LGTM: Correct dependency injection setup.The test module configuration properly provides both the resolver and the mocked DisplayService, following NestJS testing patterns.
61-64: LGTM: Good test isolation practices.Adding the displayService getter and mock clearing ensures proper test isolation and prevents test interference.
69-69: LGTM: Appropriate service definition test.Adding the displayService definition assertion ensures the mock is properly injected.
72-79: LGTM: Comprehensive display method test.The test properly verifies that the resolver calls the service method and returns the expected data, ensuring the delegation pattern works correctly.
81-90: LGTM: Thorough subscription test coverage.The test correctly verifies the subscription creation with proper channel and return value, ensuring the pubsub integration works as expected.
api/src/unraid-api/graph/resolvers/info/info.service.spec.ts (8)
1-91: Well-structured mock setup with comprehensive external dependency coverage.The extensive mocking setup properly isolates the service under test from external dependencies. The mock implementations are appropriate and realistic for testing purposes.
107-133: Proper test module setup following NestJS testing patterns.The beforeEach setup correctly resets mocks and creates a fresh testing module for each test, ensuring test isolation.
139-169: Comprehensive testing of Docker container statistics.The tests cover both the normal case with container data and the error case with empty results, ensuring robust error handling.
171-191: OS information test validates proper data mapping.The test correctly verifies that system information is combined with hostname and uptime data from different sources.
193-256: Thorough CPU information testing with edge cases.Excellent coverage including proper field mapping (physicalCores → cores, cores → threads), missing speed value handling, and CPU flags error scenarios.
258-280: Version information test covers integration points.The test properly validates that both Unraid-specific and system software versions are retrieved and combined correctly.
282-335: Memory testing includes complex dmidecode fallback logic.Good coverage of memory layout retrieval, error handling, and the fallback mechanism when dmidecode is unavailable.
337-345: ```shell
#!/bin/bashFetch implementation of generateDevices to determine its complexity
rg -n "async generateDevices" -A 50 api/src/unraid-api/graph/resolvers/info/info.service.ts
</details> <details> <summary>api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts (1)</summary> `12-382`: **Excellent test coverage!** The comprehensive test suite properly covers all resolver methods, including error scenarios. This addresses the previous concern about missing tests. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| .filter((device) => device.class === 'vga' && !device.allowed) | ||
| .map((entry) => { | ||
| const gpu: Gpu = { | ||
| id: `gpu/${entry.id}`, | ||
| blacklisted: entry.allowed, |
There was a problem hiding this comment.
Fix contradictory blacklist logic.
The filter selects devices where !device.allowed but then sets blacklisted: entry.allowed. This means non-allowed devices get blacklisted: false, which seems backwards.
- .filter((device) => device.class === 'vga' && !device.allowed)
+ .filter((device) => device.class === 'vga')
.map((entry) => {
const gpu: Gpu = {
id: `gpu/${entry.id}`,
- blacklisted: entry.allowed,
+ blacklisted: !entry.allowed,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .filter((device) => device.class === 'vga' && !device.allowed) | |
| .map((entry) => { | |
| const gpu: Gpu = { | |
| id: `gpu/${entry.id}`, | |
| blacklisted: entry.allowed, | |
| .filter((device) => device.class === 'vga') | |
| .map((entry) => { | |
| const gpu: Gpu = { | |
| id: `gpu/${entry.id}`, | |
| blacklisted: !entry.allowed, | |
| }; | |
| return gpu; | |
| }) |
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/info/devices.service.ts around lines 31 to
35, the code filters devices where allowed is false but then sets blacklisted to
entry.allowed, causing contradictory logic. Fix this by setting blacklisted to
the negation of entry.allowed so that devices not allowed are correctly marked
as blacklisted.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
🤖 I have created a release *beep* *boop* --- ## [4.9.0](v4.8.0...v4.9.0) (2025-07-08) ### Features * add graphql resource for API plugins ([#1420](#1420)) ([642a220](642a220)) * add management page for API keys ([#1408](#1408)) ([0788756](0788756)) * add rclone ([#1362](#1362)) ([5517e75](5517e75)) * API key management ([#1407](#1407)) ([d37dc3b](d37dc3b)) * api plugin management via CLI ([#1416](#1416)) ([3dcbfbe](3dcbfbe)) * build out docker components ([#1427](#1427)) ([711cc9a](711cc9a)) * docker and info resolver issues ([#1423](#1423)) ([9901039](9901039)) * fix shading in UPC to be less severe ([#1438](#1438)) ([b7c2407](b7c2407)) * info resolver cleanup ([#1425](#1425)) ([1b279bb](1b279bb)) * initial codeql setup ([#1390](#1390)) ([2ade7eb](2ade7eb)) * initialize claude code in codebse ([#1418](#1418)) ([b6c4ee6](b6c4ee6)) * move api key fetching to use api key service ([#1439](#1439)) ([86bea56](86bea56)) * move to cron v4 ([#1428](#1428)) ([b8035c2](b8035c2)) * move to iframe for changelog ([#1388](#1388)) ([fcd6fbc](fcd6fbc)) * native slackware package ([#1381](#1381)) ([4f63b4c](4f63b4c)) * send active unraid theme to docs ([#1400](#1400)) ([f71943b](f71943b)) * slightly better watch mode ([#1398](#1398)) ([881f1e0](881f1e0)) * upgrade nuxt-custom-elements ([#1461](#1461)) ([345e83b](345e83b)) * use bigint instead of long ([#1403](#1403)) ([574d572](574d572)) ### Bug Fixes * activation indicator removed ([5edfd82](5edfd82)) * alignment of settings on ManagementAccess settings page ([#1421](#1421)) ([70c790f](70c790f)) * allow rclone to fail to initialize ([#1453](#1453)) ([7c6f02a](7c6f02a)) * always download 7.1 versioned files for patching ([edc0d15](edc0d15)) * api `pnpm type-check` ([#1442](#1442)) ([3122bdb](3122bdb)) * **api:** connect config `email` validation ([#1454](#1454)) ([b9a1b9b](b9a1b9b)) * backport unraid/webgui[#2269](https://github.com/unraid/api/issues/2269) rc.nginx update ([#1436](#1436)) ([a7ef06e](a7ef06e)) * bigint ([e54d27a](e54d27a)) * config migration from `myservers.cfg` ([#1440](#1440)) ([c4c9984](c4c9984)) * **connect:** fatal race-condition in websocket disposal ([#1462](#1462)) ([0ec0de9](0ec0de9)) * **connect:** mothership connection ([#1464](#1464)) ([7be8bc8](7be8bc8)) * console hidden ([9b85e00](9b85e00)) * debounce is too long ([#1426](#1426)) ([f12d231](f12d231)) * delete legacy connect keys and ensure description ([22fe91c](22fe91c)) * **deps:** pin dependencies ([#1465](#1465)) ([ba75a40](ba75a40)) * **deps:** pin dependencies ([#1470](#1470)) ([412b329](412b329)) * **deps:** storybook v9 ([#1476](#1476)) ([45bb49b](45bb49b)) * **deps:** update all non-major dependencies ([#1366](#1366)) ([291ee47](291ee47)) * **deps:** update all non-major dependencies ([#1379](#1379)) ([8f70326](8f70326)) * **deps:** update all non-major dependencies ([#1389](#1389)) ([cb43f95](cb43f95)) * **deps:** update all non-major dependencies ([#1399](#1399)) ([68df344](68df344)) * **deps:** update dependency @types/diff to v8 ([#1393](#1393)) ([00da27d](00da27d)) * **deps:** update dependency cache-manager to v7 ([#1413](#1413)) ([9492c2a](9492c2a)) * **deps:** update dependency commander to v14 ([#1394](#1394)) ([106ea09](106ea09)) * **deps:** update dependency diff to v8 ([#1386](#1386)) ([e580f64](e580f64)) * **deps:** update dependency dotenv to v17 ([#1474](#1474)) ([d613bfa](d613bfa)) * **deps:** update dependency lucide-vue-next to ^0.509.0 ([#1383](#1383)) ([469333a](469333a)) * **deps:** update dependency marked to v16 ([#1444](#1444)) ([453a5b2](453a5b2)) * **deps:** update dependency shadcn-vue to v2 ([#1302](#1302)) ([26ecf77](26ecf77)) * **deps:** update dependency vue-sonner to v2 ([#1401](#1401)) ([53ca414](53ca414)) * disable file changes on Unraid 7.2 ([#1382](#1382)) ([02de89d](02de89d)) * do not start API with doinst.sh ([7d88b33](7d88b33)) * do not uninstall fully on 7.2 ([#1484](#1484)) ([2263881](2263881)) * drop console with terser ([a87d455](a87d455)) * error logs from `cloud` query when connect is not installed ([#1450](#1450)) ([719f460](719f460)) * flash backup integration with Unraid Connect config ([#1448](#1448)) ([038c582](038c582)) * header padding regression ([#1477](#1477)) ([e791cc6](e791cc6)) * incorrect state merging in redux store ([#1437](#1437)) ([17b7428](17b7428)) * lanip copy button not present ([#1459](#1459)) ([a280786](a280786)) * move to bigint scalar ([b625227](b625227)) * node_modules dir removed on plugin update ([#1406](#1406)) ([7b005cb](7b005cb)) * omit Connect actions in UPC when plugin is not installed ([#1417](#1417)) ([8c8a527](8c8a527)) * parsing of `ssoEnabled` in state.php ([#1455](#1455)) ([f542c8e](f542c8e)) * pin ranges ([#1460](#1460)) ([f88400e](f88400e)) * pr plugin promotion workflow ([#1456](#1456)) ([13bd9bb](13bd9bb)) * proper fallback if missing paths config modules ([7067e9e](7067e9e)) * rc.unraid-api now cleans up older dependencies ([#1404](#1404)) ([83076bb](83076bb)) * remote access lifecycle during boot & shutdown ([#1422](#1422)) ([7bc583b](7bc583b)) * sign out correctly on error ([#1452](#1452)) ([d08fc94](d08fc94)) * simplify usb listing ([#1402](#1402)) ([5355115](5355115)) * theme issues when sent from graph ([#1424](#1424)) ([75ad838](75ad838)) * **ui:** notifications positioning regression ([#1445](#1445)) ([f73e5e0](f73e5e0)) * use some instead of every for connect detection ([9ce2fee](9ce2fee)) ### Reverts * revert package.json dependency updates from commit 711cc9a for api and packages/* ([94420e4](94420e4)) --- 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>
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Refactor