Closed Bug 718456 Opened 12 years ago Closed 12 years ago

The "approved" mail should have the approver and the changes in the body

Categories

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

Tracking

(Not tracked)

VERIFIED FIXED
2012-01-31

People

(Reporter: atopal, Assigned: willkg)

Details

(Whiteboard: u=contributor c=wiki s=2012.2 p=1)

Currently we display the approver only in the subject line, but that gets cut off when the article name is too long. Please put the approver name in the body.

Also, the diff with the last approved version is never shown, not sure if it's a regression or if it never worked.
Whiteboard: u=contributor c=wiki s=2012.2 p= → u=contributor c=wiki s=2012.2 p=1
Grabbing this one.
Assignee: nobody → willkg
Just to be sure, this bug has probably the same cause as bug 705317
(In reply to Kadir Topal [:atopal] from comment #2)
> Just to be sure, this bug has probably the same cause as bug 705317

I'll look at that when working on the second part.
I'm working on the first issue and I can't reproduce the problem. It's entirely possible I don't understand what the steps involved are.

I:

1. went into the contributor dashboard
2. picked an article in the "needs review" section
3. clicked on the "Review" link in the status column
4. approved it with a message and got an email like this:

===
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Subject: Your revision has been approved: Will Firefox work on my mobile device?
From: notifications@support.mozilla.org
To: user463178@example.com
Date: Thu, 19 Jan 2012 19:50:20 -0000
Message-ID: <20120119195020.16084.74340@saturn.mv.mozilla.com>

Your revision has been reviewed.

wkahngreene has approved your revision to the document
Will Firefox work on my mobile device?.

Message from the reviewer:

sweet!

To view the history of this document, click the following
link, or paste it into your browser's location bar:

https://support-local.allizom.org/en-US/kb/will-firefox-work-my-mobile-devi=
ce/history
===

That shows who approved the change in the body of the message.

Does that sound like the right set of steps to reproduce?
Nevermind--Ricky walked me through the steps so I can reproduce the issue now.
(In reply to Kadir Topal [:atopal] from comment #0)
> Currently we display the approver only in the subject line, but that gets
> cut off when the article name is too long. Please put the approver name in
> the body.

I've got this fixed.


> Also, the diff with the last approved version is never shown, not sure if
> it's a regression or if it never worked.

It looks like the entire new revision is shown. Do you want me to take out the new revision and replace it with a diff between the approved revision and the previous one?
The mail should show the diff first and then the complete new revision, same as bug 705317. Maybe you can even lift the code verbatim. Currently the template says (if diff), but apparently the logic behind that is false, so there is never a diff to be included.
Priority: -- → P1
Code changes are now in a pull request:

https://github.com/mozilla/kitsune/pull/449
Priority: P1 → --
Whoops--nixed the Priority field by accident.
Priority: -- → P1
Fixed in master in https://github.com/mozilla/kitsune/commit/7aaa103 .

QA:

1. make sure account A has "email me when revisions are approved" checked
2. with account B, pick a kb article revision ready for review in the dashboard
3. approve the article
4. account A will get an email about the newly approved revision

The email that account A gets has:

1. the name of the approver at the top of the body of the email
2. the complete diff between the approved revision and the previous approved revision in the body of the email


Closing this out.

This ended up taking me a day or so and turned into a 2 pter because I was unfamiliar with how this stuff works and the code involved.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2012-01-31
I approved an edit and the email I got did not have the complete diff- is there another step I'm missing or a reason it'd be different on stage? Email text:

'Your revision has been reviewed.

rbillings has approved your revision to the document
absolutely.

Message from the reviewer:

two lines of change

To view the history of this document, click the following
link, or paste it into your browser's location bar:

https://support.allizom.org/en-US/kb/absolutely/history'
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Rebecca Billings from comment #11)
> I approved an edit and the email I got did not have the complete diff...

That is the email that goes out to the revision creator. This bug is about the email that goes out to users (usually localizers) that are watching for approvals (go to the https://support.allizom.org/contributors to sign up for them under "Email me when revisions are..."). One little caveat is that the user used to watch for approvals should be different than the user submitting the revision because in that case one of the emails gets skipped so we don't email the user twice.
Verified with user who signed up to get approval notices via the contributors dashboard
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.