Skip to content

Fix selected element overwrites

Erwan Rouchet requested to merge fix-selected-element-overwrite into master

Closes #328 (closed)

Scenarios

Selecting another element

  1. Annotate two elements A and B on any image
  2. 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
  3. Before the PATCH is completed, move element B or move one point of element B
    • state.selectedElement becomes a clone of Element B
  4. 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
  5. 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

  1. Annotate two elements A and B on any image
  2. 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
  3. 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
    • Click on an empty space on the image to deselect element B
      • state.selectedElement becomes empty.
  4. 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
  5. Trying to update A again causes HTTP 405 due to a call to PATCH /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 match PartialUpdateElement's regex, which requires a UUID, and a 404 page only exists for GET.

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

Merge request reports

Loading