feat(disks): add isSpinning field to Disk type #1527
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
api/src/unraid-api/graph/resolvers/disks/disks.model.ts(2 hunks)api/src/unraid-api/graph/resolvers/disks/disks.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/disks/disks.service.spec.ts(1 hunks)api/src/unraid-api/graph/resolvers/disks/disks.service.ts(6 hunks)
🧰 Additional context used
🧠 Learnings (4)
api/src/unraid-api/graph/resolvers/disks/disks.model.ts (5)
Learnt from: pujitm
PR: #1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Learnt from: elibosley
PR: #1352
File: packages/unraid-api-plugin-connect/src/config.entity.ts:37-39
Timestamp: 2025-04-21T18:44:24.383Z
Learning: The project prefers using class-validator decorators directly on entity classes rather than separate Zod schemas to maintain type consistency between GraphQL entities and validation rules.
Learnt from: elibosley
PR: #1352
File: packages/unraid-api-plugin-connect/src/config.entity.ts:16-26
Timestamp: 2025-04-21T18:44:15.414Z
Learning: For the Unraid API project, class-validator should be used for validation to avoid mismatches between different validation schemas (like Zod).
Learnt from: pujitm
PR: #982
File: web/helpers/apollo-cache/index.ts:142-142
Timestamp: 2024-12-13T17:28:15.384Z
Learning: In web/helpers/apollo-cache/index.ts, it's acceptable to use structuredClone since the project targets Node.js v20 and browsers where structuredClone is supported.
Learnt from: elibosley
PR: #1181
File: web/store/theme.ts:0-0
Timestamp: 2025-02-24T14:51:21.328Z
Learning: In the Unraid API project's theme system, exact TypeScript type definitions are preferred over index signatures for theme variables to ensure better type safety.
api/src/unraid-api/graph/resolvers/disks/disks.service.spec.ts (14)
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to /test/store//*.ts : Mock external dependencies appropriately in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to /test/store//*.ts : Test getter dependencies are properly mocked in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to /test/store//*.ts : Verify state changes by updating the store in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to /test/store//*.ts : Verify state changes after actions in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to /test/store//*.ts : Test action side effects and state changes in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/*.test.ts : Mock external dependencies and services in tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to /test/store//*.ts : Test store interactions with other stores in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to /test/store//*.ts : Override specific action implementations when needed in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/*.test.ts : Mock external services and API calls in tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to /test/store//*.ts : Test action side effects if not stubbed in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to /test/store//*.ts : Do not mock the store being tested in the test file; use createPinia()
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/*.test.ts : Use createTestingPinia() for mocking stores in component tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to /test/store//*.ts : Set initial state for focused testing in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to /test/store//*.ts : Test async operations completely in store tests
api/src/unraid-api/graph/resolvers/disks/disks.resolver.ts (1)
Learnt from: mdatelle
PR: #942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts, methods like removeRoleFromApiKey rely on service-level error handling.
api/src/unraid-api/graph/resolvers/disks/disks.service.ts (9)
Learnt from: elibosley
PR: #1068
File: api/src/unraid-api/auth/api-key.service.ts:122-137
Timestamp: 2025-01-22T18:34:06.925Z
Learning: The store in @app/store is implemented using Redux's configureStore, where dispatch operations for config updates are synchronous in-memory state updates that cannot fail, making transaction-like patterns unnecessary.
Learnt from: pujitm
PR: #1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a set() method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.
Learnt from: elibosley
PR: #1211
File: api/src/unraid-api/graph/connect/connect-settings.service.ts:48-53
Timestamp: 2025-03-13T16:17:55.820Z
Learning: In NestJS services, dynamic imports should be wrapped in try/catch blocks with proper error logging and fallback handling to prevent unhandled rejections from crashing the application.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to /test/store//*.ts : Mock external dependencies appropriately in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to /test/store//*.ts : Verify proper error handling in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to /test/store//*.ts : Override specific action implementations when needed in store tests
Learnt from: elibosley
PR: #1063
File: api/src/unraid-api/cli/sso/validate-token.command.ts:76-81
Timestamp: 2025-01-23T21:26:17.766Z
Learning: In the unraid/api codebase, store dispatches using Redux Toolkit's unwrap() method already handle errors appropriately and don't require additional try-catch blocks.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to /test/store//*.ts : Test store interactions with other stores in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to /test/store//*.ts : When testing Store files use createPinia and setActivePinia
🪛 GitHub Check: Test API
api/src/unraid-api/graph/resolvers/disks/disks.service.ts
[failure] 2-2:
Delete Store·}·from·'@reduxjs/toolkit';⏎import·{·
[failure] 18-18:
Replace toBoolean·}·from·'@app/core/utils/index with batchProcess·}·from·'@app/utils
[failure] 17-17:
Replace type·{·IniSlot·}·from·'@app/store/state-parsers/slots with {⏎····Disk,⏎····DiskFsType,⏎····DiskInterfaceType,⏎····DiskSmartStatus,⏎}·from·'@app/unraid-api/graph/resolvers/disks/disks.model
[failure] 135-135:
Replace ⏎············(d)·=>·d.id.trim()·===·disk.serialNum.trim()⏎········ with (d)·=>·d.id.trim()·===·disk.serialNum.trim()
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build Web App
- GitHub Check: Build API
🔇 Additional comments (5)
api/src/unraid-api/graph/resolvers/disks/disks.resolver.ts (1)
43-46: LGTM!The resolver field correctly exposes the
isSpinningproperty following the established pattern.api/src/unraid-api/graph/resolvers/disks/disks.model.ts (1)
148-150: LGTM!The field is properly decorated and validated following the project's class-validator pattern.
api/src/unraid-api/graph/resolvers/disks/disks.service.ts (3)
135-147: LGTM!The disk matching logic correctly handles whitespace and provides a sensible default.
158-160: LGTM!Efficient implementation that reads the ini file once and reuses it for all disk parsing.
147-147: Verify spundown field interpretation
- The code treats
spundown === '0'as meaning the disk is spinning (isSpinning: true).- No type definitions, documentation comments, or tests referencing
spundownwere found in the repo.- Please confirm against the Unraid disks.ini specification or example configs that a value of
'0'indeed indicates a non-spun-down (spinning) disk.
elibosley
force-pushed
the
feature/disk-spinning-status
branch
from
89fd014 to
47decbd
Compare
|
Claude finished @elibosley's task —— View job Reviewing PR ChangesAnalyzed the DIFF/CHANGESET for critical issues in the changed lines only:
✅ No critical issues found in changes The changes properly add the |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ing logic - Added `isSpinning` field to the `ArrayDisk` model to indicate the spinning state of disks. - Updated the `parseDisk` method to utilize the new `isSpinning` field from the state instead of the previous ini file parsing. - Enhanced unit tests to cover scenarios for spinning and spun down disks, ensuring accurate state representation in the API.
- Updated unit tests in `emhttp.test.ts` and `slots.test.ts` to include the `isSpinning` field for various disk states, ensuring accurate representation of disk activity. - Enhanced `disks.resolver.spec.ts` to reflect the `isSpinning` status in mock disk data, improving test coverage for disk behavior.
…e tests - Removed dependency injection for the store in `DisksService`, directly utilizing the store from the application context. - Updated unit tests in `disks.service.spec.ts` to mock the store's state and ensure proper functionality without the injected store. - Enhanced test coverage by verifying interactions with the store's state in various scenarios.
…Service - Refactored `DisksService` to utilize `ConfigService` for accessing disk state data instead of the previously injected store. - Updated unit tests in `disks.service.spec.ts` to mock `ConfigService` and verify interactions, ensuring proper functionality with the new implementation. - Enhanced test coverage by checking various scenarios for disk retrieval using the configuration service.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1527 +/- ##
==========================================
+ Coverage 58.50% 58.53% +0.02%
==========================================
Files 624 624
Lines 30717 30732 +15
Branches 3606 3617 +11
==========================================
+ Hits 17972 17989 +17
+ Misses 12708 12706 -2
Partials 37 37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
🤖 I have created a release *beep* *boop* --- ## [4.20.0](v4.19.1...v4.20.0) (2025-09-08) ### Features * **disks:** add isSpinning field to Disk type ([#1527](#1527)) ([193be3d](193be3d)) ### Bug Fixes * better component loading to prevent per-page strange behavior ([095c222](095c222)) * **deps:** pin dependencies ([#1669](#1669)) ([413db4b](413db4b)) * **plugin:** add fallback for unraid-api stop in deprecation cleanup ([#1668](#1668)) ([797bf50](797bf50)) * prepend 'v' to API version in workflow dispatch inputs ([f0cffbd](f0cffbd)) * progress frame background color fix ([#1672](#1672)) ([785f1f5](785f1f5)) * properly override header values ([#1673](#1673)) ([aecf70f](aecf70f)) --- 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