origo-map / origo

Origo
Other
48 stars 54 forks source link

Feature refresh for improved concurrent editing #1390

Open MattiasSp opened 2 years ago

MattiasSp commented 2 years ago

Is your feature request related to a problem? Please describe. With multiple users editing the same features in a layer, if they stay in the same session they will overwrite each other's edits.

Describe the solution you'd like When selecting or starting editing of a feature, its geometry and attributes should be refreshed so that the current version of the feature is shown when editing starts. If the refreshed feature differs from the user's current version, the user should be alerted with e g a dialog that allows the user to retain their old version if they choose (with the update being presented as the default behaviour).

At the moment, every user starting a session loads their own version of the layer that they can edit during the session. So any sessions that overlap in time will risk overwriting each other's edits.

While far from a complete solution to concurrent editing, a simple feature refresh before opening the attribute edit form, or when selecting the geometry during edit mode, would greatly reduce the risk of issues. Multiple users would have to have the same feature's attribute edit form open or edit the geometry at the same time for their edits to collide. This would be a great improvement.

Describe alternatives you've considered Ideally, layers whose underlying datasource changes should update immediately in the map to show the current version. Likewise, the attribute edit form should receive updates for attributes edited by another user in real-time. These features would require back-end support in e g Origo Server with Socket.io or something similar, so I'll leave them outside the scope of this issue.

MattiasSp commented 2 years ago

A simple solution would be to call origo.api().getLayer("layername").getSource().refresh(), but this reloads the entire layer, which could introduce a nasty delay when opening the attribute form or starting editing of a geometry, especially for large layers.

Another thought; comparing the feature with the source should be done again before saving edits so that the user is alerted if the source changed during the editing.

steff-o commented 2 years ago

I have written a PR (almost done) for editing attributes in related tables. In that work I have implemented functionality to ensure that a feature is loaded when the attributes edit form is displayed as a related object is not necessarily loaded if using BBOX strategy . The same concept would be applicable to this issue by reloading individual features from source.

My solution is based on implementing a new vector source class that extends VectorSource to be able to access the source from a layer. That class can have these kind of methods and can be access by getlayer(currentLayer).getSource().ensureLoaded(FID) or similar . As a bonus this approach makes it possible to change source parameters that are set at startup but at present can't be changed without creating a new loader. Another approach would to break out the loader function code to a utility library that picks up the configuration for a layer and can be called by something like loader.ensureLoaded(currentLayer, FID), but that is a bit messy in my opinion.

If that PR ever sees the light of day, all the heavy lifting is done when it comes to refresh, at least for WFS as I have not implemented for AGS, but that would follow the same pattern.

MattiasSp commented 2 years ago

@steff-o That would solve this issue elegantly! I did go looking for just such a function in the OpenLayers VectorSource source code and was surprised that it doesn't seem possible to refresh individual features.

Extending VectorSource seems like a good solution for Origo, but taking it another step back, perhaps we should post an issue over at the OpenLayers repo and find out it they would be willing to implement this kind of feature in the OL core?

jokd commented 2 years ago

While at it, maybe #658 could be fixed as well?

steff-o commented 2 years ago

After looking in the repo i realized there already is a utility function getfeature() that fetches individual features from both WFS and AGS. It could be used to replace a feature in a layer source without having to mess with the source class. But somehow I think the source and the getfeature() function should be consolidated in some way. The loader function in wfs.js is essentially the same as getFeature().

MattiasSp commented 2 years ago

After mulling this over for a bit and checking out the awesome new ability to go layer.getSource().getFeatureById(id) (thanks @steff-o), I have a new suggestion for solving this issue.

We could add a new config option for wfs layers, e g refreshFeatureOnSelect, that when true causes every selection to trigger a re-read from source of the selected feature(s).

When editing, we want to keep the option of not rereading the selected feature. So ideally, while editing, the re-read features should be compared to the currently loaded features. If found to be different, the user should be prompted and be able to choose whether or not to keep the current feature or replace it with the new version.

Any thoughts on this, or ideas on where to put the re-read check?

steff-o commented 2 years ago

In general I would prefer if the re-read check was as close to the source as possible in order to make it reusable. It will probably be of interest in more places to have an easy access to features not yet loaded or updated from server.

One approach would be extending WfsSource with a method that ensures that features are re-read and returns a list of those who were updated, added and deleted. The drawback with this solution is that this functionality must be implemented in both WFS and AGS. If you're in to an object orientated approach, an abstract base class could be introduced between VectorSource and WfsSource/AGSSource where common functionality is implemented and only the source specific part goes into Wfs/AGS to avoid double implementation.

The simplest solution would of course be to ask the user beforehand if the feature should be refreshed withou actually check if it is updated.

Just a word of caution: the ensureLoaded function in WfsSource does not actually refresh features as the super.addFeatures() call only adds new items. So you must still rewrite something in there. Probably move out the addFeature() to callers and deal with the returned feature list from _loaderHelper in each caller.