Skip to content
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

[BUGFIX] ensure ManyArray state is in-sync with relationship state #5462

Merged
merged 1 commit into from
May 21, 2018

Conversation

runspired
Copy link
Contributor

@runspired runspired commented May 1, 2018

This PR extracts the non-diff changes from #5269. In general this is just a nice cleanup PR.

It ideally would include a test to show the issue with ManyArray losing sync that it addresses; however, because we do not currently correctly diff when flushing canonical state (thereby losing local changes), this out-of-sync issue is very hard to manifest as in most scenarios we would be in the same incorrect state even though we were drawing our state from the incorrect source. To be out-of-sync required the currentState to be different from canonicalState post flushCanonical for more changes than just isNew records.

I've taken the opportunity to port the tests from #5269 and converted them to todo. While fixing the ManyArray state won't resolve those tests, it will move our ability to fix "currentState" in ManyArray forward, and this fix was needed for the full solution in that PR.

@runspired
Copy link
Contributor Author

cc @fivetanley this may help you, but it's just a guess

@runspired runspired force-pushed the cleanup-manyarray-state branch 2 times, most recently from c143aed to 38b7a47 Compare May 9, 2018 07:50
@runspired runspired changed the title [WIP][BUGFIX] ensure ManyArray state is in-sync with relationship state [BUGFIX] ensure ManyArray state is in-sync with relationship state May 21, 2018
port tests but skip them

make a TODO that works with testem

leave explanation

print failed assertions if possible

granular todos

remove dead loop
@runspired runspired merged commit e3a24f6 into master May 21, 2018
@runspired runspired deleted the cleanup-manyarray-state branch May 21, 2018 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant