-
Notifications
You must be signed in to change notification settings - Fork 41
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
fix: undo multiple y [PT-187833677] #1380
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1380 +/- ##
==========================================
+ Coverage 85.63% 85.72% +0.09%
==========================================
Files 522 522
Lines 25582 25584 +2
Branches 6534 6967 +433
==========================================
+ Hits 21906 21933 +27
+ Misses 3522 3376 -146
- Partials 154 275 +121
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Test summaryRun details
Flakiness
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud |
3085861
to
95f3251
Compare
95f3251
to
916463a
Compare
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.
👍🏻LGTM
[PT-187833677]
The crash occurred on undoing the drop of a second (or more) Y attribute because
GraphDataConfigurationModel.filterCase
can get called for the "missing" attribute and crashed when it encountered anundefined
attribute description. Fixing that was straightforward, but revealed an underlying issue that the undo/redo of adding an additional y attribute still didn't work. The fix there was to add a MobX reaction to synchronize thefilteredCases
array with the attribute configuration. This meant that some bespoke code to manage that synchronization in response to particular events could be eliminated. The new reaction also relies on makingfilteredCases
a MobX shallow-observable array so that additions/removals to/from it can trigger the necessary reaction(s).