787360 - Do not send emails on 1st level changes.

Status: RESOLVED FIXED

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

Description

[Kadir Topal [:atopal]]

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).

Comment 1

[Ricky Rosario [:rrosario, :r1cky]]

This looks like an easy condition to add. 1pt

Comment 2

[Joshua Smith [:joshua-s]]

I am interested in taking this, but I'd need help knowing what to modify.

Comment 3

[Kadir Topal [:atopal]]

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?

Comment 4

[Joshua Smith [:joshua-s]]

I would need assistance in navigating and modifying the code, so I don't know if I would really be a help...

Comment 5

[Berker Peksag [:berkerpeksag]]

(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

Comment 6

[Joshua Smith [:joshua-s]]

So would I change line 29 to:
> if ready_for_l10n and l10n.count() > 2:

Comment 7

[Ricky Rosario [:rrosario, :r1cky]]

(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.

Comment 8

[Joshua Smith [:joshua-s]]

Ya, I can't take this one...Sorry :(

Comment 9

[Shuhao]

I'll take this bug.

Comment 10

[Ricky Rosario [:rrosario, :r1cky]]

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.

Comment 11

[Shuhao]

PR: https://github.com/mozilla/kitsune/pull/1396

Comment 12

[Shuhao]

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

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

Comment 13

[Shuhao]

Just pushed to production.

Comment 14

[Andrew Truong [:feer56] [:andrewtruong] [:atruong]]

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.

Comment 15

[Ricky Rosario [:rrosario, :r1cky]]

(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.