Include repo name in review request emails

Review Request #3768 — Created Feb. 2, 2013 and submitted

Information

Review Board
master

Reviewers

Include repo name in review request emails

For Issue 2483. The review request email notification template was altered to include the repository name along with the review group. This was requested since it would be useful for groups with multiple repositories.

The template was altered so that if a repository is associated with the review request, then the name will appear in parentheses after the review group. If no repository was included, then nothing new will appear.
I set up emails to be sent through gmail smtp on my local test server and also created a repository to test with. I then created two review requests, one with the test repo and one with no repo. 

The emails for the test repo review request sent emails saying 'Review Request for Katkat (TestRepo) ' and the emails for the review request with no repo sent emails saying 'Review Request for Katkat'. This was the expected result.

Description From Last Updated

For HTML e-mails, we try to mimic the look of review request boxes. So we have some fields, like the …

chipx86chipx86

If there's a repository, it must have a name, so you don't need to check repository.name. You only need to …

chipx86chipx86

You already have a margin-top style on the parent element. I don't think you need this additionally on the

daviddavid

This can be removed. It's really only useful when we have whole sections with lots of content below it, but …

chipx86chipx86
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
      Ignored Files:
        reviewboard/templates/notifications/review_request_email.html
        reviewboard/templates/notifications/review_request_email.txt
    
    
  2. 
      
KA
SM
  1. This looks fine, but I'm not sure if it's the best way to list the repository. Maybe a separate line that says something like:
    
    when there is no repo:
        "Repository: None"
    or when there is:
        "Repository: <repo_name>.
  2. 
      
chipx86
  1. 
      
  2. Show all issues
    For HTML e-mails, we try to mimic the look of review request boxes. So we have some fields, like the Bugs field. We should be adding one for the Repository. That means it'll end up looking different from the .txt template, but that's okay.
    1. Where would you recommend putting the Repository field? Below the Bugs field, or somewhere else?
  3. Show all issues
    If there's a repository, it must have a name, so you don't need to check repository.name. You only need to do the outer check.
    
    I'm not sure that this is the right place to put it the repository name, though. This line can end up getting long, and there's no real context saying that the thing in parens is a repository name.
    
    I think what would work better is to have a separate line, if the repository exists, saying:
    
    "Repository: <name>"
  4. 
      
KA
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
      Ignored Files:
        reviewboard/templates/notifications/review_request_email.html
        reviewboard/templates/notifications/review_request_email.txt
    
    
  2. 
      
david
  1. 
      
  2. Show all issues
    You already have a margin-top style on the parent element. I don't think you need this additionally on the <b>
  3. 
      
chipx86
  1. One tiny thing, and then we're good! :)
  2. Show all issues
    This can be removed. It's really only useful when we have whole sections with lots of content below it, but in this case I see it as more of a field under the previous section.
  3. 
      
chipx86
  1. Would you be able to provide screenshots of both types of diffs?
    
    There's a handy URL you can use:
    
    http://<site>/r/<id>/preview-email/text/
    
    And:
    
    http://<site>/r/<id>/preview-email/html/
  2. 
      
KA
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
      Ignored Files:
        reviewboard/templates/notifications/review_request_email.html
        reviewboard/templates/notifications/review_request_email.txt
    
    
  2. 
      
mike_conley
  1. Looking at these screenshots....does anybody else feel like "Repository" should be the first thing? Before the review request description?
    1. I discussed this with Mike and David, the plan is to move both the Bugs and Repository sections to be above Description in both the html and text versions. 
    2. Maybe that should be done as part of this change, then. Get it all there in one commit.
  2. 
      
KA
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
      Ignored Files:
        reviewboard/templates/notifications/review_request_email.html
        reviewboard/templates/notifications/review_request_email.txt
    
    
  2. 
      
mike_conley
  1. This looks good to me. Thanks Katkat!
  2. 
      
KA
Review request changed
Status:
Completed
Change Summary:
Pushed to release-1.7.x (daf2016). Thanks!