Page MenuHomePhabricator

Add link: Edits on Add link articles are automatically removed by subsequent Add link edits
Closed, ResolvedPublic

Description

Tested on testwiki wmf.5.

  1. On an article with suggested Add links, click on Edit tab link and open it in a different tab. So there are two tabs -the tab #1 in Add link mode and the tab # with VE mode.
  2. In the browser tab #2 (with the article opened in VE mode), make some edits. Publish them (save the edits).
  3. In the browser tab #1 (with the article opened in Add link mode), answer 'Yes' or 'No' and save your edits.

Saving Add link answers will remove the previous edits. The View history displays: Link suggestions: 0 accepted, 1 rejected, 1 skipped.

Screen Shot 2021-05-18 at 12.53.57 PM.png (1×2 px, 376 KB)

The links to compare the revisions:

Notes:

  • Tag Newcomer task Suggested: add links is added to an edit made outside the suggested Add link interface
  • there might be many ways when a user makes edits on an article that has a suggested Add link: find it randomly when reading and start editing; seeing an article in SE module and open it for editing in a separate window and edit it etc.

Event Timeline

This task might be a subtask of T269653: Add a link: edit mode toggle (machine suggestions & visual)
However, some of the specs on T269653 might need more details.
For example,

Users cannot combine link suggestions with other kinds of edits. They can do one or the other. If they leave "AI suggestions" mode, their progress is gone. Same with if they toggle over to visual or source mode, make some changes, and then toggle back to AI suggestions mode: their progress is gone.

Although, presently, the toggle between Edit and Edit source is disabled, there are other ways to make edits on the Add link articles; edits can be done before adding links as described in this task. In this case, for a user it's difficult to make a connection between two modes - and losing their progress seems abrupt.

Etonkovidova renamed this task from Add link: Edits on Add link articles are automatically removed by subseqeunt Add link edits to Add link: Edits on Add link articles are automatically removed by subsequent Add link edits.May 19 2021, 3:02 AM

Is it different from what happens when you have VE in both tabs? This sounds like a failure to raise an edit conflict, and I don't think we change anything related to that.

Is it different from what happens when you have VE in both tabs? This sounds like a failure to raise an edit conflict, and I don't think we change anything related to that.

I suppose we could look at the edit before save and inspect the previous edit (was it done with add links? and/or by the previous user?) and reject the edit if needed. But it sounds like more effort than it is worth, IMO. cc @MMiller_WMF

Add Link edits accidentally damaging the article is somewhat serious, IMO. VE should show an error to the user when the save failed due to an edit conflict - maybe the customizations to the save form somehow broke that.

kostajh triaged this task as Medium priority.May 26 2021, 1:20 PM

Add Link edits accidentally damaging the article is somewhat serious, IMO. VE should show an error to the user when the save failed due to an edit conflict - maybe the customizations to the save form somehow broke that.

The scenario @Etonkovidova listed has the same results with regular VisualEditor as it does when the AddLink interface is involved.

Some changes we could make:

  • before save, check that the content is from the latest revision and fail if there's a newer revision (would require instrumentation plus UI to inform the user about what is going on)
  • if the user has done reject/skip all actions (no saves) then load the most recent revision before saving the document to avoid overwriting an older document
MMiller_WMF raised the priority of this task from Medium to High.Jun 7 2021, 5:11 PM
kostajh added a subscriber: RHo.

