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.
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.
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.
Until now, it was displayed on the path (line or polygon)
"center". So when zoomed in, if the center is not on the
screen, the tooltip will not be visible.
When we introduced the DataLayer.settings property, we did not run
a migration for existing datalayers (because there are millions in the
French server).
Now that we simplified the `DataLayer.isLoaded()` logic in the client,
we want to be sure that created DataLayer on the client have full
metadata.
Co-authored-by: David Larlet <david@larlet.fr>
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.
When moving a path, this will put a once time listener on "moveend". If
we delete this feature, and move the map, we should not execute the
event callback.
A better fix would be to cancel the event listener on delete, but that
is much more work given how we deal with Leaflet events right now.
When moving a path, this will put a once time listener on "moveend".
If we delete this feature, and move the map, we should not execute the
event callback.
A better fix would be to cancel the event listener on delete, but that
is much more work given how we deal with Leaflet events right now.
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).
Otherwise, when an infinite alert replace an alert with a finite duration,
this first alert timeout will close the second alert.
Co-authored-by: David Larlet <david@larlet.fr>
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.
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.
