Page MenuHomePhabricator

Implement Fallback version
Open, HighPublic

Description

This fallback has no limit checks .. it is a replacement of the current summary message "Item changed"

Template

Changed label, description and/or alias in XX languages

Example:
Changed label, description and/or alias in 60 languages

To do

Final AC

Given Item or Property UI page

When I update only some terms in different languages
Then I should see the following summary message in item's history and recent changes:
Changed label, description and/or alias in XX languages where XX is the number of distinct languages changed

When I update some other non-term parts
Then I should see the following summary message in item's history and recent changes:
Item changed


Given API user of wbeditentity api

When I update terms and other parts of entity
Then I should see the following summary message in item's history and recent changes:
Changed label, description and/or alias in XX languages, and other parts where XX is the number of distinct languages changed

Related Objects

View Standalone Graph
This task is connected to more than 200 other tasks. Only direct parents and subtasks are shown here. Use View Standalone Graph to show more of the graph.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

@alaa_wmde -- back to doing -- Does it mean there is available feedback please?

@Rosalie_WMDE not yet we don't have any feedback on https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/518721 so far.
I move this task back to Doing probably by mistake though.. the patch attached to it seems ready for some feedback. so move it back to Peer Review

ADR for changing ChangeOp is now approved. This can be restarted!

Change 519078 abandoned by Rosalie Perside (WMDE):
[WIP] Add definition of getState() ChangeOp method to classes Implementing ChangeOP

Reason:
New aproach: we will make changeOp return changeOpResult instead of giving changeOp states. See https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Wikibase/ /518721/

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

Change 517454 abandoned by Rosalie Perside (WMDE):
WIP. Add state concept to ChangeOp.

Reason:
New aproach: we will make changeOp return changeOpResult instead of giving changeOp states. See https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Wikibase/ /518721/

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

why is this in Stalled?

waiting on patches from parent task. Also did not want to have it in Doing with the parent task.

Change 524498 had a related patch set uploaded (by Rosalie Perside (WMDE); owner: Rosalie Perside (WMDE)):
[mediawiki/extensions/Wikibase@master] [WIP]Implement ChangeOpResult for ChangeOpAliases, ChangeOpDescription and ChangeOpLabel

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

Change 524498 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Implement ChangeOpResult for ChangeOpAliases, ChangeOpDescription and ChangeOpLabel

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

@alaa_wmde For the next change, I guess we should count the languages from the changeOps array then another change will be to modify the edit summary based on the counts result.
I am working on the language count now. Let me know if am missing something please.

Change 526672 had a related patch set uploaded (by Rosalie Perside (WMDE); owner: Rosalie Perside (WMDE)):
[mediawiki/extensions/Wikibase@master] Make ChangeOps::apply() return instance of changeOpResult

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

@alaa_wmde For the next change, I guess we should count the languages from the changeOps array then another change will be to modify the edit summary based on the counts result.

That makes a lot of sense to separate all things we need to do with ChangeOpResult related stuff, and then do the edit summary at the end 👍

Change 526672 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Make ChangeOps::apply() return instance of changeOpResult Add Test for ChangeOps::apply() return value

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

Change 529325 had a related patch set uploaded (by Rosalie Perside (WMDE); owner: Rosalie Perside (WMDE)):
[mediawiki/extensions/Wikibase@master] Make Summary class hold all languages of edit.

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

Change 529956 had a related patch set uploaded (by Rosalie Perside (WMDE); owner: Rosalie Perside (WMDE)):
[mediawiki/extensions/Wikibase@master] Introduce LanguageBoundChangeOpResult following ISP

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

Change 529325 abandoned by Alaa Sarhan:
[WIP]Make Summary class hold all languages of edit.

Reason:
Paired on a different approach with Rosalie, and agreed on abandoning this one

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

alaa_wmde updated the task description. (Show Details)Mon, Aug 19, 9:39 AM

(mimicking gerrit 😄)

Change 530116 had a related patch set uploaded (by Rosalie Perside (WMDE); owner: Rosalie Perside (WMDE)):
[mediawiki/extensions/Wikibase@master] Add service for counting distinct changed languages in a given changeOpResults tree.

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

