diff --git a/reviewboard/reviews/forms.py b/reviewboard/reviews/forms.py index 6f786a334afa8733e8dd5ad3f592144dc916e94f..0f3ee9124efc6088dfe65f15982e0e90d6cf068a 100644 --- a/reviewboard/reviews/forms.py +++ b/reviewboard/reviews/forms.py @@ -6,7 +6,7 @@ from django import forms from django.contrib.admin.widgets import FilteredSelectMultiple from django.contrib.auth.models import User from django.core.exceptions import ValidationError -from django.utils.translation import ugettext_lazy as _ +from django.utils.translation import ugettext, ugettext_lazy as _ from reviewboard.admin.form_widgets import RelatedUserWidget from reviewboard.diffviewer import forms as diffviewer_forms @@ -153,6 +153,30 @@ class UploadDiffForm(diffviewer_forms.UploadDiffForm): except DiffSet.DoesNotExist: pass + def clean(self): + """Clean the form. + + This ensures that the associated review request was not created with + history. + + Returns: + dict: + The cleaned form data. + + Raises: + django.core.exceptions.ValidationError: + The form cannot be validated. + """ + super(UploadDiffForm, self).clean() + + if self.review_request.created_with_history: + raise ValidationError(ugettext( + 'The review request was created with history support and ' + 'DiffSets cannot be attached in this way. Instead, attach ' + 'DiffCommits.')) + + return self.cleaned_data + def create(self, attach_to_history=False): """Create the DiffSet and optionally attach it to the history. diff --git a/reviewboard/reviews/managers.py b/reviewboard/reviews/managers.py index ea8f6ba275ef52267c5aed4f03f82aedf436b112..071d6b94c0e9860613c019000302b8a14ca35efd 100644 --- a/reviewboard/reviews/managers.py +++ b/reviewboard/reviews/managers.py @@ -111,10 +111,50 @@ class ReviewRequestManager(ConcurrencyManager): return ReviewRequestQuerySet(self.model) def create(self, user, repository, commit_id=None, local_site=None, - create_from_commit_id=False): - """ - Creates a new review request, optionally filling in fields based off - a commit ID. + create_from_commit_id=False, create_with_history=False): + """Create a new review request. + + Args: + user (django.contrib.auth.models.User): + The user creating the review request. They will be tracked as + the submitter. + + repository (reviewboard.scmtools.Models.Repository): + The repository, if any, the review request is associated with. + + If ``None``, diffs cannot be added to the review request. + + commit_id (unicode, optional): + An optional commit ID. + + local_site (reviewboard.site.models.LocalSite, optional): + An optional LocalSite to associate the review request with. + + create_from_commit_id (bool, optional): + Whether or not the given ``commit_id`` should be used to + pre-populate the review request data. If ``True``, the given + ``repository`` will be used to do so. + + create_with_history (bool, optional): + Whether or not the created review request will support + attaching multiple commits per diff revision. + + If ``False``, it will not be possible to use the + :py:class:`~reviewboard.webapi.resources.diff.DiffResource` to + upload diffs; the + :py:class:`~reviewboard.webapi.resources.DiffCommitResource` + must be used instead. + + Returns: + reviewboard.reviews.models.review_request.ReviewRequest: + The created review request. + + Raises: + reviewboard.scmtools.errors.ChangeNumberInUseError: + The commit ID is already in use by another review request. + + ValueError: + An invalid value was passed for an argument. """ from reviewboard.reviews.models import ReviewRequestDraft @@ -142,6 +182,14 @@ class ReviewRequestManager(ConcurrencyManager): except (ObjectDoesNotExist, TypeError, ValueError): pass + if create_with_history: + if repository is None: + raise ValueError('create_with_history requires a repository.') + elif create_from_commit_id: + raise ValueError( + 'create_from_commit_id and create_with_history cannot ' + 'both be set to True.') + # Create the review request. We're not going to actually save this # until we're confident we have all the data we need. review_request = self.model( @@ -152,6 +200,8 @@ class ReviewRequestManager(ConcurrencyManager): diffset_history=DiffSetHistory(), local_site=local_site) + review_request.created_with_history = create_with_history + if commit_id: review_request.commit = commit_id diff --git a/reviewboard/reviews/models/review_request.py b/reviewboard/reviews/models/review_request.py index 4f7afa495e2b3bbee4c35003a7b8d8d1f72bcfe6..4df1e1bebefc2747f32ebd60d5ad9a20c9b209b9 100644 --- a/reviewboard/reviews/models/review_request.py +++ b/reviewboard/reviews/models/review_request.py @@ -147,6 +147,9 @@ class ReviewRequest(BaseReviewRequestDetails): request. Some fields are user-modifiable, while some are used for internal state. """ + + _CREATED_WITH_HISTORY_EXTRA_DATA_KEY = '__created_with_history' + PENDING_REVIEW = "P" SUBMITTED = "S" DISCARDED = "D" @@ -268,7 +271,6 @@ class ReviewRequest(BaseReviewRequestDetails): _('dropped issue count'), initializer=_initialize_issue_counts) - issue_verifying_count = CounterField( _('verifying issue count'), initializer=_initialize_issue_counts) @@ -417,6 +419,38 @@ class ReviewRequest(BaseReviewRequestDetails): def owner(self, new_owner): self.submitter = new_owner + @property + def created_with_history(self): + """Whether or not this review request was created with commit support. + + This property can only be changed before the review request is created + (i.e., before :py:meth:`save` is called and it has a primary key + assigned). + """ + return (self.extra_data is not None and + self.extra_data.get(self._CREATED_WITH_HISTORY_EXTRA_DATA_KEY, + False)) + + @created_with_history.setter + def created_with_history(self, value): + """Set whether this review request was created with commit support. + + This can only be used during review request creation (i.e., before + :py:meth:`save` is called). + + Raises: + ValueError: + The review request has already been created. + """ + if self.pk: + raise ValueError('created_with_history cannot be changed once ' + 'the review request has been created.') + + if self.extra_data is None: + self.extra_data = {} + + self.extra_data[self._CREATED_WITH_HISTORY_EXTRA_DATA_KEY] = value + def get_participants(self): """Returns a list of users who have discussed this review request.""" # See the comment in Review.get_participants for this list diff --git a/reviewboard/reviews/tests/test_forms.py b/reviewboard/reviews/tests/test_forms.py index fde3046efb9a3fe48c5560db2d526486fea299ce..bfc04d88b4fbe7f13ee61978e643b8f2ca678d83 100644 --- a/reviewboard/reviews/tests/test_forms.py +++ b/reviewboard/reviews/tests/test_forms.py @@ -1,8 +1,10 @@ from __future__ import unicode_literals from django.contrib.auth.models import User +from django.core.files.uploadedfile import SimpleUploadedFile -from reviewboard.reviews.forms import DefaultReviewerForm, GroupForm +from reviewboard.reviews.forms import (DefaultReviewerForm, GroupForm, + UploadDiffForm) from reviewboard.reviews.models import Group from reviewboard.scmtools.models import Repository, Tool from reviewboard.site.models import LocalSite @@ -169,3 +171,46 @@ class GroupFormTests(TestCase): }) self.assertTrue(form.is_valid()) + + +class UploadDiffFormTests(TestCase): + """Unit tests for UploadDiffForm.""" + + fixtures = ['test_users', 'test_scmtools'] + + def test_clean_with_no_history(self): + """Testing UploadDiffForm.clean with a review request created without + history support + """ + review_request = self.create_review_request(create_repository=True) + form = UploadDiffForm( + review_request=review_request, + data={ + 'basedir': '', + }, + files={ + 'path': SimpleUploadedFile('diff', + self.DEFAULT_GIT_FILEDIFF_DATA), + }) + + self.assertTrue(form.is_valid()) + + def test_clean_with_history(self): + """Testing UploadDiffForm.clean with a review request created with + history support + """ + review_request = self.create_review_request(create_repository=True, + create_with_history=True) + + form = UploadDiffForm( + review_request=review_request, + data={ + 'basedir': '', + }, + files={ + 'path': SimpleUploadedFile('diff', + self.DEFAULT_GIT_FILEDIFF_DATA), + }) + + self.assertFalse(form.is_valid()) + self.assertNotEqual(form.non_field_errors, []) diff --git a/reviewboard/reviews/tests/test_review_request.py b/reviewboard/reviews/tests/test_review_request.py index ffe3eae06d50ad68015476302cf0814397797744..722767bf7d63375987625325eaaa1184c4e62f53 100644 --- a/reviewboard/reviews/tests/test_review_request.py +++ b/reviewboard/reviews/tests/test_review_request.py @@ -422,6 +422,62 @@ class ReviewRequestTests(SpyAgency, TestCase): self.assertEqual(updated_object, review_request) self.assertEqual(timestamp, changedesc.timestamp) + @add_fixtures(['test_scmtools']) + def test_create_with_history_and_commit_id(self): + """Testing ReviewRequest.objects.create when create_with_history=True + and create_from_commit_id=True + """ + user = User.objects.get(username='doc') + repository = self.create_repository() + + msg = ('create_from_commit_id and create_with_history cannot both be ' + 'set to True.') + + with self.assertRaisesMessage(ValueError, msg): + ReviewRequest.objects.create(repository=repository, + user=user, + commit_id='0' * 40, + create_from_commit_id=True, + create_with_history=True) + + @add_fixtures(['test_scmtools']) + def test_created_with_history_cannot_change_when_true(self): + """Testing ReviewRequest.created_with_history cannot change after + creation when False + """ + user = User.objects.get(username='doc') + repository = self.create_repository() + + review_request = ReviewRequest.objects.create(repository=repository, + user=user) + + self.assertFalse(review_request.created_with_history) + + msg = ('created_with_history cannot be changed once the review ' + 'request has been created.') + + with self.assertRaisesMessage(ValueError, msg): + review_request.created_with_history = True + + @add_fixtures(['test_scmtools']) + def test_created_with_history_cannot_change_when_false(self): + """Testing ReviewRequest.created_with_history cannot change after + creation when True + """ + user = User.objects.get(username='doc') + repository = self.create_repository() + review_request = ReviewRequest.objects.create(repository=repository, + user=user, + create_with_history=True) + + self.assertTrue(review_request.created_with_history) + + msg = ('created_with_history cannot be changed once the review ' + 'request has been created.') + + with self.assertRaisesMessage(ValueError, msg): + review_request.created_with_history = False + class GetLastActivityInfoTests(TestCase): """Unit tests for ReviewRequest.get_last_activity_info""" diff --git a/reviewboard/testing/testcase.py b/reviewboard/testing/testcase.py index 564c454962b7885e43b3c700181b25bfe991e068..e8eabcec6e9a700b977a3ab0bf6feb13d4847991 100644 --- a/reviewboard/testing/testcase.py +++ b/reviewboard/testing/testcase.py @@ -238,11 +238,11 @@ class TestCase(FixturesCompilerMixin, DjbletsTestCase): **kwargs (dict): Keyword arguments to be passed to the - :py:class:`~reviewboard.diffviewer.models.diff_commit.DiffCommit` - initializer. + :py:class:`~reviewboard.diffviewer.models.diffcommit. + DiffCommit` initializer. Returns: - reviewboard.diffviewer.models.diff_commit.DiffCommit: + reviewboard.diffviewer.models.diffcommit.DiffCommit: The resulting DiffCommit. """ assert isinstance(diff_contents, bytes) @@ -728,18 +728,105 @@ class TestCase(FixturesCompilerMixin, DjbletsTestCase): repository=None, id=None, create_repository=False, target_people=None, - target_groups=None): + target_groups=None, + create_with_history=False, + **kwargs): """Create a ReviewRequest for testing. - The ReviewRequest may optionally be attached to a LocalSite. It's also - populated with default data that can be overridden by the caller. + Args: + with_local_site (bool, optional): + Whether or not the review request should be associated with a + LocalSite. + + local_site (reviewboard.site.models.LocalSite, optional): + The LocalSite to associate the review request with. + + If not provided, the LocalSite with the name specified in + ``self.local_site_name`` will be used. + + summary (unicode, optional): + The contents of the Summary field. + + description (unicode, optional): + The contents of the Description field. + + testing_done (unicode, optional): + The contents of the Testing Done field. + + submitter (unicode or django.contrib.auth.models.User, optional): + Either the username or the actual + :py:class:`django.contrib.auth.models.User` instance of the + submitter. + + branch (unicode, optional): + The branch the review request was created against. + + local_id (int, optional): + The per-LocalSite ID for the review request. + + bugs_closed (unicode, optional): + A comma-separated list of the bugs closed. + + status (unicode, optional): + The status of the review request. + + See :py:attr:`ReviewRequest.STATUSES ` for a list of + valid values. + + public (bool, optional): + Whether or not the review request is marked as public (i.e., if + it has already been published once). + + publish (bool, optional): + Whether or not to publish after creating the review request. - If create_repository is True, a Repository will be created - automatically. If set, a custom repository cannot be provided. + commit_id (unicode, optional): + An optional commit ID to associate with the review request. + + changenum (unicode, optional): + An optional change number to associate with the review request. + + time_added (datetime.datetime, optional): + When the review request was created. + + last_updated (datetime.datetime, optional): + When the review request was last updated. + + repository (reviewboard.scmtools.models.Repository, optional): + The repository to use for this review request. + + If provided, ``create_repository`` must be ``False``. + + id (int, optional): + The desired primary key. + + create_repository (bool, optional): + Whether or not to create a repository for this review request. + + If ``True``, ``repository`` must be ``None``. - The provided submitter may either be a username or a User object. + target_people (list, optional): + The list of people to assign the review request to. - If publish is True, ReviewRequest.publish() will be called. + target_groups (list, optional): + The list of groups to assign the review request to. + + create_with_history (bool, optional): + Whether or not the review request should support multiple + commits. + + **kwargs (dict): + Additional keyword arguments to pass to the review request + initializer. + + Returns: + reviewboard.reviews.models.review_request.ReviewRequest: + The created review request. + + Raises: + ValueError: + An invalid value was provided during initialization. """ if not local_site: if with_local_site: @@ -773,7 +860,10 @@ class TestCase(FixturesCompilerMixin, DjbletsTestCase): commit_id=commit_id, changenum=changenum, bugs_closed=bugs_closed, - status=status) + status=status, + **kwargs) + + review_request.created_with_history = create_with_history # Set this separately to avoid issues with CounterField updates. review_request.id = id diff --git a/reviewboard/webapi/resources/diff.py b/reviewboard/webapi/resources/diff.py index b4e296fcfb7871a8aaafd990fa3adec338f547ed..6073d2886e21e655a452e89a908c70b8eab6f50c 100644 --- a/reviewboard/webapi/resources/diff.py +++ b/reviewboard/webapi/resources/diff.py @@ -283,7 +283,13 @@ class DiffResource(WebAPIResource): if review_request.repository is None: return INVALID_ATTRIBUTE, { 'reason': 'This review request was created as attachments-' - 'only, with no repository.' + 'only, with no repository.', + } + elif review_request.created_with_history: + return INVALID_ATTRIBUTE, { + 'reason': 'This review request was created with support for ' + 'multiple commits. A regular diff cannot be ' + 'uploaded.', } form_data = request.POST.copy() diff --git a/reviewboard/webapi/resources/review_request.py b/reviewboard/webapi/resources/review_request.py index e0394d9aa2fc7af3cc31577d18e4e1449a6c7060..4a0488fe2ae22d421a74b3ab460c90bee71b019f 100644 --- a/reviewboard/webapi/resources/review_request.py +++ b/reviewboard/webapi/resources/review_request.py @@ -14,6 +14,7 @@ from djblets.webapi.decorators import (webapi_login_required, webapi_response_errors, webapi_request_fields) from djblets.webapi.errors import (DOES_NOT_EXIST, + INVALID_FORM_DATA, NOT_LOGGED_IN, PERMISSION_DENIED) from djblets.webapi.fields import (BooleanFieldType, @@ -30,6 +31,7 @@ from reviewboard.admin.server import build_server_url from reviewboard.diffviewer.errors import (DiffTooBigError, DiffParserError, EmptyDiffError) +from reviewboard.diffviewer.features import dvcs_feature from reviewboard.reviews.errors import (CloseError, PermissionError, PublishError, @@ -134,6 +136,15 @@ class ReviewRequestResource(MarkdownFieldsMixin, WebAPIResource): '``close_description`` field.', 'added_in': '2.0.12', }, + 'created_with_history': { + 'type': BooleanFieldType, + 'description': 'Whether or not the review request was created ' + 'with history support.\n' + '\n' + 'A value of true indicates that the review request ' + 'will have commits attached.', + 'added_in': '4.0', + }, 'depends_on': { 'type': ResourceListFieldType, 'resource': 'reviewboard.webapi.resources.review_request.' @@ -555,6 +566,36 @@ class ReviewRequestResource(MarkdownFieldsMixin, WebAPIResource): else: return False + def serialize_object(self, obj, request=None, *args, **kwargs): + """Serialize a review request. + + This method excludes fields from features that are not enabled. + + Args: + obj (reviewboard.reviews.models.review_request.ReviewRequest): + The review request to serialize. + + request (django.http.HttpRequest, optional): + The HTTP request from the client. + + *args (tuple): + Additional positional arguments. + + **kwargs (dict): + Additional keyword arguments. + + Returns: + dict: + The serialized review request. + """ + result = super(ReviewRequestResource, self).serialize_object( + obj, request=request, *args, **kwargs) + + if not dvcs_feature.is_enabled(request=request): + result.pop('created_with_history') + + return result + def serialize_bugs_closed_field(self, obj, **kwargs): return obj.get_bug_list() @@ -636,9 +677,21 @@ class ReviewRequestResource(MarkdownFieldsMixin, WebAPIResource): 'description': 'If true, and if ``commit_id`` is provided, ' 'the review request information and (when ' 'supported) the idff will be based on the ' - 'commit ID.', + 'commit ID.\n' + '\n' + 'This field cannot be set if ' + '"create_with_history" is set.', 'added_in': '2.0', }, + 'create_with_history': { + 'type': BooleanFieldType, + 'description': 'Whether or not to create the review request ' + 'with support for history.\n' + '\n' + 'This field cannot be set if ' + '"create_from_commit_id" is set.', + 'added_in': '4.0', + }, 'force_text_type': { 'type': ChoiceFieldType, 'choices': MarkdownFieldsMixin.TEXT_TYPES, @@ -666,7 +719,8 @@ class ReviewRequestResource(MarkdownFieldsMixin, WebAPIResource): ) def create(self, request, repository=None, submit_as=None, changenum=None, commit_id=None, local_site_name=None, - create_from_commit_id=False, extra_fields={}, *args, **kwargs): + create_from_commit_id=False, create_with_history=False, + extra_fields={}, *args, **kwargs): """Creates a new review request. The new review request will start off as private and pending, and @@ -723,6 +777,9 @@ class ReviewRequestResource(MarkdownFieldsMixin, WebAPIResource): if not user: return INVALID_USER + if not dvcs_feature.is_enabled(request=request): + create_with_history = False + if repository is not None: try: try: @@ -754,7 +811,8 @@ class ReviewRequestResource(MarkdownFieldsMixin, WebAPIResource): try: review_request = ReviewRequest.objects.create( user, repository, commit_id, local_site, - create_from_commit_id=create_from_commit_id) + create_from_commit_id=create_from_commit_id, + create_with_history=create_with_history) if extra_fields: try: @@ -802,6 +860,10 @@ class ReviewRequestResource(MarkdownFieldsMixin, WebAPIResource): return REPO_INFO_ERROR except ValidationError: return COMMIT_ID_ALREADY_EXISTS + except ValueError as e: + return INVALID_FORM_DATA, { + 'reason': six.text_type(e), + } @webapi_check_local_site @webapi_login_required diff --git a/reviewboard/webapi/tests/test_diff.py b/reviewboard/webapi/tests/test_diff.py index 2369c6f07b553b68a83d4b16837facce4178e03d..7fb3ae1d24eb2c93ca89f0578de414e1bf42c977 100644 --- a/reviewboard/webapi/tests/test_diff.py +++ b/reviewboard/webapi/tests/test_diff.py @@ -2,9 +2,11 @@ from __future__ import unicode_literals import os +from django.core.files.uploadedfile import SimpleUploadedFile from django.utils import six from djblets.webapi.errors import (INVALID_ATTRIBUTE, INVALID_FORM_DATA, PERMISSION_DENIED) +from djblets.webapi.testing.decorators import webapi_test_template from reviewboard import scmtools from reviewboard.diffviewer.models import DiffSet @@ -208,6 +210,34 @@ class ResourceListTests(ExtraDataListMixin, ReviewRequestChildListMixin, self.assertEqual(rsp['stat'], 'fail') self.assertEqual(rsp['err']['code'], INVALID_ATTRIBUTE.code) + @webapi_test_template + def test_post_with_history(self): + """Testing the POST API with a diff and a review request created + with history support + """ + review_request = self.create_review_request(submitter=self.user, + create_repository=True, + create_with_history=True) + + diff = SimpleUploadedFile('diff', + self.DEFAULT_GIT_FILEDIFF_DATA, + content_type='text/x-patch') + + rsp = self.api_post( + get_diff_list_url(review_request), + { + 'path': diff, + 'basedir': '', + }, + expected_status=400) + + self.assertEqual(rsp['stat'], 'fail') + self.assertEqual(rsp['err']['code'], INVALID_ATTRIBUTE.code) + self.assertEqual( + rsp['reason'], + 'This review request was created with support for multiple ' + 'commits. A regular diff cannot be uploaded.') + @six.add_metaclass(BasicTestsMetaclass) class ResourceItemTests(ExtraDataItemMixin, ReviewRequestChildItemMixin, diff --git a/reviewboard/webapi/tests/test_draft_diff.py b/reviewboard/webapi/tests/test_draft_diff.py index 15fcc3d92ec69bd1193caffdeb568464272a6c8c..92803237936a2776e1fd50edac76d7cfafec16ff 100644 --- a/reviewboard/webapi/tests/test_draft_diff.py +++ b/reviewboard/webapi/tests/test_draft_diff.py @@ -2,8 +2,10 @@ from __future__ import unicode_literals import os +from django.core.files.uploadedfile import SimpleUploadedFile from django.utils import six -from djblets.webapi.errors import INVALID_FORM_DATA +from djblets.webapi.errors import INVALID_ATTRIBUTE, INVALID_FORM_DATA +from djblets.webapi.testing.decorators import webapi_test_template from reviewboard import scmtools from reviewboard.diffviewer.models import DiffSet @@ -170,6 +172,31 @@ class ResourceListTests(ExtraDataListMixin, BaseWebAPITestCase): self.assertEqual(rsp['max_size'], self.siteconfig.get('diffviewer_max_diff_size')) + @webapi_test_template + def test_post_with_history(self): + """Testing the POST API with a diff and a review request created + with history support + """ + review_request = self.create_review_request(submitter=self.user, + create_repository=True, + create_with_history=True) + + rsp = self.api_post( + get_draft_diff_list_url(review_request), + { + 'path': SimpleUploadedFile('diff', + self.DEFAULT_GIT_FILEDIFF_DATA), + 'basedir': '', + }, + expected_status=400) + + self.assertEqual(rsp['stat'], 'fail') + self.assertEqual(rsp['err']['code'], INVALID_ATTRIBUTE.code) + self.assertEqual( + rsp['reason'], + 'This review request was created with support for multiple ' + 'commits. A regular diff cannot be uploaded.') + @six.add_metaclass(BasicTestsMetaclass) class ResourceItemTests(ExtraDataItemMixin, BaseWebAPITestCase): diff --git a/reviewboard/webapi/tests/test_review_request.py b/reviewboard/webapi/tests/test_review_request.py index 9c7d5d6aa097c3a74bcd11074700cd35dfecee91..8f3c7dc83a557e19135217edbab480048007fbe4 100644 --- a/reviewboard/webapi/tests/test_review_request.py +++ b/reviewboard/webapi/tests/test_review_request.py @@ -6,6 +6,7 @@ from django.db.models import Q from django.utils import six from django.utils.timezone import get_current_timezone from djblets.db.query import get_object_or_none +from djblets.features.testing import override_feature_check from djblets.testing.decorators import add_fixtures from djblets.webapi.errors import (DOES_NOT_EXIST, INVALID_FORM_DATA, @@ -17,6 +18,7 @@ from pytz import timezone from reviewboard.accounts.backends import AuthBackend from reviewboard.accounts.models import LocalSiteProfile from reviewboard.admin.server import build_server_url +from reviewboard.diffviewer.features import dvcs_feature from reviewboard.reviews.models import (BaseComment, ReviewRequest, ReviewRequestDraft) from reviewboard.reviews.signals import (review_request_closing, @@ -53,8 +55,9 @@ class ResourceListTests(SpyAgency, ExtraDataListMixin, BaseWebAPITestCase): def compare_item(self, item_rsp, review_request): self.assertEqual(item_rsp['id'], review_request.display_id) self.assertEqual(item_rsp['summary'], review_request.summary) - self.assertEqual(item_rsp['extra_data'], review_request.extra_data) - + self.assertEqual( + item_rsp['extra_data'], + self.resource.serialize_extra_data_field(review_request)) # # HTTP GET tests # @@ -818,7 +821,7 @@ class ResourceListTests(SpyAgency, ExtraDataListMixin, BaseWebAPITestCase): self.create_diffset(review_request) self.create_diffset(review_request) - with self.assertNumQueries(13): + with self.assertNumQueries(14): rsp = self.api_get(get_review_request_list_url(), expected_mimetype=review_request_list_mimetype) @@ -1159,6 +1162,154 @@ class ResourceListTests(SpyAgency, ExtraDataListMixin, BaseWebAPITestCase): self.assertEqual(rsp['stat'], 'fail') self.assertEqual(rsp['err']['code'], PERMISSION_DENIED.code) + @add_fixtures(['test_scmtools']) + @webapi_test_template + def test_post_create_with_history_enabled(self): + """Testing the POST API with create_with_history=True when the + DVCS feature is enabled + """ + repository = self.create_repository(tool_name='Git') + + with override_feature_check(dvcs_feature.feature_id, enabled=True): + rsp = self.api_post( + get_review_request_list_url(), + { + 'repository': repository.path, + 'create_with_history': True, + }, + expected_mimetype=review_request_item_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + + item_rsp = rsp['review_request'] + review_request = ReviewRequest.objects.get(pk=item_rsp['id']) + + self.assertTrue(review_request.created_with_history) + self.compare_item(item_rsp, review_request) + + @add_fixtures(['test_scmtools']) + @webapi_test_template + def test_post_create_with_history_disabled(self): + """Testing the POST API with create_with_history=True when the + DVCS feature is disabled + """ + repository = self.create_repository(tool_name='Git') + + with override_feature_check(dvcs_feature.feature_id, enabled=False): + rsp = self.api_post( + get_review_request_list_url(), + { + 'repository': repository.path, + 'create_with_history': True, + }, + expected_mimetype=review_request_item_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + + item_rsp = rsp['review_request'] + review_request = ReviewRequest.objects.get(pk=item_rsp['id']) + + self.assertNotIn('created_with_history', item_rsp) + self.assertFalse(review_request.created_with_history) + self.compare_item(item_rsp, review_request) + + @add_fixtures(['test_scmtools']) + @webapi_test_template + def test_post_create_with_history_enabled_and_create_from_commit_id(self): + """Testing the POST API with create_with_history=True and + create_from_commit_id=True + """ + repository = self.create_repository(tool_name='Git') + + with override_feature_check(dvcs_feature.feature_id, enabled=True): + rsp = self.api_post( + get_review_request_list_url(), + { + 'repository': repository.path, + 'create_with_history': True, + 'create_from_commit_id': True, + 'commit_id': '0' * 40, + }, + expected_status=400) + + self.assertEqual(rsp['stat'], 'fail') + self.assertEqual(rsp['err']['code'], INVALID_FORM_DATA.code) + self.assertEqual(rsp['reason'], + 'create_from_commit_id and create_with_history ' + 'cannot both be set to True.') + + @add_fixtures(['test_scmtools']) + @webapi_test_template + def test_post_create_with_history_disabled_and_create_from_commit_id(self): + """Testing the POST API with create_with_history=True and + create_from_commit_id=True when the DVCS feature is disabled + """ + repository = self.create_repository(tool_name='Test') + + with override_feature_check(dvcs_feature.feature_id, enabled=False): + rsp = self.api_post( + get_review_request_list_url(), + { + 'repository': repository.path, + 'create_with_history': True, + 'create_from_commit_id': True, + 'commit_id': '0' * 40, + }, + expected_mimetype=review_request_item_mimetype, + expected_status=201) + + self.assertEqual(rsp['stat'], 'ok') + self.assertIn('review_request', rsp) + + item_rsp = rsp['review_request'] + review_request = ReviewRequest.objects.get(pk=item_rsp['id']) + self.assertFalse(review_request.created_with_history) + + self.compare_item(item_rsp, review_request) + + @add_fixtures(['test_scmtools']) + @webapi_test_template + def test_post_create_with_history_enabled_no_repository(self): + """Testing the POST API with no repository when the DVCS feature + is enabled + """ + with override_feature_check(dvcs_feature.feature_id, enabled=True): + rsp = self.api_post( + get_review_request_list_url(), + { + 'create_with_history': True, + }, + expected_status=400) + + self.assertEqual(rsp['stat'], 'fail') + self.assertEqual(rsp['err']['code'], INVALID_FORM_DATA.code) + self.assertEqual(rsp['reason'], + 'create_with_history requires a repository.') + + @add_fixtures(['test_scmtools']) + @webapi_test_template + def test_post_create_with_history_disabled_no_repository(self): + """Testing the POST API with nor repository when the DVCS feature + is disabled + """ + with override_feature_check(dvcs_feature.feature_id, enabled=False): + rsp = self.api_post( + get_review_request_list_url(), + { + 'create_with_history': True, + }, + expected_mimetype=review_request_item_mimetype, + expected_status=201) + + self.assertEqual(rsp['stat'], 'ok') + self.assertIn('review_request', rsp) + + item_rsp = rsp['review_request'] + review_request = ReviewRequest.objects.get(pk=item_rsp['id']) + self.assertFalse(review_request.created_with_history) + + self.compare_item(item_rsp, review_request) + def test_get_or_create_user_auth_backend(self): """Testing the POST review-requests/?submit_as= API with AuthBackend.get_or_create_user failure @@ -1234,7 +1385,9 @@ class ResourceItemTests(ExtraDataItemMixin, BaseWebAPITestCase): def compare_item(self, item_rsp, review_request): self.assertEqual(item_rsp['id'], review_request.display_id) self.assertEqual(item_rsp['summary'], review_request.summary) - self.assertEqual(item_rsp['extra_data'], review_request.extra_data) + self.assertEqual( + item_rsp['extra_data'], + self.resource.serialize_extra_data_field(review_request)) self.assertEqual(item_rsp['absolute_url'], self.base_url + review_request.get_absolute_url()) @@ -1465,6 +1618,37 @@ class ResourceItemTests(ExtraDataItemMixin, BaseWebAPITestCase): self.assertIn('issue_resolved_count', rr) self.assertIn('issue_verifying_count', rr) + @webapi_test_template + def test_get_dvcs_feature_enabled(self): + """Testing the GET API includes DVCS-specific fields in + the response when the DVCS feature is enabled + """ + with override_feature_check(dvcs_feature.feature_id, enabled=True): + review_request = self.create_review_request(publish=True) + rsp = self.api_get(get_review_request_item_url(review_request.pk), + expected_mimetype=review_request_item_mimetype) + + self.assertIn('review_request', rsp) + item_rsp = rsp['review_request'] + + self.assertIn('created_with_history', item_rsp) + self.assertFalse(item_rsp['created_with_history']) + + @webapi_test_template + def test_get_dvcs_feature_disabled(self): + """Testing the GET API does not include DVCS-specific fields in + the response when the DVCS feature is disabled + """ + with override_feature_check(dvcs_feature.feature_id, enabled=False): + review_request = self.create_review_request(publish=True) + rsp = self.api_get(get_review_request_item_url(review_request.pk), + expected_mimetype=review_request_item_mimetype) + + self.assertIn('review_request', rsp) + item_rsp = rsp['review_request'] + + self.assertNotIn('created_with_history', item_rsp) + # # HTTP PUT tests # diff --git a/reviewboard/webapi/tests/test_review_request_draft.py b/reviewboard/webapi/tests/test_review_request_draft.py index 089b50d611791cef6ce6ff411229afba39a4699d..0955c6c13c6687f63ab7057ac6ae54a7ab48365b 100644 --- a/reviewboard/webapi/tests/test_review_request_draft.py +++ b/reviewboard/webapi/tests/test_review_request_draft.py @@ -41,7 +41,8 @@ class ResourceTests(SpyAgency, ExtraDataListMixin, ExtraDataItemMixin, self.assertEqual(item_rsp['description'], draft.description) self.assertEqual(item_rsp['testing_done'], draft.testing_done) - self.assertEqual(item_rsp['extra_data'], draft.extra_data) + self.assertEqual(item_rsp['extra_data'], + self.resource.serialize_extra_data_field(draft)) self.assertEqual(item_rsp['changedescription'], changedesc.text) if changedesc.rich_text: