Add the ability to revoke Ship Its on published reviews.

Review Request #8995 — Created June 5, 2017 and submitted

Information

Review Board
release-3.0.x
14be143...

Reviewers

A long-requested feature has been to undo a Ship It that was posted
accidentally or prematurely. Such Ship Its could previously only be
negated by opening an issue (which had to be on a file or diff in
previous releases as well).

This change introduces this feature by offering a "X" button on the
"Ship It" label on reviews. When the owner of the change clicks the
button, they'll be prompted to confirm that they want to revoke the
Ship It. Once confirmed, an API request will be made that clears the
Ship It flag, alters the Ship It counter on the review request, and
also changes the "Ship It!" text on Ship It-only reviews to appear
with a strikethrough. This will be instantly shown on the review.

The Ship It on a "Fix it, then Ship It!" review can also be removed.
Hovering over the "Fix it!" label will cause the "Ship It" to slide
down, which may also help new users wondering what the little green
label sticking out might be. The "Ship It" there can be revoked the
same way as above.

During the process of revoking the Ship It, a "revoking" signal will
be emitted that extensions can hook into. They'll have the ability
to abort the revoke process if they choose. If nothing blocks it and
the revoke can go through, a "revoked" signal will also be emitted,
which is used just to notify callers.

Reviews that have had their Ship It revoked will also have a
"revoked_ship_it" key set in extra_data, which can be used by API
consumers and extensions for any analytics that might be needed.

Tested revoking Ship Its on reviews posted using the "Ship It"
button, reviews with custom body text, and reviews marking Ship It
with open issues.

Tested the presence of the button for different users and anonymous
users.

Python and JavaScript unit tests pass.


Description From Last Updated

Col: 56 Missing semicolon.

reviewbotreviewbot

Could be a tiny bit nicer as: setTimeout(() => this._clearRevokingShipIt(), 900);

daviddavid

typo: dict0

daviddavid

Should probably be "Keyword arguments" instead of "Dictionary arguments"

daviddavid

Trailing comma?

daviddavid

Need a trailing comma here, too.

daviddavid
Checks run (1 failed, 1 succeeded)
flake8 passed.
JSHint failed.

JSHint

chipx86
chipx86
david
  1. 
      
  2. Show all issues

    Could be a tiny bit nicer as:

    setTimeout(() => this._clearRevokingShipIt(), 900);
    
  3. reviewboard/webapi/resources/review.py (Diff revision 3)
     
     
    Show all issues

    typo: dict0

  4. reviewboard/webapi/resources/review.py (Diff revision 3)
     
     
    Show all issues

    Should probably be "Keyword arguments" instead of "Dictionary arguments"

  5. reviewboard/webapi/resources/review.py (Diff revision 3)
     
     
    Show all issues

    Trailing comma?

  6. 
      
chipx86
david
  1. 
      
  2. reviewboard/webapi/resources/review.py (Diff revisions 3 - 4)
     
     
    Show all issues

    Need a trailing comma here, too.

  3. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to release-3.0.x (537491b)