@RHo the proposed change to the workflow (that the user doesn't see) is, when "Save" is pressed (the edit summary is loading), and again just before Publish is pressed (from the edit summary), we'd check to see if the suggestions being added correspond to the latest revision of the article.

If there's a mismatch, it means someone edited the article while the user had the add link interface open. What should the user see, and should we redirect the user back to Special:Homepage?

We can potentially try to fetch new link suggestions on demand if this scenario comes up. So the user sees a message like "The article was edited, do you want to get new suggestions for the updated article?", and in the background we call the link recommendation service. That could potentially return no results, but would probably be more likely to return new suggestions. The downside is that it would take up to 10 seconds to fetch suggestions. And the user would have to start the yes/no/skip process over again.

Maybe the simplest thing for now is to surface an error message to the user, and take them back to Special:Homepage.

@RHo the proposed change to the workflow (that the user doesn't see) is, when "Save" is pressed (the edit summary is loading), and again just before Publish is pressed (from the edit summary), we'd check to see if the suggestions being added correspond to the latest revision of the article.

If there's a mismatch, it means someone edited the article while the user had the add link interface open. What should the user see, and should we redirect the user back to Special:Homepage?

We can potentially try to fetch new link suggestions on demand if this scenario comes up. So the user sees a message like "The article was edited, do you want to get new suggestions for the updated article?", and in the background we call the link recommendation service. That could potentially return no results, but would probably be more likely to return new suggestions. The downside is that it would take up to 10 seconds to fetch suggestions. And the user would have to start the yes/no/skip process over again.

Maybe the simplest thing for now is to surface an error message to the user, and take them back to Special:Homepage.

Hi @kostajh - up to 10secs to return new suggestions seems like quite a long time, plus there being a chance that no new suggestions would be found. So I agree the better option is to show an error message and take them back to the homepage.
Maybe something like Another edit was while you were reviewing this article, and your changes could not be saved. with the CTA to go "Back to the homepage"?

Hi @kostajh - up to 10secs to return new suggestions seems like quite a long time, plus there being a chance that no new suggestions would be found. So I agree the better option is to show an error message and take them back to the homepage.
Maybe something like Another edit was while you were reviewing this article, and your changes could not be saved. with the CTA to go "Back to the homepage"?

That sounds good.

The default error flow in VisualEditor when a problem occurs after "Publish changes" is pressed is the following:

image.png (1×1 px, 1 MB)

So we would replace the text in the red box with Another edit was while you were reviewing this article, and your changes could not be saved. and the "Dismiss" with "Go back to Special:Homepage"? Would you still want the red box and icon styling?

Hi @kostajh - up to 10secs to return new suggestions seems like quite a long time, plus there being a chance that no new suggestions would be found. So I agree the better option is to show an error message and take them back to the homepage.
Maybe something like Another edit was while you were reviewing this article, and your changes could not be saved. with the CTA to go "Back to the homepage"?

That sounds good.
The default error flow in VisualEditor when a problem occurs after "Publish changes" is pressed is the following:

image.png (1×1 px, 1 MB)

So we would replace the text in the red box with Another edit was while you were reviewing this article, and your changes could not be saved. and the "Dismiss" with "Go back to Special:Homepage"? Would you still want the red box and icon styling?

Replacing the text and button sounds good, but without the red box and icon styling please. Alternatively, could we use the standard simple message dialog that we use when users come to an article without suggestions?

image.png (448×844 px, 32 KB)

Also, maybe it's better to have "back to suggested edits" as the CTA for consistency (with the dialog when there are no suggested links), and since on Mobile we want to return them to the full-screen suggested edits module rather than the homepage.

If we use the alert dialog we use elsewhere, it looks a little awkward stacked on top of the VE dialog

image.png (1×2 px, 516 KB)

IMO it would probably fit better to override the contents of the "Something went wrong" dialog shown in VE.

If we use the alert dialog we use elsewhere, it looks a little awkward stacked on top of the VE dialog

image.png (1×2 px, 516 KB)

IMO it would probably fit better to override the contents of the "Something went wrong" dialog shown in VE.

Ahh I was hoping it could appear in place of instead of being stacked on top of the VE dialog. Happy to go with your original plan of overriding the "Something went wrong" dialog then.

If we use the alert dialog we use elsewhere, it looks a little awkward stacked on top of the VE dialog

image.png (1×2 px, 516 KB)

IMO it would probably fit better to override the contents of the "Something went wrong" dialog shown in VE.

Ahh I was hoping it could appear in place of instead of being stacked on top of the VE dialog. Happy to go with your original plan of overriding the "Something went wrong" dialog then.

Hmm, well, I'll try first to hide the publish dialog and raise the alert dialog, hopefully that works OK.

Change 699409 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/VisualEditor@master] Introduce VisualEditorApiVisualEditorPreSaveHook

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

Change 699410 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/GrowthExperiments@master] [WIP] AddLink: Ensure revision has suggestions before saving

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

After reviewing the task, I did more testing I to see how (or if) VE raises conflict for normal edits. VE differentiates between edits done by one user making edits, say, in two different tabs and two different users trying to save edits on the same article.

For a user who does editing in two different tabs - the saving is straightforwardly logical. Only the most recent edit (based on the timestamp) will be saved - this is what @kostajh wrote in https://phabricator.wikimedia.org/T283109#7119068 . For 'Add link' tasks the behavior is exactly the same as for normal VE edits. This, in a sense, invalidates the issue. Arguably, some improvements should be made - from a user point of view, adding links task might look totally different from normal editing, so to see edits disappear is more confusing in this case. It'd be great if it'd be improved.

