-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
prevent reference cycles in complex forms #1401
Conversation
…as deleted if creating it would result in a reference cycle
C# Unit Tests104 tests 104 ✅ 5s ⏱️ Results for commit da576bc. ♻️ This comment has been updated with latest results. |
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! Very elegant algorithm. 🤓
I suppose we should figure out how to bubble this up to the UI if it actually happens while working in the app. Otherwise, nothing will happen and there will be no explanation as to why not. Though fortunately for the activity view you should be able to figure it out somewhat easily.
backend/FwLite/LcmCrdt/Changes/Entries/AddEntryComponentChange.cs
Outdated
Show resolved
Hide resolved
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.
Actually, there's a problem: deleted Components seem to still get used when looking for cycles. That's just what I'm observing as I try it.
I'm guessing that should be solved upstream? I.e. Harmony shouldn't return deleted objects?
yeah we should ignore them here. But no I don't think we should change harmony we just need to take that into account, though there's an argument for having that be a filter parameter which defaults to false which is what we usually want. |
this will avoid issues syncing with Fw where lcm will throw if you attempt to make a reference cycle