Page MenuHomePhabricator

Non-JS error handling when a conflict is submitted with no column selected
Open, Stalled, Needs TriagePublic3 Estimated Story Points

Description

Steps:

  • Check (and possibly fix) what happens if no side is selected, and the page submitted. It should not be emptied!
  • In the no-JS case, just reload the page if one of the radio buttons had no selection. Including the red markers (see the JS ticket for that).

Event Timeline

OK first things I want to mention here:

  • If the user does not select anything and submits the form then the default is that nothing happens. The article stays untouched. This is also true for a case where there is a third edit by another user while solving the conflict. In that case the article will then show the version with the third edit.
  • If the user has multiple selections and selects only one, then we will overwrite the article and blocks where nothing was selected will be removed.

I will first add a fallback for that in any case where the selection is incomplete, we will keep the article untouched.

@WMDE-Fisch Sorry to step on yr toes here, but I wanted to mention that I'm adding Less styles and JS for the other half of this task, so you might want to just do the server-side validation and set the mw-twocolconflict-no-selection class on the row.

If the user does not select anything and submits the form then the default is that nothing happens. The article stays untouched.

Does this mean that the user lost their changes?

If the user does not select anything and submits the form then the default is that nothing happens. The article stays untouched.

Does this mean that the user lost their changes?

Yes. But just to be aware here: I'll add this basic "last-resort" fallback that we at least do not blindly overwrite the current revision with something bogus. Using this as a basis I would add the detailed error handling that kicks in before that and could eventually completely replace the fallback.

I don't quite understand, but we have the option of not deploying any changes to make the radio buttons unselected, until after the correct server-side error handling is in place.

@WMDE-Fisch Sorry to step on yr toes here, but I wanted to mention that I'm adding Less styles and JS for the other half of this task, so you might want to just do the server-side validation and set the mw-twocolconflict-no-selection class on the row.

+1 good idea to split the work here.

Change 573544 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/TwoColConflict@master] Do not edit the page when selection is incomplete

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

Change 573555 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/TwoColConflict@master] Error style for conflict rows

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

Change 573555 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Error style for conflict rows

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

WMDE-Fisch moved this task from Doing to Sprint Backlog on the WMDE-QWERTY-Sprint-2020-02-19 board.
WMDE-Fisch subscribed.

Change 574799 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] [POC] Non-JS validation of a POSTed conflict resolution

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

Change 574813 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Remove extra submit flag in favor of content values

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

Change 574813 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Remove extra submit flag in favor of content values

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

thiemowmde subscribed.

This is in a bad position. The implementation turns out to be non-trivial no matter what we decide. Needs a product perspective. Needs iteration on UX requirements.

awight changed the task status from Open to Stalled.Mar 4 2020, 11:02 AM

This is in a bad position. The implementation turns out to be non-trivial no matter what we decide. Needs a product perspective. Needs iteration on UX requirements.

@Lena_WMDE @Hanna_Petruschat_WMDE ^ needs some input. Thanks!

Change 573544 abandoned by WMDE-Fisch:
Do not edit the page when selection is incomplete

Reason:
Fixed with a better approach in I2c8a403d6d2fb9fa3c481dc274312d921a31a715

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

Change 578322 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/TwoColConflict@master] [WIP][POC] Resume to conflict when incomplete

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

Change 578479 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/core@master] [WIP][POC] Force edit conflict to compare older versions

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

Together with @Lena_WMDE and @awight we decided on stalling that issue for the moment. Since all currently considered solutions would still need more work and might even still leave some uncertainty.

We also decided that for now, the default of pre-selecting the "other" side should be switch to the "your" side for the following reasons:

  • if the "other" side is pre-selected and users are blindly submitting the page their edits are completely lost
  • if the "your" side is pre-selected and users are blindly submitting the page the other edits are overwritten but at least could be recovered from the history

We also decided that for now, the default of pre-selecting the "other" side should be switch to the "your" side […]

That's a bad decision with ugly social, almost moral implications. I'm very sorry, but I can not support this. I will absolutely explain this, but would like to do this in person, not here.

Change 578516 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/TwoColConflict@master] Select the "your" side by default

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

Change 578516 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Select the "your" side by default

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

Finally, PM and I had a chance to talk. We decided to continue with the decision previously made (that is selecting the right side by default, as done by patch https://gerrit.wikimedia.org/r/578516), collect user input during the small default phase, and possibly change the decision before the feature becomes the default for all wikis. Main motivation for this way forward is that we want to collect user feedback for the "your side selected" solution right now, and possibly collect more user input for the "nothing selected" solution later.

Change 580177 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/TwoColConflict@master] Add browser test for nojs handling

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

Should this be brought into the current sprint? There are still a few points of work remaining for the browser test and maybe other final details.

thiemowmde changed the point value for this task from 8 to 3.

The browser test is done. Should we consider this done, or is there a specific cleanup you still want to do?

Change 580177 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Add browser test for nojs handling

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

The task is about the non-js error handling. So I guess its not done when looking at the bigger picture. But it's done for what we wanted to/could do for now. So if we close this ticket we should create at least a new one with a summary on the ideas / options.

Change 578322 abandoned by WMDE-Fisch:
[WIP][POC] Resume to conflict when incomplete

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

This needs demo instructions, and maybe should be declined. It's a bit tricky because the no-JS form doesn't allow this use case any more.

  • Disable Javascript in the browser.
  • Enter the conflict workflow.
  • Go into the browser and edit the DOM (!) to remove the "selected" attribute of any column selector.
  • Submit the form.

It should return exactly the same view, with contents unchanged, and all selectable rows highlighted in red. Call to action text should appear.

Stalled, so moved to watching. The only "new" thing that happened in this sprint is the browser test. Nothing to demo here.