diff --git a/reviewboard/reviews/models/review_request.py b/reviewboard/reviews/models/review_request.py index 3f5ded37a06529054b74d9bbbd76aa1cef7bfa82..e1ffe938ecf61afd2a04911e201c164d5626c850 100644 --- a/reviewboard/reviews/models/review_request.py +++ b/reviewboard/reviews/models/review_request.py @@ -588,6 +588,74 @@ class ReviewRequest(BaseReviewRequestDetails): This will return the last object updated, along with the timestamp of that object. It can be used to judge whether something on a review request has been made public more recently. + + Deprecated: + 4.0: + See :py:meth:`get_last_activity_info` instead. + + Args: + diffsets (list of reviewboard.diffviewer.models.DiffSet, optional): + The list of diffsets to compare for latest activity. + + If not provided, this will be populated with the last diffset. + + reviews (list of reviewboard.reviews.models.Review, optional): + The list of reviews to compare for latest activity. + + If not provided, this will be populated with the latest review. + + Returns: + tuple: + + * The timestamp the review request was last updated + (:py:class:`~datetime.datetime`). + * The object that was updated + (:py:class:`~reviewboard.reviews.models.Review`, + :py:class:`~reviewboard.reviews.models.ReviewRequest`, or + :py:class:`~reviewboard.diffviewer.models.DiffSet`). + """ + warnings.warn( + 'ReviewRequest.get_last_activity is deprecated in Review Board ' + '4.0 and will be removed in a future release. Please use ' + 'ReviewRequest.get_last_activity_info instead.', + DeprecationWarning) + + info = self.get_last_activity_info(diffsets=diffsets, reviews=reviews) + return info['timestamp'], info['updated_object'] + + def get_last_activity_info(self, diffsets=None, reviews=None): + """Return the last public activity information on the review request. + + Args: + diffsets (list of reviewboard.diffviewer.models.DiffSet, optional): + The list of diffsets to compare for latest activity. + + If not provided, this will be populated with the last diffset. + + reviews (list of reviewboard.reviews.models.Review, optional): + The list of reviews to compare for latest activity. + + If not provided, this will be populated with the latest review. + + Returns: + dict: + A dictionary with the following keys: + + ``timestamp``: + The :py:class:`~datetime.datetime` that the object was updated. + + ``updated_object``: + The object that was updated. This will be one of the following: + + * The :py:class:`~reviewboard.reviews.models.ReviewRequest` + itself. + * A :py:class:`~reviewboard.reviews.models.Review`. + * A :py:class:`~reviewboard.diffviewer.models.DiffSet`. + + ``changedesc``: + The latest + :py:class:`~reviewboard.changedescs.models.ChangeDescription`, + if any. """ timestamp = self.last_updated updated_object = self @@ -618,7 +686,19 @@ class ReviewRequest(BaseReviewRequestDetails): timestamp = review.timestamp updated_object = review - return timestamp, updated_object + changedesc = None + + if updated_object is self or isinstance(updated_object, DiffSet): + try: + changedesc = self.changedescs.latest() + except ChangeDescription.DoesNotExist: + pass + + return { + 'changedesc': changedesc, + 'timestamp': timestamp, + 'updated_object': updated_object, + } def changeset_is_pending(self, commit_id): """Returns whether the associated changeset is pending commit. diff --git a/reviewboard/reviews/tests/test_review_request.py b/reviewboard/reviews/tests/test_review_request.py index fa15cd7499398f900df32c0fbff9ee5172ab4ffd..ffe3eae06d50ad68015476302cf0814397797744 100644 --- a/reviewboard/reviews/tests/test_review_request.py +++ b/reviewboard/reviews/tests/test_review_request.py @@ -423,6 +423,134 @@ class ReviewRequestTests(SpyAgency, TestCase): self.assertEqual(timestamp, changedesc.timestamp) +class GetLastActivityInfoTests(TestCase): + """Unit tests for ReviewRequest.get_last_activity_info""" + + fixtures = ['test_scmtools', 'test_users'] + + def setUp(self): + super(GetLastActivityInfoTests, self).setUp() + + doc = User.objects.get(username='doc') + self.review_request = self.create_review_request( + create_repository=True, + publish=True, + target_people=[doc]) + + def test_get_last_activity_info(self): + """Testing ReviewRequest.get_last_activity_info""" + self.assertEqual( + self.review_request.get_last_activity_info(), + { + 'changedesc': None, + 'timestamp': self.review_request.last_updated, + 'updated_object': self.review_request, + }) + + def test_get_last_activity_info_draft(self): + """Testing ReviewRequest.get_last_activity_info after updating the + draft + """ + draft = ReviewRequestDraft.create(self.review_request) + draft.summary = 'A new summary appears' + draft.save() + + self.assertEqual( + self.review_request.get_last_activity_info(), + { + 'changedesc': None, + 'timestamp': self.review_request.last_updated, + 'updated_object': self.review_request, + }) + + def test_get_last_activity_info_update(self): + """Testing ReviewRequest.get_last_activity_info after an update""" + draft = ReviewRequestDraft.create(self.review_request) + draft.summary = 'A new summary appears' + draft.save() + + self.review_request = ReviewRequest.objects.get( + pk=self.review_request.pk) + self.review_request.publish(user=self.review_request.submitter) + changedesc = self.review_request.changedescs.latest() + + self.assertEqual( + self.review_request.get_last_activity_info(), + { + 'changedesc': changedesc, + 'timestamp': changedesc.timestamp, + 'updated_object': self.review_request, + }) + + def test_get_last_activity_info_diff_update(self): + """Testing ReviewRequest.get_last_activity_info after a diff update""" + diffset = self.create_diffset(review_request=self.review_request, + draft=True) + self.review_request.publish(user=self.review_request.submitter) + diffset = DiffSet.objects.get(pk=diffset.pk) + + self.assertEqual( + self.review_request.get_last_activity_info(), + { + 'changedesc': self.review_request.changedescs.latest(), + 'timestamp': diffset.timestamp, + 'updated_object': diffset, + }) + + def test_get_last_activity_info_review(self): + """Testing ReviewRequest.get_last_activity_info after a review""" + review = self.create_review(review_request=self.review_request, + publish=True) + + self.assertEqual( + self.review_request.get_last_activity_info(), + { + 'changedesc': None, + 'timestamp': review.timestamp, + 'updated_object': review, + }) + + def test_get_last_activity_info_review_reply(self): + """Testing ReviewRequest.get_last_activity_info after a review and + a reply + """ + review = self.create_review(review_request=self.review_request, + publish=True) + + reply = self.create_reply(review=review, publish=True) + + self.assertEqual( + self.review_request.get_last_activity_info(), + { + 'changedesc': None, + 'timestamp': reply.timestamp, + 'updated_object': reply, + }) + + def test_get_last_activity_info_update_and_review(self): + """Testing ReviewRequest.get_last_activity_info after an update and a + review + """ + draft = ReviewRequestDraft.create(self.review_request) + draft.summary = 'A new summary appears' + draft.save() + + # self.review_request = ReviewRequest.objects.get( + # pk=self.review_request.pk) + self.review_request.publish(user=self.review_request.submitter) + + review = self.create_review(review_request=self.review_request, + publish=True) + + self.assertEqual( + self.review_request.get_last_activity_info(), + { + 'changedesc': None, + 'timestamp': review.timestamp, + 'updated_object': review, + }) + + class IssueCounterTests(TestCase): """Unit tests for review request issue counters.""" diff --git a/reviewboard/webapi/resources/review_request_last_update.py b/reviewboard/webapi/resources/review_request_last_update.py index 4744ff5a50ff065c668d0e437e54e0310ee8a2ce..95d8f48ee1f1119d446b5c9082f26d792f8a0a58 100644 --- a/reviewboard/webapi/resources/review_request_last_update.py +++ b/reviewboard/webapi/resources/review_request_last_update.py @@ -20,6 +20,7 @@ class ReviewRequestLastUpdateResource(WebAPIResource): Clients can periodically poll this to see if any new updates have been made. """ + name = 'last_update' policy_id = 'review_request_last_update' singleton = True @@ -77,45 +78,51 @@ class ReviewRequestLastUpdateResource(WebAPIResource): review_request): return self.get_no_access_error(request) - timestamp, updated_object = review_request.get_last_activity() + info = review_request.get_last_activity_info() + timestamp = info['timestamp'] + updated_object = info['updated_object'] + changedesc = info['changedesc'] etag = encode_etag('%s:%s' % (timestamp, updated_object.pk)) if etag_if_none_match(request, etag): return HttpResponseNotModified() - user = None summary = None update_type = None if isinstance(updated_object, ReviewRequest): - user = updated_object.submitter - if updated_object.status == ReviewRequest.SUBMITTED: - summary = _("Review request submitted") + summary = _('Review request submitted') elif updated_object.status == ReviewRequest.DISCARDED: - summary = _("Review request discarded") + summary = _('Review request discarded') else: - summary = _("Review request updated") + summary = _('Review request updated') - update_type = "review-request" + update_type = 'review-request' elif isinstance(updated_object, DiffSet): - summary = _("Diff updated") - update_type = "diff" + summary = _('Diff updated') + update_type = 'diff' elif isinstance(updated_object, Review): - user = updated_object.user - if updated_object.is_reply(): - summary = _("New reply") - update_type = "reply" + summary = _('New reply') + update_type = 'reply' else: - summary = _("New review") - update_type = "review" + summary = _('New review') + update_type = 'review' else: # Should never be able to happen. The object will always at least # be a ReviewRequest. assert False + if changedesc: + user = changedesc.get_user(review_request) + else: + # There is no changedesc which means this review request hasn't + # been changed since it was first published, so this change must + # be due to the original submitter. + user = review_request.submitter + return 200, { self.item_result_key: { 'timestamp': timestamp, diff --git a/reviewboard/webapi/tests/mimetypes.py b/reviewboard/webapi/tests/mimetypes.py index b7d45d6fe01470104a88b52b4ff029afe2299c4a..044f2a3fa647c0080b4fd87417866038e58851ec 100644 --- a/reviewboard/webapi/tests/mimetypes.py +++ b/reviewboard/webapi/tests/mimetypes.py @@ -119,6 +119,9 @@ review_reply_list_mimetype = _build_mimetype('review-replies') review_reply_item_mimetype = _build_mimetype('review-reply') +review_request_last_update_mimetype = _build_mimetype('last-update') + + review_reply_diff_comment_list_mimetype = \ _build_mimetype('review-reply-diff-comments') review_reply_diff_comment_item_mimetype = \ 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..e4184e0309e2c99a0036c673207f06baa907f54c --- /dev/null +++ b/reviewboard/webapi/tests/test_review_request_last_update.py @@ -0,0 +1,358 @@ +"""Tests for the ReviewRequestLastUpdateResource.""" + +from __future__ import unicode_literals + +from dateutil.parser import parse as parse_date +from django.contrib.auth.models import User +from django.utils import six +from djblets.webapi.testing.decorators import webapi_test_template + +from reviewboard.diffviewer.models import DiffSet +from reviewboard.reviews.models import (Review, ReviewRequest, + ReviewRequestDraft) +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_url + + +@six.add_metaclass(BasicTestsMetaclass) +class ResourceTests(BaseWebAPITestCase): + """Testing ReviewRequestLastUpdateResource APIs.""" + + fixtures = ['test_users', 'test_scmtools'] + sample_api_url = 'review-requests//last-update/' + test_http_methods = ('GET',) + resource = resources.review_request_last_update + + def compare_item(self, item_rsp, review_request, expected_user=None): + """Compare a response to a review request. + + Args: + item_rsp (dict): + A dictionary of serialized data from the resource. + + review_request (reviewboard.reviews.models.review_request. + ReviewRequest): + The review request whose latest activity will be used for + comparison. + + expected_user (django.contrib.auth.models.User, optional): + The user we expected to be serialized in ``item_rsp``. + + If not provided, this defaults to ``review_request.submitter``. + + Raises: + AssertionError: + The serialized response does not correspond with the given + review request or user. + """ + if expected_user is None: + expected_user = review_request.submitter + + item_type = item_rsp['type'] + last_updated = review_request.last_updated.replace(microsecond=0) + + last_activity = review_request.get_last_activity_info() + updated_object = last_activity['updated_object'] + changedesc = last_activity['changedesc'] + + rsp_timestamp = parse_date(item_rsp['timestamp']).replace( + microsecond=0) + + if item_type == 'review-request': + self.assertIsInstance(updated_object, ReviewRequest) + self.assertEqual(updated_object, review_request) + object_timestamp = updated_object.last_updated + elif item_type == 'diff': + self.assertIsInstance(updated_object, DiffSet) + object_timestamp = updated_object.timestamp + elif item_type in ('review', 'reply'): + self.assertIsInstance(updated_object, Review) + object_timestamp = updated_object.timestamp + + self.assertIsNone(last_activity['changedesc']) + else: + self.fail('Unknown item type "%s"' % item_type) + + object_timestamp = object_timestamp.replace(microsecond=0) + + self.assertEqual(last_updated, object_timestamp) + self.assertEqual(last_updated, rsp_timestamp) + + if changedesc: + self.assertEqual(expected_user, + changedesc.get_user(review_request)) + + self.assertEqual(expected_user.pk, + item_rsp['user']['id']) + + # + # HTTP GET tests + # + + def setup_basic_get_test(self, user, with_local_site=False, + local_site_name=None, update_review_request=False, + update_diffset=False, update_review=False, + update_reply=False, publish_user=None): + """Setup a basic HTTP GET test. + + Args: + user (django.contrib.auth.models.User): + The user that will be used to retrieve the resource. + + They will also be the submitter/author of all objects (although + not necessary the publisher). + + with_local_site (boolean, optional): + Whether or not a LocalSite-specific resource will be used. + + local_site_name (unicode, optional): + The name of the LocalSite to use. + + update_review_request (boolean, optional): + Whether or not the review request should be updated after + publishing. + + update_diffset (boolean, optional): + Whether or not the review request should be updated with a diff + after publishing. + + update_review (boolean, optional): + Whether or not the review request should be reviewed after + publishing. + + update_reply (boolean, optional): + Whther or not to create a reply. + + This implies ``update_review``. + + publish_user (django.contrib.auth.models.User, optional): + The user that will trigger the update (i.e., they will publish + all requested objects). + + If not provided, ``user`` will be used. + + Returns: + tuple: + A 3-tuple of: + + * The URL to request. + * The expected mimetype. + * The review request to use for comparison. + """ + update_review = update_review or update_reply + publish_user = publish_user or user + review_request = self.create_review_request( + create_repository=True, + submitter=user, + with_local_site=with_local_site, + target_people=[user]) + + review_request.publish(user=publish_user) + + if update_review_request: + draft = ReviewRequestDraft.create(review_request) + draft.summary = '%s updated' % review_request.summary + draft.save() + + review_request.publish(user=publish_user) + + if update_diffset: + self.create_diffset(review_request=review_request, draft=True) + review_request.publish(user=publish_user) + + if update_review: + review = self.create_review(review_request=review_request, + publish=False) + review.publish(user=publish_user) + + if update_reply: + reply = self.create_reply(review) + reply.publish(user=publish_user) + + return ( + get_review_request_last_update_url(local_site_name=local_site_name, + review_request=review_request), + review_request_last_update_mimetype, + review_request, + ) + + @webapi_test_template + def test_get_other_user(self): + """Testing the GET API when the review request has been published + by another user + """ + publish_user = User.objects.get(username='admin') + url, expected_mimetype, review_request = self.setup_basic_get_test( + self.user, + publish_user=publish_user) + rsp = self.api_get(url, expected_mimetype=expected_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + item_rsp = rsp['last_update'] + + self.assertEqual(item_rsp['type'], 'review-request') + + # We have no way of knowing what user published based on only the + # model, so it will always be the submitter. + self.compare_item(item_rsp, review_request, expected_user=self.user) + + @webapi_test_template + def test_get_review_request_updated(self): + """Testing the GET API when the review request has been updated + """ + url, expected_mimetype, review_request = self.setup_basic_get_test( + self.user, + update_review_request=True) + rsp = self.api_get(url, expected_mimetype=expected_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + item_rsp = rsp['last_update'] + + self.assertEqual(item_rsp['type'], 'review-request') + self.compare_item(item_rsp, review_request) + + @webapi_test_template + def test_get_review_request_updated_other_user(self): + """Testing the GET API when a review request has been updated + by a different user + """ + publish_user = User.objects.get(username='admin') + url, expected_mimetype, review_request = self.setup_basic_get_test( + self.user, + update_review_request=True, + publish_user=publish_user) + rsp = self.api_get(url, expected_mimetype=expected_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + item_rsp = rsp['last_update'] + + self.assertEqual(item_rsp['type'], 'review-request') + self.compare_item(item_rsp, review_request, expected_user=publish_user) + + @webapi_test_template + def test_get_diff_updated(self): + """Testing the GET API when a new diff has been published""" + url, expected_mimetype, review_request = self.setup_basic_get_test( + self.user, + update_diffset=True) + rsp = self.api_get(url, expected_mimetype=expected_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + item_rsp = rsp['last_update'] + + self.assertEqual(item_rsp['type'], 'diff') + self.compare_item(rsp['last_update'], review_request) + + @webapi_test_template + def test_get_diff_updated_other_user(self): + """Testing the GET API when a new diff has been published by + another user + """ + publish_user = User.objects.get(username='admin') + url, expected_mimetype, review_request = self.setup_basic_get_test( + self.user, + update_diffset=True, + publish_user=publish_user) + rsp = self.api_get(url, expected_mimetype=expected_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + item_rsp = rsp['last_update'] + + self.assertEqual(item_rsp['type'], 'diff') + self.compare_item(item_rsp, review_request, expected_user=publish_user) + + @webapi_test_template + def test_get_review_published(self): + """Testing the GET API when a review has been published""" + url, expected_mimetype, review_request = self.setup_basic_get_test( + self.user, + update_review=True) + rsp = self.api_get(url, expected_mimetype=expected_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + item_rsp = rsp['last_update'] + + self.assertEqual(item_rsp['type'], 'review') + self.compare_item(item_rsp, review_request) + + @webapi_test_template + def test_get_review_published_other_user(self): + """Testing the GET API when a review has been published by + another user + """ + publish_user = User.objects.get(username='admin') + url, expected_mimetype, review_request = self.setup_basic_get_test( + self.user, + update_review=True, + publish_user=publish_user) + rsp = self.api_get(url, expected_mimetype=expected_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + item_rsp = rsp['last_update'] + + self.assertEqual(item_rsp['type'], 'review') + + # We have no way of knowing what user published based on only the + # model, so it will always be the review author. + self.compare_item(item_rsp, review_request, expected_user=self.user) + + @webapi_test_template + def test_get_reply_published(self): + """Testing the GET API when a review reply has been published""" + url, expected_mimetype, review_request = self.setup_basic_get_test( + self.user, + update_reply=True) + rsp = self.api_get(url, expected_mimetype=expected_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + item_rsp = rsp['last_update'] + + self.assertEqual(item_rsp['type'], 'reply') + self.compare_item(item_rsp, review_request) + + @webapi_test_template + def test_get_reply_published_other_user(self): + """Testing the GET API when a review reply has been published by + another user + """ + publish_user = User.objects.get(username='admin') + url, expected_mimetype, review_request = self.setup_basic_get_test( + self.user, + update_reply=True, + publish_user=publish_user) + rsp = self.api_get(url, expected_mimetype=expected_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + item_rsp = rsp['last_update'] + + self.assertEqual(item_rsp['type'], 'reply') + + # We have no way of knowing what user published based on only the + # model, so it will always be the review author. + self.compare_item(item_rsp, review_request, expected_user=self.user) + + @webapi_test_template + def test_get_review_request_updated_review_published_other_user(self): + """Testing the GET API when a review request has been updated + and a review request has been published + """ + publish_user = User.objects.get(username='admin') + url, expected_mimetype, review_request = self.setup_basic_get_test( + self.user, + update_review_request=True, + update_review=True, + publish_user=publish_user) + rsp = self.api_get(url, expected_mimetype=expected_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + item_rsp = rsp['last_update'] + + self.assertEqual(item_rsp['type'], 'review') + + # We have no way of knowing what user published based on only the + # model, so it will always be the review author. + self.compare_item(item_rsp, review_request, expected_user=self.user) diff --git a/reviewboard/webapi/tests/urls.py b/reviewboard/webapi/tests/urls.py index fd80cb108032f16b5369acd6dad88bd20be529cf..a72e3b75ae813f4576b6f3ea627c7f5459daaf3c 100644 --- a/reviewboard/webapi/tests/urls.py +++ b/reviewboard/webapi/tests/urls.py @@ -667,6 +667,15 @@ def get_review_request_draft_url(review_request, local_site_name=None): review_request_id=review_request.display_id) +# +# ReviewRequestLastUpdateResource +# +def get_review_request_last_update_url(review_request, local_site_name=None): + return resources.review_request_last_update.get_item_url( + local_site_name=local_site_name, + review_request_id=review_request.display_id) + + # # ReviewScreenshotCommentResource #