Optimistic conflicts resolution mecanism

[yohanboniface]

original code from @Biondilbiondo (Thanks! :) )

Basically:

• when there is a conflict (i.e. someone try to save with an old ETag)
• we compare new version with its reference version and with the latest known
• if we see only simple case (features added both side, removal on one side…),
  then we merge
• otherwise we raise the conflict as before

[yohanboniface]

One of the keys, if we go that direction, is to keep the relevant versions while there is an editing session going, so maybe instead of keeping the n last versions, we should combine that with the versions from the last hour (but this implies some cron to delete versions, which is less KISS).

[almet]

I've just rebased this but I'm getting an (unrelated ?) error:

______________________________________________________________ test_can_change_owner[chromium] _______________________________________________________________

map = <Map: test map>, live_server = <LiveServer listening at http://localhost:52830>, login = <function login.<locals>.do_login at 0x113f3d120>
user = <User: Joe>

    def test_can_change_owner(map, live_server, login, user):
        page = login(map.owner)
        page.goto(f"{live_server.url}{map.get_absolute_url()}?edit")
        edit_permissions = page.get_by_title("Update permissions and editors")
        edit_permissions.click()
        close = page.locator(".umap-field-owner .close")
        close.click()
        input = page.locator("input.edit-owner")
        input.type(user.username)
        input.press("Tab")
        save = page.get_by_role("button", name="Save")
        expect(save).to_be_visible()
        save.click()
        sleep(1)  # Let save ajax go
        modified = Map.objects.get(pk=map.pk)
>       assert modified.owner == user
E       assert <User: Gabriel> == <User: Joe>
E         +<User: Gabriel>
E         -<User: Joe>

Do you have an idea where this could come from?

[almet]

Happens also on the latest master, I've opened an issue #1404

[almet]

I've pushed an updated version of the code and tests.

It's still doing some "magic" stuff with the form that I'm not super happy with, but I would gladly see a review of these changes already.

[almet]

We might actually do a copy.deepcopy here to avoid editing objects elsewhere.

[almet]

Maybe we could just return if_unmodified_since and self.last_modified != if_unmodified_since

[almet]

I'm not a fan of this "let's overwrite what the client sent us", do you have a better idea?

[yohanboniface]

Sounds good to me!
Didn't you said you wanted to use sets instead ?

[almet]

Didn't you said you wanted to use sets instead ?

Actually, sets needs to have hashable elements inside, which is not our case at the moment, because we're storying dicts. and dicts of lists, so it's not possible :-)

[almet]

There is some magic done with the form handling here. It's useful in order to circumvent the way we use Django model forms.

Changing this would probably mean using a separated "update datalayer form", not tied to the model, so we can validate the form without altering the model (which seem to be how django does).

Here are some notes for later usage if needed:

---

While trying to change how the Form is used in a DataLayer update, I've understood how things work, so here are some notes.

I wanted the submitted form to be processed and validated, and then merged with a document already present in the database.

However, since the form is a ModelForm, it must be linked to something in the database which doesn't seem to be the case here. On data validation the FileField is considered a new field, meaning that the geojson object is overwritten by the new one, and self.path, being derived from self.object.geojson is updated.

[yohanboniface]

BAM! 🎉 🚀