Link: https://www.wikidata.org/w/index.php?title=Q230497&diff=2100448177.
The summary says Removed claim: date of birth (P569): 670, but the actual value was 999.
Note that the previous change, made almost two minutes earlier, was a change from 670 (displayed value) to 999 (removed value).
Not sure how often it happens, just randomly came across this change.
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| Fix edit summary to match the action | mediawiki/extensions/Wikibase | master | +72 -8 |
I can reproduce this with 2 browser tabs open https://www.wikidata.org/w/index.php?diff=2265291151&oldid=2265291140&title=Q4115189
Steps to reproduce...
Edit summaries should be slightly confusing
I expect this is due to on edit, the baserevid of the loaded entity is used to create the summary, when probably the latest revision should liekly be used (perhaps slightly more complex than that, but thats the general thing I believe I'm seeing)
Not sure if this is exactly how the IPs would be triggering this situation, but it could essentially have the same fix
Change #1204885 had a related patch set uploaded (by Dumbledore; author: Dumbledore):
[mediawiki/extensions/Wikibase@master] Fix edit summary to match the action
Shouldn’t we report this as an edit conflict instead? I think that would be my expectation as a UI or API user: I’ve loaded the entity, reviewed the existing statement, and (manually or based on some automatic criteria) decided that it should be removed (e.g. incorrect, less precise than other statement, duplicate of other statement, etc.), and so I want to delete the exact statement I’m seeing. If the statement has been edited in the meantime, then whatever checking I did (manually or automatically) is moot; the statement might have been improved to the point where it shouldn’t be deleted anymore. But the API will still delete it anyway (which makes me wonder what the point of the baserevid parameter of this module even is… does it ever have an effect? it’s not like wbremoveclaims can do a million different things to the statement).
Note that the reproduction steps in T361444#10264672 are using the same user to make both edits, and MediaWiki / Wikibase sometimes suppresses edit conflicts between the same user. But as far as I can tell, wbremoveclaims still behaves the same even when different users are making the edit – this change and this removal came from different accounts, and the second edit was made with baserevid=2429864369 (i.e. the revision ID of the parent of the first edit), but it still removed the statement despite the intermediate change.
Hey @Lucas_Werkmeister_WMDE, You're totally right to call this out. I think I've been fixing the symptom instead of the actual problem.
The real issue:
So right now, my patch just makes sure the edit summary shows the correct value by loading the latest revision. But like you said - if the statement changed in between, we're still deleting something the user never even saw. That doesn't feel right at all.
What probably makes more sense:
If someone's trying to remove a statement and they've passed a baserevid, we should actually check if that statement has changed since then. If it has, we should throw an edit conflict instead of just going ahead with the deletion. Otherwise, what's even the point of having baserevid as a parameter?
I can update my patch to do this properly, but wanted to check with you first - does that sound like the right approach? Or are there cases where we'd want to keep the current behavior?
To me it sounds like a rephrasing of my comment, so I’m hardly going to disagree with it, but it also doesn’t get us anywhere because the decision about how the API module should behave isn’t up to either of us anyway.
@Lydia_Pintscher or @Arian_Bozorg: product question – how should wbremoveclaims behave when an old baserevid is specified and the specified statement has been changed in the meantime? Should it remove the statement anyway (with the “old” or “new” value in the edit summary?), report an edit conflict, or do something else?
Also – the claim parameter of that module can actually take multiple statement IDs. What if some but not all of the statements were edited in the meantime? We could remove only the unchanged statements and emit warnings about the other ones, but to me that seems confusing… I think it’s probably better to report an edit conflict and not save any changes in this case.
I am leaning towards edit conflict.
Adding @Ifrahkhanyaree_WMDE also to check what the REST API does in this case. Or is that even a thing there? 🤔
We've not had this come up so far, but can we can see if we can reproduce it there but I'll defer the final judgement on what to do in this situation to y'all.
Alright, stalled while waiting for T410487: Try and reproduce potential edit conflict bug in REST API.