diff --git a/reviewboard/static/rb/css/pages/reviews.less b/reviewboard/static/rb/css/pages/reviews.less
index e41972b61b9594a7b0585309681d69313890da2d..e1a231aafb8f6aa4d06c20e3aedb7f235a1d824c 100644
--- a/reviewboard/static/rb/css/pages/reviews.less
+++ b/reviewboard/static/rb/css/pages/reviews.less
@@ -789,7 +789,7 @@
margin-bottom: 8px;
padding: 4px 8px;
position: relative;
- width: 4.5em;
+ width: 5.5em;
top: -8px;
.border-radius(@box-border-radius 0 @box-border-radius 0);
diff --git a/reviewboard/static/rb/js/views/reviewBoxView.js b/reviewboard/static/rb/js/views/reviewBoxView.js
index 3ebd6668504d899aff27d7117b5d48d714b2d0fa..cbfecc40a288d98456fe0ed6c26e821cb186e134 100644
--- a/reviewboard/static/rb/js/views/reviewBoxView.js
+++ b/reviewboard/static/rb/js/views/reviewBoxView.js
@@ -5,6 +5,10 @@
* comments, screenshots, and file attachments.
*/
RB.ReviewBoxView = RB.CollapsableBoxView.extend({
+ events: {
+ 'click .remove-shipit': '_removeShipIt'
+ },
+
initialize: function() {
this._$shipIt = null;
this._reviewReply = null;
@@ -31,7 +35,6 @@ RB.ReviewBoxView = RB.CollapsableBoxView.extend({
bugTrackerURL = reviewRequest.get('bugTrackerURL'),
review = this.model,
loadReviewID;
-
RB.CollapsableBoxView.prototype.render.call(this);
// Expand the box if the review is current being linked to
@@ -48,6 +51,14 @@ RB.ReviewBoxView = RB.CollapsableBoxView.extend({
this._$banners = this.$('.banners');
this._$shipIt = this.$('.shipit');
+ if(this.model.attributes.shipIt){
+ this.listenToOnce(this.model, 'saved', function(){
+ if(!this.model.shipIt){
+ this._removeShipItLabel();
+ }
+ });
+ }
+
_.each(this.$('.review-comments .issue-indicator'), function(el) {
var $issueState = $('.issue-state', el),
issueStatus,
@@ -115,6 +126,8 @@ RB.ReviewBoxView = RB.CollapsableBoxView.extend({
bugTrackerURL: bugTrackerURL
});
});
+
+
},
/*
@@ -234,5 +247,29 @@ RB.ReviewBoxView = RB.CollapsableBoxView.extend({
.addClass('with-issues')
.text(gettext('Fix it, then Ship it!'));
}
+ },
+ /*
+ * Revokes the ship it label if the remove link is clicked
+ *
+ * This function changes the review model to reflect a ship-it
+ * value of false, and triggers a saved event
+ */
+ _removeShipIt: function() {
+ this.model.ready({
+ ready: function() {
+ this.model.set({shipIt: false});
+ this.model.save({shipIt: this.model.get('shipIt')});
+ var reviewRequest = this.model.get('parentObject');
+ }
+ }, this);
+ return false;
+ },
+ /*
+ * Removes the ship-it label and updates the review request's timestamp
+ */
+ _removeShipItLabel: function() {
+ var reviewRequest = this.model.get('parentObject');
+ reviewRequest.markUpdated(this.model.get('timestamp'));
+ this._$shipIt.remove();
}
});
diff --git a/reviewboard/templates/reviews/boxes/review.html b/reviewboard/templates/reviews/boxes/review.html
index 0008e2962a1b7129fb6358b04b7dfd42d6bb634b..c323633a199bd74f9e43ab2be62b18b2a72e2621 100644
--- a/reviewboard/templates/reviews/boxes/review.html
+++ b/reviewboard/templates/reviews/boxes/review.html
@@ -15,7 +15,7 @@
{% if entry.issue_open_count > 0 %}
{% trans "Fix it, then Ship it!" %}
{% else %}
- {% trans "Ship it!" %}
+
{% endif %}
{% endif %}
diff --git a/reviewboard/webapi/resources/base_review.py b/reviewboard/webapi/resources/base_review.py
index bb263c0478f5c43300c0291fd0c3d7c11bbd7989..a9befefe0026abf00e8f87c486b3a0709f6c5cfc 100644
--- a/reviewboard/webapi/resources/base_review.py
+++ b/reviewboard/webapi/resources/base_review.py
@@ -1,6 +1,7 @@
from __future__ import unicode_literals
from django.core.exceptions import ObjectDoesNotExist
+from djblets.db.fields import CounterField
from django.db.models import Q
from django.utils import six
from djblets.util.decorators import augment_method_from
@@ -177,6 +178,15 @@ class BaseReviewResource(MarkdownFieldsMixin, WebAPIResource):
# This will be overridden by MarkdownFieldsMixin.
return None
+ def has_revokable_shipit_permissions(self, request,
+ review, *args, **kwargs):
+ return ((review.user == request.user or request.user.is_superuser) and
+ review.review_request.is_accessible_by(request.user) and
+ review.public)
+
+
+
+
@webapi_check_local_site
@webapi_login_required
@webapi_response_errors(DOES_NOT_EXIST, NOT_LOGGED_IN, PERMISSION_DENIED)
@@ -245,7 +255,8 @@ class BaseReviewResource(MarkdownFieldsMixin, WebAPIResource):
The only special field is ``public``, which, if set to true, will
publish the review. The review will then be made publicly visible. Once
- public, the review cannot be modified or made private again.
+ public, the review cannot be modified or made private again
+ with the exception of removing a ship it from review.
"""
try:
resources.review_request.get_object(request, *args, **kwargs)
@@ -285,8 +296,21 @@ class BaseReviewResource(MarkdownFieldsMixin, WebAPIResource):
"""Common function to update fields on a draft review."""
if not self.has_modify_permissions(request, review):
# Can't modify published reviews or those not belonging
- # to the user.
- return self._no_access_error(request.user)
+ # to the user, unless we're revoking ship its
+ if self.has_revokable_shipit_permissions(request, review):
+ # Signifies that the ship it has a false value it
+ # has been revoked, so we decrement the counter
+ if 'ship_it' in kwargs and review.ship_it \
+ and not kwargs['ship_it']:
+ CounterField.decrement_many(
+ review.review_request,
+ {
+ 'shipit_count': 1,
+ })
+ else:
+ return self._no_access_error(request.user)
+ else:
+ return self._no_access_error(request.user)
if 'ship_it' in kwargs:
review.ship_it = kwargs['ship_it']
diff --git a/reviewboard/webapi/tests/test_review.py b/reviewboard/webapi/tests/test_review.py
index 83a0f79087b50a8c0088cb16875605352d7f49cd..742874c6fa6a2c10c39b87d388f0a9fcffb28644 100644
--- a/reviewboard/webapi/tests/test_review.py
+++ b/reviewboard/webapi/tests/test_review.py
@@ -263,6 +263,19 @@ class ResourceItemTests(ReviewItemMixin, ReviewRequestChildItemMixin,
{'ship_it': True},
expected_status=403)
+ def test_put_with_published_and_revoked_shipIts(self):
+ """Testing the PUT review-requests//reviews// API
+ with pre-published review and removing a ship-it
+ """
+ review_request = self.create_review_request(publish=True)
+ review = self.create_review(review_request, user=self.user,
+ ship_it=True, publish=True)
+
+ self.api_put(
+ get_review_item_url(review_request, review.id),
+ {'ship_it': False},
+ expected_mimetype=review_item_mimetype)
+
@add_fixtures(['test_site'])
def test_put_publish(self):
"""Testing the PUT review-requests//reviews//?public=1 API"""
diff --git a/reviewboard/webapi/tests/test_review_request.py b/reviewboard/webapi/tests/test_review_request.py
index bf4d33575ec4b4c0a2b6ccf1e265ed8399dc30f5..114b21c090cb457938d0dfb121c9c1950c3937f8 100644
--- a/reviewboard/webapi/tests/test_review_request.py
+++ b/reviewboard/webapi/tests/test_review_request.py
@@ -17,12 +17,14 @@ from reviewboard.site.models import LocalSite
from reviewboard.webapi.errors import INVALID_REPOSITORY
from reviewboard.webapi.resources import resources
from reviewboard.webapi.tests.base import BaseWebAPITestCase
-from reviewboard.webapi.tests.mimetypes import (review_request_item_mimetype,
+from reviewboard.webapi.tests.mimetypes import (review_item_mimetype,
+ review_request_item_mimetype,
review_request_list_mimetype)
from reviewboard.webapi.tests.mixins import BasicTestsMetaclass
from reviewboard.webapi.tests.mixins_extra_data import (ExtraDataItemMixin,
ExtraDataListMixin)
from reviewboard.webapi.tests.urls import (get_repository_item_url,
+ get_review_item_url,
get_review_request_item_url,
get_review_request_list_url,
get_user_item_url)
@@ -564,6 +566,25 @@ class ResourceListTests(SpyAgency, ExtraDataListMixin, BaseWebAPITestCase):
extra_query=Q(shipit_count__gt=0))
self.assertEqual(len(rsp['review_requests']), q.count())
+ # Tests for determining if ship it counter works with removable ship-its
+ def _setup_removable_ship_it_count_tests(self):
+ review_request = self.create_review_request(publish=True)
+ review= self.create_review(review_request, user=self.user,
+ ship_it=True, publish=True)
+ self.api_put(
+ get_review_item_url(review_request, review.id),
+ {'ship_it': False},
+ expected_mimetype=review_item_mimetype)
+ self.create_review(review_request, ship_it=True, publish=True)
+
+ def test_get_with_removable_ship_it_count_equals(self):
+ """Testing the GET review-requests/?ship-it-count= API
+ with removable ship it"""
+ self._setup_removable_ship_it_count_tests()
+ self._test_get_with_field_count('ship-it-count', 2, 0)
+ self._test_get_with_field_count('ship-it-count', 1, 1)
+
+
def test_get_with_time_added_from(self):
"""Testing the GET review-requests/?time-added-from= API"""
start_index = 3