Reviewers should be able to veto "Mark other translations as out of date"

VERIFIED FIXED in 1.3

Status

support.mozilla.org
Knowledge Base Software
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: cilias, Assigned: paulc)

Tracking

unspecified

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: sumo_only)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
If someone edits an article, and check marks "Mark other translations as out of date", the reviewer should be able to uncheck before approving it.

Updated

9 years ago
Assignee: nobody → smirkingsisyphus
Target Milestone: --- → 1.3

Comment 1

9 years ago
IMO when someone check marks that checkbox, approvers and locale leaders should also be warned by email, since that check could easily slip away unseen.

Comment 2

9 years ago
How could we make sure that the reviewer sees the option being used when reviewing the change? Where in the review process should it be shown? Right now there is just an "Approve changes" link.

How about this: when you're viewing a staging article (as a reviewer), at the top of the article we could show that checkbox and allow people to uncheck it before approving. Let me attach screenshot mockup of what I mean.

Comment 3

9 years ago
Created attachment 384372 [details]
Mockup UI

Rather than just showing a small line of text with a small link to "Approve changes", we could make it more of a full-size form with the ability to both veto a previous "significant edit" option, or add it (if the editor forgot to check it and it is indeed significant).

Comment 4

9 years ago
Created attachment 384373 [details]
If the editor didn't check the checkbox

This is how it would look like if the checkbox wasn't checked by the article editor. The reviewer can still check the checkbox before approving the changes.

Note also that several people could have made edits on an article before the reviewer approves/rejects. If person A edits and marks the edit as significant, then person B corrects a typo on the staging copy and doesn't mark his/her change as significant, the overall approve will still be significant, so the checkbox should be checked for the approver.

Comment 5

9 years ago
Can we decide - for example - that only an approver (and higher) can judge if
an edit is "significant"?

In other words: an editor changes the staging copy of a page, the approver
looks at the page and, if he decides that the changes is "significant", check
marks the "Mark other translations as out of date" box.

In my (personal) vision of the process, an editor never sees that checkbox with staging copies of an article.

Comment 6

9 years ago
Created attachment 384378 [details]
Off-topic: Article approval/rejection mockup

OK, I started to think some more about the functionality we'd like and created a mockup that doesn't just solve this bug, but rejecting edits as well.

* It allows you to select whether you want to approve or reject the edit
* If you approve it, you also have the ability to correct the (already entered) change summary - maybe the original person making the edits didn't specify a clear enough message? Having a clear summary of what was changes is especially important if the edit marks translations as outdated.
* If you reject it, you have the ability to specify why it was rejected. This message should of course be sent to the person(s) making the edit(s).

Submitting this patch here in lack of a better place. This fix would involve more than this bug.

Comment 7

9 years ago
Created attachment 384380 [details]
Off-topic: Article approval/rejection mockup

Better version.
Attachment #384378 - Attachment is obsolete: true

Comment 8

9 years ago
(In reply to comment #5)
> Can we decide - for example - that only an approver (and higher) can judge if
> an edit is "significant"?

What would be the benefit of that? I view the editor's checkbox as a "nomination", and ultimately it should be the reviewer that has to decide (as shown in the mockup I made). 

We could remove the possibility for an editor to check the checkbox altogether, but then we would lose the ability for the editor to "alert" the reviewer. Essentially, the reviewer would then have to pay more attention so an important edit is really making translations outdated. Otherwise, an important edit could be made and no localizers would ever be notified.

So:

* by keeping the option for editors, the risk is that a reviewer doesn't pay attention to the checked checkbox and marks all translations as outdated by accident. 

* by removing the option for editors, the risk is that a reviewer doesn't notice that the edit is significant, and no localizers are informed of the change.

Chris, do you have any opinion here?

Comment 9

9 years ago
Sorry David,

The problem is only the fact that now this checkbox is too small and hidden to make a visible warn for the reviewer.

If this check triggers a notification and is visible enough (as it is with the mockup you made), there is no problem to leave it for both the editor and the reviewer.

Bye :-)
Yeah, so my comment above was assuming that the mockup I made and attached to this bug would be implemented. I totally agree that the situation today is not good. :) 

The question is if we still would like to remove the option to mark a translation as outdated for non-reviewers/admins even if the above UI is implemented, but my opinion is that it won't be necessary as long as there is a way for reviewers to veto it.
(Assignee)

Comment 11

9 years ago
As per attachment 384380 [details], the UI will be added in bug 446082.

I think the approach here should be to simply remove that option from editing staging pages. The "out of date" action can then only be taken on approval. Sound good?
Assignee: smirkingsisyphus → paul.craciunoiu
(Assignee)

Comment 12

9 years ago
Since bug 467554 removes the option, the ability to veto won't be required. Reviewers can decide when to mark articles out of date, which makes sense to me. If that's okay, then this bug will be solved in bug 467554.
(Assignee)

Comment 13

9 years ago
Calling this fixed as per bugs 467554 (this removes the ability to mark staging articles as out of date) and 446082 (this has the UI).
Henceforth, it's all up to the approver when to mark out of date. Sounds good?
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Chris, would you mind taking a look at this?  Thanks!
(Reporter)

Comment 15

9 years ago
I tried this on <https://support-stage.mozilla.org/en-US/kb/Closing+the+only+tab+closes+the+window>.

It seems to have worked, but the es version still doesn't have the warning. <https://support-stage.mozilla.org/en-US/kb/Cerrar+la+%C3%BAnica+pesta%C3%B1a+cierra+la+ventana?bl=n>
And the nl version
<https://support-stage.mozilla.org/en-US/kb/Cerrar+la+%C3%BAnica+pesta%C3%B1a+cierra+la+ventana?bl=n>.

Most other translations of that article have the warning. Maybe the reason it is not on the es and nl versions is due to another bug. I don't know.
(Assignee)

Comment 16

9 years ago
Strange indeed. Can you try on older articles? (before July 1st, my dump is old -- I'll try to update it Monday) and perhaps on sumotools (I'd like to not resync the stage database so often)

Post the articles here and I'll try to reproduce on my copy and see what's up.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 18

9 years ago
It seems like maybe this is a different bug -- some locales don't get a warning. I filed bug 512630 to reflect this.

I think is fixed. Only reviewers can mark articles as out of date on approving an article edit. Or do we want to provide a way to remove the warning? If so, does that functionality require removing it on a per-translation basis or for all translations?
(Assignee)

Comment 19

9 years ago
s/is fixed/this bug is fixed
(Assignee)

Updated

9 years ago
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
Chris, are we happy with this now, with the information from comment 18?
(Reporter)

Comment 21

9 years ago
(In reply to comment #20)
> Chris, are we happy with this now, with the information from comment 18?

Yes
Thx -- verified.
Status: RESOLVED → VERIFIED
Whiteboard: tiki_bug
No link to commit or patch attached. Can't upstream.

If this is part of staging & approval, it might already be upstreamed.
Whiteboard: tiki_bug → tiki_bug, tiki_discuss
(Assignee)

Comment 24

9 years ago
LPH: see comment 13
comment 11, comment 12, comment 18 all have links to relevant bugs, as well.
Will track the other bugs instead. This one is just a shell.
Whiteboard: tiki_bug, tiki_discuss → sumo_only
You need to log in before you can comment on or make changes to this bug.