fix(data-views): handle record merging - #8624
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughQuery results now report record deletions and merge completion through optional callbacks. Data Views clears selected record IDs and resets the preview index after these events. After a merge, it also refreshes query results. ChangesData Views merge refresh
Suggested reviewers: Priority: ➖ Normal Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The merge flow now reconciles removed records and refreshes Data Views without an identified remaining merge risk. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change is narrowly scoped to keeping selection and preview state consistent with record changes. No new access or mutation capability was identified, but recovery after interrupted or unsuccessful merges remains only partially established. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
Full details: Automatic TestsExplanation Automatic tests are necessary for this behavior change. The PR changes merge and deletion callback flow across Resolution Add automatic Jest/React Testing Library regression tests. Verify that a merge clears the Data Views selected IDs and preview state before the refreshed results arrive, triggers the refresh callback, and that deleted records invoke local result removal plus the parent deletion callback. Also verify that Full details: Testing InstructionsExplanation The instructions clearly cover the Data Views merge flow, but they do not cover the Query Builder path. The PR changes the shared Resolution Add a Query Builder test. Open Query Builder, run an Agent query, select at least two records, merge the records, and confirm that the results rerun without an error. Also confirm that the selected rows and record preview clear after the merge in both Data Views and Query Builder. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kwhuber
left a comment
There was a problem hiding this comment.
- Open Data Views
- Select Agent.
- Select two or more Agent records.
- Click Merge Records and complete the merge.
- Confirm the Data View query reruns without crashing.
Still receiving the same crash after merge on ojsmnh and auburn.
Specify 7 Crash Report - 2026-10-02T16_23_50.698Z.txt
|
@kwhuber Thanks for testing! Your crash shows that the agent request was happening during the deletion but before the post-merge request happens... Data Views now clears its selection immediately! Should be fixed 🤞 |
JDAM2k4
left a comment
There was a problem hiding this comment.
Testing instructions
- Open Data Views
- Select Agent.
- Select two or more Agent records.
- Click Merge Records and complete the merge.
- Confirm the Data View query reruns without crashing.
This didn't work with naturkundemuseum, as it gave me the error Unknown column 'spmerging.status' in 'SELECT both with and without the auto-populate checkbox selected. I think that's due to a separate error though.
Specify 7 Crash Report - 2026-10-02T19_19_30.765Z.txt
I tried this again in the pri database and it worked fine, regardless of the auto-populate checkbox.
@JDAM2k4 That is a migration error, correct! I would stop using that database in favor of one that is currently up-to-date with all migrations. |
gabek96
left a comment
There was a problem hiding this comment.
Testing instructions
- Open Data Views
- Select Agent.
- Select two or more Agent records.
- Click Merge Records and complete the merge.
- Confirm the Data View query reruns without crashing.
Works great!
Fixes #8623
This PR solves the issue introduced with Data Views when merging records. It should now:
Testing instructions
Summary by CodeRabbit