[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
gerritbot added a comment. Change 270777 merged by jenkins-bot: Simplify SpecialSetLabelDescriptionAliases after ChangeOps fix https://gerrit.wikimedia.org/r/270777 TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: daniel, gerritbot Cc: thiemowmde, gerritbot, Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, D3r1ck01, Izno, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
gerritbot added a comment. Change 270777 had a related patch set uploaded (by Thiemo Mättig (WMDE)): Simplify SpecialSetLabelDescriptionAliases after ChangeOps fix https://gerrit.wikimedia.org/r/270777 TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: daniel, gerritbot Cc: thiemowmde, gerritbot, Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Izno, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
gerritbot added a comment. Change 269718 merged by jenkins-bot: Make SpecialSetLabelDescriptionAliases use ChangeOps. https://gerrit.wikimedia.org/r/269718 TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: gerritbot Cc: thiemowmde, gerritbot, Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Izno, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
gerritbot added a comment. Change 260585 abandoned by Aude: Use common baserevid for label and description changes [WIP] Reason: we probably want to ultimately solve this by having one api request (wbeditentity), but then the edit summaries might not be as fine-grained as they are now. https://gerrit.wikimedia.org/r/260585 TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: gerritbot Cc: thiemowmde, gerritbot, Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Izno, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
daniel added a comment. Turns out SpecialSetLabelDescriptionAliases also needed fixing TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: daniel Cc: thiemowmde, gerritbot, Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Izno, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
gerritbot added a comment. Change 269718 had a related patch set uploaded (by Daniel Kinzler): Make SpecialSetLabelDescriptionAliases use ChangeOps. https://gerrit.wikimedia.org/r/269718 TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: gerritbot Cc: thiemowmde, gerritbot, Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Izno, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
gerritbot added a comment. Change 269461 merged by jenkins-bot: Make ChangeOp::validate run against the current revision. https://gerrit.wikimedia.org/r/269461 TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: gerritbot Cc: thiemowmde, gerritbot, Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Izno, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
gerritbot added a comment. Change 269461 had a related patch set uploaded (by Daniel Kinzler): Make ChangeOp::validate run against the current revision. https://gerrit.wikimedia.org/r/269461 TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: gerritbot Cc: thiemowmde, gerritbot, Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Izno, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
daniel added a comment. In https://phabricator.wikimedia.org/T121395#2008222, @aude wrote: > @daniel hmm... Using the latest base revision would definitely help, though > wonder if relying on that, we still might have a race condition. For two concurrent API requests, there is a race condition even if you do the check after the patch/merge. The only way to avoid this is to supply the //expected current revision// (the one we used for validation) when doing the actual saving. WikiPage has a mechanism for erroring out if the current revision was changed by a concurrent request. I'm afraid getting this right will require us to rewrite some of the logic in EditPage. TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: daniel Cc: thiemowmde, gerritbot, Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Izno, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
aude added a comment. @daniel hmm... Using the latest base revision would definitely help, though wonder if relying on that, we still might have a race condition. TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: aude Cc: thiemowmde, gerritbot, Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Izno, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
aude added a comment. @daniel using the latest revision for validating sounds ok. also, per https://phabricator.wikimedia.org/T121395#1898302 (and really wonder how I was unclear there?), if there is an edit conflict, then the patch / result of resolving the conflict should be validated (perhaps by creating change op for it) and it would be nice if saving multiple labels + descriptions + aliases could be one api request, with one base revision. Right now, saving multiple terms *always* produces an edit conflict and a warning saying such in the browser console. TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: aude Cc: thiemowmde, gerritbot, Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Izno, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
daniel added a comment. Suspicion (1) seems to be unfounded: - api/EditEntity calls ModifyEntity::applyChangeOp on a ChangeOps object. - ModifyEntity::applyChangeOp calls ChangeOp(s)::validate - For each ChangeOp a ChangeOps instances contains, ChangeOps::validate will first call validate on that change op, then apply it. - So if there is a LabelChangeOp and a DescriptionChangeOp for the same language, in any order, the second op would check against the entity with the first op applied, detecting any conflict. - Note that validation of the first op could fail due to a false positive, because of a conflict that would be resolved by applying the second op. A better way to do this would be to use a combind LabelAndDescriptionChangeOp. That would also remove the overhead of running the same constraint check twice. TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: daniel Cc: thiemowmde, gerritbot, Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Izno, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
thiemowmde added a subscriber: thiemowmde. thiemowmde added a comment. We discussed this during todays story time. Suspicions: 1. With one API request (or two with no explicit base revision), both ChangeOps are first validated independently, and then applied. The second is validated without the first being applied. 2. With two API requests, both ChangeOps are validated against the same base revision. The second edit does not "see" the first edit because it uses the same base revision as the first edit for comparison, but merges the edit into the latest revision. Expected behavior when the UI does two API requests: Q1 does have label/description "A/A". Q2 does have "B/A". You want to change Q2 to "A/B". This means: First edit changes Q2 to "A/A". BOOM! Second edit changes Q2 to "A/B", which is fine. TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: thiemowmde Cc: thiemowmde, gerritbot, Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
gerritbot added a subscriber: gerritbot. gerritbot added a comment. Change 260585 had a related patch set uploaded (by Aude): Use common baserevid for label and description changes [WIP] https://gerrit.wikimedia.org/r/260585 TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: gerritbot Cc: gerritbot, Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
aude added a comment. to fix in the backend, one possible solution might be to generate changeops for the edit conflict patch, and apply validation at this point. https://github.com/wikimedia/mediawiki-extensions-Wikibase/blob/master/repo/includes/EditEntity.php#L440 (probably some of this code could also be factored out and be made nicer, but that's aside from the bug fix) TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: aude Cc: Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
aude added a comment. problem is that (in some cases) we are validating an old version of an entity, with a specific change (label or description) patched into the entity. the api call keeps passing the same baserevid even after subsequent edits in the term box. ModifyEntity uses baserevid when looking up the entity. e.g. I change the en label to "FOO", click save (with e.g. baserevid = 1), it validates "FOO" (label) + "old description" then change en description to "Wikipedia disambiguation page", click save (still baserevid = 1), it validates "old label" + "Wikipedia disambiguation page". then have an edit conflict, but it is able to be resolved. no further validation is done at this point, at least for label + description uniqueness. We should: 1. fix the UI to pass the correct baserevid 2. fix the backend so that validation is done when an entity gets patched due to edit conflict. (otherwise, there still is a way around the constraint if someone is directly using the api) TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: aude Cc: Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
aude added a comment. i can reproduce the issue and looking into this TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: aude Cc: Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
aude added a comment. see: https://www.wikidata.org/w/index.php?title=Q4115189&oldid=284854784 https://www.wikidata.org/w/index.php?title=Q3737270&oldid=283412652 TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: aude Cc: Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
aude added a comment. i suspect this is (unfortunately) a case where we run in to limitations of TermSqlIndex and maxConflicts it finds: * @note: This implementation does not guarantee that all matches are returned. * The maximum number of conflicts returned is controlled by $this->maxConflicts. https://github.com/wikimedia/mediawiki-extensions-Wikibase/blob/master/lib/includes/store/sql/TermSqlIndex.php#L830-L831 i would like to add test cases for this and poke some more, though not sure how much we can 'fix' the current wb_terms and TermSqlIndex for this vs. what daniel suggests. TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: aude Cc: Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T121395: [Bug] Term constraints not working
Bene added a subscriber: Bene. Bene added a comment. Both items have their labels tracked correctly in the `wb_terms` table. select * from wb_terms where term_entity_id in (3737270, 4115189) and term_language = 'en' TASK DETAIL https://phabricator.wikimedia.org/T121395 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: Bene Cc: Bene, Lydia_Pintscher, hoo, aude, Aklapper, daniel, Jonas, StudiesWorld, Wikidata-bugs, Mbch331 ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
