search

fabric8-services / fabric8-wit

wit stands for Work Item Tracker
http://devdoc.almighty.io/
Apache License 2.0
45 stars 86 forks source link

Should we get rid of the version field in workitems? #2186

Open jarifibrahim opened 6 years ago

jarifibrahim commented 6 years ago

Every workitem right now has a version field and this does more harm than good. When two users concurrently try to update a workitem, one of them will get a Failed to update workitem error on the UI because the version number has changed. The other user now has to refresh the page so that he gets the new version number. The user will loose all the newer unsaved data due to the refresh.

This problem will be more visible when the board view is in place. It is highly probable that two users might try to move the same card and in that case the action would fail due to version conflict.

This is not at all a good user experience. I suggest we get rid of the version number and use some kind of version hash (similar to git hash) to keep track of the revisions.

With the version hash, we will never have version conflict. It is possible that two users simultaneously update the workitem title and only the newer one shows up. This is the expected behaviour. This is what happens when two users simultaneously update the description on Github.

rgarg1 commented 6 years ago

@aslakknutsen @kwk @michaelkleinhenz

kwk commented 6 years ago

@jarifibrahim good points. But can you elaborate how a hash makes things easier? Shouldn't we really start using push notification through web sockets to update clients? That should solve the problem as well.

jarifibrahim commented 6 years ago

@kwk

@jarifibrahim good points. But can you elaborate how a hash makes things easier?

Using a hash would make things very easy. First, we would not compare hashes the way we right now compare version numbers (new version = old version + 1). A user can update a workitem from whatever state they are. It's fine if two users try to update the description and only the latest one is saved.

Shouldn't we really start using push notification through web sockets to update clients? That should solve the problem as well.

Push notifications would solve a completely different problem. The current problem is related to the version field being an integer.

kwk commented 6 years ago

It's fine if two users try to update the description and only the latest one is saved.

@jarifibrahim how is that fine at all? I have encountered the situation multiple times in Github that I couldn't save my changes to a description because it has been already modified by somebody else. It wasn't nice because my changes were lost, but my change should really respect what was there. In the end I appreciated that I didn't destroy somebody else's work although the experience wasn't nice.

But again, what is "the latest change"? Out of two changes is it the one where the user pressed the submit button after the other user pressed it?

jarifibrahim commented 6 years ago

@jarifibrahim how is that fine at all? I have encountered the situation multiple times in Github that I couldn't save my changes to a description because it has been already modified by somebody else. It wasn't nice because my changes were lost, but my change should really respect what was there. In the end I appreciated that I didn't destroy somebody else's work although the experience wasn't nice.

@kwk I am confused about what kind of behaviour do we want. I initially thought saving the second update is a valid behavior but now I am not sure. The current implementation feels more appropriate now. We just have to ensure frontend shows appropriate messages on version conflict. Right now it shows Failed to update workitem which is not at all helpful.

But again, what is "the latest change"? Out of two changes is it the one where the user pressed the submit button after the other user pressed it?

I like the way Github does this. They have a nice edited button that shows all the revisions. We should add this feature in future.

michaelkleinhenz commented 6 years ago

We should prevent conflicts as far as it is possible without going completely for complying to ACID in my opinion. A lost update might be acceptable for some fields while not for others. I'd think that the schema definition might have some impact on a merging strategy. At least there might be more semantics involved than just overwriting values. So I would agree with @jarifibrahim, this might be a good idea.

kwk commented 6 years ago

This PR presents an idea to deal with Versioning more easily, less error-prone and centralized: https://github.com/fabric8-services/fabric8-wit/pull/2263