diff --git a/reviewboard/diffviewer/diffutils.py b/reviewboard/diffviewer/diffutils.py index 6692c034cb7dbcdcbf3603b8d8b3eddd7051bf49..313409fba4f6dd771ad7ae73c2324a834b5702a0 100644 --- a/reviewboard/diffviewer/diffutils.py +++ b/reviewboard/diffviewer/diffutils.py @@ -600,7 +600,7 @@ def get_diff_files(diffset, filediff=None, interdiffset=None, else: filediffs = diffset.files.select_related().all() - filediffs = list(filediffs) + # filediffs = list(filediffs) filediffs = exclude_filediff_ancestors(filediffs, diffset, diffset_file_graph) diff --git a/reviewboard/hostingsvcs/github.py b/reviewboard/hostingsvcs/github.py index 931ad1d35252eb292b6c4dd511a2a37f46124054..69de021fab6e1fb5c50abc4a93e3369847df1e33 100644 --- a/reviewboard/hostingsvcs/github.py +++ b/reviewboard/hostingsvcs/github.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +import dateutil import hashlib import hmac import json @@ -8,9 +9,11 @@ import re import uuid from collections import defaultdict +import ipdb from django import forms from django.conf import settings from django.conf.urls import patterns, url +from django.contrib.auth.models import User from django.contrib.sites.models import Site from django.core.cache import cache from django.core.exceptions import ObjectDoesNotExist @@ -25,6 +28,8 @@ from django.views.decorators.http import require_POST from djblets.siteconfig.models import SiteConfiguration from reviewboard.admin.server import build_server_url, get_server_url +from reviewboard.diffviewer.forms import EmptyDiffForm +from reviewboard.diffviewer.models import DiffCommit from reviewboard.hostingsvcs.bugtracker import BugTracker from reviewboard.hostingsvcs.errors import (AuthorizationError, HostingServiceError, @@ -41,6 +46,7 @@ from reviewboard.hostingsvcs.service import (HostingService, HostingServiceClient) from reviewboard.hostingsvcs.utils.paginator import (APIPaginator, ProxyPaginator) +from reviewboard.reviews.models import ReviewRequest, ReviewRequestDraft from reviewboard.scmtools.core import Branch, Commit from reviewboard.scmtools.errors import FileNotFoundError, SCMError from reviewboard.site.urlresolvers import local_site_reverse @@ -472,6 +478,7 @@ class GitHub(HostingService, BugTracker): supports_repositories = True supports_two_factor_auth = True supports_list_remote_repositories = True + supports_pull_requests = True supported_scmtools = ['Git'] has_repository_hook_instructions = True @@ -483,7 +490,10 @@ class GitHub(HostingService, BugTracker): url(r'^hooks/close-submitted/$', 'reviewboard.hostingsvcs.github.post_receive_hook_close_submitted', - name='github-hooks-close-submitted') + name='github-hooks-close-submitted'), + url(r'^hooks/pull-request/$', + 'reviewboard.hostingsvcs.github.post_receive_hook_pull_request', + name='github-hooks-pull-request') ) # This should be the prefix for every field on the plan forms. @@ -494,6 +504,14 @@ class GitHub(HostingService, BugTracker): '-granting-organization-access-on-github' ) + # Possible statuses allowed by GitHub's Status API + PR_STATUS_SUCCESS = 'success' + PR_STATUS_ERROR = 'error' + PR_STATUS_PENDING = 'pending' + PR_STATUS_FAILURE = 'failure' + PR_STATUSES = [PR_STATUS_SUCCESS, PR_STATUS_ERROR, + PR_STATUS_FAILURE, PR_STATUS_PENDING] + def get_api_url(self, hosting_url): """Returns the API URL for GitHub. @@ -962,6 +980,34 @@ class GitHub(HostingService, BugTracker): 'hook_uuid': repository.get_or_create_hooks_uuid(), })) + def update_pull_request(self, review_request): + """Updates the status of the pull request created by the review request. + + This will set the status of the latest commit in the pull request + on GitHub to either 'success' (if the review request is approved), + 'error' (if any open issues exist), or 'pending' otherwise. + + Args: + review_request (reviewboard.scmtools.models.ReviewRequest): + The review request that was created from a pull request. + """ + if review_request.approved: + status = self.PR_STATUS_SUCCESS + elif review_request.issue_open_count > 0: + status = self.PR_STATUS_ERROR + else: + status = self.PR_STATUS_PENDING + + ipdb.set_trace() + + target_url = build_server_url(review_request.get_absolute_url()) + + self.api_set_status_for_commit(review_request.repository, + review_request.pull_request_commit_head, + status, + description='Posted from ReviewBoard', + target_url=target_url) + def _reset_authorization(self, client_id, client_secret, token): """Resets the authorization info for an OAuth app-linked token. @@ -978,6 +1024,54 @@ class GitHub(HostingService, BugTracker): username=client_id, password=client_secret) + def api_set_status_for_commit(self, repository, commit, state, + description='', target_url=''): + """Set the status for a given commit. + + GitHub allows commits to be marked with a status that is shown + in pull requests containing those commits. + + Args: + repository (reviewboard.scmtools.models.Repository): + The repository that the commit belongs to. + + commit (unicode): + The full ID of the commit for which to set the status. + + state (unicode): + The status to set for the commit. + + This must be one of the following values: + + * :py:attr:`PR_STATUS_SUCCESS` + * :py:attr:`PR_STATUS_ERROR` + * :py:attr:`PR_STATUS_FAILURE` + * :py:attr:`PR_STATUS_PENDING` + + description (unicode): + The description to associate with the status. + + target_url (unicode, optional): + The HTTP(S) URL associated with the status. + """ + + if state not in self.PR_STATUSES: + raise ValueError("Must be one of: %s" % ', ' + .join(self.PR_STATUSES)) + + status_url = self._build_api_url('%s/statuses/%s' % ( + self._get_repo_api_url(repository), + commit + )) + + data = { + 'state': state, + 'description': description, + 'target_url': target_url + } + + return self.client.json_post(status_url, json.dumps(data)) + def _delete_auth_token(self, auth_id, password, two_factor_auth_code=None): """Requests that an authorization token be deleted. @@ -1032,6 +1126,109 @@ class GitHub(HostingService, BugTracker): @require_POST +def post_receive_hook_pull_request(request, local_site_name=None, + repository_id=None, + hosting_service_id=None): + """Create or update review requests from pull requests. + + Args: + request (django.http.HttpRequest) + The HttpRequest that called this hook. + local_site_name (Optional[unicode]) + The local site name. + repository_id (Optional[Integer]) + The ID that uniquely identifies a repository. + hosting_service_id (Optional[unicode]) + The name of the hosting service the repository is hosted at. + + Returns: + django.http.HttpResponse + A HttpResponse if the review request was updated successfully, or + a HttpResponseBadRequest with the error message otherwise. + """ + + hook_event = request.META.get('HTTP_X_GITHUB_EVENT') + + if hook_event == 'ping': + # A ping event is sent to verify the webhook works. + return HttpResponse() + elif hook_event != 'pull_request': + # We only want pull_request events, as they contain the right data. + return HttpResponseBadRequest( + 'Only "pull_request" events are supported.') + + repository = get_repository_for_hook(repository_id, hosting_service_id, + local_site_name) + + invalid_response = verify_hook_identity(repository, request) + if invalid_response is not None: + return invalid_response + + try: + payload = json.loads(request.body) + except ValueError: + return HttpResponseBadRequest('Invalid payload format.') + + pr_data = payload['pull_request'] + + if payload['action'] == 'opened': + try: + submitter = _get_review_request_creator_from_pull_request( + repository, payload) + except ObjectDoesNotExist: + return HttpResponseBadRequest( + 'No ReviewBoard user found matching the submitter ' + 'of the pull request.') + + rr = ReviewRequest.objects.create(submitter, repository) + rr.pull_request_id = pr_data['id'] + rr.pull_request_commit_head = pr_data['head']['sha'] + + draft = rr.get_draft() + draft.summary = 'PR from Github - %s' % pr_data['title'] + draft.description = pr_data['body'] + + try: + draft.diffset = _create_diffset_from_commits( + rr, repository, payload, request) + except SCMError as e: + logging.error('Unable to fetch commits: %s', e) + return HttpResponseBadRequest('Commits could not be retrieved.') + + draft.save() + rr.save() + elif payload['action'] == 'synchronize': + try: + rr = ReviewRequest.objects.get(pull_request_id=pr_data['id'], + repository=repository) + except ReviewRequest.DoesNotExist: + return HttpResponseBadRequest( + 'The corresponding review request does not exist.') + + rr.pull_request_commit_head = pr_data['head']['sha'] + + # TODO: Should the summary and description be updated too? + draft = ReviewRequestDraft.create(rr) + + try: + draft.diffset = _create_diffset_from_commits( + rr, repository, payload, request) + except SCMError as e: + logging.error('Unable to fetch commits: %s', e) + return HttpResponseBadRequest('Commits could not be retrieved.') + + draft.save() + rr.save() + + repository.hosting_service.api_set_status_for_commit( + repository, rr.pull_request_commit_head, 'pending') + else: + return HttpResponseBadRequest('Invalid action.') + + return HttpResponse() + + +@require_POST def post_receive_hook_close_submitted(request, local_site_name=None, repository_id=None, hosting_service_id=None): @@ -1049,18 +1246,9 @@ def post_receive_hook_close_submitted(request, local_site_name=None, repository = get_repository_for_hook(repository_id, hosting_service_id, local_site_name) - # Validate the hook against the stored UUID. - m = hmac.new(bytes(repository.get_or_create_hooks_uuid()), request.body, - hashlib.sha1) - - sig_parts = request.META.get('HTTP_X_HUB_SIGNATURE').split('=') - - if sig_parts[0] != 'sha1' or len(sig_parts) != 2: - # We don't know what this is. - return HttpResponseBadRequest('Unsupported HTTP_X_HUB_SIGNATURE') - - if m.hexdigest() != sig_parts[1]: - return HttpResponseBadRequest('Bad signature.') + invalid_response = verify_hook_identity(repository, request) + if invalid_response is not None: + return invalid_response try: payload = json.loads(request.body) @@ -1080,6 +1268,135 @@ def post_receive_hook_close_submitted(request, local_site_name=None, return HttpResponse() +def verify_hook_identity(repository, request): + """Validate the webhook request by checking it against the stored UUID. + + This ensures the web hook was actually called by GitHub after it was + set up by a user, not by anyone who makes a request to the hook URL. + + Args: + repository (reviewboard.scamtools.models.Repository) + The repository the hook was registered in. + request (django.http.HttpRequest) + The HTTP request that called the hook. + + Returns: + django.http.HttpResponseBadRequest + None if the hook was successfully validated, or an instance of + HttpResponseBadRequest containing the error message if unsuccessful. + """ + m = hmac.new(bytes(repository.get_or_create_hooks_uuid()), request.body, + hashlib.sha1) + + sig_parts = request.META.get('HTTP_X_HUB_SIGNATURE').split('=', 1) + + if sig_parts[0] != 'sha1': + # We don't know what this is. + logging.error('Unknown HTTP_X_HUB_SIGNATURE: %s', + request.META.get('HTTP_X_HUB_SIGNATURE')) + return HttpResponseBadRequest('Unsupported HTTP_X_HUB_SIGNATURE') + + if m.hexdigest() != sig_parts[1]: + return HttpResponseBadRequest('Bad signature.') + + return None + + +def _get_review_request_creator_from_pull_request(repository, payload): + """Returns the user that the review request should be associated with. + + The user will be the submitter of the review request that was automatically + created from the corresponding pull request. + + Args: + repository (reviewboard.scmtools.models.Repository) + The repository the pull request was created in. + payload (Dictionary) + The payload sent by GitHub when the pull request hook was called. + + Returns: + django.contrib.auth.models.User + The user that created the review request. + """ + + # TODO: We need a better way of linking users. + + # For now, just use the email from the creator of the PR + user_profile, _ = repository.hosting_service.client.json_get( + payload['pull_request']['head']['user']['url']) + return User.objects.get(email=user_profile['email']) + + +def _create_diffset_from_commits(review_request, repo, payload, request=None): + """Create a DiffSet to represent all the commits in a pull request. + + Args: + review_request (reviewboard.reviews.models.ReviewRequest): + The review request associated with the pull request that + triggered the webhook. + + repo (reviewboard.scmtools.models.Repository): + The repository that the review request is associated with. + + payload (dict): + The request payload from Github as the result of triggering + the pull request webhook. + + request (django.http.HttpRequest, optional): + The request the payload originated from. Defaults to None. + + Returns: + reviewboard.diffviewer.models.DiffSet: + A DiffSet that represents all the commits in a pull request at the time + of the request. + + Raises: + reviewboard.scmtools.errors.SCMError: + Raised when any of the commits referenced in the payload + could not be retrieved, or one of those commits are a merge commit. + """ + diff_form = EmptyDiffForm(review_request, request=request) + diffset = diff_form.create() + + commits_url = payload['pull_request']['commits_url'] + commits_data = repo.hosting_service.client.api_get(commits_url) + + for commit_data in commits_data: + is_merge = len(commit_data['parents']) > 1 + + if is_merge: + # Merge commits are currently not supported by RB. + raise SCMError('Merge commits are not supported.') + + commit = commit_data['commit'] + author = commit['author'] + committer = commit['committer'] + parents = commit_data['parents'] + + DiffCommit.objects.create_from_data( + repository=repo, + diff_file_name='diff', + diff_file_contents=repo.hosting_service.get_change( + repo, commit_data['sha']).diff, + parent_diff_file_name=None, + parent_diff_file_contents=None, + request=request, + commit_id=commit_data['sha'], + parent_id=parents[0]['sha'], + merge_parent_ids=None, + author_name=author['name'], + author_email=author['email'], + author_date=dateutil.parser.parse(author['date']), + committer_name=committer['name'], + committer_email=committer['email'], + committer_date=dateutil.parser.parse(committer['date']), + description=commit['message'], + commit_type='change', + diffset=diffset) + + return diffset + + def _get_review_request_id_to_commits_map(payload, server_url, repository): """Returns a dictionary, mapping a review request ID to a list of commits. diff --git a/reviewboard/hostingsvcs/service.py b/reviewboard/hostingsvcs/service.py index 2185e7b679c93a042bd5635e3e8017e12564cd1f..294d5706fe44b11c4ee525f06ecebd3c40292e49 100644 --- a/reviewboard/hostingsvcs/service.py +++ b/reviewboard/hostingsvcs/service.py @@ -178,6 +178,7 @@ class HostingService(object): supports_two_factor_auth = False supports_list_remote_repositories = False has_repository_hook_instructions = False + supports_pull_requests = False self_hosted = False repository_url_patterns = None @@ -428,6 +429,19 @@ class HostingService(object): """ raise NotImplementedError + def update_pull_request(self, review_request): + """Updates the pull request's status. + + This performs an update against the pull request + that created the review request. + + Args: + review_request (reviewboard.scmtools.models.ReviewRequest): + The review request that was created from a pull request. + """ + if not self.supports_pull_requests: + raise NotImplementedError + @classmethod def get_bug_tracker_requires_username(cls, plan=None): if not cls.supports_bug_trackers: diff --git a/reviewboard/hostingsvcs/tests/test_github.py b/reviewboard/hostingsvcs/tests/test_github.py index ca553ce591d1f2eee0e2e303dc03353f2a1c9693..2dcea072f705fb094107d5658cca0d70ece77e4f 100644 --- a/reviewboard/hostingsvcs/tests/test_github.py +++ b/reviewboard/hostingsvcs/tests/test_github.py @@ -6,6 +6,7 @@ import json from hashlib import md5 from textwrap import dedent +from django.contrib.auth.models import User from django.core.exceptions import ObjectDoesNotExist from django.utils import six from django.utils.six.moves import cStringIO as StringIO @@ -13,7 +14,9 @@ from django.utils.six.moves.urllib.error import HTTPError from django.utils.six.moves.urllib.parse import urlparse from djblets.testing.decorators import add_fixtures +from reviewboard.diffviewer.models import DiffSet from reviewboard.scmtools.core import Branch +from reviewboard.hostingsvcs import github from reviewboard.hostingsvcs.models import HostingServiceAccount from reviewboard.hostingsvcs.repository import RemoteRepository from reviewboard.hostingsvcs.tests.testcases import ServiceTests @@ -33,6 +36,7 @@ class GitHubTests(ServiceTests): self.assertTrue(self.service_class.supports_bug_trackers) self.assertTrue(self.service_class.supports_repositories) self.assertFalse(self.service_class.supports_ssh_key_association) + self.assertTrue(self.service_class.supports_pull_requests) def test_public_field_values(self): """Testing the GitHub public plan repository field values""" @@ -1190,6 +1194,229 @@ class GitHubTests(ServiceTests): HTTP_X_GITHUB_EVENT=event, HTTP_X_HUB_SIGNATURE='sha1=%s' % m.hexdigest()) + @add_fixtures(['test_users', 'test_scmtools']) + def test_pull_request_hook_ping(self): + """Testing GitHub pull_request hook ping""" + account = self._get_hosting_account() + account.save() + + repository = self.create_repository(hosting_account=account) + + url = local_site_reverse( + 'github-hooks-pull-request', + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'github', + }) + + response = self._post_pull_request_payload( + url, repository.get_or_create_hooks_uuid(), 'opened', 0, + event='ping') + self.assertEqual(response.status_code, 200) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_pull_request_hook_with_invalid_signature(self): + """Testing GitHub pull_request hook with invalid signature""" + account = self._get_hosting_account() + account.save() + + repository = self.create_repository(hosting_account=account) + + url = local_site_reverse( + 'github-hooks-pull-request', + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'github', + }) + + response = self._post_pull_request_payload( + url, 'bad-secret', 'opened', 0, + event='synchronize') + self.assertEqual(response.status_code, 400) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_pull_request_hook_with_invalid_action(self): + """Testing GitHub pull_request hook with an invalid action""" + account = self._get_hosting_account() + account.save() + + repository = self.create_repository(hosting_account=account) + + url = local_site_reverse( + 'github-hooks-pull-request', + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'github', + }) + + response = self._post_pull_request_payload( + url, repository.get_or_create_hooks_uuid(), 'foo', 0) + self.assertEqual(response.status_code, 400) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_pull_request_hook_with_invalid_event(self): + """Testing GitHub pull_request hook with non-pull request event""" + account = self._get_hosting_account() + account.save() + + repository = self.create_repository(hosting_account=account) + + url = local_site_reverse( + 'github-hooks-pull-request', + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'github', + }) + + response = self._post_pull_request_payload( + url, repository.get_or_create_hooks_uuid(), 'opened', 0, + event='bar') + self.assertEqual(response.status_code, 400) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_pull_request_hook_creates_review_request(self): + """Testing GitHub pull_request hook creates a review request""" + review_request = self._create_review_request_from_pull_request() + self.assertIsNotNone(review_request) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_pull_request_hook_updates_review_request(self): + """Testing GitHub pull_request hook updates a review request""" + review_request = self._create_review_request_from_pull_request() + repository = review_request.repository + user = User.objects.get(email='admin@example.com') + review_request.publish(user=user) + + self.assertIsNone(review_request.get_draft()) + + url = local_site_reverse( + 'github-hooks-pull-request', + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'github', + }) + + self._post_pull_request_payload( + url, repository.get_or_create_hooks_uuid(), 'synchronize', 1337) + + self.assertIsNotNone(review_request.get_draft()) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_pull_request_hook_sets_pending_status_on_new_review_request(self): + """Testing GitHub pull request created with a pending status.""" + review_request = self._create_review_request_from_pull_request() + repository = review_request.repository + + self.assertTrue(github.GitHub.api_set_status_for_commit.called_with( + repository, review_request.pull_request_commit_head, 'pending')) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_pull_request_hook_sets_error_status_with_open_issue(self): + """Testing GitHub pull request set with error status with open issue""" + review_request = self._create_review_request_from_pull_request() + repository = review_request.repository + + # Attach an open issue to this review request + user = User.objects.get(email='admin@example.com') + review = self.create_review(review_request, user=user) + self.create_general_comment(review, issue_opened=True) + review.publish() + + self.assertEqual( + github.GitHub.api_set_status_for_commit.last_call.args, + (repository, review_request.pull_request_commit_head, 'error')) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_pull_request_hook_sets_success_status_with_approval(self): + """Testing GitHub pull request set with success status when approved""" + review_request = self._create_review_request_from_pull_request() + repository = review_request.repository + + # Add a ship-it to make it approved + self.assertTrue(review_request.issue_open_count == 0) + user = User.objects.get(email='admin@example.com') + review = self.create_review(review_request, user=user, ship_it=True) + review.publish() + + self.assertEqual( + github.GitHub.api_set_status_for_commit.last_call.args, + (repository, review_request.pull_request_commit_head, 'success')) + + def _create_review_request_from_pull_request(self): + """Creates a review request when a pull request is created.""" + def _get_change(*args, **kwargs): + class FakeCommit: + def __init__(self): + self.diff = ( + """diff --git a/asdfasdfasfd.txt b/asdfasdfasfd.txt + new file mode 100644 + index 0000000..6afd275 + --- /dev/null + +++ b/asdfasdfasfd.txt + @@ -0,0 +1,9 @@ + +I""") + + return FakeCommit() + + def _create_from_data(*args, **kwargs): + return None + + def _fake_user(*args): + return User.objects.get(email='admin@example.com') + + account = self._get_hosting_account() + account.save() + + repository = self.create_repository(hosting_account=account) + + self.spy_on(github._get_review_request_creator_from_pull_request, + call_fake=_fake_user) + self.spy_on(github.GitHub.get_change, call_fake=_get_change) + self.spy_on(github.GitHub.api_set_status_for_commit, + call_original=False) + self.spy_on(DiffSet.objects.create_from_data, + call_fake=_create_from_data) + + url = local_site_reverse( + 'github-hooks-pull-request', + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'github', + }) + + response = self._post_pull_request_payload( + url, repository.get_or_create_hooks_uuid(), 'opened', 1337) + self.assertEqual(response.status_code, 200) + review_request = ReviewRequest.objects.get(pull_request_id=1337) + + return review_request + + def _post_pull_request_payload(self, url, secret, action, pr_id, + event='pull_request'): + payload = json.dumps({ + # NOTE: This payload only contains the content we make + # use of in the hook. + 'action': action, + 'pull_request': { + 'id': pr_id, + 'state': 'open', + 'title': 'Test PR', + 'head': { + 'sha': 'fc393664cc4e94f0bdd1807b146176589a9a74eb' + }, + 'body': 'Test body' + } + }) + + m = hmac.new(bytes(secret), payload, hashlib.sha1) + + return self.client.post( + url, + payload, + content_type='application/json', + HTTP_X_GITHUB_EVENT=event, + HTTP_X_HUB_SIGNATURE='sha1=%s' % m.hexdigest()) + def _test_check_repository(self, expected_user='myuser', **kwargs): def _http_get(service, url, *args, **kwargs): self.assertEqual( diff --git a/reviewboard/reviews/builtin_fields.py b/reviewboard/reviews/builtin_fields.py index 1e351350e5e149f4c6db45d18c92713a9faefca9..b97e62325d209bf158a5ed29c81e7b8dce0b1db0 100644 --- a/reviewboard/reviews/builtin_fields.py +++ b/reviewboard/reviews/builtin_fields.py @@ -482,6 +482,23 @@ class CommitField(BuiltinFieldMixin, BaseReviewRequestField): return escape(commit_id) +class PullRequestField(BuiltinFieldMixin, BaseReviewRequestField): + """ The Pull Request field on a review request. + + This displays the ID of the pull request the review request + was created from. + """ + + field_id = 'pull_request_id' + label = _('Pull Request') + + def should_render(self, pull_request_id): + return bool(pull_request_id) + + def render_value(self, pull_request_id): + return escape(pull_request_id) + + class DiffField(ReviewRequestPageDataMixin, BaseReviewRequestField): """Represents a newly uploaded diff on a review request. @@ -986,6 +1003,7 @@ class InformationFieldSet(BaseReviewRequestFieldSet): BlocksField, ChangeField, CommitField, + PullRequestField, ] diff --git a/reviewboard/reviews/evolutions/__init__.py b/reviewboard/reviews/evolutions/__init__.py index 2f1314422fb393ad984f69d05c37d00705d2a5e4..64a0ead915ca7f1e6935cea4e500f4c1b36d130d 100644 --- a/reviewboard/reviews/evolutions/__init__.py +++ b/reviewboard/reviews/evolutions/__init__.py @@ -35,4 +35,5 @@ SEQUENCE = [ 'general_comments', 'add_owner_to_draft', 'status_update_timeout', + 'pull_request' ] diff --git a/reviewboard/reviews/evolutions/pull_request.py b/reviewboard/reviews/evolutions/pull_request.py new file mode 100644 index 0000000000000000000000000000000000000000..8c97799491e7ce25af6ebafb07d98b07a6ca8cc2 --- /dev/null +++ b/reviewboard/reviews/evolutions/pull_request.py @@ -0,0 +1,13 @@ +from django_evolution.mutations import AddField, ChangeMeta +from django.db import models + + +MUTATIONS = [ + AddField('ReviewRequest', 'pull_request_id', models.CharField, + max_length=64, null=True, db_index=True), + ChangeMeta('ReviewRequest', 'unique_together', + ((u'commit_id', u'repository'), + (u'changenum', u'repository'), + (u'local_site', u'local_id'), + (u'pull_request_id', u'repository'))) +] diff --git a/reviewboard/reviews/models/base_comment.py b/reviewboard/reviews/models/base_comment.py index 0e5866e89daa1ae81992fb4e297289433d5d3a9a..ec4bd00263803087defc2dee066ab56085b4915f 100644 --- a/reviewboard/reviews/models/base_comment.py +++ b/reviewboard/reviews/models/base_comment.py @@ -8,6 +8,7 @@ from django.utils.encoding import python_2_unicode_compatible from django.utils.translation import ugettext_lazy as _ from djblets.db.fields import CounterField, JSONField from djblets.db.managers import ConcurrencyManager +from reviewboard.hostingsvcs.models import HostingServiceAccount @python_2_unicode_compatible @@ -138,13 +139,17 @@ class BaseComment(models.Model): # the review. review = self.get_review() + review_request = review.review_request + hosting_svc_account = HostingServiceAccount.objects.get( + repositories__review_requests__reviews=review) + if not review.public: review.timestamp = self.timestamp review.save() else: if (not self.is_reply() and - self.issue_opened and - self._loaded_issue_status != self.issue_status): + self.issue_opened and + self._loaded_issue_status != self.issue_status): # The user has toggled the issue status of this comment, # so update the issue counts for the review request. old_field = ReviewRequest.ISSUE_COUNTER_FIELDS[ @@ -159,8 +164,10 @@ class BaseComment(models.Model): new_field: 1, }) - q = ReviewRequest.objects.filter(pk=review.review_request_id) - q.update(last_review_activity_timestamp=self.timestamp) + hosting_svc_account.service.update_pull_request( + review_request) + review_request.last_review_activity_timestamp = self.timestamp + review_request.save() except ObjectDoesNotExist: pass diff --git a/reviewboard/reviews/models/review.py b/reviewboard/reviews/models/review.py index 75008ed7fa0872550b5ada7c49352895e0a222cd..cab6f350c19c31665ad5b6b1565d946d401db078 100644 --- a/reviewboard/reviews/models/review.py +++ b/reviewboard/reviews/models/review.py @@ -11,6 +11,7 @@ from djblets.db.fields import CounterField, JSONField from djblets.db.query import get_object_or_none from reviewboard.diffviewer.models import DiffSet +from reviewboard.hostingsvcs.models import HostingServiceAccount from reviewboard.reviews.managers import ReviewManager from reviewboard.reviews.models.base_comment import BaseComment from reviewboard.reviews.models.diff_comment import Comment @@ -273,6 +274,13 @@ class Review(models.Model): user=user, review=self, to_submitter_only=to_submitter_only) + review_request = self.review_request + hosting_svc_account = HostingServiceAccount.objects.get( + repositories__review_requests__reviews=self) + + hosting_svc_account.service.update_pull_request( + review_request) + def delete(self): """Deletes this review. diff --git a/reviewboard/reviews/models/review_request.py b/reviewboard/reviews/models/review_request.py index a1ed3adc86639db35f81ebe7c4c7805a8f8ab5a3..9fa4b42b593557b9da734eaee9631363b6a46e74 100644 --- a/reviewboard/reviews/models/review_request.py +++ b/reviewboard/reviews/models/review_request.py @@ -232,6 +232,9 @@ class ReviewRequest(BaseReviewRequestDetails): verbose_name=_('Dependencies'), related_name='blocks') + pull_request_id = models.CharField(_('Pull Request ID'), + max_length=64, blank=True, + null=True, db_index=True) # Review-related information # The timestamp representing the last public activity of a review. @@ -324,8 +327,6 @@ class ReviewRequest(BaseReviewRequestDetails): ReviewRequest.objects.filter(pk=self.pk).update( commit_id=six.text_type(self.changenum)) - return self.commit_id - return None def set_commit(self, commit_id): @@ -368,6 +369,33 @@ class ReviewRequest(BaseReviewRequestDetails): return self._approval_failure + @property + def pull_request_commit_head(self): + """Return the commit ID of the head of the pull request. + + Returns: + unicode + The sha of the latest commit in the pull request. + """ + if self.extra_data is None: + self.extra_data = {} + + return self.extra_data.get('pull_request_commit_head') + + @pull_request_commit_head.setter + def pull_request_commit_head(self, commit_id): + """Set the commit ID of the head of the pull request. + + Args: + commit_id (unicode) + The sha of the commit to set as the latest commit + of the pull request. + """ + if self.extra_data is None: + self.extra_data = {} + + self.extra_data['pull_request_commit_head'] = commit_id + 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 @@ -1083,9 +1111,12 @@ class ReviewRequest(BaseReviewRequestDetails): class Meta: app_label = 'reviews' ordering = ['-last_updated', 'submitter', 'summary'] - unique_together = (('commit_id', 'repository'), - ('changenum', 'repository'), - ('local_site', 'local_id')) + unique_together = ( + ('commit_id', 'repository'), + ('changenum', 'repository'), + ('local_site', 'local_id'), + ('pull_request_id', 'repository') + ) permissions = ( ("can_change_status", "Can change status"), ("can_submit_as_another_user", "Can submit as another user"),