refactor(multivariate): move-mv-option-saves-to-rtk-query#7760
Conversation
Stage 1 of the MV-options RTK migration: a new multivariateOption service replaces the raw data.put/post/delete calls in editFeatureMv and createFlag. Orchestration, store events and per-option error attribution are unchanged; failed option creation during createFlag is no longer silently swallowed.
…utation Stage 2 of the MV-options RTK migration. saveMultivariateOptions is a single mutation whose queryFn diffs the input against the server's current options (fetched fresh, so stale client state can never turn an update into a duplicate create), runs the PUT/POST/DELETE requests, and returns the saved options in input order plus per-option errors keyed by index. On success it invalidates the feature's ProjectFlag tag so subscribers refetch API-validated data. editFeatureMv becomes a thin wrapper that maps the composite result onto the existing store contracts: per-card error surfacing via store.error.multivariate_options and the positional onComplete callback. Standard flags skip the round-trip entirely.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
the
front-end
Docker builds report
|
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
|
@gemini-code-assist review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new useMultivariateOption service to handle creating, updating, and deleting multivariate options sequentially, ensuring stable creation order and preventing duplicate options. It refactors feature-list-store.ts to use these new endpoints and adds comprehensive E2E tests to verify multivariate option stability. Feedback on the changes suggests normalizing IDs to strings when searching for existing options to avoid type mismatch issues during strict equality checks.
| if (v.id) { | ||
| original = serverOptions.find((m) => m.id === v.id) |
There was a problem hiding this comment.
To prevent potential type mismatch issues (e.g., if v.id is passed as a string from legacy form inputs or client state while m.id is a number from the server), it is safer to compare them by normalizing both to strings or numbers. Strict equality === will fail if one is a string and the other is a number, which would result in creating duplicate options instead of updating the existing ones.
| if (v.id) { | |
| original = serverOptions.find((m) => m.id === v.id) | |
| if (v.id) { | |
| original = serverOptions.find((m) => String(m.id) === String(v.id)) |
3 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Migrates the multivariate option save flow from the legacy Flux store to a single RTK Query mutation.
saveMultivariateOptionsmutation diffs submitted options against the server's current state before upserting/deleting, so stale client state can no longer create duplicate variants.editFeatureMvbecomes a thin adapter preserving the existing contracts: store events, per-card error attribution, and the positional option ordering consumed by both v1 and v2 feature-state saves. All failing options now surface errors, not just the first.orderingfix will follow separately).Out of scope (follow-up): retiring the store's event system and converting the modal to mutation hooks.
How did you test this code?