Fix selected element overwrites
Closes #328 (closed)
Scenarios
Selecting another element
- Annotate two elements A and B on any image
- With the
👆 tool, move element A or move one point of element A- When pressing the mouse button,
state.selectedElement
becomes a clone of Element A - When releasing the button,
state.selectedElement
's polygon is already updated and a PATCH is triggered
- When pressing the mouse button,
- Before the
PATCH
is completed, move element B or move one point of element B-
state.selectedElement
becomes a clone of Element B
-
- Once A's patch is completed, B is shown to have A's polygon and A is not updated
- After the
PATCH
, the selected element's polygon was always updated, without checking its ID or its existence, causing element B to have element A's new polygon
- After the
- Once B's patch is completed, B is shown to have B's polygon again.
- B, the selected element, gets its polygon back after the second PATCH since it gets updated again.
Selecting nothing
- Annotate two elements A and B on any image
- With the
👆 tool, move element A or move one point of element A- When pressing the mouse button,
state.selectedElement
becomes a clone of Element A - When releasing the button,
state.selectedElement
's polygon is already updated and a PATCH is triggered
- When pressing the mouse button,
- Before the
PATCH
is completed:- Move element B or move one point of element B
- When pressing the mouse button,
state.selectedElement
becomes a clone of Element B - When releasing the button,
state.selectedElement
's polygon is already updated and a PATCH is triggered
- When pressing the mouse button,
- Click on an empty space on the image to deselect element B
-
state.selectedElement
becomes empty.
-
- Move element B or move one point of element B
- Once A's patch is completed, A is shown to have A's polygon, as expected
-
state.selectedElement
becomes{ zone: { polygon: [......] } }
, without any ID - A still appears to be selected due to reactivity issues linked to this state, since
Boolean(state.selectedElement) === true
even without an ID
-
- Trying to update A again causes
HTTP 405
due to a call toPATCH /api/v1/element/undefined/
- Without an ID, the ID sent to the API becomes
undefined
. We get a 405 because/api/v1/element/undefined/
does not matchPartialUpdateElement
's regex, which requires a UUID, and a 404 page only exists forGET
.
- Without an ID, the ID sent to the API becomes
Solution
The main part of this fix is to automatically update state.selectedElement
whenever the elementsv2/set
or elementsv2/bulkSet
mutations are called, only for elements with matching UUIDs. Unit tests have been updated to have a case where the selected element gets updated.
This removes the need for the InteractiveImage
component to do the updating itself, therefore allowing us to add an early return
statement inside editSelected
and avoiding the polygon
update at the end of the method when it is not needed.
Suggested refactorings
- Try to move the entire InteractiveImage event handling methods into a store module, or into a classic JavaScript ES6
class
, making unit tests much easier, especially for those edge cases #336 (closed) - Get rid of the
if
-else if
statements all over said methods as those dramatically decrease the code's readability and have been a main cause for this error #337 (closed)
Edited by Erwan Rouchet