"Trivial" publishes for review requests

Review Request #7022 - Created March 7, 2015 and submitted

Jessica Qian
Review Board
master
253d8e5...
reviewboard
Each time a review request is published, it will send an e-mail (assuming e-mails are turned on for the server as a whole). We want to add a checkbox with the label "Send E-Mail", in the green draft header. If the user un-checks this box before publishing, it will publish without sending an e-mail.

Tested with new diff, checked and unchecked for "Send E-Mail".

Tested with attachment only draft, checked and unchecked for "Send E-Mail".

  • 0
  • 0
  • 30
  • 38
  • 68
Description From Last Updated
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/settings.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/templates/reviews/review_header.html
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/settings.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/templates/reviews/review_header.html
    
    
  2. reviewboard/notifications/email.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (82 > 79 characters)
    
  3. reviewboard/reviews/signals.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (86 > 79 characters)
    
  4. reviewboard/settings.py (Diff revision 1)
     
     
     'django_reset' imported but unused
    
  5. reviewboard/settings.py (Diff revision 1)
     
     
     'from settings_local import *' used; unable to detect undefined names
    
  6. reviewboard/settings.py (Diff revision 1)
     
     
     'PIPELINE_JS' imported but unused
    
  7. reviewboard/settings.py (Diff revision 1)
     
     
     'PIPELINE_CSS' imported but unused
    
  8. reviewboard/settings.py (Diff revision 1)
     
     
    Col: 33
     W292 no newline at end of file
    
  9. Col: 18
     E231 missing whitespace after ':'
    
  10. 
      
Jessica Qian
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/settings.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/templates/reviews/review_header.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/settings.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/templates/reviews/review_header.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
  2. reviewboard/notifications/email.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (82 > 79 characters)
    
  3. reviewboard/reviews/signals.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (86 > 79 characters)
    
  4. reviewboard/settings.py (Diff revision 2)
     
     
     'django_reset' imported but unused
    
  5. reviewboard/settings.py (Diff revision 2)
     
     
     'from settings_local import *' used; unable to detect undefined names
    
  6. reviewboard/settings.py (Diff revision 2)
     
     
     'PIPELINE_CSS' imported but unused
    
  7. reviewboard/settings.py (Diff revision 2)
     
     
     'PIPELINE_JS' imported but unused
    
  8. reviewboard/settings.py (Diff revision 2)
     
     
    Col: 33
     W292 no newline at end of file
    
  9. Col: 18
     E231 missing whitespace after ':'
    
  10. 
      
Jessica Qian
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/templates/reviews/review_header.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/templates/reviews/review_header.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
  2. reviewboard/reviews/signals.py (Diff revision 3)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  3. reviewboard/reviews/signals.py (Diff revision 3)
     
     
    Col: 1
     W191 indentation contains tabs
    
  4. reviewboard/reviews/signals.py (Diff revision 3)
     
     
    Col: 15
     E128 continuation line under-indented for visual indent
    
  5. 
      
Jessica Qian
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/templates/reviews/review_header.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/templates/reviews/review_header.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
  2. 
      
David Trowbridge
  1. Can you attach a screenshot?

  2. We should hide this if siteconfig.get("mail_send_review_mail") is False.

  3. Please un-do this line.

  4. You could just add trivial=None in the argument list instead of this.

  5. 
      
Jessica Qian
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/templates/reviews/review_header.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/templates/reviews/review_header.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
  2. 
      
Christian Hammond
  1. 
      
  2. reviewboard/notifications/email.py (Diff revision 5)
     
     

    No parens after not. Should just be ... and not trivial.

  3. Space before the />.

  4. No comma after "not".

  5. 
      
Jessica Qian
Jessica Qian
Jessica Qian
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/templates/reviews/review_header.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/templates/reviews/review_header.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
  2. 
      
Barret Rennie
  1. 
      
  2. reviewboard/notifications/email.py (Diff revision 6)
     
     

    Can you change this to use single quotes?

    1. The other methods of email.py all use double quotes for siteconfig.get() method.
    2. In the past we have used both double and single quotes, but we are moving towards using only single quotes in the code base (with some exceptions).

  3. reviewboard/reviews/signals.py (Diff revision 6)
     
     

    Can you change these to use single quotes?

    1. The other methods of signals.py all use double quotes.

  4. reviewboard/reviews/views.py (Diff revision 6)
     
     

    Single quotes.

    1. The other methods of views.py all use double quotes for siteconfig.get() method.

  5. See next issue. This should be something like !this.$('#not-trivial').prop('checked') to make more sense semantically.

  6. So the property is trivial and the id for the checkbox is #trivial. However, the label associated for the checkbox is "Send E-Mail."

    Semantically, I believe that the ID for the field should be #not-trivial so that the state of the check box is not the inverse of the state of the publish being trivial.

  7. This should read something like "Determines if the review request's publish will not send an E-Mail."

  8. 
      
Jessica Qian
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/templates/reviews/review_header.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/templates/reviews/review_header.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
  2. 
      
