Page MenuHomePhabricator

After aplying baserevid to add form, it's impossible to add two forms without refresh
Closed, InvalidPublic

Description

If we apply the given patches in parents tasks then it will be impossible to add more than one form without refreshing the page and you would get errors like "L1-F1" is already assigned which is understandable but UX-wise, interrupting. We have several options now:

  • Refresh the page when someone saves the page (very hacky but doable in five minutes)
  • Update the baserevid when edit is saved (that might be some work because the api modules don't return the saved revid, so you need to do a round trip API call to update but that sorta defies the whole point of conflict detection in the first place)
  • Do not support conflict detection for adding form and senses and add them regardless but it wouldn't solve the issue of not being to able edit a form twice without refreshing.

Event Timeline

  • Refresh the page when someone saves the page (very hacky but doable in five minutes)

That seems to be worse than no conflict detection at all.

  • Update the baserevid when edit is saved (that might be some work because the api modules don't return the saved revid, so you need to do a round trip API call to update but that sorta defies the whole point of conflict detection in the first place)

I'm conflicted on that one. Wouldn't the ideal state be that we try to make a 3-way-merge and only report a conflict if that fails?

  • Do not support conflict detection for adding form and senses and add them regardless but it wouldn't solve the issue of not being to able edit a form twice without refreshing.

That would mean to not merge the parent patch at all. Why wouldn't that solve the issue? It seems to work fine right now 🤔

That seems to be worse than no conflict detection at all.

Yeah

I'm conflicted on that one. Wouldn't the ideal state be that we try to make a 3-way-merge and only report a conflict if that fails?

So we do a 3-way merge but the problem is that it fails for two additions of forms because they would claim "L1-F1" at the same time.

  • Do not support conflict detection for adding form and senses and add them regardless but it wouldn't solve the issue of not being to able edit a form twice without refreshing.

That would mean to not merge the parent patch at all. Why wouldn't that solve the issue? It seems to work fine right now 🤔

it means also reverting some parts in API but my frontend patch also sends baserevid for removing a form or editing them.

Change 510149 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/WikibaseLexeme@master] Change the way LexemeDiffer and LexemePatcher work with nextFormId and nextSenseId

https://gerrit.wikimedia.org/r/510149

I might not understand the particular problem clear enough, but regarding options listed

Update the baserevid when edit is saved

This is about base id in the front end code?
Isn't it what the UI code is expected to always do?

I discussed this with @Michael earlier today, and it seems that correct thing to do would be to have frontend code update base rev ID after the edit has been made. This might require some work on syncing the base ID between different widgets on the single page, as current they all seem to track base ID separately.
Also, it seems that base revision ID is not really necessary for wbladdform and wbladdsense end points. In those cases conflict detection might seem hard to define, and given the nature of action performed, does not really seem necessary.

Change 510149 abandoned by Ladsgroup:
Change the way LexemeDiffer and LexemePatcher work with nextFormId and nextSenseId

Reason:
There is no need to do that.

https://gerrit.wikimedia.org/r/510149

So we decided not to include baserevid for addform and addsense. Thus this issue should never happen