Do not send emails on 1st level changes.

RESOLVED FIXED in 2013Q2

Status

support.mozilla.org
Knowledge Base Software
P3
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: atopal, Assigned: Pwnna)

Tracking

unspecified
2013Q2

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: u=contributor c=wiki p=1 s=2013.backlog)

(Reporter)

Description

6 years ago
Localizers can opt-in to receive emails when English articles are marked as ready for L10n. Please change this so they do not receive emails on trivial changes (the first of 3 levels).
This looks like an easy condition to add. 1pt
Whiteboard: u=contributor c=wiki p= → u=contributor c=wiki p=1
Priority: -- → P3
Target Milestone: --- → 2012Q4
I am interested in taking this, but I'd need help knowing what to modify.
(Reporter)

Comment 3

5 years ago
Joshua, can you specify what kind of information you are looking for? Are you unsure what the bug is about or do you need assistance to navigate the code?
Whiteboard: u=contributor c=wiki p=1 → u=contributor c=wiki p=1 s=2013.backlog
Target Milestone: 2012Q4 → 2013Q2
I would need assistance in navigating and modifying the code, so I don't know if I would really be a help...
(In reply to Joshua Smith [:joshua-s] from comment #4)
> I would need assistance in navigating and modifying the code, so I don't
> know if I would really be a help...

I think you need to change the context_dict() function in apps/wiki/events.py:

https://github.com/mozilla/kitsune/blob/master/apps/wiki/events.py#L29
https://github.com/mozilla/kitsune/blob/master/apps/wiki/events.py#L175
So would I change line 29 to:
> if ready_for_l10n and l10n.count() > 2:
(In reply to Joshua Smith [:joshua-s] from comment #6)
> So would I change line 29 to:
> > if ready_for_l10n and l10n.count() > 2:

This is a bit more complicated than that. There are different levels of changes (SIGNIFICANCES):
https://github.com/mozilla/kitsune/blob/master/apps/wiki/config.py#L141

Basically, we want to let the TYPO_SIGNIFCANCE revisions go through without email. But I'm pretty sure it's more complicated than just looking at that.
Ya, I can't take this one...Sorry :(
(Assignee)

Comment 9

5 years ago
I'll take this bug.
(Assignee)

Updated

5 years ago
Assignee: nobody → shwu
Pwnna points out that revisions have a `can_be_readied_for_localization()` method. It is used properly in the `mark_ready_for_l10n_revision()` view but not in the `review_revision()` view. Turns out that revision can be marked as ready during the review process too.

See:
https://github.com/mozilla/kitsune/blob/master/apps/wiki/views.py#L473
https://github.com/mozilla/kitsune/blob/master/apps/wiki/templates/wiki/includes/review_form.html#L49

There should be calls to `can_be_readied_for_localization()` around there.
(Assignee)

Comment 11

5 years ago
PR: https://github.com/mozilla/kitsune/pull/1396
Status: NEW → ASSIGNED
(Assignee)

Comment 12

5 years ago
Landed in: https://github.com/mozilla/kitsune/commit/1de9134f33d9a2c44d0abc0a1d4253b1b3653db3

Just needs to push to production! That will happen tomorrow, though.
(Assignee)

Comment 13

5 years ago
Just pushed to production.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
What is changed is that if you select level 1 change when reviewing the "ready for localization" tick box is removed, if this what we wanted? What I thought is going to happen is that we could still mark as ready but the email wont be sent - removing the tick box basically represents us avoiding it the main issue.
(In reply to Feer56 (Andrew T.) from comment #14)
> What is changed is that if you select level 1 change when reviewing the
> "ready for localization" tick box is removed, if this what we wanted? What I
> thought is going to happen is that we could still mark as ready but the
> email wont be sent - removing the tick box basically represents us avoiding
> it the main issue.

Level 1 changes are for typos, they never should be meant to be marked as ready for l10n. In fact, on the history page, you can't do it. It was a bug we were allowing it during the review process.
You need to log in before you can comment on or make changes to this bug.