Revokable ship it labels
Review Request #7170 — Created April 5, 2015 and discarded
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.
| Description | From | Last Updated |
|---|---|---|
|
Col: 13 E115 expected an indented block (comment) |
 reviewbot
|
|
|
Missing one empty line |
XU xuanyi | |
|
How about just _revokeShipIt ? |
 brennie
|
|
|
Missing one space between the key and value |
XU xuanyi | |
|
Missing one empty line |
XU xuanyi | |
|
How about _removeShipItLabel ? |
brennie
|
|
|
We should also be able to remove Ship-Its from "Fix it, then Ship it" reviews. |
brennie
|
|
|
The import should be in alphabetical order |
XU xuanyi | |
|
This needs to be updated to reflect the fact that the review can be updated to revoke the ship-it ness … |
brennie
|
|
|
Why this logic? By the time we get here, we've already determined that the user has to have modify permissions … |
brennie
|
|
|
Col: 13 E115 expected an indented block (comment) |
reviewbot
|
|
|
What if review.ship_it = False prior to assining to it? I believe this can result in incorrect ship-it counters. As … |
brennie
|
|
|
Left over from debugging? |
brennie
|
|
|
Col: 80 E501 line too long (81 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (86 > 79 characters) |
reviewbot
|
|
|
Col: 9 E115 expected an indented block (comment) |
reviewbot
|
|
|
Col: 61 E231 missing whitespace after ',' |
reviewbot
|
|
|
Col: 80 E501 line too long (84 > 79 characters) |
reviewbot
|
|
|
Col: 38 E127 continuation line over-indented for visual indent |
reviewbot
|
|
|
Col: 15 E225 missing whitespace around operator |
reviewbot
|
|
|
Col: 15 E225 missing whitespace around operator |
reviewbot
|
|
|
Col: 15 E225 missing whitespace around operator |
reviewbot
|
- Summary:
-
Changed ship it label model[WIP] Revokable ship it labels
- Description:
-
[WIP] 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.
+ Updated model after model is revoked.
+ A few things to work out:
After the model is saved need to refresh that review to show that the label is no longer there Update ship it counters on all reviews page ~ Perhaps, on reviews that are purely just "Ship It" we can add revoked to body text ~ Perhaps, on reviews that are purely just "Ship It" we can add revoked to body text. + Style corrections on where X is placed.
- Change Summary:
-
Added Event Listeners for saved model. When Ship it is Revoked, decremented ship it counter.
- Description:
-
~ [WIP] 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.
~ [WIP] 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.
+ Ship its can now 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: ~ Updated model after model is revoked.
~ ~ A few things to work out:
~ Things to do still:
~ Style Remove on Ship it Label ~ Add tests - After the model is saved need to refresh that review to show that the label is no longer there - Update ship it counters on all reviews page - Perhaps, on reviews that are purely just "Ship It" we can add revoked to body text. - Style corrections on where X is placed. - Commit:
-
ff2ac9f1a66c8e24f748f996d74ad19c8a302c8505cb666cc8e20ae7fd53a824a48d3cea0580ec8a
-
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 -
-
-
-
-
-
This needs to be updated to reflect the fact that the review can be updated to revoke the ship-it ness of the review.
-
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_itkey mandatory for updating the review. I don't think that is necessary. -
What if
review.ship_it = Falseprior to assining to it? I believe this can result in incorrect ship-it counters.As I understand it, the
ReviewResource(which inherits fromBaseReviewResource) is also used for review drafts (theReviewDraftResourceis just a redirection to it). Won't toggling theshipit-ness of a draft review end up decrementing the ship-it count? -
- Change Summary:
-
Added Tests and fixed minor logic bugs
- Description:
-
~ [WIP] 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.
~ 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.
Ship its can now 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: ~ updated(decremented) when a ship it is revoked. Things to do still:
~ Style Remove on Ship it Label ~ Fix style on ship it label - Add tests - Testing Done:
-
~ Just manual testing done so far on simple creation and deletion of ship its
~ 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. - Commit:
-
05cb666cc8e20ae7fd53a824a48d3cea0580ec8af8364ef2cf182e773a055fc9b439fc9717d351e6
-
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 -
-
-
-
-
-
-
-
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 -
- Change Summary:
-
Fixed Style issues to ship it label
- Summary:
-
[WIP] Revokable ship it labelsRevokable ship it labels
- Description:
-
~ 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.
~ Ship its can now be revoked and the changes seen immediately. Furthermore, the ship-it counter corrresponding to the review request is duly ~ 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. - updated(decremented) when a ship it is revoked. - - Things to do still:
- Fix style on ship it label - Commit:
-
a14b981a1e87310fd99ffe76d97a9bbe3d6490a0dc70fe8bc926b8157675d78fcd9b27f6a2a9e2db
- Diff:
-
Revision 5 (+102 -7)
-
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 -
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