Include branch field in review request emails

Review Request #10147 — Created Sept. 16, 2018 and submitted

bolariinwa
Review Board
release-3.0.x
4707
f21537d...
reviewboard, students

With a lot of review request emails, prioritizing review requests emails
becomes uneasy.

Review request e-mails will now contain a branch field.

Tested by manually checking generated review request emails for the
inclusion of the branch field.

Loading file attachments...

  • 0
  • 0
  • 9
  • 0
  • 9
Description From Last Updated
brennie
  1. 
      
  2. Can you wrap your description at 72 characters instead of 100+?

    Same for testing done.

  3. Can you add a screenshot of the rendered HTML e-mail view with the new content?

  4. 
      
bolariinwa
bolariinwa
brennie
  1. 
      
  2. I know this is a small change, but can you take a peek at our guide for writing good change descriptions. Specifically, the description and testing done should consist of complete sentences.

    Also the description should read more like

    E-mails will now contain the contents of the branch field ....
    

    i.e., the sentences should be descriptive not summaritive.

  3. 
      
bolariinwa
chipx86
  1. 
      
  2. reviewboard/templates/notifications/review_request_email.txt (Diff revision 1)
     
     
     
     
     
     
     
     

    We probably want these one right after the other. Unfortunately, the Django templating language is kinda messy when it comes to newlines (since it's really built for HTML where it rarely matters). Each newline in there, including the ones after the template tags, will be included in the plain-text e-mail. That means you're going to get something like:

    Repository: my-repo
    
    
    
    Branch: my-branch
    
    
    Description
    -----------
    

    (This is only a problem in plain-text e-mails.)

    Instead, we'll want to collapse those. The good news is, we have a handy tag you can use. called condense. This would look like:

    {% condense 0 %}
    {%  if review_request.repository %}
    Repository: ...
    {%  endif %}
    {%  if review_request.branch %}
    Branch: ...
    {%  endif %}
    {% endcondense %}
    
    Description.....
    

    Make sure you run the preview-email view with this and check the output to see that there's consistency in spacing between each section and each important thing within it. We'll want to see that in the testing.

  3. 
      
bolariinwa
brennie
  1. 
      
  2. reviewboard/templates/notifications/review_request_email.html (Diff revision 2)
     
     
     
     
     
     
     
     
     
     

    Indentation here is wrong. We do our {% blocktag %} indentation inside the tags themselves and do not increase the indentation of nest tags.

    This should be formatted as:

    <div style="margin-top: 1.5em">
    {%   if review_request.repository %}
     <strong style="....">Repository: </strong>
     {{review_request.repository.name}}
    {%   endif %}
    {%   if review_request.branch %}
     <strong style="...">Branch: </strong>
     {{review_request.branch}}
    {%   endif %}
    </div>
    
  3. 
      
bolariinwa
david
  1. 
      
  2. This doesn't look right. What we want is:

    Repository: Review Board
    Branch: release-3.0.x
    
  3. It's also not quite right here. Probably need to include a <br>.

  4. 
      
bolariinwa
Review request changed

Change Summary:

Addressed issues

Commit:

-25652f9bb18e6a2fefa3993e3c03764d22a4d73c
+7942f7203bfd549abc3e5d51badaf2d073b9e270

Diff:

Revision 4 (+136 -1042)

Show changes

Added Files:

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

bolariinwa
david
  1. Ship It!
  2. 
      
bolariinwa
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (6f4a415)
Loading...