Page MenuHomePhabricator

[Story] Create better edit summaries for wbeditentity API endpoint
Open, HighPublic13 Story Points

Description

Motivation
For mobile edits, we don't want to create every action atomically. Reasons for this are:

  • The number of edits currently created by one save creates rather cluttering watchlists
  • It would create a lot of requests, and this would increase needed band width
  • It would create "part error states", where some part of an edit is saved, but some other part is not

However, editors should still be able to understand as much as possible about an edit without having to look at the diff. Thus we need edit summaries that are as specific as possible, and at the same time not overwhelming.

As a Wikidata or a Wikipedia patroler (both are affected!)
I want to understand what an edit was about just by reading the edit summary
so that I identify vandalism right from my watchlist

Acceptance Criteria

  • Provide custom generated edit summaries, if only termbox contents was changed, and if no custom edit summary has been provided
    • As long as 5 or less terms (item, description or alias) were changed, display the expanded version of the edit summary (see definition below)
    • If more than 5 items were changed, use the shortened version. (see definition below)
    • If more than 50 languages received changes, use the fallback version. (see definition below)
  • If anything outside of the termbox was changed, don't use the new edit summary mechanism

Technical thoughts

  • In order to create a general "edit summary creation" point, the edit summary should be generated as part of the wbeditentity endpoint.
  • Dealing with change operations was a struggle in the lexeme project, maybe we can learn something from there?

Edit summary versions
Expanded version
Show each change seperated by comma, in the following format:
<Added/Changed/Removed> [<language code>] <label/description/alias>: <new label/description contents>,
Example:
Added [en] label: Frame of Notre-Dame de Paris, Changed [en] label: Maja, Added [fr] alias: Marie

The wording is attempted to be as close to the current edit summary formulation as possible

Shortened version
Changes for <all language codes that changed the same types>: Label, Description, Alias [whichever of these were changed] [whole line repeats for all variations changed of label, description, alias]
Example:
Changed in fr, es, en, pl zh: label, description, alias. Changed in it, hu: label

Fallback version
Changed label, description and/or alias [whichever of them has been touched anywhere] in XX languages
Example:
Chenged label, description and/or alias in 60 languages

Event Timeline

Lea_WMDE created this task.Apr 11 2019, 1:30 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 11 2019, 1:30 PM
Lea_WMDE updated the task description. (Show Details)Apr 11 2019, 1:46 PM
Lea_WMDE renamed this task from Create better edit summaries for wbeditentity endpoint to Create better Termbox edit summaries for wbeditentity endpoint.Apr 11 2019, 2:06 PM
Lea_WMDE updated the task description. (Show Details)
Lea_WMDE updated the task description. (Show Details)
Lea_WMDE moved this task from Incoming to Ready to estimate on the Wikidata-Campsite board.
Lydia_Pintscher moved this task from Ready to estimate to Needs Work on the Wikidata-Campsite board.
Lydia_Pintscher added a subscriber: Lydia_Pintscher.

Let's quickly check in on the shortened version of the edit summary.

alaa_wmde added a subscriber: alaa_wmde.EditedMay 13 2019, 6:23 AM

I'm adding another motivation for saving in one http request instead of separate ones, that is device-independent.

In short, one or more of the separate requests might fail due to validation errors. The other requests will pass and persist their data. However, our UI will show one failure message for whole save operation as having failed, which is not true. The editor will not know that some of their edits have actually been saved until they refresh the item page as the UI won't be updated with those persisted values.

