@Jakob_WMDE I think if you wanted you could maybe make this a train blocker.

It's not the most crucial thing in the world because hopefully we don't have that many entities that have a matching label and description. Also, using wbsetdescription does work as a workaround. On the other hand if we can be sure to catch it before the train that would probably be for the best.

I notice that this is still blocking the train, but no one has addressed this yet.

From the task description:

I checked this is the case by reverting just this commit.

Is that a reasonable course of action we could take to remove this as a train blocker? i.e. reverting https://gerrit.wikimedia.org/r/506182 ?

@thcipriani yes it has been reverted already

In T222621#5164600, @alaa_wmde wrote:

@thcipriani yes it has been reverted already

ah, hadn't realized, thank you!

I went through the code and couldn't find anything that does a full check on the entity. I even wrote some tests to make sure it's not happening (Failed TDD I guess), then cherry-picked the given change that got reverted and couldn't reproduce it locally (tried, changing another language, tried changing it to different label in the same language that has the issue, tried introducing new issue), all work as expected (I didn't test anything in API, I tested everything in desktop GUI). Can I get more info on how to reproduce the issue?

Change 509135 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] Add tests in LabelDescriptionNotEqualValidator for cases that it's only checking one language

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

Sorry for the lack of explicit reproduction :/. The steps below use httpie and the mediawiki-docker-dev setup.

I didn't test anything in API, I tested everything in desktop GUI

I think in desktop GUI different endpoints are used (wbsetlabel, wbsetdescription) not wbeditentity. Here is are steps to reproduce; sorry that they weren't clearly there before.

1. in Wikibase git checkout 336207887deea6ad3644c222124233a132db7f58 At the time of writing this is: origin/wmf/1.34.0-wmf.4
2. make a new item e.g. http -f POST default.web.mw.localhost:8080/mediawiki/api.php action==wbeditentity new==item data=='{ "labels": { "en":{"language": "en", "value": "foo"} }, "descriptions": { "en":{"language": "en", "value": "foo"} } }' token=' \'
3. Apply the reverted patch git cherry-pick b71b5b2af611e593816bbe6b949e216b88e1dcf8
4. try and change the duplicated terms: http -f POST default.web.mw.localhost:8080/mediawiki/api.php action==wbeditentity id=Q314 data=='{ "labels": { "en":{"language": "en", "value": "foo"} }, "descriptions": { "en":{"language": "en", "value": "NOTfoo"} } }' token=' \'
5. Get returned error with info Label and description for language code en can not have the same value.

@alaa_wmde and I tested it with wmf.4 and master and with all the given acceptance criteria, also followed your steps to reproduce (did the API call using API sandbox though but it shouldn't make any difference) and still we can't reproduce it :'(

I was able to reproduce the issue, I needed to remove clear= bit. I double check where this is coming from

Thanks for digging into it so much. Seems perhaps somehow I wandered into a very edge case set-up by mistake :(. As I mentioned I only found it by testing our new termbox on a random item on beta; seems impossibly bad/good luck that I ran into it with the first request I made.

I dug deep into the issue and it's really interesting, first, if you try to edit another language, it doesn't fail using wbeditentity. This is an example I got:

{
    "entity": {
        "aliases": {},
        "claims": {},
        "descriptions": {
            "de": {
                "language": "de",
                "value": "foo"
            },
            "en": {
                "language": "en",
                "value": "foo0"
            }
        },
        "id": "Q29",
        "labels": {
            "de": {
                "language": "de",
                "value": "nfoo"
            },
            "en": {
                "language": "en",
                "value": "foo0"
            }
        },
        "lastrevid": 111,
        "sitelinks": {},
        "type": "item"
    },
    "success": 1
}

(I added de using httpie and wbeditentity later)

It also doesn't fail if instead of { "labels": { "en":{"language": "en", "value": "foo"} }, "descriptions": { "en":{"language": "en", "value": "NOTfoo"} } }, you set '{ "labels": { "en":{"language": "en", "value": "NOTfoo"} }, "descriptions": { "en":{"language": "en", "value": "foo"} } } which gives away why it's happening, the changeOps are atomic, the system tries to apply the changeOp that doesn't change anything (in our case label, and it fails. If you apply the description first, the entity is untangled and then you can simply apply the empty changeop. I can add a basic sanity check to shorten out and don't try to apply an empty change op and that would fix our very funny edge case.

Change 509373 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] Do not validate against same label and description when change is noop

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

Change 509374 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] Revert "Revert "Use LabelDescriptionNotEqualValidator in TermValidatorFactory""

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

Change 509373 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Do not validate against same label and description when change is noop

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

Change 509374 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Revert "Revert "Use LabelDescriptionNotEqualValidator in TermValidatorFactory""

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

Change 509135 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Add tests in LabelDescriptionNotEqualValidator for cases that it's only checking one language

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