Page MenuHomePhabricator

Patching after edit conflicts
Closed, ResolvedPublic

Description

When there are a detected edit conflict it is possible to use the base revision (or a later stored field-specific revision) to generate a diff and then patch the latest revision with an appropriate applicable diff.

This attempt on patching the entity can be done before or after the collision detection in "userWasLastToEdit". If the patching is done after the collision detection it should probably be done early in ApiModifyItem (and similar), probably before it is necessary to make the decision on which revision to modify.


Version: unspecified
Severity: normal
Whiteboard: storypoints: 5
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=37685

Details

Reference
bz39836

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 12:53 AM
bzimport set Reference to bz39836.
bzimport added a subscriber: Unknown Object (MLST).

It seems like the code in the API will be somewhat simpler if the collision detection (aka "userWasLastToEdit") is done before an attempt to patch, and it is also somewhat easier to check for edit conflicts than a failing patching.

  • Bug 40130 has been marked as a duplicate of this bug. ***

Implemented in I344d7681 as follows:

  • modifications must always be based on the revision that is indicated as the

base revision. I.e. if ApiModifyEntity gets a baserevid parameter, it must loat
that version of the entity, and then modify that, not the current revision.

  • If the current revision is the base revsision, or there is no base revision,

then just save the new content as is (no conflict).

  • Otherwise, EditPage creates a patch by generating a diff from the base

content to the new content (the content provided by the caller).

  • Then, a "clean" patch is generated by removing all changes from the patch

that conflict with (are not applicable to) the current revision.

  • If the clean patch is different from the original patch, there is a conflict.
  • If there is a conflict but the current user was the only one to edit since

the base revision, the conflict is ignored. Otherwise, saving is aborted.

  • If saving was not aborted, we now have either a clean patch, or a patch with

conflicts against the user's own edits.

  • Apply that path to the current revision to get a fresh version of the new

content which has all the intended changes performed on top of the current
revision instead of the base revision. (in git terms, this is a rebase).

This should get us a clean result.

Fix for a nasty edge case when adding the first alias for a given language: I53d046c1

I344d7681 is merged, story is pending demo.

It seems to be one important case missing handling. This is where there is detected an edit conflict with the user itself, but there are some intermediate edits by another user.

The only way to handle this seems to be to create an applicable diff for each revision from the base revision up until the last revision, and if the only none-empty diffs are owned by the user itself there is no edit conflict. Otherwise if non-empty applicable diffs are owned by other users, then there is an edit conflict.

The proposed solution is heavy and should only be used if all other methods indicates an edit conflict, and if there is indications that user was not the only one to edit since the base revision.

To reproduce this failure open two different browsers, with to different users or one non-logged in user. Set both browsers to operate in the same language. Edit the description with browser A, then edit the label two times with browser B. There should now be a warning about an edit conflict in browser B even if the user in that browser only have a conflict with himself.

Another case that fails involves removal of sitelinks or aliases, where the user has an edit conflict with himself.

Load an item in a single browser window. Add a new sitelink that does not exist already in the item, and save the sitelink. Now click on edit for the same sitelink and remove it, and then save the sitelink. You will now get an adit conflict.

Veriefied in Wikidata demo for sprint 24