flake8
passed.
JSHint
failed.
Add the ability to revoke Ship Its on published reviews.
Review Request #8995 — Created June 5, 2017 and submitted
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. |
 reviewbot
|
|
|
Could be a tiny bit nicer as: setTimeout(() => this._clearRevokingShipIt(), 900); |
 david
|
|
|
typo: dict0 |
david
|
|
|
Should probably be "Keyword arguments" instead of "Dictionary arguments" |
david
|
|
|
Trailing comma? |
david
|
|
|
Need a trailing comma here, too. |
david
|
- Change Summary:
-
Fixed a jshint error.
- Commit:
-
5f98fa7fbcf53ba6bfc601590bae639ccaf06f3233d22e7e3c2f05378aa39bfb0b4311c9d1948b55
- Diff:
-
Revision 2 (+699 -33)
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
- Change Summary:
-
Added a missing method that snuck into another change.
- Commit:
-
33d22e7e3c2f05378aa39bfb0b4311c9d1948b5556363d44ae2cf307ba88624ccb0acf1e7aba9050
- Diff:
-
Revision 3 (+704 -33)
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
- Change Summary:
-
- Fixed typos and improved docs.
- Added a trailing comma.
- Replaced a
_.bind()call with a fat arrow function.
- Commit:
-
56363d44ae2cf307ba88624ccb0acf1e7aba905014be1431bc6955a3560ab00d4cb99a07f08b822c
- Diff:
-
Revision 4 (+703 -33)
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.