Presently for two different users making normal edits on the same article, the warning would appear - even if edits are just adding separate pieces of content which do not conflict per se.

Screen Shot 2021-06-11 at 12.12.51 PM.png (444×1 px, 73 KB)

I did not realize this before, but this is probably not an Add Image or VisualEditor issue but a MediaWiki (anti)feature that's planned to be removed: T175745: Do not overwrite edits when conflicting with self

I did not realize this before, but this is probably not an Add Image or VisualEditor issue but a MediaWiki (anti)feature that's planned to be removed: T175745: Do not overwrite edits when conflicting with self

It doesn't sound like the use case discussed there (T175745#6772289) of "clicking the back button and re-editing the page" is that relevant to this task, though.

It doesn't sound like the use case discussed there (T175745#6772289) of "clicking the back button and re-editing the page" is that relevant to this task, though.

From the server's POV it's the same thing: two subsequent edits from the same user, with the same base revision ID.

(On some browsers, anyway. Depends on whether the edit form is fetched again from the server when the back button is pressed, or loaded from bfcache which preserves form field values, including the hidden field holding the revision ID.)

The error in logstash on arwiki: https://logstash.wikimedia.org/goto/732fe88dea85f6d00a9d4cba55a931f1

normalized_message
[{reqId}] {exception_url}   UnexpectedValueException: Tried to submit link suggestion reviews for revision "50457154" which has already been reviewed.
exception.trace
from /srv/mediawiki/php-1.37.0-wmf.9/extensions/GrowthExperiments/includes/NewcomerTasks/AddLink/AddLinkSubmissionHandler.php(103)
#0 /srv/mediawiki/php-1.37.0-wmf.9/extensions/GrowthExperiments/includes/VisualEditorHooks.php(66): GrowthExperiments\NewcomerTasks\AddLink\AddLinkSubmissionHandler->run(Title, User, integer, NULL, array)
#1 /srv/mediawiki/php-1.37.0-wmf.9/includes/HookContainer/HookContainer.php(160): GrowthExperiments\VisualEditorHooks->onVisualEditorApiVisualEditorEditPostSave(MediaWiki\Page\PageIdentityValue, User, string, array, array, array, array)
#2 /srv/mediawiki/php-1.37.0-wmf.9/extensions/VisualEditor/includes/VisualEditorHookRunner.php(50): MediaWiki\HookContainer\HookContainer->run(string, array, array)
#3 /srv/mediawiki/php-1.37.0-wmf.9/extensions/VisualEditor/includes/ApiVisualEditorEdit.php(488): MediaWiki\Extension\VisualEditor\VisualEditorHookRunner->onVisualEditorApiVisualEditorEditPostSave(MediaWiki\Page\PageIdentityValue, User, string, array, array, array, array)
#4 /srv/mediawiki/php-1.37.0-wmf.9/includes/api/ApiMain.php(1714): ApiVisualEditorEdit->execute()
#5 /srv/mediawiki/php-1.37.0-wmf.9/includes/api/ApiMain.php(684): ApiMain->executeAction()
#6 /srv/mediawiki/php-1.37.0-wmf.9/includes/api/ApiMain.php(655): ApiMain->executeActionWithErrorHandling()
#7 /srv/mediawiki/php-1.37.0-wmf.9/api.php(90): ApiMain->execute()
#8 /srv/mediawiki/php-1.37.0-wmf.9/api.php(45): wfApiMain()
#9 /srv/mediawiki/w/api.php(3): require(string)
#10 {main}

@Etonkovidova this comment was intended for another task, right? I don't think the error is related to the edit conflict issue.

@Etonkovidova this comment was intended for another task, right? I don't think the error is related to the edit conflict issue.

I suppose it's related in the sense that the presave hook would prevent us from reaching this point.

Change 701376 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[schemas/event/secondary@master] link_suggestion_interaction: Add outdatedsuggestions_dialog interface

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

@Etonkovidova this comment was intended for another task, right? I don't think the error is related to the edit conflict issue.

I suppose it's related in the sense that the presave hook would prevent us from reaching this point.

There is now a dedicated task for it: T287103: UnexpectedValueException: Tried to submit link suggestion reviews which has already been reviewed.

Change 701376 merged by jenkins-bot:

[schemas/event/secondary@master] link_suggestion_interaction: Add outdatedsuggestions_dialog interface

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

Change 699409 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Introduce VisualEditorApiVisualEditorPreSaveHook

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

Change 699410 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] AddLink: Ensure revision has suggestions before saving

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

