That fix does not really fix the original issue, but it makes it
impactless, and I think it's safer anyway to have upsert idempotent.
The pattern to reproduce is:
- peer A create a synced map, add a datalayer, save it
- peer B loads the map, click on edit
- at this time, peer B have twice the datalayer data, once from the
server AND once from the sync
So a better fix would be to make that peer B send a meaningfull HLC to
peer A I guess.
For this we may save the last HLC is the map properties, or maybe try to
merge the "reference_version" and the HLC.
I first tried to handle this on Leaflet.Editable side, to make it fire
the "editable:edited" event we use to trigger the sync, but deciding
what to do with a feature on escape needs some decisions that seems hard
to implement in a generic way in Leaflet.Editable.
We call stopDrawing, which then calls cancelDrawing, so here one need to
decide if cancelDrawing should keep the already drawn line (but cancel
the point being drawn) or cancel everything.
This is why I end up making this change in uMap itself.
That fix does not really fix the original issue, but it make it impactless.
The pattern is:
- peer A create a synced map, add a datalayer, save it
- peer B loads the map, click on edit
- at this time, peer B have twice the datalayer data, once from the server
AND once from the sync
So a better fix would be to make that peer B send a meaningfull HLC to
peer A I guess.
For this we may save the last HLC is the map properties, or maybe try
to merge the "reference_version" and the HLC.
I first tried to handle this on Leaflet.Editable side, to make
it fire the "editable:edited" event we use to trigger the sync,
but deciding what to do with a feature on escape needs some
decisions that seems hard to implement in a generic way in
Leaflet.Editable.
We call stopDrawing, which then calls cancelDrawing, so here
one need to decide if cancelDrawing should keep the already
drawn line (but cancel the point being drawn) or cancel
everything.
This is why I end up making this change in uMap itself.
To reproduce:
- create a map
- saved it
- change the "syncEnabled" setting to on
- save again
- open another tab with this map
- switch on edit mode
In this case, the second client will try to authenticate:
- once switch on edit mode
- and once receiving the operation message from peer A about the
syncEnabled (which calls render, which calls initSyncEngine in
in this case)
I think we want to keep render to call initSyncEngine, so if a map
owner switch off the syncEnabled setting, this will (should) disconnect
the other peers.
When a peer save the map, other peers in dirty state should not need to
save the map anymore.
That implementation uses the lastKnownHLC as a reference, but it changes
all dirty states at once. Another impementation could be to have each
object sync its dirty state, but in this case we do not have a HLC per
object as reference, and it also creates more messages.
ping @almet if you're around and wanna have a fresh look. :)
And remove Leaflet.Toolbar dependency.
This also teach ContextMenu how to display icons instead of
text and how to render in horizontal orientation instead of
vertical.
At the end, we only need two states: has this datalayer loaded the
data it should load ? yes / no.
Whether it local or remote data should not be a matter.
The scenario to reproduce is:
- peer A creates a datalayer
- peer B add a marker on that datalayer
- peer B saves the datalayer
Before this fix, after the save, peer A would get a new _referenceVersion
for this datalayer, and the render method would make a hide/show,
which would refetch the data from the server, duplicating it.
So forcing the _loaded to be true in this situation prevent this.
We may want to rework the "_loaded" pattern, maybe with the server
returning in a datalayer metadata the length of the data it get for
this given datalayer, so the client knows if it is worth getting
data for a layer when itself (the client) does not have any.
Co-authored-by: David Larlet <david@larlet.fr>
When a peer save the map, other peers in dirty state should not need
to save the map anymore.
That implementation uses the lastKnownHLC as a reference, but it changes
all dirty states at once. Another impementation could be to have each
object sync its dirty state, but in this case we do not have a HLC per
object as reference, and it also creates more messages.
Co-authored-by: David Larlet <david@larlet.fr>
This is more unified between markers and paths, and it allows paths to
be synced as soon as they have been drawn (instead of when closing the
edit panel, which created a race condition when changing its properties
that were then synced to other while the feature itself was not).
This also:
- change the "set center and zoom" to be a panel instead of
a direct action (including the "defaultView" setting
- refactor the "get started dialog"
This is a first step to remove our dependency to the unmaintained
Leaflet.Toolbar plugin.
This is more unified between markers and paths, and it allows paths to
be synced as soon as they have been drawn (instead of when closing the
edit panel, which created a race condition when changing its properties
that were then synced to other while the feature itself was not).
This is not the same as the short `SITE_NAME` which is displayed as the
title of the (home)page for instance.
The plan is to set `SITE_DESCRIPTION` as `uMap OpenStreetMap` for the
OSM instance and `uMap agents publics` for the ANCT one.
When deleting a datalayer, it will now be moved to a state "deleted", and
it will only be deleted for real when running the command `umap empty_trash`.
This is what we already do for the map itself, but until now if a user
deleted a only a datalayer by mistake (not the map itself) it could not retrieve
it.
fix#2280
That's a first step, which:
- internalize Formbuilder as a bunch of modules
- use Javascript classes instead of Leaflet ones
- remove dependencies to Leaflet (L.DomUtil…)
- replaces `L.FormBuilder` by `Form` (in theory generic, but not quite)
and `U.FormBuilder` by `MutatingForm` (knows about isDirty,
`inheritable` and such)
There is much more room for refactor, but let's do it step by step!
fix#2268
There is a tricky choice to do: the delete actually occurs in two
times, first the datalayer is hidden from the UI and set as "deleted"
(this can then be undone) then at next "save" it will totally removed.
When syncing, given we removed the "reset/undo" feature for now, and
because it was simpler, I decide to do both step in once.
When working on a proper "undo/redo", we may challenge this choice
again.
fix#2268
There is a tricky choice to do: the delete actually occurs in two
times, first the datalayer is hidden from the UI and set as "deleted"
(this can then be undone) then at next "save" it will totally removed.
When syncing, given we removed the "reset/undo" feature for now, and
because it was simpler, I decide to do both step in once.
When working on a proper "undo/redo", we may challenge this choice
again.
