Update ListAttributes identically when removing edges or nodes
#2280
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cbentejac
requested review from
fabiencastan,
demoulinv and
servantftechnicolor
cbentejac
commented
Jan 15, 2024
fabiencastan
requested changes
Jan 16, 2024
cbentejac
force-pushed
the
fix/removeEdges
branch
from
c78e5e0 to
66bb35a
Compare
cbentejac
force-pushed
the
fix/removeEdges
branch
2 times, most recently
from
0829252 to
6a01610
Compare
fabiencastan
approved these changes
Jan 31, 2024
The behaviour regarding `ListAttributes` was different depending on whether an edge or a node was removed: - if an edge was removed, the `ListAttribute` that was the destination of that edge had its corresponding element completely removed; - if a node was removed, the `ListAttribute` that was the destination of one of the node's edges had its corresponding element reset, but not removed. With this behaviour, a user had different UIDs depending on whether a single edge or the whole node had been removed where the UID should always be identical.
cbentejac
force-pushed
the
fix/removeEdges
branch
from
6a01610 to
e9ee9f2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The behaviour regarding
ListAttributeswas different depending on whether an edge or a node was removed:ListAttributethat was the destination of that edge had its corresponding element completely removed;ListAttributethat was the destination of one of the node's edges had its corresponding element reset, but not removed.With this behaviour, a user had different UIDs depending on whether a single edge or the whole node had been removed where the UID should always be identical.
This PR ensures that removing a node has the same effect on a connected node as removing an edge: it thus removes completely an element connected to a
ListAttributewhen the node is removed.Implementation remarks
When collecting the outgoing edges, we now check whether the edge is connected to a
ListAttribute. If it is, we associate the attribute of the outgoing edge to a tuple containing theListAttributeit is connected to and its value (if it has any). We then remove the corresponding element in theListAttributethe edge is connected to.When recreating a node that has been removed (either by performing an "Undo" action or by upgrading a node), this map is used to check whether the edge that will be re-added is to be connected to a
ListAttribute. If it is, a new element is added to theListAttributewith the value stored in the map. Re-creating the entry in the list is necessary: if it does not exist, the edge will not have any destination attribute to connect to.