421: Enhancement request: nag owners about stale review requests

shanka******@gmai***** (Google Code) (Is this you? Claim this profile.)
What's the URL of the page containing the problem?
N/A

What steps will reproduce the problem?
1. Open a review request
2. Let it sit around for months, without action
3. Reviewboard cluttered up!

What is the expected output? What do you see instead?

It would be nice if the server sent out daily nagging mail to everyone who
had a review open which has had no activity in more than a week or so;
reminding them to either mark it submitted or discarded.

Ideally, one mail per user containing all their stale requests, but one
mail per stale request is also ok, I suppose.
chipx86
#1 chipx86
This seems like a good candidate for an extension in 2.0.
  • -Type-Defect
    +Type-Enhancement
    +Milestone-Extensions
    +Component-EMail
    +Component-Reviews
#2 ryan.ga*******@gmai***** (Google Code) (Is this you? Claim this profile.)
An extension/plug-in sounds like a good idea.

I personally nag about once a week those with review requests that have been marked
with "Ship It!" for over 4 days or so but haven't been marked submitted yet.  

As such, I'd prefer to set up reviewboard to nag with this rule plus the one for
those not marked "Ship It!" but open (and perhaps inactive?) for over 1.5 weeks or so.

One other thing though, is that in setting up these rules, it would be nice if
reviewboard could observer "working" days (i.e. ignore weekends).  In general, many
reviews look older than they really are because reviewboard doesn't "ignore"
weekends.  e.g. you submit a review Friday, often no one will look at it until Monday
but by then it already looks old (red).  Perhaps this should be for a separate
enhancement request though as I'm sure there could be a lot of disagreement on it or
concerns about locales of groups (i.e. ignoring holidays).
#3 bens*****@yaho***** (Google Code) (Is this you? Claim this profile.)
I'd rather just have the ability in my incoming review page to filter out the reviews
marked with "Ship It!". Then it's up to the submitter to submit their reviews.
#4 kevin.******@beatpo****** (Google Code) (Is this you? Claim this profile.)
Isn't this the same as bug 221?
chipx86
#5 chipx86
Slightly different. This one is talking about automatic nagging of owners of open
review requests that are marked as "ship it" to tell them they should close it out in
some way.
chipx86
#6 chipx86
This actually just needs to be implemented as a management command and run from a
cron job. This could make it in before extensions. Let's say 1.1 optimistically,
though this may move to 1.2. If someone wants to contribute a patch, this will
certainly get in sooner.
  • -Milestone-Extensions
    +Milestone-Release1.1
#7 jeff****@gmai***** (Google Code) (Is this you? Claim this profile.)
I've created a standalone python script that does this and just uses the API URLs to figure everything out.  It currently 
nags you if you haven't commented on a review request (because it could be the reviewer who hasn't responded to your review's 
comments.)  It also doesn't look at age.  If you post a review at 9:59 and the script runs at 10, you'll get pinged.

It uses smtplib to send the mail.  No idea if this matches everyone's vision for this extension, but what I wrote works for 
our group.

I have a couple of hardcoded variables, our mail server and the RB URL, in the script.  Maybe that's ok, though, since it'll 
just be a cron job.  If hardcoded values work for you, it's already done.  =)  If you need it to be flexible and want to 
remove the hardcoded stuff, I will not be getting around to it any time soon...
chipx86
#8 chipx86
Hi Jeff,

If you don't mind licensing it as MIT, we can modify it, get it into shape, and get
it in for 1.1.
#9 jeff****@gmai***** (Google Code) (Is this you? Claim this profile.)
#  The MIT License
#  
#  Copyright (c) 2009 Jeffrey Lamb, Genie Industries
#  
#  Permission is hereby granted, free of charge, to any person obtaining a copy
#  of this software and associated documentation files (the "Software"), to deal
#  in the Software without restriction, including without limitation the rights
#  to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
#  copies of the Software, and to permit persons to whom the Software is
#  furnished to do so, subject to the following conditions:
#  
#  The above copyright notice and this permission notice shall be included in
#  all copies or substantial portions of the Software.
#  
#  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
#  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
#  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
#  AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
#  LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
#  OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
#  THE SOFTWARE.

