Last modified: 2013-07-24 07:07:38 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T51699, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 49699 - VisualEditor: Add FlaggedRevs "accept pending revisions" to save dialog
VisualEditor: Add FlaggedRevs "accept pending revisions" to save dialog
Status: RESOLVED FIXED
Product: VisualEditor
Classification: Unclassified
MediaWiki integration (Other open bugs)
unspecified
All All
: High enhancement
: VE-deploy-2013-07-25
Assigned To: Roan Kattouw
:
: 50852 51833 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-17 16:10 UTC by James Forrester
Modified: 2013-07-24 07:07 UTC (History)
10 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description James Forrester 2013-06-17 16:10:45 UTC
Alongside the "minor" and "watch" checkboxes in the save dialog, for pages with FlaggedRevs switched on and outstanding unaccepted revisions, add a checkbox to "accept pending revisions".
Comment 1 James Forrester 2013-07-06 21:21:39 UTC
*** Bug 50852 has been marked as a duplicate of this bug. ***
Comment 2 James Alexander 2013-07-06 21:51:16 UTC
Just as an addition to this: I'm not sure if just the checkbox is what is needed. The current work flow has a whole separate page because we need to show the diff involved for the review. I imagine that can be done without going to a new page but either way they should be able to review the diff (and preferably leave a comment like you can in the review page now) before checking any kind of approve box since they become somewhat responsible for those edits.
Comment 3 Risker 2013-07-06 22:06:27 UTC
(In reply to comment #0)
> Alongside the "minor" and "watch" checkboxes in the save dialog, for pages
> with
> FlaggedRevs switched on and outstanding unaccepted revisions, add a checkbox
> to
> "accept pending revisions".

I don't find that an acceptable solution, because there is a requirement that reviewers actually review the edit they are accepting; they cannot do that if they're just ticking a box.  

See https://commons.wikimedia.org/wiki/File:Cropped_edit_source_admin_edit_with_existing_PC.png for what the screen looks like using Edit Source.  Note that the diff includes ALL unreviewed changes.
Comment 4 James Forrester 2013-07-06 22:17:02 UTC
(In reply to comment #2)
> Just as an addition to this: I'm not sure if just the checkbox is what is
> needed. The current work flow has a whole separate page because we need to
> show the diff involved for the review. I imagine that can be done without
> going to a new page but either way they should be able to review the diff
> (and preferably leave a comment like you can in the review page now) before
> checking any kind of approve box since they become somewhat responsible for
> those edits.

Yes, indeed; I was imagining:

* on load, an alert in the edit notices saying "You will need to accept the outstanding changes before saving" (or whatever the current message is) - possibly with a link to the below?
* on entering the save dialog, the save button will be disabled with another copy of the "You will need to accept the outstanding changes before saving" text (in the same place as the edit conflict / CAPTCHA / DOM corruption warnings)
* in the line of buttons ("Save page", "Review your changes") there should be a third button, "Review outstanding changes", which on click pops up the diff of outstanding changes, with a text box for the optional review comment and an "accept these changes" button that takes you back to the save dialog, now with the save button enabled.

Feels a little clunky, though - perhaps we could make the process in-line with the edit summary message?
Comment 5 Krinkle 2013-07-16 20:55:09 UTC
(In reply to comment #4)
> (In reply to comment #2)
> > Just as an addition to this: I'm not sure if just the checkbox is what is
> > needed. The current work flow has a whole separate page because we need to
> > show the diff involved for the review. I imagine that can be done without
> > going to a new page but either way they should be able to review the diff
> > (and preferably leave a comment like you can in the review page now) before
> > checking any kind of approve box since they become somewhat responsible for
> > those edits.
> 
> Yes, indeed; I was imagining:
> 
> * on load, an alert in the edit notices saying "You will need to accept the
> outstanding changes before saving" (or whatever the current message is) -
> possibly with a link to the below?
> * on entering the save dialog, the save button will be disabled with another
> copy of the "You will need to accept the outstanding changes before saving"
> text (in the same place as the edit conflict / CAPTCHA / DOM corruption
> warnings)
> * in the line of buttons ("Save page", "Review your changes") there should
> be a
> third button, "Review outstanding changes", which on click pops up the diff
> of
> outstanding changes, with a text box for the optional review comment and an
> "accept these changes" button that takes you back to the save dialog, now
> with
> the save button enabled.
> 
> Feels a little clunky, though - perhaps we could make the process in-line
> with
> the edit summary message?

Since this is going to need to be fully in javascript for VisualEditor, and the interface for this in FlaggedRevisions is already mostly in javascript (a small bit is done server-side through EditPage hooks), I recommend we not duplicate all of this logic and API handling for:
* "advertise you are reviewing this"
* API handling "leave comments"
 "get the diff against the latest stable version"
* "mark as reviewed" 
* etc.

And instead depend on the logic being in FlaggedRevisions in a re-usable way that we can just call from within VisualEditor.

Ideally they would also make their styling more pretty so that we can actually re-use it. Alternatively they'll have to make the logic re-usable separate from the UI as the current UI is not going to fit in (not just visually foreign to VisualEditor or usable interfaces in general, but also technically due to the DOM structure and layout properties).
Comment 6 Risker 2013-07-17 17:08:43 UTC
(In reply to comment #0)
> Alongside the "minor" and "watch" checkboxes in the save dialog, for pages
> with
> FlaggedRevs switched on and outstanding unaccepted revisions, add a checkbox
> to
> "accept pending revisions".

I note this has been posted as an enhancement; however, as flagged revisions are applied to *all* pages in article space on certain Wikipedias (de and ru in particular) I'd urge that this be resolved prior to introducing VisualEditor to those projects.  For them, the absence of this would be more than the minor inconvenience it is on English Wikipedia, where PC is applied to a very small number of pages and could be switched to semi-protection if really needed.
Comment 7 James Forrester 2013-07-17 17:12:33 UTC
(In reply to comment #6)
> I note this has been posted as an enhancement; however, as flagged revisions
> are applied to *all* pages in article space on certain Wikipedias (de and ru
> in particular) I'd urge that this be resolved prior to introducing VisualEditor
> to those projects. For them, the absence of this would be more than the minor
> inconvenience it is on English Wikipedia, where PC is applied to a very small
> number of pages and could be switched to semi-protection if really needed.

Absolutely.
Comment 8 James Forrester 2013-07-24 06:01:20 UTC
*** Bug 51833 has been marked as a duplicate of this bug. ***
Comment 9 Gerrit Notification Bot 2013-07-24 06:46:55 UTC
Change 75555 had a related patch set uploaded by Catrope:
Render check boxes from EditPage

https://gerrit.wikimedia.org/r/75555
Comment 10 Gerrit Notification Bot 2013-07-24 07:07:32 UTC
Change 75555 merged by jenkins-bot:
Render check boxes from EditPage

https://gerrit.wikimedia.org/r/75555
Comment 11 James Forrester 2013-07-24 07:07:38 UTC
Merged and will get deployed in a few hours' time.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links