Submitter:	Jonathan Ellis	Reviewers
Branch:		Groups:	reviewboard
Bugs:		People:
Change Number:	None	Repository:	Review Board SVN

Description:

the main point of this change is to allow a staff user to specify submit_as=a-different-user when creating a review request so that user will be the one notified of reviews, etc.  this enables using a post-commit hook to auto-create reviews for all or part of a repository.  there are some side concerns related to this:
 
 * basedir must be allowed to be empty when creating diffs against a svn repository as opposed to a working copy.  that is what the diffviewer change does; for the web interface the functionality is identical, but the json manually rips out the basedir when necessary, allowing a svn repo diff to be treated as "absolute."
 * added is_visible_to and is_mutable_by methods to the Request models to encapsulate the notion that "staff can always view and modify requests"
 * moved sending of mail into the models to avoid duplication of code in json and view
 * json new_diff includes traceback when something unexpected goes wrong
 * add get_changeset to svn to allow update_from_changenum for that scm

Testing Done:

Diff revision 1 (Latest)

Files Changed:

/trunk/reviewboard/diffviewer/forms.py: 3 changes [ 1 2 3 ]
/trunk/reviewboard/reviews/models.py: 5 changes [ 1 2 3 4 5 ]
/trunk/reviewboard/reviews/views.py: 4 changes [ 1 2 3 4 ]
/trunk/reviewboard/scmtools/svn.py: 4 changes [ 1 2 3 4 ]
/trunk/reviewboard/webapi/json.py: 21 changes [ 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 ]

Expand changes

         if len(files) == 0:
             raise EmptyDiffError(_("The diff file is empty"))
         # Check that we can actually get all these files.
-        if tool.get_diffs_use_absolute_paths():
+        if 'basedir' in self.fields:
-            basedir = ''
-        else:
             basedir = smart_unicode(self.cleaned_data['basedir'])
         else:
             basedir = ''
         for f in files:
             f2, revision = tool.parse_diff_revision(f.origFile, f.origInfo)
             filename = os.path.join(basedir, f2)

 import os
 import re
 from datetime import datetime
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.db import models
 from django.db.models import Q, permalink
 from django.utils.html import escape
 from django.utils.safestring import mark_safe
 from reviewboard.diffviewer.models import DiffSet, DiffSetHistory, FileDiff
 from reviewboard.scmtools.models import Repository
 from reviewboard.utils.fields import ModificationTimestampField
 from reviewboard.utils.templatetags.htmlutils import crop_image, thumbnail
 from reviewboard.reviews.email import mail_review, mail_review_request, \
                                       mail_reply
 class InvalidChangeNumberError(Exception):
     def __init__(self):
         Exception.__init__(self, None)
             # If this is not a pending review request now, delete any
             # and all ReviewRequestVisit objects.
             self.visits.all().delete()
         super(ReviewRequest, self).save()
     def is_visible_to(self, user):
         return self.public or self.is_mutable_by(user)
     def is_mutable_by(self, user):
         return self.submitter == user or user.is_staff
     class Admin:
         list_display = ('summary', 'submitter', 'status', 'public', \
                         'last_updated')
         list_filter = ('public', 'status', 'time_added', 'last_updated')
         else:
             changes['diff'] = False
         request.save()
         if settings.SEND_REVIEW_MAIL and changes:
             mail_review_request(request.submitter, request, changes)
         return changes
     class Admin:
         list_display = ('summary', 'submitter', 'last_updated')
         list_filter = ('last_updated',)
         for comment in self.screenshot_comments.all():
             comment.timetamp = self.timestamp
             comment.save()
         if settings.SEND_REVIEW_MAIL:
             if self.is_reply():
                 mail_reply(self.user, self)
             else:
                 mail_review(self.user, self)
     def delete(self):
         """
         Deletes this review.
         This will enforce that all contained comments are also deleted.

     Publishes a new review request or the changes on a draft for a review
     request.
     """
     review_request = get_object_or_404(ReviewRequest, pk=review_request_id)
-    if review_request.submitter == request.user:
+    if review_request.is_mutable_by(request.user):
         if not review_request.target_groups and \
            not review_request.target_people:
-            pass # FIXME show an error
+            # FIXME error nicely
             raise Exception('cannot publish with no reviewers set')
         try:
             draft = review_request.reviewrequestdraft_set.get()
             # This will in turn save the review request, so we'll be done.
             # The draft didn't exist, so we must save the review request
             # ourselves.
             review_request.public = True
             review_request.save()
-        if settings.SEND_REVIEW_MAIL:
-            mail_review_request(request.user, review_request)
         return HttpResponseRedirect(review_request.get_absolute_url())
     else:
         raise HttpResponseForbidden() # XXX Error out

 import re
 try:
     from pysvn import ClientError, Revision, opt_revision_kind
 except ImportError:
     pass
 from reviewboard.diffviewer.parser import DiffParser
 from reviewboard.scmtools.core import \
-    SCMError, FileNotFoundError, SCMTool, HEAD, PRE_CREATION, UNKNOWN
+    ChangeSet, SCMError, FileNotFoundError, SCMTool, HEAD, PRE_CREATION, UNKNOWN
 class SVNTool(SCMTool):
     def __init__(self, repository):
         self.repopath = repository.path
         if self.repopath[-1] == '/':
             self.repopath = self.repopath[:-1]
         SCMTool.__init__(self, repository)
         import pysvn
         self.client = pysvn.Client()
         def callback_ssl(trust_dict):
             return True, trust_dict['failures'], False
         self.client.callback_ssl_server_trust_prompt = callback_ssl
         if repository.username:
             self.client.set_default_username(str(repository.username))
         if repository.password:
             self.client.set_default_password(str(repository.password))
                                    self.__normalize_revision(revision))
         except ClientError, e:
             stre = str(e)
             if 'File not found' in stre:
                 raise FileNotFoundError(path, revision, str(e))
