Revokable ship it labels

Review Request #7170 - Created April 5, 2015 and updated

Rohan Meringenti
Review Board
master
dc70fe8...
reviewboard, students

Added removable ship it labels such that a ship it label can be revoked only by the person who endorsed it in the first place. To revoke a ship it, click
on the X to the right of a ship it label. Ship its can be revoked and the changes seen immediately. Furthermore, the ship-it counter corrresponding to the review request is duly updated(decremented) when a ship it is revoked.

Manual testing using different users and reviews, and checking if appropriate permissions are met.
Manual tests for ship its being revoked on click, and asynchronously. Furthermore, any checks to see if
review request is up to date returns true.
Unit tests for if ship it counter is being updated appropriately.

  • 2
  • 0
  • 19
  • 1
  • 22
Description From Last Updated
We should also be able to remove Ship-Its from "Fix it, then Ship it" reviews. Barret Rennie Barret Rennie
Col: 15 E225 missing whitespace around operator Review Bot Review Bot
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/base_review.py
    
    Ignored Files:
        reviewboard/templates/reviews/boxes/review.html
        reviewboard/static/rb/js/views/reviewBoxView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/base_review.py
    
    Ignored Files:
        reviewboard/templates/reviews/boxes/review.html
        reviewboard/static/rb/js/views/reviewBoxView.js
    
    
  2. Col: 13
     E115 expected an indented block (comment)
    
  3. 
      
Rohan Meringenti
Rohan Meringenti
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/base_review.py
    
    Ignored Files:
        reviewboard/templates/reviews/boxes/review.html
        reviewboard/static/rb/js/views/reviewBoxView.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/base_review.py
    
    Ignored Files:
        reviewboard/templates/reviews/boxes/review.html
        reviewboard/static/rb/js/views/reviewBoxView.js
    
    
  2. Col: 13
     E115 expected an indented block (comment)
    
  3. 
      
Xuanyi Lin
  1. 
      
  2. Missing one empty line

  3. Missing one space between the key and value

  4. Missing one empty line

  5. The import should be in alphabetical order

  6. 
      
Barret Rennie
  1. 
      
  2. How about just _revokeShipIt ?

  3. How about _removeShipItLabel ?

  4. We should also be able to remove Ship-Its from "Fix it, then Ship it" reviews.

  5. reviewboard/webapi/resources/base_review.py (Diff revision 2)
     
     
     
     

    This needs to be updated to reflect the fact that the review can be updated to revoke the ship-it ness of the review.

  6. reviewboard/webapi/resources/base_review.py (Diff revision 2)
     
     
     

    Why this logic?
    By the time we get here, we've already determined that the user has to have modify permissions on the review.

    Also, this makes the ship_it key mandatory for updating the review. I don't think that is necessary.

  7. reviewboard/webapi/resources/base_review.py (Diff revision 2)
     
     
     
     
     
     
     
     
     
     

    What if review.ship_it = False prior to assining to it? I believe this can result in incorrect ship-it counters.

    As I understand it, the ReviewResource (which inherits from BaseReviewResource) is also used for review drafts (the ReviewDraftResource is just a redirection to it). Won't toggling the shipit-ness of a draft review end up decrementing the ship-it count?

  8. Left over from debugging?

  9. 
      
Christian Hammond
  1. Make sure the students group is added! :)

  2. 
      
Rohan Meringenti
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/tests/test_review.py
        reviewboard/webapi/resources/base_review.py
        reviewboard/webapi/tests/test_review_request.py
    
    Ignored Files:
        reviewboard/templates/reviews/boxes/review.html
        reviewboard/static/rb/js/views/reviewBoxView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/tests/test_review.py
        reviewboard/webapi/resources/base_review.py
        reviewboard/webapi/tests/test_review_request.py
    
    Ignored Files:
        reviewboard/templates/reviews/boxes/review.html
        reviewboard/static/rb/js/views/reviewBoxView.js
    
    
  2. Col: 80
     E501 line too long (81 > 79 characters)
    
  3. Col: 80
     E501 line too long (86 > 79 characters)
    
  4. Col: 9
     E115 expected an indented block (comment)
    
  5. Col: 61
     E231 missing whitespace after ','
    
  6. Col: 80
     E501 line too long (84 > 79 characters)
    
  7. reviewboard/webapi/tests/test_review.py (Diff revision 3)
     
     
    Col: 38
     E127 continuation line over-indented for visual indent
    
  8. Col: 15
     E225 missing whitespace around operator
    
  9. 
      
Rohan Meringenti
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/tests/test_review.py
        reviewboard/webapi/resources/base_review.py
        reviewboard/webapi/tests/test_review_request.py
    
    Ignored Files:
        reviewboard/templates/reviews/boxes/review.html
        reviewboard/static/rb/js/views/reviewBoxView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/tests/test_review.py
        reviewboard/webapi/resources/base_review.py
        reviewboard/webapi/tests/test_review_request.py
    
    Ignored Files:
        reviewboard/templates/reviews/boxes/review.html
        reviewboard/static/rb/js/views/reviewBoxView.js
    
    
  2. Col: 15
     E225 missing whitespace around operator
    
  3. 
      
Rohan Meringenti
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/tests/test_review.py
        reviewboard/webapi/resources/base_review.py
        reviewboard/webapi/tests/test_review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/reviewBoxView.js
        reviewboard/templates/reviews/boxes/review.html
        reviewboard/static/rb/css/pages/reviews.less
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/tests/test_review.py
        reviewboard/webapi/resources/base_review.py
        reviewboard/webapi/tests/test_review_request.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/reviewBoxView.js
        reviewboard/templates/reviews/boxes/review.html
        reviewboard/static/rb/css/pages/reviews.less
    
    
  2. Col: 15
     E225 missing whitespace around operator
    
  3. 
      
Rohan Meringenti
Rohan Meringenti
Review request changed

Groups:

+reviewboard
Loading...