David Trowbridge
  1. 
      
  2. reviewboard/reviews/views.py (Diff revision 7)
     
     

    Can we call this context varable send_email?

  3. Because trivial is internally a boolean value, can you use options.trivial ? 1 : 0 ?

  4. Because trivial is internally a boolean value, can you use options.trivial ? 1 : 0 ?

  5. Please verify that trivial is false when e-mail is turned off (in case other people add handlers for the signal on the backend).

  6. reviewboard/templates/reviews/review_header.html (Diff revision 7)
     
     
     
     
     

    Can you test to make sure that there are no javascript errors if e-mail is turned off?

  7. 
      
Jessica Qian
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        solution_template.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        solution_template.py
    
    
    WARNING: Number of comments exceeded maximum, showing 30 of 38.
  2. solution_template.py (Diff revision 8)
     
     
    Col: 56
     W291 trailing whitespace
    
  3. solution_template.py (Diff revision 8)
     
     
    Col: 8
     W291 trailing whitespace
    
  4. solution_template.py (Diff revision 8)
     
     
    Col: 38
     E226 missing whitespace around arithmetic operator
    
  5. solution_template.py (Diff revision 8)
     
     
    Col: 74
     W291 trailing whitespace
    
  6. solution_template.py (Diff revision 8)
     
     
    Col: 21
     E231 missing whitespace after ','
    
  7. solution_template.py (Diff revision 8)
     
     
    Col: 35
     E226 missing whitespace around arithmetic operator
    
  8. solution_template.py (Diff revision 8)
     
     
    Col: 25
     E231 missing whitespace after ','
    
  9. solution_template.py (Diff revision 8)
     
     
    Col: 22
     E225 missing whitespace around operator
    
  10. solution_template.py (Diff revision 8)
     
     
    Col: 34
     E226 missing whitespace around arithmetic operator
    
  11. solution_template.py (Diff revision 8)
     
     
    Col: 34
     E226 missing whitespace around arithmetic operator
    
  12. solution_template.py (Diff revision 8)
     
     
    Col: 18
     W291 trailing whitespace
    
  13. solution_template.py (Diff revision 8)
     
     
    Col: 67
     E225 missing whitespace around operator
    
  14. solution_template.py (Diff revision 8)
     
     
    Col: 80
     E501 line too long (99 > 79 characters)
    
  15. solution_template.py (Diff revision 8)
     
     
    Col: 24
     E228 missing whitespace around modulo operator
    
  16. solution_template.py (Diff revision 8)
     
     
    Col: 65
     E228 missing whitespace around modulo operator
    
  17. solution_template.py (Diff revision 8)
     
     
    Col: 26
     E225 missing whitespace around operator
    
  18. solution_template.py (Diff revision 8)
     
     
    Col: 46
     E231 missing whitespace after ','
    
  19. solution_template.py (Diff revision 8)
     
     
    Col: 50
     E226 missing whitespace around arithmetic operator
    
  20. solution_template.py (Diff revision 8)
     
     
    Col: 55
     E226 missing whitespace around arithmetic operator
    
  21. solution_template.py (Diff revision 8)
     
     
    Col: 38
     E226 missing whitespace around arithmetic operator
    
  22. solution_template.py (Diff revision 8)
     
     
    Col: 44
     E226 missing whitespace around arithmetic operator
    
  23. solution_template.py (Diff revision 8)
     
     
    Col: 71
     E228 missing whitespace around modulo operator
    
  24. solution_template.py (Diff revision 8)
     
     
    Col: 30
     E225 missing whitespace around operator
    
  25. solution_template.py (Diff revision 8)
     
     
    Col: 68
     E226 missing whitespace around arithmetic operator
    
  26. solution_template.py (Diff revision 8)
     
     
    Col: 80
     E501 line too long (105 > 79 characters)
    
  27. solution_template.py (Diff revision 8)
     
     
    Col: 28
     E228 missing whitespace around modulo operator
    
  28. solution_template.py (Diff revision 8)
     
     
    Col: 73
     E225 missing whitespace around operator
    
  29. solution_template.py (Diff revision 8)
     
     
    Col: 25
     E226 missing whitespace around arithmetic operator
    
  30. solution_template.py (Diff revision 8)
     
     
    Col: 44
     E231 missing whitespace after ','
    
  31. solution_template.py (Diff revision 8)
     
     
    Col: 38
     E226 missing whitespace around arithmetic operator
    
  32. 
      
Jessica Qian
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/templates/reviews/review_header.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/templates/reviews/review_header.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
  2. 
      
David Trowbridge
  1. Hey Jessica,

    There are a bunch of open issues in old reviews. Can you go through and mark them as fixed/dropped as appropriate? That way new reviewers will know what to look at.

  2. 
      
David Trowbridge
  1. 
      
  2. The "for" name no longer matches the ID of the input element.

  3. 
      
Jessica Qian
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/templates/reviews/review_header.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/notifications/email.py
        reviewboard/webapi/resources/review_request_draft.py
        reviewboard/reviews/signals.py
        reviewboard/reviews/models/review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
        reviewboard/static/rb/js/models/reviewRequestEditorModel.js
        reviewboard/templates/reviews/review_header.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
    
    
  2. 
      
Barret Rennie
  1. 
      
  2. Undo this change. A trailing comma in a JS object causes an error in some browsers.

  3. 
      
Jessica Qian
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.5.x (051b134)
Loading...