digital-fuesim-manv icon indicating copy to clipboard operation
digital-fuesim-manv copied to clipboard

A failing move proposal doesn't revert the position of the respective element

Open Dassderdie opened this issue 2 years ago • 4 comments

Bug report:

On #291

If the proposal to move an image fails, the image remains on the invalid position instead of being reverted to its original position.

Direct response from @Dassderdie

The debugging was more difficult than I thought, but I think I got it now.

  1. On this branch, you weren't using optimistic updates.
            apiService.proposeAction(
                {
                    type: '[MapImage] Move MapImage',
                    mapImageId: mapImage.id + 'a',
                    targetPosition,
                },
                // this must be true to be optimistic
                true
            );

I'd propose we remove the default for this parameter. However, against my original prediction, we use optimistic updates quite a lot. Therefore a conscious choice should be made.

Actual problem

  1. There is a possible desynchronization between the state and OpenLayers. We drag a feature before we propose the action. Suppose we don't use optimistic updates and the proposal gets rejected. In that case, we have a desync between OpenLayers and the client state because we only pass changes in the state to OpenLayers.

  2. If we use optimistic updates and the action got applied locally, sent to the server, got rejected, and the local changes get reverted, the state changes in between. Therefore OpenLayers should update the position of the element afterward.

This is (most of the time) not the case. The local action is probably not applied because we share the same reducers on the server and the client. If the server rejects the proposal, the client will do it too (Error while applying server action. This is expected if an optimistic update has been applied). Therefore the state never changes.

TLDR: The effects you saw are not our custom optimistic updates not working, but the optimistic updates we do in OpenLayers by changing the position of elements during the dragging without respecting the state in the store. The client and server are in sync, but OpenLayers and the client state are not.

Solution

Handling OpenLayers optimistic updates nicely encapsulated as we do in the store is not possible (I think). Instead, we could implement a handler specifically for the case the proposal doesn't go through in the translateHandler.

On the other hand, I believe that if we propose a valid action there that doesn't get rejected by the client, too, the state should change two times and, therefore, automatically redraw the element to the correct position.

Dassderdie avatar Apr 18 '22 20:04 Dassderdie