Checked in testwikiwmf.20 and tested betalabs with the latest patch. In a sense, the fix is in place - the issue when normal edits are overwritten with subsequent link recommendations edits is fixed.

However, the two scenarios ( below have some issues:
(1) editing a recommended link article independently from link recommendation mode

  • On Homepage a user sees the article "Title A" as a link recommendation SE task
  • does not click on it, but searches for the article "Title A" and edit it in a VE/Source editor - saves the edits
  • returns to Homepage. The article "Title A" is not removed from the list of suggested articles
  • clicking on it - will load "Back to suggested edits" message

(2) editing a recommended link article in a different tab

  • On Homepage a user sees "Title A" article as a link recommendation SE task
  • a user clicks on the card - "Title A" article opens in link recommendation mode
  • a user clicks on Edit tab and opens the article in VE mode in a different tab
  • user makes some edits in VE mode and saves them.
  • returns to the tab with "Title A" article is opened in link recommendation mode and answers 'yes'/'no' and tries to publish. The edit conflict message is displayed:

Screen Shot 2021-08-24 at 5.34.01 PM.png (1×2 px, 737 KB)

Checked in testwikiwmf.20 and tested betalabs with the latest patch. In a sense, the fix is in place - the issue when normal edits are overwritten with subsequent link recommendations edits is fixed.

🎉 thanks for the review!

However, the two scenarios ( below have some issues:
(1) editing a recommended link article independently from link recommendation mode

  • On Homepage a user sees the article "Title A" as a link recommendation SE task
  • does not click on it, but searches for the article "Title A" and edit it in a VE/Source editor - saves the edits
  • returns to Homepage. The article "Title A" is not removed from the list of suggested articles
  • clicking on it - will load "Back to suggested edits" message

Yeah, we haven't done anything about the cached articles for the user. I think it is too much effort for too little benefit, but we could monitor how often this error comes up and do something about it if it seems like it's a common problem?

(2) editing a recommended link article in a different tab

  • On Homepage a user sees "Title A" article as a link recommendation SE task
  • a user clicks on the card - "Title A" article opens in link recommendation mode
  • a user clicks on Edit tab and opens the article in VE mode in a different tab
  • user makes some edits in VE mode and saves them.
  • returns to the tab with "Title A" article is opened in link recommendation mode and answers 'yes'/'no' and tries to publish. The edit conflict message is displayed:

Screen Shot 2021-08-24 at 5.34.01 PM.png (1×2 px, 737 KB)

I think this works as expected -- is there another behavior you would expect?

(2) editing a recommended link article in a different tab

  • On Homepage a user sees "Title A" article as a link recommendation SE task
  • a user clicks on the card - "Title A" article opens in link recommendation mode
  • a user clicks on Edit tab and opens the article in VE mode in a different tab
  • user makes some edits in VE mode and saves them.
  • returns to the tab with "Title A" article is opened in link recommendation mode and answers 'yes'/'no' and tries to publish. The edit conflict message is displayed:

Screen Shot 2021-08-24 at 5.34.01 PM.png (1×2 px, 737 KB)

I think this works as expected -- is there another behavior you would expect?

Thanks! I realized that I forgot to add to the description of the cases that they are the edge cases. Generally, I agree that it's too much effort for too little benefit.

Regarding "this works as expected":

  • going through links recommendation and doing normal editing are two separate editing venues, so the edit conflict warning appears quite unexpectedly. I didn't create clever conflicts, e.g. when in VE I would add the same link that was suggested by the recommended link surface. The edit conflict warning appears for a simpler situation, i.e. some text is added to a page.
  • I checked how VE handles the edit conflict. If the same user does some edits in different tabs, it's not possible to see the edit conflict warning.

Since the scope of the task is done, I'm marking it as Resolved. I've added the task to the tasks that I'll be monitoring, in case, the scenarios described would happen often and interfere with newcomer editing workflows.

@Etonkovidova pointed out in T289722: [testwiki-wmf.20] Add link: Edit conflict is displayed when user attempts to submit recommended link edits after submitting edits in VE that this also happens when those two actions happen in the same tab, and that's pretty confusing behavior.

The scenario described here is fine, IMO. MediaWiki tries to handle potential edit conflicts by merging conflicting changes; that's complicated and we don't want to deal with it.

I checked how VE handles the edit conflict. If the same user does some edits in different tabs, it's not possible to see the edit conflict warning.

That's more of a bug than a feature IMO (or MediaWiki core; it's not VE-specific); and it might go away at some point, per T175745.