-            elif 'callback_ssl_server_trust_prompt required' in stre:
-                raise SCMError(
-                    'HTTPS certificate not accepted.  Please ensure that ' +
-                    'the proper certificate exists in ~/.subversion/auth ' +
-                    'for the user that reviewboard is running as.')
             else:
                 raise SCMError(e)
     def get_changeset(self, changesetid):
         r = self.__normalize_revision(changesetid)
         log = self.client.log(self.repopath, r, r, True)[0]
         cs = ChangeSet()
         cs.changenum = changesetid
         cs.summary = log['message']
         def pathinfo(path):
             return path['action'] + ' ' + path['path']
         cs.description = '\n'.join(pathinfo(path) for path in log['changed_paths'])
         cs.username = log['author']
         return cs
     def parse_diff_revision(self, file_str, revision_str):
         if revision_str == "(working copy)":
             return file_str, HEAD
         # Binary diffs don't provide revision information, so we set a fake

 from djblets.util.misc import get_object_or_none
 from reviewboard.accounts.models import Profile
 from reviewboard.diffviewer.forms import UploadDiffForm, EmptyDiffError
 from reviewboard.diffviewer.models import FileDiff, DiffSet
-from reviewboard.reviews.email import mail_review, mail_review_request, \
-                                      mail_reply
 from reviewboard.reviews.forms import UploadScreenshotForm
 from reviewboard.reviews.models import ChangeNumberInUseError, \
                                        InvalidChangeNumberError, \
                                        ReviewRequest, Review, Group, Comment, \
                                        ReviewRequestDraft, Screenshot, \
 INVALID_ATTRIBUTE         = JsonError(102, "Invalid attribute")
 NOT_LOGGED_IN             = JsonError(103, "You are not logged in")
 LOGIN_FAILED              = JsonError(104, "The username or password was " +
                                            "not correct")
 INVALID_FORM_DATA         = JsonError(105, "One or more fields had errors")
 INVALID_USER              = JsonError(106, "User does not exist")
 UNSPECIFIED_DIFF_REVISION = JsonError(200, "Diff revision not specified")
 INVALID_DIFF_REVISION     = JsonError(201, "Invalid diff revision")
 INVALID_ACTION            = JsonError(202, "Invalid action specified")
 INVALID_CHANGE_NUMBER     = JsonError(203, "The change number specified " +
 @require_POST
 def new_review_request(request):
     try:
         repository_path = request.POST.get('repository_path',
                                            settings.DEFAULT_REPOSITORY_PATH)
-        repository_id = request.POST.get('repository_id', None)
+        repository_id = request.POST.get('repository_id')
         submit_as = request.POST.get('submit_as')
         if submit_as:
             if not request.user.is_staff:
                 return JsonResponseError(request, PERMISSION_DENIED)
             try:
                 user = User.objects.filter(username=submit_as)[0]
             except IndexError:
                 return JsonResponseError(request, INVALID_USER)
         else:
             user = request.user
         if repository_path == None and repository_id == None:
             return JsonResponseError(request, MISSING_REPOSITORY)
         repository = Repository.objects.get(
             Q(path=repository_path) |
             Q(mirror_path=repository_path))
         review_request = ReviewRequest.objects.create(
-            request.user, repository, request.POST.get('changenum', None))
+            user, repository, request.POST.get('changenum', None))
         return JsonResponse(request, {'review_request': review_request})
     except Repository.DoesNotExist, e:
         return JsonResponseError(request, INVALID_REPOSITORY,
                                  {'repository_path': repository_path})
     """
     Returns the review request with the specified ID.
     """
     review_request = get_object_or_404(ReviewRequest, pk=review_request_id)
-    if not review_request.public and review_request.submitter != request.user:
+    if not review_request.is_visible_to(request.user):
         return JsonResponseError(request, PERMISSION_DENIED)
     return JsonResponse(request, {'review_request': review_request})
 @json_login_required
 def review_request_by_changenum(request, repository_id, changenum):
     try:
         review_request = ReviewRequest.objects.get(changenum=changenum,
                                                    repository=repository_id)
-        if not review_request.public and \
+        if not review_request.is_visible_to(request.user):
-           review_request.submitter != request.user:
             return JsonResponseError(request, PERMISSION_DENIED)
         return JsonResponse(request, {'review_request': review_request})
     except ReviewRequest.DoesNotExist:
         return JsonResponseError(request, INVALID_CHANGE_NUMBER)
     try:
         draft = ReviewRequestDraft.objects.get(review_request=review_request)
     except ReviewRequestDraft.DoesNotExist:
         return JsonResponseError(request, DOES_NOT_EXIST)
-    if review_request.submitter != request.user:
+    if not review_request.is_mutable_by(request.user):
         return JsonResponseError(request, PERMISSION_DENIED)
     draft.delete()
     return JsonResponse(request)
         draft = ReviewRequestDraft.objects.get(review_request=review_request_id)
         review_request = draft.review_request
     except ReviewRequestDraft.DoesNotExist:
         return JsonResponseError(request, DOES_NOT_EXIST)
-    if review_request.submitter != request.user:
+    if not review_request.is_mutable_by(request.user):
         return JsonResponseError(request, PERMISSION_DENIED)
-    changes = draft.save_draft()
+    draft.save_draft()
     draft.delete()
-    if settings.SEND_REVIEW_MAIL and changes:
-        mail_review_request(request.user, review_request, changes)
     return JsonResponse(request)
 def find_user(username):
     try:
                 return user
     return None

Review Board

beta

This change has been marked as submitted.

Improve json usefulness for post-commit hook

Diff revision 1 (Latest)