Closed Bug 469188 Opened 11 years ago Closed 10 years ago

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

Categories

(support.mozilla.org :: Knowledge Base Software, task)

task
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: cilias, Assigned: paulc)

References

Details

(Whiteboard: sumo_only)

Attachments

(3 files, 1 obsolete file)

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.
Assignee: nobody → smirkingsisyphus
Target Milestone: --- → 1.3
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.
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.
Attached image 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).
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.
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.
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.
Better version.
Attachment #384378 - Attachment is obsolete: true
(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?
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.
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
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.
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
Closed: 11 years ago
Resolution: --- → FIXED
Chris, would you mind taking a look at this?  Thanks!
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.
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 → ---
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?
s/is fixed/this bug is fixed
Status: REOPENED → RESOLVED
Closed: 11 years ago10 years ago
Resolution: --- → FIXED
Chris, are we happy with this now, with the information from comment 18?
(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
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.