Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Multiple sync-related fixes #1936

Merged
merged 3 commits into from
Jun 26, 2024
Merged

Multiple sync-related fixes #1936

merged 3 commits into from
Jun 26, 2024

Conversation

almet
Copy link
Member

@almet almet commented Jun 24, 2024

I found some issues with the current implementation of the sync mechanism, which this PR fixes.

  • The "move a feature from one datalayer to another" feature was not thought of, at all, and as a result introduced the bug that was referenced in Polyline disapears on layer movement #1931.
  • It turns out that the syncing of this operation wasn't working either, creating shape clones along the way.
  • The previous (buggy) implementation was not sending the proper layer id, resulting to features hosted by the wrong layers.
  • Operations were triggered when loading data from the original server, which shouldn't happen.

Generally speaking, the current implementation misses some cross-datalayer tests, but I suggest these should come as a separate pull request.

yohanboniface
yohanboniface reviewed Jun 25, 2024
View reviewed changes
umap/static/umap/js/umap.js Outdated Show resolved Hide resolved
umap/static/umap/js/umap.layer.js Show resolved Hide resolved
umap/static/umap/js/umap.features.js Outdated Show resolved Hide resolved
@almet
fix(sync): pass the correct datalayer uuid
d1ed8b7
@almet
fix(sync): sync feature layer changes
bf59026 
This commits adds a way to differenciate "end of edit" events that are
triggered rightfully and the one which are triggered during the deletion
of a feature.

Also, directly call the `sync.upsert()` method after the move happened.
@almet
Copy link
Member Author

almet commented Jun 25, 2024

Rebased on latest master, and incorporated the proposed changed from reviews. We should be ready to go.

@almet
fix(sync): do not send messages when loading data from the server
a8be174 
This introduces a `sync=true/false` parameter that can be passed along
when loading data from our own server, in which case we don't want to
send "operation" messages.
@almet almet merged commit 7a7977c into master Jun 26, 2024
3 of 4 checks passed
@almet almet deleted the fix-datalayer-move branch June 26, 2024 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidbgk davidbgk davidbgk left review comments

@yohanboniface yohanboniface yohanboniface approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@almet @davidbgk @yohanboniface