-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Clone on copy #1707
Clone on copy #1707
Conversation
b07db46
to
e6f448d
Compare
Clone will copy elements in a side-effect free manner, i.e. not perform any wiring.
98d290d
to
92f2b16
Compare
Ensures copy is unaffected by edit operations after copy. Closes #798
Updated as discussed. Using |
To be released as a new major version. |
You mean for adding root elements? Then we'll add this ability in the future. |
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.
We should consider fixing this.
Simply re-use the existing cache to retrieve a references entry. Global state, while it may work here, is an anti pattern in our code base.
* properly copy category and category value * create category value when creating group label
This causes unintended side-effects, for example copies `processRef` with every participant. On top actual root element copy is being implemented via `RootElementReferenceBehavior`.
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.
✅
This fixes a fundamental flaw in our copy mechanism.
Until today we would carry around a mutable copy of
oldBusinessObject
andoldDi
that would change what has been copied if the copied element changes after the copy operation:Unfortunately the PR got a little bigger than intended, fixing numerous small issues and addition additional test coverage to test copy + paste end-to-end.
Contributions
Closes #798
Closes #1708