(copy-paste a previous comment https://phabricator.wikimedia.org/T212869#5135995)

The problem comes from the fact that UI is making two separate requests for what appears to be an atomic action (save all changed terms) to the editor. While that haven't caused any issues before now, and probably was decided to be done that way so that changing labels, descriptions and aliases get their own edit revisions separate from each other (for easier rollbacks and more atomic history on level of term edits), bubbling up this kind of separation up to the UI is problematic in this case.
Why is that a problem now?
Let's assume that second request (description in this case) is "fixed" and will look at the parent revision id instead of baserevid and it will detect the duplication correctly and fail.
In that case, the label will be changed, but the description will not, which is not the right thing to do here from UX perspective. The editor is editing both, and press save expecting both to be saved or nothing to be saved if they did a mistake or didn't know they can't use same label and description. Their next step will be to go and rollback their edit to the label, that if they even noticed that it was saved after they get an error message (due to the failing second request)
How to solve UX issue?
If that part of editing does not work without javascript anyway, we can add some client-side validation. If not, then we have to do one request for saving (can we use wbeditentity endpoint instead for instance?).


of course, we can solve the issue I'm describing here by changing our UI to respond to these separate requests separately as they return and update accordingly. Though that's likely to be an epic work and not a trivial one. So I'd go for one request per "save" button (meaning all edited fields that button is supposed to save) for now.

alaa_wmde added a comment.EditedMay 15 2019, 8:47 AM

@alaa_wmde

  • checks if we can split changing summaries workding from the requirement of doing single request in UI to wbeditentity (i.e. wbeditentity generating separate revisions/summaries)

@Lydia_Pintscher

  • checks with @Lea_WMDE regarding the wording/detail of shortend version
Lea_WMDE updated the task description. (Show Details)May 15 2019, 9:52 AM
Lea_WMDE updated the task description. (Show Details)

@alaa_wmde: @Lydia_Pintscher and I synced on the text, the shortened version is updated now. Concerning

@alaa_wmde

  • checks if we can split changing summaries workding from the requirement of doing single request in UI to wbeditentity (i.e. wbeditentity generating separate revisions/summaries)

For the Termbox project, we are expecting one edit in total, not multiple ones, even if they come in a single request. One of the reasons for that: We don't want to deal with states where one request went through, but others did not.
If you feel strongly about wanting to go in that direction now, could you come to our storytime today at 4 pm?

WMDE-leszek renamed this task from Create better Termbox edit summaries for wbeditentity endpoint to Create better edit summaries for wbeditentity endpoint.May 15 2019, 2:30 PM
WMDE-leszek renamed this task from Create better edit summaries for wbeditentity endpoint to Create better edit summaries for wbeditentity API endpoint.

The actual limits after which we move from one version to another ( Expanded -> Shortend -> fallback) is to be checked and detailed during implementation.

For example: it could be that one might try to generate one version and fallback to the next only if the generated test exceeded storage limits.

WMDE-leszek set the point value for this task to 13.May 21 2019, 12:51 PM
alaa_wmde updated the task description. (Show Details)May 21 2019, 1:24 PM

The actual limits after which we move from one version to another ( Expanded -> Shortend -> fallback) is to be checked and detailed during implementation.

Why is that? Do you think it likely the backend will not be able to handle the requests of the acceptance criteria?
I would not want us to increase the limits. The 5 was chosen, because most activities are below that line, and 5 is still a "small enough" number to overlook. The 50 might actually be way too big.
So please don't increase either of the two numbers, and please stay with using number of terms/ languages as deciding factors

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

Change 518721 had a related patch set uploaded (by Alaa Sarhan; owner: Alaa Sarhan):
[mediawiki/extensions/Wikibase@master] Add ADR explaning the change to ChangeOp design.

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

The actual limits after which we move from one version to another ( Expanded -> Shortend -> fallback) is to be checked and detailed during implementation.

Why is that? Do you think it likely the backend will not be able to handle the requests of the acceptance criteria?
I would not want us to increase the limits. The 5 was chosen, because most activities are below that line, and 5 is still a "small enough" number to overlook. The 50 might actually be way too big.
So please don't increase either of the two numbers, and please stay with using number of terms/ languages as deciding factors

That was because we weren't sure about the limit as well during estimating and taking in this task. We can certainly stay with those numbers if that's important. Why is it important to stick to those numbers? I reckon one reason might be to keep it consistent between different edits, but can't see it as an important factor in here. I can be missing something else of course. Are there other reasons?

Rosalie_WMDE added a subscriber: Rosalie_WMDE.EditedJul 4 2019, 3:29 PM

@alaa_wmde and I made some changes to the ADR for this task here.

A short summary of the change.
  • We decided to proceed with the visitor design pattern (Stateless).
  • Extended the discussion dateline (Now, 2019-07-09).
  • Added Some implementation description to the choice made.

Change 518721 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Add ADR explaning the change to ChangeOp design.

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

Change 522154 had a related patch set uploaded (by Rosalie Perside (WMDE); owner: Rosalie Perside (WMDE)):
[mediawiki/extensions/Wikibase@master] [WIP]Add ChangeOpResult interface

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

Change 522430 had a related patch set uploaded (by Rosalie Perside (WMDE); owner: Rosalie Perside (WMDE)):
[mediawiki/extensions/WikibaseLexeme@master] ChangeOp::apply() should return ChangeOpResult

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

Change 522154 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Add ChangeOpResult interface

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

Change 522430 merged by jenkins-bot:
[mediawiki/extensions/WikibaseLexeme@master] ChangeOp::apply() should return ChangeOpResult

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

Change 524200 had a related patch set uploaded (by Rosalie Perside (WMDE); owner: Rosalie Perside (WMDE)):
[mediawiki/extensions/Wikibase@master] ChangeOp::apply() should return ChangeOpResult

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

Change 524200 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] ChangeOp::apply() should return ChangeOpResult

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

alaa_wmde renamed this task from Create better edit summaries for wbeditentity API endpoint to [Story ]Create better edit summaries for wbeditentity API endpoint.Jul 22 2019, 9:47 AM

In doing as it is a story and has parts of it in progress already

Sorry, this ticket slipped through for me :/ Where do we currently stand with the limitations? Did you come to strongly different conclusions than what is spelled out in the acceptance criteria?

We haven't yet reached a place to deal with limits. We are implementing the fallback version first, and once done with that we should next move to the shortend version where we will have to figure out the limits for it. Will ping you with conclusions once we have any then to get some product feedback on them.

DannyS712 renamed this task from [Story ]Create better edit summaries for wbeditentity API endpoint to [Story] Create better edit summaries for wbeditentity API endpoint.Fri, Jul 26, 3:36 PM
alaa_wmde triaged this task as High priority.Tue, Aug 20, 2:52 PM