Page MenuHomePhabricator

Add baserevid to WikibaseLexeme API modules
Closed, ResolvedPublic5 Story Points

Description

As user of the WikibaseLexeme API I want to be able to provide a baserevid for the edit i am making while using the WikibaseLexeme API modules.

A user of the WikibaseLexeme API = our UI

API modules that needs adjusting:

  • wbladdform
  • wbladdsense
  • wbleditformelements
  • wbleditsenseelements
  • wblremoveform
  • wblremovesense

Acceptance criteria:

All listed API modules now have the baserevid parameter

When baserevid is not valid (for a different entity .. etc)
Then API request should fail

When there's a conflict around baserevid (same behavior as wbeditentity endpoint)
Then API request should fail

Related Objects

Event Timeline

Addshore created this task.Feb 27 2019, 2:04 PM
Restricted Application added a project: Wikidata. · View Herald TranscriptFeb 27 2019, 2:04 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
alaa_wmde updated the task description. (Show Details)Mar 5 2019, 4:05 PM
alaa_wmde set the point value for this task to 5.

Change 495908 had a related patch set uploaded (by Alaa Sarhan; owner: Alaa Sarhan):
[mediawiki/extensions/WikibaseLexeme@master] lexeme.api: require and check conflict on baserevid param

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

Change 495908 had a related patch set uploaded (by Alaa Sarhan; owner: Alaa Sarhan):
[mediawiki/extensions/WikibaseLexeme@master] lexeme.api: require and check conflict on baserevid param

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

Tarrow added a subscriber: Tarrow.Mar 27 2019, 10:37 AM

I took a little look at this and I'm not feeling super clear on what the acceptance criteria are. My immediate thoughts would be that baseRevId not equalling the latest id shouldn't immediately result in a conflict.

@Addshore could you maybe give some examples in the acceptance criteria to make it explicit what the expected behaviour is?

Trying to mimic similar functionality from existing modules is hard because the checks are apparently spread about a bit. e.g. this red herring in https://github.com/wikimedia/mediawiki-extensions-Wikibase/blob/a7db4c9fd88bc1ddfdf3b5266b399cff7e808954/repo/includes/Api/EditEntity.php#L198 which makes the check look much simpler than I would expect it is

From Mattermost: according to @Addshore WikiPageEntityStore is the place to look for hints as to the existing logic

alaa_wmde added a comment.EditedMar 28 2019, 1:41 PM

findings from reading through code again:

current logic and status quo:
in wikibase, explained here T126231
in lexeme, there was no such logic in lexeme -> we added very basic baserevid !== latestrevid

If I understand correctly, I think wikibase is not doing what we think it is. Wikibase only checks that the patch is valid on both base and current revisions, but does not do a two- or three-way diff (as far as I could see in code).

If that is true, then the current logic we did for lexeme is a better edit conflict prevention than what's in wikibase, and probably we can go with it for this iteration and leave implementing more sophisticated diffing and conflict detection for a later one.

But that might not be true. If someone knows from the top of their head I'd appreciate a shout. Otherwise, I'll have to go deeper later and figure out for sure.

alaa_wmde updated the task description. (Show Details)Apr 10 2019, 8:38 AM

As @Tarrow pointed out the current patch has lot simpler solution compared to the existing logic on wbeditentity endpoint, so we have to change the endpoints covered by this task so that they behave consistently with wbeditentity behavior.

alaa_wmde removed alaa_wmde as the assignee of this task.May 7 2019, 3:26 PM
alaa_wmde added a subscriber: alaa_wmde.
Restricted Application added a project: User-Ladsgroup. · View Herald TranscriptMay 8 2019, 11:56 AM
This comment was removed by Ladsgroup.

Change 508851 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/WikibaseLexeme@master] Add baserevid and conflict detection to AddForm API module

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

I found an easy and funny way to use Wikibase's edit conflict detection. I tested it and it works completely fine.

Change 508851 merged by jenkins-bot:
[mediawiki/extensions/WikibaseLexeme@master] Add baserevid and conflict detection to AddForm API module

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

Change 509933 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/WikibaseLexeme@master] Send the patched lexeme based on the baserevid sent in wbladdform

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

Change 509933 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/WikibaseLexeme@master] Send the patched lexeme based on the baserevid sent in wbladdform
https://gerrit.wikimedia.org/r/509933

This is not merged :D

Change 495908 abandoned by Alaa Sarhan:
lexeme.api: require and check conflict on baserevid param

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

Change 518052 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] Make conflict detection more strict

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

Addshore moved this task from incoming to in progress on the Wikidata board.Jun 21 2019, 11:25 PM

Change 509933 abandoned by Ladsgroup:
Send the patched lexeme based on the baserevid sent in wbladdform

Reason:
There's no need to add base rev id to add form

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

Change 518052 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Make conflict detection more strict

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

alaa_wmde closed this task as Resolved.Tue, Oct 1, 2:47 PM