-
Couldn't load subscription status.
- Fork 11
fix: incorrect state merging in redux store #1437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
omit stale config entries while merging in new config state
WalkthroughThe Redux slices Changes
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (6)
✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
api/src/store/modules/emhttp.ts (1)
167-174: Improve type safety of the custom merge function.The custom merge function correctly implements array replacement logic, but the
anytypes reduce type safety.Consider improving type safety:
-const mergeReplaceArrays = (object: any, source: any, ...rest: any[]) => - mergeWith(object, source, ...rest, (objValue: unknown, srcValue: unknown) => { +const mergeReplaceArrays = <T extends Record<string, unknown>>( + object: T, + source: Partial<T>, + ...rest: Partial<T>[] +) => + mergeWith(object, source, ...rest, (objValue: unknown, srcValue: unknown) => { if (Array.isArray(srcValue)) { return srcValue; } return undefined; });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
api/src/store/modules/emhttp.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`api/**/*`: Use pnpm ONLY for package management. Always run scripts from api/pa...
api/**/*: Use pnpm ONLY for package management.
Always run scripts from api/package.json unless requested.
Test suite is VITEST, do not use jest.
Run tests with: pnpm --filter ./api test.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
api/src/store/modules/emhttp.ts
⏰ 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: Test API
- GitHub Check: Build API
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
api/src/store/modules/emhttp.ts (5)
5-5: Import change enables custom merge behavior.The switch from
mergetomergeWithis correct and necessary for implementing the custom array replacement logic.
188-188: Correct usage of custom merge function in reducer.The replacement of
mergewithmergeReplaceArraysproperly implements the array replacement behavior for state updates.
197-197: Correct usage for successful state loading.The custom merge function correctly handles the fulfilled case where
action.payloadcontains valid state data.
206-206: Correct usage for single state file loading.The custom merge function properly handles the conditional merging when
action.payloadcontains valid state data.
201-201: ```shell
#!/bin/bash1. Extract full createAsyncThunk payloadCreator for loadStateFiles
rg -A 40 -B 5 "export const loadStateFiles" api/src/store/modules/emhttp.ts
2. Find mergeReplaceArrays import in emhttp.ts to see its source
rg -n "mergeReplaceArrays" api/src/store/modules/emhttp.ts
3. Locate the implementation of mergeReplaceArrays in the repo
rg -A 10 -B 0 "export function mergeReplaceArrays" -t ts .
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
api/src/store/modules/dynamix.ts (1)
3-3: Remove unused import.The
mergeimport fromlodash-esis no longer used after switching toObject.assign.-import { merge } from 'lodash-es';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
api/src/store/modules/dynamix.ts(2 hunks)api/src/store/modules/emhttp.ts(4 hunks)api/src/store/modules/registration.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/store/modules/emhttp.ts
🧰 Additional context used
📓 Path-based instructions (1)
`api/**/*`: Use pnpm ONLY for package management. Always run scripts from api/pa...
api/**/*: Use pnpm ONLY for package management.
Always run scripts from api/package.json unless requested.
Test suite is VITEST, do not use jest.
Run tests with: pnpm --filter ./api test.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
api/src/store/modules/registration.tsapi/src/store/modules/dynamix.ts
⏰ 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/store/modules/dynamix.ts (1)
23-23: Verify behavior change from deep merge to shallow merge.The change from
merge()toObject.assign()shifts from deep merging to shallow merging. This means nested objects will be completely replaced rather than recursively merged.While this aligns with the PR objective to "omit stale configuration entries," ensure this doesn't break existing functionality that may rely on nested object merging.
#!/bin/bash # Search for nested object usage in DynamixConfig type and related code ast-grep --pattern 'updateDynamixConfig($_)' rg -A 5 -B 5 'DynamixConfig.*{' rg -A 10 'updateDynamixConfig'Also applies to: 32-32, 36-39
api/src/store/modules/registration.ts (1)
50-50: Consistent shallow merge implementation.The changes correctly implement shallow merging with
Object.assign(), consistent with the pattern in other Redux slices. The simpler state structure here (primitive types) makes the shallow vs deep merge distinction less critical.#!/bin/bash # Verify usage patterns for updateRegistrationState to ensure no breaking changes ast-grep --pattern 'updateRegistrationState($_)' rg -A 5 -B 5 'updateRegistrationState'Also applies to: 59-59, 63-63
|
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>
omit stale config entries while merging in new config state
Summary by CodeRabbit