diff --git a/reviewboard/static/rb/js/resources/models/reviewRequestModel.es6.js b/reviewboard/static/rb/js/resources/models/reviewRequestModel.es6.js index 94b4a092b4a4456eda054bb6e8c90aafb47eb606..f9e647a3adc2629b69fa9f842e5853fd9505c29f 100644 --- a/reviewboard/static/rb/js/resources/models/reviewRequestModel.es6.js +++ b/reviewboard/static/rb/js/resources/models/reviewRequestModel.es6.js @@ -428,6 +428,9 @@ RB.ReviewRequest = RB.BaseResource.extend({ prefix: this.get('sitePrefix'), noActivityIndicator: true, url: this.get('links').last_update.href, + data: { + timestamp: this._lastUpdateTimestamp, + }, success: rsp => { const lastUpdate = rsp.last_update; diff --git a/reviewboard/testing/testcase.py b/reviewboard/testing/testcase.py index 34347a2d75e21c58b32b75e0b2e5f77d8f920200..b48e9352ae70a83d63add886b617f7a8aa0cc237 100644 --- a/reviewboard/testing/testcase.py +++ b/reviewboard/testing/testcase.py @@ -174,7 +174,7 @@ class TestCase(FixturesCompilerMixin, DjbletsTestCase): return file_attachment def create_diffset(self, review_request=None, revision=1, repository=None, - draft=False, name='diffset'): + draft=False, name='diffset', timestamp=None): """Creates a DiffSet for testing. The DiffSet defaults to revision 1. This can be overriden by the @@ -191,6 +191,9 @@ class TestCase(FixturesCompilerMixin, DjbletsTestCase): repository=repository, diffcompat=DiffCompatVersion.DEFAULT) + if timestamp: + diffset.timestamp = timestamp + if review_request: if draft: review_request_draft = \ @@ -502,7 +505,7 @@ class TestCase(FixturesCompilerMixin, DjbletsTestCase): def create_review(self, review_request, user='dopey', username=None, body_top='Test Body Top', body_bottom='Test Body Bottom', - ship_it=False, publish=False): + ship_it=False, publish=False, timestamp=None): """Creates a Review for testing. The Review is tied to the given ReviewRequest. It's populated with @@ -522,6 +525,9 @@ class TestCase(FixturesCompilerMixin, DjbletsTestCase): body_bottom=body_bottom, ship_it=ship_it) + if timestamp: + review.timestamp = timestamp + if publish: review.publish() diff --git a/reviewboard/webapi/resources/review_request_last_update.py b/reviewboard/webapi/resources/review_request_last_update.py index 27b4c3903c933728bcbc81841f74bc34977405c6..a08b8319b8a7fd480637474d269c8a716e19632b 100644 --- a/reviewboard/webapi/resources/review_request_last_update.py +++ b/reviewboard/webapi/resources/review_request_last_update.py @@ -1,13 +1,22 @@ from __future__ import unicode_literals +from datetime import datetime + from django.core.exceptions import ObjectDoesNotExist from django.http import HttpResponseNotModified from django.utils import six +from django.utils.timezone import utc from django.utils.translation import ugettext as _ from djblets.util.http import encode_etag, etag_if_none_match +from djblets.webapi.decorators import webapi_request_fields from djblets.webapi.errors import DOES_NOT_EXIST from reviewboard.diffviewer.models import DiffSet -from reviewboard.reviews.models import Review, ReviewRequest +from reviewboard.reviews.models import (Comment, + FileAttachmentComment, + GeneralComment, + Review, + ReviewRequest, + ScreenshotComment) from reviewboard.webapi.base import WebAPIResource from reviewboard.webapi.decorators import (webapi_check_local_site, webapi_check_login_required) @@ -50,11 +59,54 @@ class ReviewRequestLastUpdateResource(WebAPIResource): 'type': six.text_type, 'description': 'The user who made the last update.', }, + 'diffset_updates': { + 'type': ['reviewboard.diffviewer.models.DiffSet'], + 'description': 'The collection of diffset objects newer than the ' + 'timestamp provided.', + 'added_in': '3.0', + }, + 'review_updates': { + 'type': ['reviewboard.reviews.models.Review'], + 'description': 'The collection of review objects newer than the ' + 'timestamp provided that do not have a base ' + 'review.', + 'added_in': '3.0', + }, + 'reply_updates': { + 'type': ['reviewboard.reviews.models.Review'], + 'description': 'The collection of review objects newer than the ' + 'timestamp provided that have a base review.', + 'added_in': '3.0', + }, + 'comment_updates': { + 'type': ['reviewboard.reviews.models.BaseComment'], + 'description': 'The collection of review request comments ' + 'newer than the timestamp provided.', + 'added_in': '3.0', + }, + 'field_updates': { + 'type': ['reviewboard.changedescs.models.ChangeDescription'], + 'description': 'The collection of changes made to the review ' + 'request since the timestamp provided.', + 'added_in': '3.0', + } } + @webapi_request_fields( + optional={ + 'timestamp': { + 'type': six.text_type, + 'description': 'The timestamp of the most recent update ' + 'received by the client. Used to identify ' + 'out-of-date objects. Timestamp should follow' + 'the format: --
T::Z', + 'added_in': '3.0', + } + } + ) @webapi_check_login_required @webapi_check_local_site - def get(self, request, *args, **kwargs): + def get(self, request, timestamp=None, *args, **kwargs): """Returns the last update made to the review request. This shows the type of update that was made, the user who made the @@ -76,9 +128,9 @@ class ReviewRequestLastUpdateResource(WebAPIResource): review_request): return self.get_no_access_error(request) - timestamp, updated_object = review_request.get_last_activity() + server_timestamp, updated_object = review_request.get_last_activity() - etag = encode_etag('%s:%s' % (timestamp, updated_object.pk)) + etag = encode_etag('%s:%s' % (server_timestamp, updated_object.pk)) if etag_if_none_match(request, etag): return HttpResponseNotModified() @@ -115,16 +167,104 @@ class ReviewRequestLastUpdateResource(WebAPIResource): # be a ReviewRequest. assert False + if timestamp is None: + return 200, { + self.item_result_key: { + 'timestamp': server_timestamp, + 'user': user, + 'summary': summary, + 'type': update_type, + } + }, { + 'ETag': etag, + } + + try: + timestamp = datetime.strptime( + timestamp, '%Y-%m-%dT%H:%M:%SZ').replace(tzinfo=utc) + except ValueError: + # Default response for cases where a timestamp is incorrectly + # formatted. + return 200, { + self.item_result_key: { + 'timestamp': server_timestamp, + 'user': user, + 'summary': summary, + 'type': update_type, + } + }, { + 'ETag': etag, + } + + # Split reviews in to replies and base reviews. + new_reviews = [] + new_replies = [] + + for review in Review.objects.filter(review_request=review_request, + timestamp__gt=timestamp, + public=True): + if review.base_reply_to: + new_replies.append(review) + else: + new_reviews.append(review) + + new_diffsets = DiffSet.objects.filter( + history__pk=review_request.diffset_history_id, + timestamp__gt=timestamp, review_request_draft=None) + new_comments = self._get_comments_since_timestamp(new_reviews, + timestamp) + + change_descriptions = review_request.changedescs.filter( + timestamp__gt=timestamp) + field_updates = [cd.fields_changed for cd in change_descriptions] + return 200, { self.item_result_key: { - 'timestamp': timestamp, + 'timestamp': server_timestamp, 'user': user, 'summary': summary, 'type': update_type, + 'diffset_updates': new_diffsets, + 'review_updates': new_reviews, + 'reply_updates': new_replies, + 'comment_updates': new_comments, + 'field_updates': field_updates, } }, { 'ETag': etag, } + def _get_comments_since_timestamp(self, reviews, timestamp): + """ + Return all comments associated with the given collection of reviews + that are newer than the provided timestamp. + + Args: + reviews list of reviewboard.reviews.models.Review]: + The collection of reviews to scan for comment updates. + + timestamp (datetime.datetime): + The timestamp to compare comments against to determine if there + are unaccounted updates. + + Returns: list of reviewboard.reviews.models.BaseComment: + New and updated comments attached to the given reviews. + """ + comment_types = (Comment, + FileAttachmentComment, + GeneralComment, + ScreenshotComment + ) + + review_pks = [review.pk for review in reviews] + new_comments = [] + + for comment_class in self.comment_types: + new_comments.extend(comment_class.objects.filter( + review__pk__in=review_pks, + timestamp__gt=timestamp)) + + return new_comments + review_request_last_update_resource = ReviewRequestLastUpdateResource() diff --git a/reviewboard/webapi/tests/mimetypes.py b/reviewboard/webapi/tests/mimetypes.py index da92fe21cab17842a2e2c83aba7bbd3a9df6c941..1dcbf93b88d370b0192b83bf233c161964622866 100644 --- a/reviewboard/webapi/tests/mimetypes.py +++ b/reviewboard/webapi/tests/mimetypes.py @@ -139,6 +139,9 @@ review_request_item_mimetype = _build_mimetype('review-request') review_request_draft_item_mimetype = _build_mimetype('review-request-draft') +review_request_last_update_mimetype = _build_mimetype('last-update') + + root_item_mimetype = _build_mimetype('root') diff --git a/reviewboard/webapi/tests/test_review_request_last_update.py b/reviewboard/webapi/tests/test_review_request_last_update.py new file mode 100644 index 0000000000000000000000000000000000000000..2d7461477cb613bdc016d661109b374eada7b377 --- /dev/null +++ b/reviewboard/webapi/tests/test_review_request_last_update.py @@ -0,0 +1,232 @@ +"""Unit tests for the ReviewRequestLastUpdateResource API requests""" + +from __future__ import unicode_literals + +from datetime import timedelta + +from django.utils import six +from django.utils import timezone + +from reviewboard.changedescs.models import ChangeDescription +from reviewboard.diffviewer.models import DiffSet +from reviewboard.reviews.models import Review +from reviewboard.webapi.resources import resources +from reviewboard.webapi.tests.base import BaseWebAPITestCase +from reviewboard.webapi.tests.mimetypes import review_request_last_update_mimetype +from reviewboard.webapi.tests.mixins import BasicTestsMetaclass +from reviewboard.webapi.tests.urls import get_review_request_last_update_resource_url + + +@six.add_metaclass(BasicTestsMetaclass) +class ResourceTests(BaseWebAPITestCase): + """Unit tests for the updating of out of date review request objects.""" + + fixtures = ['test_users', 'test_scmtools'] + test_http_methods = ('GET',) + sample_api_url = 'review-requests//last-update/' + resource = resources.review_request_last_update + + def compare_item(self, item_rsp, response): + self.assertEqual(item_rsp['timestamp'], + response.last_updated.strftime('%Y-%m-%dT%H:%M:%SZ')) + self.assertEqual(item_rsp['timestamp'], + response.last_review_activity_timestamp.strftime( + '%Y-%m-%dT%H:%M:%SZ')) + self.assertEqual(item_rsp['user']['username'], response.submitter.username) + + # + # HTTP GET tests + # + + def setup_basic_get_test(self, user, with_local_site, local_site_name): + review_request = self.create_review_request( + create_repository=True, + with_local_site=with_local_site, + submitter=user, + publish=True) + + self.create_review(review_request, publish=True) + + return (get_review_request_last_update_resource_url( + review_request, local_site_name), + review_request_last_update_mimetype, + review_request) + + def test_get_diffsets_since_timestamp(self): + """Testing the GET review-requests//last-update/ API with new + diffsets since timestamp + """ + review_request = self.create_review_request( + publish=True, repository=self.create_repository()) + yesterday = timezone.now() - timedelta(days=1) + + self.create_diffset(review_request) + self.create_diffset(review_request) + DiffSet.objects.update(timestamp=yesterday) + + rsp = self.api_get( + get_review_request_last_update_resource_url(review_request), { + 'timestamp': '1970-01-01T00:00:00Z', + }, expected_mimetype=review_request_last_update_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + self.assertEqual(len(rsp['last_update']['diffset_updates']), 2) + + def test_get_reviews_since_timestamp(self): + """Testing the GET review-requests//last-update/ API with new + reviews since timestamp + """ + review_request = self.create_review_request(publish=True) + yesterday = timezone.now() - timedelta(days=1) + + self.create_review(review_request, publish=True) + self.create_review(review_request, publish=True) + Review.objects.update(timestamp=yesterday) + + rsp = self.api_get( + get_review_request_last_update_resource_url(review_request), { + 'timestamp': '1970-01-01T00:00:00Z', + }, expected_mimetype=review_request_last_update_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + self.assertEqual(len(rsp['last_update']['review_updates']), 2) + + def test_get_replies_since_timestamp(self): + """Testing the GET review-requests//last-update/ API with new + replies since timestamp + """ + review_request = self.create_review_request(publish=True) + yesterday = timezone.now() - timedelta(days=1) + + review_1 = self.create_review(review_request, publish=True) + review_2 = self.create_review(review_request, publish=True) + self.create_reply(review_1, publish=True) + self.create_reply(review_2, publish=True) + Review.objects.update(timestamp=yesterday) + + rsp = self.api_get( + get_review_request_last_update_resource_url(review_request), { + 'timestamp': '1970-01-01T00:00:00Z', + }, expected_mimetype=review_request_last_update_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + self.assertEqual(len(rsp['last_update']['reply_updates']), 2) + + def test_get_comments_since_timestamp(self): + """Testing the GET review-requests//last-update/ API with new + comments since timestamp + """ + review_request = self.create_review_request( + publish=True, repository=self.create_repository()) + yesterday = timezone.now() - timedelta(days=1) + + review = self.create_review(review_request, publish=True) + Review.objects.update(timestamp=yesterday) + + self.create_general_comment(review) + + diffset = self.create_diffset(review_request) + DiffSet.objects.update(timestamp=yesterday) + filediff = self.create_filediff(diffset) + self.create_diff_comment(review, filediff) + + screenshot = self.create_screenshot(review_request) + self.create_screenshot_comment(review, screenshot) + + attachment = self.create_file_attachment(review_request) + self.create_file_attachment_comment(review, attachment) + + rsp = self.api_get( + get_review_request_last_update_resource_url(review_request), { + 'timestamp': '1970-01-01T00:00:00Z', + }, expected_mimetype=review_request_last_update_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + self.assertEqual(len(rsp['last_update']['comment_updates']), 4) + + def test_get_field_updates_since_timestamp(self): + """Testing the GET review-requests//last-update/ API with field + updates since timestamp + """ + review_request = self.create_review_request( + publish=True, repository=self.create_repository()) + + first_change = ChangeDescription(public=True, + timestamp=timezone.now()) + first_change.record_field_change('summary', 'foo', 'bar') + first_change.save() + review_request.changedescs.add(first_change) + + second_change = ChangeDescription(public=True, + timestamp=timezone.now()) + second_change.record_field_change('description', 'my task', + 'is now done') + second_change.save() + review_request.changedescs.add(second_change) + + rsp = self.api_get( + get_review_request_last_update_resource_url(review_request), { + 'timestamp': '1970-01-01T00:00:00Z', + }, expected_mimetype=review_request_last_update_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + self.assertEqual(len(rsp['last_update']['field_updates']), 2) + self.assertEqual(rsp['last_update']['field_updates'][1], + { + u'summary': + { + u'new': [u'bar'], + u'old': [u'foo'], + } + } + ) + self.assertEqual(rsp['last_update']['field_updates'][0], + { + u'description': + { + u'new': [u'is now done'], + u'old': [u'my task'] + } + } + ) + + def test_get_with_improperly_formatted_timestamp(self): + """Testing the GET review-requests//last-update/ API with + improperly formatted timestamp defaults to a response excluding the + updated diffsets, reviews, replies, comments and field changes + """ + review_request = self.create_review_request( + publish=True, repository=self.create_repository()) + yesterday = timezone.now() - timedelta(days=1) + + self.create_diffset(review_request) + self.create_diffset(review_request) + DiffSet.objects.update(timestamp=yesterday) + + rsp = self.api_get( + get_review_request_last_update_resource_url(review_request), { + 'timestamp': '2016-01-01', + }, expected_mimetype=review_request_last_update_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + self.assertFalse('diffset_updates' in rsp['last_update']) + self.assertFalse('review_updates' in rsp['last_update']) + self.assertFalse('reply_updates' in rsp['last_update']) + self.assertFalse('comment_updates' in rsp['last_update']) + self.assertFalse('field_updates' in rsp['last_update']) + + def test_changes_older_than_timestamp_are_omitted(self): + review_request = self.create_review_request(publish=True) + yesterday = timezone.now() - timedelta(days=1) + + self.create_review(review_request, publish=True) + self.create_review(review_request, publish=True) + Review.objects.update(timestamp=yesterday) + + rsp = self.api_get( + get_review_request_last_update_resource_url(review_request), { + 'timestamp': timezone.now().strftime('%Y-%m-%dT%H:%M:%SZ'), + }, expected_mimetype=review_request_last_update_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + self.assertEqual(len(rsp['last_update']['review_updates']), 0) diff --git a/reviewboard/webapi/tests/urls.py b/reviewboard/webapi/tests/urls.py index 668dcd9f4cd59d81b2511c68ac99c0679b4ef5d4..5554c6df67ef3ed5b1dec2825424aeb9483af861 100644 --- a/reviewboard/webapi/tests/urls.py +++ b/reviewboard/webapi/tests/urls.py @@ -635,6 +635,29 @@ def get_review_request_draft_url(review_request, local_site_name=None): # +# ReviewRequestLastUpdateResource +# +def get_review_request_last_update_resource_url(review_request, + local_site_name=None): + """Return the URL to the last_update_resource of a given review request. + + Args: + review_request: (reviewboard.reviews.models.ReviewRequest): + The review request to scan for updates. + + local_site_name: (unicode): + The name of the local site being used for testing purposes. + + Returns: + unicode: + The resulting absolute URL to the item resource. + """ + return resources.review_request_last_update.get_item_url( + local_site_name=local_site_name, + review_request_id=review_request.display_id) + + +# # ReviewScreenshotCommentResource # def get_review_screenshot_comment_list_url(review, local_site_name=None):