Change 529956 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Introduce LanguageBoundChangeOpResult following ISP

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

Change 531296 had a related patch set uploaded (by Alaa Sarhan; owner: Alaa Sarhan):
[mediawiki/extensions/Wikibase@master] Introduce interfaces to enable visitor pattern for business logic on ChangeOpResult trees.

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

alaa_wmde added a subscriber: Rosalie_WMDE.
alaa_wmde updated the task description. (Show Details)Mon, Aug 26, 6:57 PM
alaa_wmde updated the task description. (Show Details)

Change 531296 abandoned by Alaa Sarhan:
Introduce interfaces to enable visitor pattern for business logic on ChangeOpResult trees.

Reason:
went with simpler solution

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

Change 530116 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Add service for counting distinct changed languages in a changeOpResults tree.

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

alaa_wmde removed alaa_wmde as the assignee of this task.Wed, Aug 28, 10:01 AM
alaa_wmde triaged this task as High priority.
alaa_wmde updated the task description. (Show Details)

Change 533261 had a related patch set uploaded (by Alaa Sarhan; owner: Alaa Sarhan):
[mediawiki/extensions/Wikibase@master] Use ChangedLanguagesCounter in EditEntity to generate fallback summary message

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

alaa_wmde updated the task description. (Show Details)Fri, Aug 30, 8:25 AM

@Lydia_Pintscher @Lea_WMDE
We have a question on this.

wbeditentity can be used to edit all parts of an entity. When parts other than terms have been edited through it (currently, only through API as in UI the new termbox will only do one batch for terms), it wasn't clear what the message should say then. We can do one of the following:

  • [cheapest, least ideal?] when there are any terms changed, then we use the fallback message. Otherwise, we use the old generic "Item changed" message.
  • [not that expensive, sounded decent]
    • check if only terms have been changed => use the fallback message in here
    • check if no terms has changed => use old generic "Item changed"
    • if both terms and other parts changed => use a compound comment.. e.g. combining old generic one with the new fallback for terms into smth like "Changed label, description and/or aliases in 3 languages, and change other parts"

We want to go with the second one, but would love to confirm with you on the general idea, and perhaps if you have an opinion on the combined message wording.

Change 533522 had a related patch set uploaded (by Alaa Sarhan; owner: Alaa Sarhan):
[mediawiki/extensions/Wikibase@master] Make all other ChangeOp implementations return GenericChangeOpResult

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

alaa_wmde updated the task description. (Show Details)Fri, Aug 30, 1:51 PM

We want to go with the second one, but would love to confirm with you on the general idea, and perhaps if you have an opinion on the combined message wording.

Yes agreed. We definitely need to avoid the case where the edit summary implies only terms have been changed and then other parts of the entity have been changed as well. That'd screw up quick and easy patrolling.

As for the message: how about "Changed label, description and/or aliases in 3 languages, and other parts"?

Change 533898 had a related patch set uploaded (by Jakob; owner: Jakob):
[mediawiki/extensions/Wikibase@master] GenericChangeOpResult: add tests

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

Change 533522 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Make all other ChangeOp implementations return GenericChangeOpResult

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

Change 533898 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] GenericChangeOpResult: add tests

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

Change 534127 had a related patch set uploaded (by Alaa Sarhan; owner: Alaa Sarhan):
[mediawiki/extensions/Wikibase@master] Add a service to count language unbound changes in ChangeOpResult tree.

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

Change 534127 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Add a service to count non-language-bound changes in ChangeOpResult tree.

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

As for the message: how about "Changed label, description and/or aliases in 3 languages, and other parts"?

Sounds good to me!

alaa_wmde updated the task description. (Show Details)Wed, Sep 11, 1:34 PM

Change 533261 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Generate fallback summary messages based on changed parts on entity

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

Change 535906 had a related patch set uploaded (by Alaa Sarhan; owner: Alaa Sarhan):
[mediawiki/extensions/Wikibase@master] Move new translations to correct file and keys

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