diff --git a/reviewboard/diffviewer/forms.py b/reviewboard/diffviewer/forms.py index 1232df850e9eeaf65d616aa6e1375dc0c563e7ec..67c27115710a84cfa2dc16cfb81e02c8af9377d1 100644 --- a/reviewboard/diffviewer/forms.py +++ b/reviewboard/diffviewer/forms.py @@ -2,23 +2,181 @@ from __future__ import unicode_literals +from dateutil.parser import isoparse from django import forms +from django.core.exceptions import ValidationError from django.utils.encoding import smart_unicode -from django.utils.translation import ugettext_lazy as _ +from django.utils.translation import ugettext, ugettext_lazy as _ -from reviewboard.diffviewer.models import DiffSet +from reviewboard.diffviewer.models import DiffCommit, DiffSet +from reviewboard.diffviewer.validators import (COMMIT_ID_LENGTH, + validate_commit_id) + + +class UploadCommitForm(forms.Form): + """The form for uploading a diff and creating a DiffCommit.""" + + diff = forms.FileField( + label=_('Diff'), + help_text=_('The new diff to upload.')) + + parent_diff = forms.FileField( + label=_('Parent diff'), + help_text=_('An optional diff that the main diff is based on. ' + 'This is usually used for distributed revision control ' + 'systems (Git, Mercurial, etc.).'), + required=False) + + commit_id = forms.CharField( + label=_('Commit ID'), + help_text=_('The ID of this commit.'), + max_length=COMMIT_ID_LENGTH, + validators=[validate_commit_id]) + + parent_id = forms.CharField( + label=_('Parent commit ID'), + help_text=_('The ID of the parent commit.'), + max_length=COMMIT_ID_LENGTH, + validators=[validate_commit_id]) + + commit_message = forms.CharField( + label=_('Description'), + help_text=_('The commit message.')) + + author_name = forms.CharField( + label=_('Author name'), + help_text=_('The name of the author of this commit.'), + max_length=DiffCommit.NAME_MAX_LENGTH) + + author_email = forms.CharField( + label=_('Author e-mail address'), + help_text=_('The e-mail address of the author of this commit.'), + max_length=DiffCommit.EMAIL_MAX_LENGTH, + widget=forms.EmailInput) + + author_date = forms.CharField( + label=_('Author date'), + help_text=_('The date and time this commit was authored.')) + + committer_name = forms.CharField( + label=_('Committer name'), + help_text=_('The name of the committer of this commit.'), + max_length=DiffCommit.NAME_MAX_LENGTH, + required=False) + + committer_email = forms.CharField( + label=_('Committer e-mail address'), + help_text=_('The e-mail address of the committer of this commit.'), + max_length=DiffCommit.EMAIL_MAX_LENGTH, + widget=forms.EmailInput, + required=False) + + committer_date = forms.CharField( + label=_('Committer date'), + help_text=_('The date and time this commit was committed.'), + required=False) + + def __init__(self, diffset, request=None, *args, **kwargs): + """Initialize the form. + + Args: + diffset (reviewboard.diffviewer.models.diffset.DiffSet): + The DiffSet to attach the created DiffCommit to. + + request (django.http.HttpRequest, optional): + The HTTP request from the client. + + *args (tuple): + Additional positional arguments. + + **kwargs (dict): + Additional keyword arguments. + """ + super(UploadCommitForm, self).__init__(*args, **kwargs) + + self.diffset = diffset + self.request = request + + def create(self): + """Create the DiffCommit. + + Returns: + reviewboard.diffviewer.models.diffcommit.DiffCommit: + The created DiffCommit. + """ + assert self.is_valid() + + return DiffCommit.objects.create_from_upload( + request=self.request, + diffset=self.diffset, + repository=self.diffset.repository, + diff_file=self.cleaned_data['diff'], + parent_diff_file=self.cleaned_data.get('parent_diff'), + commit_message=self.cleaned_data['commit_message'], + commit_id=self.cleaned_data['commit_id'], + parent_id=self.cleaned_data['parent_id'], + author_name=self.cleaned_data['author_name'], + author_email=self.cleaned_data['author_email'], + author_date=self.cleaned_data['author_date'], + committer_name=self.cleaned_data.get('committer_name'), + committer_email=self.cleaned_data.get('committer_email'), + committer_date=self.cleaned_data.get('committer_date')) + + def clean(self): + """Clean the form. + + Returns: + dict: + The cleaned form data. + + Raises: + django.core.exceptions.ValidationError: + The form data was not valid. + """ + super(UploadCommitForm, self).clean() + + if self.diffset.history_id is not None: + # A diffset will have a history attached if and only if it has been + # published, in which case we cannot attach further commits to it. + raise ValidationError(ugettext( + 'Cannot upload commits to a published diff.')) + + return self.cleaned_data + + def clean_author_date(self): + """Parse the date and time in the ``author_date`` field. + + Returns: + datetime.datetime: + The parsed date and time. + """ + try: + return isoparse(self.cleaned_data['author_date']) + except ValueError: + raise ValidationError(ugettext( + 'This date must be in ISO 8601 format.')) + + def clean_committer_date(self): + """Parse the date and time in the ``committer_date`` field. + + Returns: + datetime.datetime: + The parsed date and time. + """ + try: + return isoparse(self.cleaned_data['committer_date']) + except ValueError: + raise ValidationError(ugettext( + 'This date must be in ISO 8601 format.')) class UploadDiffForm(forms.Form): """The form for uploading a diff and creating a DiffSet.""" - basedir = forms.CharField( - label=_('Base Directory'), - help_text=_('The absolute path in the repository the diff was ' - 'generated in.')) path = forms.FileField( label=_('Diff'), help_text=_('The new diff to upload.')) + parent_diff_path = forms.FileField( label=_('Parent Diff'), help_text=_('An optional diff that the main diff is based on. ' @@ -26,6 +184,11 @@ class UploadDiffForm(forms.Form): 'systems (Git, Mercurial, etc.).'), required=False) + basedir = forms.CharField( + label=_('Base Directory'), + help_text=_('The absolute path in the repository the diff was ' + 'generated in.')) + base_commit_id = forms.CharField( label=_('Base Commit ID'), help_text=_('The ID/revision this change is built upon.'), @@ -36,10 +199,10 @@ class UploadDiffForm(forms.Form): Args: repository (reviewboard.scmtools.models.Repository): - The repository the DiffSet is to be created against. + The repository the diff will be uploaded against. - request (django.http.HttpRequest): - The current HTTP request. + request (django.http.HttpRequest, optional): + The HTTP request from the client. *args (tuple): Additional positional arguments. @@ -48,10 +211,11 @@ class UploadDiffForm(forms.Form): Additional keyword arguments. """ super(UploadDiffForm, self).__init__(*args, **kwargs) + self.repository = repository self.request = request - if self.repository.get_scmtool().diffs_use_absolute_paths: + if repository.get_scmtool().diffs_use_absolute_paths: # This SCMTool uses absolute paths, so there's no need to ask # the user for the base directory. del(self.fields['basedir']) @@ -95,9 +259,9 @@ class UploadDiffForm(forms.Form): return DiffSet.objects.create_from_upload( repository=self.repository, + diffset_history=diffset_history, diff_file=self.cleaned_data['path'], parent_diff_file=self.cleaned_data.get('parent_diff_path'), - diffset_history=diffset_history, basedir=self.cleaned_data.get('basedir', ''), base_commit_id=self.cleaned_data['base_commit_id'], request=self.request) diff --git a/reviewboard/diffviewer/tests/test_forms.py b/reviewboard/diffviewer/tests/test_forms.py index 9882bcc01e4237a221e5a82452e22530dccdcb0e..99b376d2c1d77ec08637a2fa7d75a73399d56cdd 100644 --- a/reviewboard/diffviewer/tests/test_forms.py +++ b/reviewboard/diffviewer/tests/test_forms.py @@ -8,11 +8,175 @@ from reviewboard.admin.import_utils import has_module from reviewboard.diffviewer.diffutils import (get_original_file, get_patched_file, patch) -from reviewboard.diffviewer.forms import UploadDiffForm +from reviewboard.diffviewer.forms import UploadCommitForm, UploadDiffForm +from reviewboard.diffviewer.models import DiffSet, DiffSetHistory from reviewboard.scmtools.models import Repository, Tool from reviewboard.testing import TestCase +class UploadCommitFormTests(SpyAgency, TestCase): + """Unit tests for UploadCommitForm.""" + + fixtures = ['test_scmtools'] + + _default_form_data = { + 'base_commit_id': '1234', + 'basedir': '/', + 'commit_id': 'r1', + 'parent_id': 'r0', + 'commit_message': 'Message', + 'author_name': 'Author', + 'author_email': 'author@example.org', + 'author_date': '1970-01-01 00:00:00+0000', + 'committer_name': 'Committer', + 'committer_email': 'committer@example.org', + 'committer_date': '1970-01-01 00:00:00+0000', + } + + def setUp(self): + super(UploadCommitFormTests, self).setUp() + + self.repository = self.create_repository(tool_name='Test') + self.spy_on(self.repository.get_file_exists, + call_fake=lambda *args, **kwargs: True) + self.diffset = DiffSet.objects.create_empty(repository=self.repository) + + def test_create(self): + """Testing UploadCommitForm.create""" + diff = SimpleUploadedFile('diff', + self.DEFAULT_GIT_FILEDIFF_DATA, + content_type='text/x-patch') + + form = UploadCommitForm( + diffset=self.diffset, + data=self._default_form_data.copy(), + files={ + 'diff': diff, + }) + + self.assertTrue(form.is_valid()) + commit = form.create() + + self.assertEqual(self.diffset.files.count(), 1) + self.assertEqual(self.diffset.commits.count(), 1) + self.assertEqual(commit.files.count(), 1) + self.assertEqual(set(self.diffset.files.all()), + set(commit.files.all())) + + def test_clean_parent_diff_path(self): + """Testing UploadCommitForm.clean() for a subsequent commit with a + parent diff + """ + diff = SimpleUploadedFile('diff', + self.DEFAULT_GIT_FILEDIFF_DATA, + content_type='text/x-patch') + parent_diff = SimpleUploadedFile('parent_diff', + self.DEFAULT_GIT_FILEDIFF_DATA, + content_type='text/x-patch') + + form = UploadCommitForm( + diffset=self.diffset, + data=self._default_form_data.copy(), + files={ + 'diff': diff, + 'parent_diff': parent_diff, + }) + + self.assertTrue(form.is_valid()) + form.create() + + form = UploadCommitForm( + diffset=self.diffset, + data=dict( + self._default_form_data, + **{ + 'parent_id': 'r1', + 'commit_id': 'r2', + } + ), + files={ + 'diff': diff, + 'parent_diff': parent_diff, + }) + + self.assertTrue(form.is_valid()) + self.assertNotIn('parent_diff', form.errors) + + def test_clean_published_diff(self): + """Testing UploadCommitForm.clean() for a DiffSet that has already been + published + """ + diff = SimpleUploadedFile('diff', + self.DEFAULT_GIT_FILEDIFF_DATA, + content_type='text/x-patch') + + form = UploadCommitForm( + diffset=self.diffset, + data=self._default_form_data, + files={ + 'diff': diff, + }) + + self.assertTrue(form.is_valid()) + form.create() + + self.diffset.history = DiffSetHistory.objects.create() + self.diffset.save(update_fields=('history_id',)) + + form = UploadCommitForm( + diffset=self.diffset, + data=dict( + self._default_form_data, + parent_id='r1', + commit_id='r0', + ), + files={ + 'diff_path': SimpleUploadedFile( + 'diff', + self.DEFAULT_GIT_FILEDIFF_DATA, + content_type='text/x-patch'), + }) + + self.assertFalse(form.is_valid()) + self.assertNotEqual(form.non_field_errors, []) + + def test_clean_author_date(self): + """Testing UploadCommitForm.clean_author_date""" + diff = SimpleUploadedFile('diff', + self.DEFAULT_GIT_FILEDIFF_DATA, + content_type='text/x-patch') + + form = UploadCommitForm( + diffset=self.diffset, + data=dict(self._default_form_data, **{ + 'author_date': 'Jan 1 1970', + }), + files={ + 'diff': diff, + }) + + self.assertFalse(form.is_valid()) + self.assertIn('author_date', form.errors) + + def test_clean_committer_date(self): + """Testing UploadCommitForm.clean_committer_date""" + diff = SimpleUploadedFile('diff', + self.DEFAULT_GIT_FILEDIFF_DATA, + content_type='text/x-patch') + + form = UploadCommitForm( + diffset=self.diffset, + data=dict(self._default_form_data, **{ + 'committer_date': 'Jun 1 1970', + }), + files={ + 'diff': diff, + }) + + self.assertFalse(form.is_valid()) + self.assertIn('committer_date', form.errors) + + class UploadDiffFormTests(SpyAgency, TestCase): """Unit tests for UploadDiffForm."""