import simplejson
from urllib import urlopen
import smtplib
import mimetypes
from email.MIMEMultipart import MIMEMultipart
from email.MIMEText import MIMEText

def sendMail(recipient, subject, htmltext, plaintext):
    UserName = 'reviewboard@review-board.org'
    
    msg = MIMEMultipart('related')
    msg['From'] = UserName
    msg['To'] = recipient
    msg['Subject'] = subject
    
    # Encapsulate the plain and HTML versions of the message body in an
    # 'alternative' part, so message agents can decide which they want to display.
    msgAlternative = MIMEMultipart('alternative')
    msg.attach(msgAlternative)
    
    # Plain text
    msgText = MIMEText(plaintext)
    msgAlternative.attach(msgText)

    # HTML text
    msgText = MIMEText(htmltext, 'html')
    msgAlternative.attach(msgText)

    mailServer = smtplib.SMTP('mail1.review-board.com')
    mailServer.sendmail(UserName, recipient, msg.as_string())
    mailServer.quit()

def main():
    allrevreq_results = urlopen('http://reviews.review-board.com/api/json/reviewrequests/all')
    allrevreq_json = simplejson.loads(allrevreq_results.read())
    allrevreq_results.close()

    for reviewreq in allrevreq_json['review_requests']:
        # Create our ship it status array
        reviewerReviewedStatus = []
        
        # append a 2 item array for each reviewer.  1 item for email, one for ship_it status
        for reviewers in reviewreq['target_people']:
            reviewerReviewedStatus.append([reviewers['email'],0])
        
        # Grab the reviews
        revreq_results = urlopen('http://reviews.review-
board.com/api/json/reviewrequests/'+str(reviewreq['id'])+'/reviews/')
        revreq_json = simplejson.loads(revreq_results.read())
        revreq_results.close()
        
        for review in revreq_json['reviews']:
            for reviewers in reviewerReviewedStatus:
                # Iterate through all reviewers for this review and find the one that posted this.
                if reviewers[0] == review['user']['email']:
                    # Set their reviewed status to 1
                    reviewers[1] = 1
        
        # Send an email if the ship_it status is 0
        for reviewers in reviewerReviewedStatus:
            if reviewers[1] == 0:
                print 'Emailing ' + str(reviewers[0]) + ' about review request #' + str(reviewreq['id']) + ' being 
stale.'
                sendMail(str(reviewers[0]), \
                'There\'s a review you haven\'t commented on.', \
                "Please look at review request #<a href=\"http://reviews.review-
board.com/r/"+str(reviewreq['id'])+"/\">"+str(reviewreq['id'])+"</a>.", \
                "Please look at review request #"+str(reviewreq['id']))

main()
chipx86
#10 chipx86
  • +Milestone-Release1.6
chipx86
#11 chipx86
  • +Priority-Low
chipx86
#12 chipx86
Pushing this out to RBTools. We should be able to create a useful script for doing this, which admins could then choose to use on the server.
  • -Component-EMail
    -Component-Reviews
    -Milestone-Release1.6
    +Component-RBTools
    +Milestone-RBTools-Release1.0
david
#13 david
  • -Milestone-RBTools-Release1.0
chipx86
#14 chipx86
  • +Project-RBTools
#15 da.****@gmai***** (Google Code) (Is this you? Claim this profile.)
This is tangential/related, I'd like to be able to abstain or remove myself from a review request. I would do this regularly for stale reviews but would occationaly do so for reviews I'm unqualified for or unable to complete for various reasons (vacation, too busy, changed teams, etc). 
david
#16 david
  • -reviewboard
    +rbtools
  • -Project:RBTools