| LEFT | RIGHT |
|---|---|
| 1 # Copyright 2008 Google Inc. | 1 # Copyright 2008 Google Inc. |
| 2 # | 2 # |
| 3 # Licensed under the Apache License, Version 2.0 (the "License"); | 3 # Licensed under the Apache License, Version 2.0 (the "License"); |
| 4 # you may not use this file except in compliance with the License. | 4 # you may not use this file except in compliance with the License. |
| 5 # You may obtain a copy of the License at | 5 # You may obtain a copy of the License at |
| 6 # | 6 # |
| 7 # http://www.apache.org/licenses/LICENSE-2.0 | 7 # http://www.apache.org/licenses/LICENSE-2.0 |
| 8 # | 8 # |
| 9 # Unless required by applicable law or agreed to in writing, software | 9 # Unless required by applicable law or agreed to in writing, software |
| 10 # distributed under the License is distributed on an "AS IS" BASIS, | 10 # distributed under the License is distributed on an "AS IS" BASIS, |
| 11 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | 11 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. |
| 12 # See the License for the specific language governing permissions and | 12 # See the License for the specific language governing permissions and |
| 13 # limitations under the License. | 13 # limitations under the License. |
| 14 | 14 |
| 15 """Views for Rietveld. | 15 """Views for Rietveld. |
| 16 | 16 |
| 17 This requires Django 0.97.pre. | 17 This requires Django 0.97.pre. |
| 18 """ | 18 """ |
| 19 | 19 |
| 20 | 20 |
| 21 ### Imports ### | 21 ### Imports ### |
| 22 | 22 |
| 23 | 23 |
| 24 # Python imports | 24 # Python imports |
| 25 import os | 25 import os |
| 26 import cgi | 26 import cgi |
| 27 import random | 27 import random |
| 28 import logging | 28 import logging |
| 29 import binascii | 29 import binascii |
| 30 import sets | |
|
GvR
2008/05/11 19:20:49
This is Python 2.5, it has a built-in set type. (E
| |
| 30 | 31 |
| 31 # AppEngine imports | 32 # AppEngine imports |
| 32 from google.appengine.api import mail | 33 from google.appengine.api import mail |
| 33 from google.appengine.api import users | 34 from google.appengine.api import users |
| 34 from google.appengine.api import urlfetch | 35 from google.appengine.api import urlfetch |
| 35 from google.appengine.ext import db | 36 from google.appengine.ext import db |
| 36 from google.appengine.ext.db import djangoforms | 37 from google.appengine.ext.db import djangoforms |
| 37 | 38 |
| 38 # DeadlineExceededError can live in two different places | 39 # DeadlineExceededError can live in two different places |
| 39 # TODO(guido): simplify once this is fixed. | 40 # TODO(guido): simplify once this is fixed. |
| 40 try: | 41 try: |
| 41 # When deployed | 42 # When deployed |
| 42 from google.appengine.runtime import DeadlineExceededError | 43 from google.appengine.runtime import DeadlineExceededError |
| 43 except ImportError: | 44 except ImportError: |
| 44 # In the development server | 45 # In the development server |
| 45 from google.appengine.runtime.apiproxy_errors import DeadlineExceededError | 46 from google.appengine.runtime.apiproxy_errors import DeadlineExceededError |
| 46 | 47 |
| 47 # Django imports | 48 # Django imports |
| 48 # TODO(guido): Don't import classes/functions directly. | 49 # TODO(guido): Don't import classes/functions directly. |
| 49 from django import newforms as forms | 50 from django import newforms as forms |
| 50 from django.http import HttpResponse, HttpResponseRedirect | 51 from django.http import HttpResponse, HttpResponseRedirect |
| 51 from django.http import HttpResponseForbidden, HttpResponseNotFound | 52 from django.http import HttpResponseForbidden, HttpResponseNotFound |
| 52 from django.shortcuts import render_to_response | 53 from django.shortcuts import render_to_response |
| 53 import django.template | 54 import django.template |
| 54 | 55 |
| 55 # Local imports | 56 # Local imports |
| 56 import models | 57 import models |
| 57 import engine | 58 import engine |
| 58 import library | 59 import library |
| 59 import patching | 60 import patching |
| 60 | 61 |
| 61 # Add our own template library. | 62 # Add our own template library. |
| 62 if not django.template.libraries.get('rietveld.library', None): | 63 if not django.template.libraries.get('rietveld.library', None): |
| 63 django.template.add_to_builtins('rietveld.library') | 64 django.template.add_to_builtins('rietveld.library') |
| 64 | 65 |
| 65 | 66 |
| 66 ### Constants ### | 67 ### Constants ### |
| 67 | 68 |
| 68 | 69 |
| 69 SENDER = 'svncodereview@gmail.com' # An administrator allowed to send mail | 70 SENDER = 'svncodereview@gmail.com' # An administrator allowed to send mail |
| 70 | 71 |
| 71 IS_DEV = os.environ['SERVER_SOFTWARE'].startswith('Dev') # Development server | 72 IS_DEV = os.environ['SERVER_SOFTWARE'].startswith('Dev') # Development server |
| 72 | 73 |
| 73 | 74 |
| 74 ### Form classes ### | 75 ### Form classes ### |
| 75 | 76 |
| 76 | 77 |
| 77 class IssueBaseForm(forms.Form): | 78 class IssueBaseForm(forms.Form): |
| 78 | 79 |
| 79 subject = forms.CharField(max_length=100, | 80 subject = forms.CharField(max_length=100, |
| (...skipping 59 matching lines...) Show 10 above Show 10 below | |
| 139 url = forms.URLField(required=False, | 140 url = forms.URLField(required=False, |
| 140 max_length=2083, | 141 max_length=2083, |
| 141 widget=forms.TextInput(attrs={'size': 60})) | 142 widget=forms.TextInput(attrs={'size': 60})) |
| 142 | 143 |
| 143 | 144 |
| 144 class UploadForm(forms.Form): | 145 class UploadForm(forms.Form): |
| 145 | 146 |
| 146 subject = forms.CharField(max_length=100) | 147 subject = forms.CharField(max_length=100) |
| 147 description = forms.CharField(max_length=10000, required=False) | 148 description = forms.CharField(max_length=10000, required=False) |
| 148 base = forms.CharField(max_length=2000) | 149 base = forms.CharField(max_length=2000) |
| 149 data = forms.FileField() | 150 data = forms.FileField() |
| 150 issue = forms.IntegerField(required=False) | 151 issue = forms.IntegerField(required=False) |
| 151 | 152 |
| 152 def get_base(self): | 153 def get_base(self): |
| 153 return self.cleaned_data.get('base') | 154 return self.cleaned_data.get('base') |
| 154 | 155 |
| 155 | 156 |
| 156 class EditForm(IssueBaseForm): | 157 class EditForm(IssueBaseForm): |
| 157 pass | 158 pass |
| 158 | 159 |
| 159 | 160 |
| 160 class RepoForm(djangoforms.ModelForm): | 161 class RepoForm(djangoforms.ModelForm): |
| 161 | 162 |
| 162 class Meta: | 163 class Meta: |
| 163 model = models.Repository | 164 model = models.Repository |
| 164 exclude = ['owner'] | 165 exclude = ['owner'] |
| 165 | 166 |
| 166 | 167 |
| 167 class BranchForm(djangoforms.ModelForm): | 168 class BranchForm(djangoforms.ModelForm): |
| 168 | 169 |
| 169 class Meta: | 170 class Meta: |
| 170 model = models.Branch | 171 model = models.Branch |
| 171 exclude = ['owner'] | 172 exclude = ['owner'] |
| 172 | 173 |
| 173 | 174 |
| 174 class PublishForm(forms.Form): | 175 class PublishForm(forms.Form): |
| 175 | 176 |
| 176 subject = forms.CharField(max_length=100, | 177 subject = forms.CharField(max_length=100, |
| 177 widget=forms.TextInput(attrs={'size': 60})) | 178 widget=forms.TextInput(attrs={'size': 60})) |
| 178 reviewers = forms.CharField(required=False, | 179 reviewers = forms.CharField(required=False, |
| 179 max_length=1000, | 180 max_length=1000, |
| 180 widget=forms.TextInput(attrs={'size': 60})) | 181 widget=forms.TextInput(attrs={'size': 60})) |
| 181 send_mail = forms.BooleanField() | 182 send_mail = forms.BooleanField() |
| 182 message = forms.CharField(required=False, | 183 message = forms.CharField(required=False, |
| 183 max_length=10000, | 184 max_length=10000, |
| 184 widget=forms.Textarea(attrs={'cols': 60})) | 185 widget=forms.Textarea(attrs={'cols': 60})) |
| 185 | 186 |
| 186 | 187 |
| 187 class MiniPublishForm(forms.Form): | 188 class MiniPublishForm(forms.Form): |
| 188 | 189 |
| 189 send_mail = forms.BooleanField() | 190 send_mail = forms.BooleanField() |
|
GvR
2008/05/11 23:27:41
The order of the fields should be the same as in P
| |
| 190 reviewers = forms.CharField(required=False, | 191 reviewers = forms.CharField(required=False, |
| 191 max_length=1000, | 192 max_length=1000, |
| 192 widget=forms.TextInput(attrs={'size': 60})) | 193 widget=forms.TextInput(attrs={'size': 60})) |
| 193 message = forms.CharField(required=False, | 194 message = forms.CharField(required=False, |
| 194 max_length=10000, | 195 max_length=10000, |
| 195 widget=forms.Textarea(attrs={'cols': 60})) | 196 widget=forms.Textarea(attrs={'cols': 60})) |
| 196 | 197 |
| 197 | 198 |
| 198 class SettingsForm(forms.Form): | 199 class SettingsForm(forms.Form): |
| 199 | 200 |
| 200 nickname = forms.CharField(max_length=30) | 201 nickname = forms.CharField(max_length=30) |
| 201 | 202 |
| 202 | 203 |
| 203 ### Helper functions ### | 204 ### Helper functions ### |
| 204 | 205 |
| 205 | 206 |
| 206 # Counter displayed (by respond()) below) on every page showing how | 207 # Counter displayed (by respond()) below) on every page showing how |
| 207 # many requests the current incarnation has handled, not counting | 208 # many requests the current incarnation has handled, not counting |
| 208 # redirects. Rendered by templates/base.html. | 209 # redirects. Rendered by templates/base.html. |
| 209 counter = 0 | 210 counter = 0 |
| 210 | 211 |
| 211 def respond(request, template, params=None): | 212 def respond(request, template, params=None): |
| 212 """Helper to render a response, passing standard stuff to the response. | 213 """Helper to render a response, passing standard stuff to the response. |
| 213 | 214 |
| 214 Args: | 215 Args: |
| 215 request: The request object. | 216 request: The request object. |
| 216 template: The template name; '.html' is appended automatically. | 217 template: The template name; '.html' is appended automatically. |
| 217 params: A dict giving the template parameters; modified in-place. | 218 params: A dict giving the template parameters; modified in-place. |
| 218 | 219 |
| 219 Returns: | 220 Returns: |
| 220 Whatever render_to_response(template, params) returns. | 221 Whatever render_to_response(template, params) returns. |
| 221 | 222 |
| 222 Raises: | 223 Raises: |
| 223 Whatever render_to_response(template, params) raises. | 224 Whatever render_to_response(template, params) raises. |
| 224 """ | 225 """ |
| 225 global counter | 226 global counter |
| 226 counter += 1 | 227 counter += 1 |
| 227 if params is None: | 228 if params is None: |
| 228 params = {} | 229 params = {} |
| 229 must_choose_nickname = False | 230 must_choose_nickname = False |
| 230 if request.user is not None: | 231 if request.user is not None: |
| 231 account = models.Account.get_account_for_user(request.user) | 232 account = models.Account.get_account_for_user(request.user) |
| 232 delta = account.created - account.modified | 233 delta = account.created - account.modified |
| 233 if delta.days < 0: | 234 if delta.days < 0: |
| 234 delta = -delta | 235 delta = -delta |
| 235 must_choose_nickname = delta.days == 0 and delta.seconds < 2 | 236 must_choose_nickname = delta.days == 0 and delta.seconds < 2 |
| 236 params['request'] = request | 237 params['request'] = request |
| 237 params['counter'] = counter | 238 params['counter'] = counter |
| 238 params['user'] = request.user | 239 params['user'] = request.user |
| 239 params['is_admin'] = request.user_is_admin | 240 params['is_admin'] = request.user_is_admin |
| (...skipping 283 matching lines...) Show 10 above Show 10 below | |
| 523 assert url | 524 assert url |
| 524 try: | 525 try: |
| 525 fetch_result = urlfetch.fetch(url) | 526 fetch_result = urlfetch.fetch(url) |
| 526 except Exception, err: | 527 except Exception, err: |
| 527 form.errors['url'] = [str(err)] | 528 form.errors['url'] = [str(err)] |
| 528 return None | 529 return None |
| 529 if fetch_result.status_code != 200: | 530 if fetch_result.status_code != 200: |
| 530 form.errors['url'] = ['HTTP status code %s' % fetch_result.status_code] | 531 form.errors['url'] = ['HTTP status code %s' % fetch_result.status_code] |
| 531 return None | 532 return None |
| 532 data = db.Blob(fetch_result.content) | 533 data = db.Blob(fetch_result.content) |
| 533 | 534 |
| 534 return data, url | 535 return data, url |
| 535 | 536 |
| 536 | 537 |
| 537 @issue_owner_required | 538 @issue_owner_required |
| 538 def add(request): | 539 def add(request): |
| 539 """/<issue>/add - Add a new PatchSet to an existing Issue.""" | 540 """/<issue>/add - Add a new PatchSet to an existing Issue.""" |
| 540 issue = request.issue | 541 issue = request.issue |
| 541 form = AddForm(request.POST, request.FILES) | 542 form = AddForm(request.POST, request.FILES) |
| 542 if not add_patchset_from_form(request, issue, form): | 543 if not add_patchset_from_form(request, issue, form): |
| 543 return show(request, issue.key().id(), form) | 544 return show(request, issue.key().id(), form) |
| 544 return HttpResponseRedirect('/%s' % issue.key().id()) | 545 return HttpResponseRedirect('/%s' % issue.key().id()) |
| 545 | 546 |
| 546 | 547 |
| 547 def add_patchset_from_form(request, issue, form, message_key='message'): | 548 def add_patchset_from_form(request, issue, form, message_key='message'): |
| 548 """Helper for add() and upload().""" | 549 """Helper for add() and upload().""" |
| 549 # TODO(guido): use a transaction like in _make_new(); may be share more code? | 550 # TODO(guido): use a transaction like in _make_new(); may be share more code? |
| 550 if form.is_valid(): | 551 if form.is_valid(): |
| 551 data_url = _get_data_url(form) | 552 data_url = _get_data_url(form) |
| 552 if not form.is_valid(): | 553 if not form.is_valid(): |
| 553 return False | 554 return False |
| 554 data, url = data_url | 555 data, url = data_url |
| 555 message = form.cleaned_data[message_key] | 556 message = form.cleaned_data[message_key] |
| 556 patchset = models.PatchSet(issue=issue, message=message, data=data, url=url, | 557 patchset = models.PatchSet(issue=issue, message=message, data=data, url=url, |
| 557 base=issue.base, owner=request.user, parent=issue) | 558 base=issue.base, owner=request.user, parent=issue) |
| 558 patchset.put() | 559 patchset.put() |
| 559 | 560 |
| 560 patches = engine.ParsePatchSet(patchset) | 561 patches = engine.ParsePatchSet(patchset) |
| 561 if not patches: | 562 if not patches: |
| 562 patchset.delete() | 563 patchset.delete() |
| 563 errkey = url and 'url' or 'data' | 564 errkey = url and 'url' or 'data' |
| 564 form.errors[errkey] = ['Patch set contains no recognizable patches'] | 565 form.errors[errkey] = ['Patch set contains no recognizable patches'] |
| 565 return False | 566 return False |
| 566 db.put(patches) | 567 db.put(patches) |
| 567 issue.put() # To update last modified time | 568 issue.put() # To update last modified time |
| 568 return True | 569 return True |
| 569 | 570 |
| 570 | 571 |
| 571 def _get_reviewers(form): | 572 def _get_reviewers(form): |
| 572 """Helper to return the list of reviewers, or None for error.""" | 573 """Helper to return the list of reviewers, or None for error.""" |
| 573 reviewers = [] | 574 reviewers = sets.Set() |
| 574 raw_reviewers = form.cleaned_data.get('reviewers') | 575 raw_reviewers = form.cleaned_data.get('reviewers') |
| 575 if raw_reviewers: | 576 if raw_reviewers: |
| 576 for reviewer in raw_reviewers.split(','): | 577 for reviewer in raw_reviewers.split(','): |
| 577 reviewer = reviewer.strip() | 578 reviewer = reviewer.strip() |
| 578 if reviewer and reviewer not in reviewers: | 579 if reviewer: |
| 579 try: | 580 try: |
| 580 reviewer = db.Email(reviewer) | 581 reviewer = db.Email(reviewer) |
| 581 if reviewer.count('@') != 1: | 582 if reviewer.count('@') != 1: |
| 582 raise db.BadValueError('Invalid email address: %s' % reviewer) | 583 raise db.BadValueError('Invalid email address: %s' % reviewer) |
| 583 head, tail = reviewer.split('@') | 584 head, tail = reviewer.split('@') |
| 584 if '.' not in tail: | 585 if '.' not in tail: |
| 585 raise db.BadValueError('Invalid email address: %s' % reviewer) | 586 raise db.BadValueError('Invalid email address: %s' % reviewer) |
| 586 except db.BadValueError, err: | 587 except db.BadValueError, err: |
| 587 form.errors['reviewers'] = [unicode(err)] | 588 form.errors['reviewers'] = [unicode(err)] |
| 588 return None | 589 return None |
| 589 reviewers.append(reviewer) | 590 reviewers.add(reviewer) |
| 590 return reviewers | 591 return list(reviewers) |
|
GvR
2008/05/11 19:20:49
I'd prefer to keep reviewers in the order that the
| |
| 591 | 592 |
| 592 | 593 |
| 593 | 594 |
| 594 @issue_required | 595 @issue_required |
| 595 def show(request, form=AddForm()): | 596 def show(request, form=AddForm()): |
| 596 """/<issue> - Show an issue.""" | 597 """/<issue> - Show an issue.""" |
| 597 issue = request.issue | 598 issue = request.issue |
| 598 patchsets = list(issue.patchset_set.order('created')) | 599 patchsets = list(issue.patchset_set.order('created')) |
| 599 issue.draft_count = 0 | 600 issue.draft_count = 0 |
| 600 issue.comment_count = 0 | 601 issue.comment_count = 0 |
| 601 for patchset in patchsets: | 602 for patchset in patchsets: |
| 602 patchset.patches = list(patchset.patch_set.order('filename')) | 603 patchset.patches = list(patchset.patch_set.order('filename')) |
| 603 patchset.n_comments = 0 | 604 patchset.n_comments = 0 |
| 604 for patch in patchset.patches: | 605 for patch in patchset.patches: |
| 605 patchset.n_comments += patch.num_comments | 606 patchset.n_comments += patch.num_comments |
| 606 issue.comment_count += patchset.n_comments | 607 issue.comment_count += patchset.n_comments |
| 607 patchset.n_drafts = 0 | 608 patchset.n_drafts = 0 |
| 608 if request.user: | 609 if request.user: |
| 609 for patch in patchset.patches: | 610 for patch in patchset.patches: |
| 610 patchset.n_drafts += patch.num_drafts | 611 patchset.n_drafts += patch.num_drafts |
| 611 issue.draft_count += patchset.n_drafts | 612 issue.draft_count += patchset.n_drafts |
| 612 last_patchset = first_patch = None | 613 last_patchset = first_patch = None |
| 613 if patchsets: | 614 if patchsets: |
| 614 last_patchset = patchsets[-1] | 615 last_patchset = patchsets[-1] |
| 615 if last_patchset.patches: | 616 if last_patchset.patches: |
| 616 first_patch = last_patchset.patches[0] | 617 first_patch = last_patchset.patches[0] |
| 617 messages = list(issue.message_set.order('date')) | 618 messages = list(issue.message_set.order('date')) |
| 618 return respond(request, 'issue.html', | 619 return respond(request, 'issue.html', |
| 619 {'issue': issue, 'patchsets': patchsets, | 620 {'issue': issue, 'patchsets': patchsets, |
| 620 'messages': messages, 'form': form, | 621 'messages': messages, 'form': form, |
| 621 'last_patchset': last_patchset, | 622 'last_patchset': last_patchset, |
| 622 'first_patch': first_patch}) | 623 'first_patch': first_patch}) |
| 623 | 624 |
| 624 | 625 |
| 625 @issue_owner_required | 626 @issue_owner_required |
| 626 def edit(request): | 627 def edit(request): |
| 627 """/<issue>/edit - Edit an issue.""" | 628 """/<issue>/edit - Edit an issue.""" |
| 628 issue = request.issue | 629 issue = request.issue |
| 629 base = issue.base | 630 base = issue.base |
| 630 | 631 |
| 631 if request.method != 'POST': | 632 if request.method != 'POST': |
| 632 form = EditForm(initial={'subject': issue.subject, | 633 form = EditForm(initial={'subject': issue.subject, |
| 633 'description': issue.description, | 634 'description': issue.description, |
| 634 'base': base, | 635 'base': base, |
| 635 'reviewers': ', '.join(issue.reviewers)}) | 636 'reviewers': ', '.join(issue.reviewers)}) |
| 636 form.set_branch_choices(base) | 637 form.set_branch_choices(base) |
| 637 return respond(request, 'edit.html', {'issue': issue, 'form': form}) | 638 return respond(request, 'edit.html', {'issue': issue, 'form': form}) |
| 638 | 639 |
| 639 form = EditForm(request.POST) | 640 form = EditForm(request.POST) |
| 640 form.set_branch_choices() | 641 form.set_branch_choices() |
| (...skipping 257 matching lines...) Show 10 above Show 10 below | |
| 898 'ORDER BY date', | 899 'ORDER BY date', |
| 899 patch=patch, lineno=lineno, left=left) | 900 patch=patch, lineno=lineno, left=left) |
| 900 comments = list(c for c in query if not c.draft or c.author == request.user) | 901 comments = list(c for c in query if not c.draft or c.author == request.user) |
| 901 if comment is not None and comment.author is None: | 902 if comment is not None and comment.author is None: |
| 902 # Show anonymous draft even though we don't save it | 903 # Show anonymous draft even though we don't save it |
| 903 comments.append(comment) | 904 comments.append(comment) |
| 904 if not comments: | 905 if not comments: |
| 905 return HttpResponse(' ') | 906 return HttpResponse(' ') |
| 906 for c in comments: | 907 for c in comments: |
| 907 c.complete(patch) | 908 c.complete(patch) |
| 908 return render_to_response('inline_comment.html', | 909 return render_to_response('inline_comment.html', |
| 909 {'user': request.user, | 910 {'user': request.user, |
| 910 'patch': patch, | 911 'patch': patch, |
| 911 'patchset': patchset, | 912 'patchset': patchset, |
| 912 'issue': issue, | 913 'issue': issue, |
| 913 'comments': comments, | 914 'comments': comments, |
| 914 'lineno': lineno, | 915 'lineno': lineno, |
| 915 'snapshot': snapshot, | 916 'snapshot': snapshot, |
| 916 'side': side}) | 917 'side': side}) |
| 917 | 918 |
| 918 | 919 |
| 919 PUBLISH_MAIL_TEMPLATE = """Dear %s, | 920 PUBLISH_MAIL_TEMPLATE = """Dear %s, |
| 920 | 921 |
| 921 New code review comments by %s have been published. | 922 New code review comments by %s have been published. |
| 922 Please go to %s to read them. | 923 Please go to %s to read them. |
| 923 | 924 |
| 924 Message: | 925 Message: |
| 925 %s | 926 %s |
| 926 | 927 |
| 927 Details: | 928 Details: |
| 928 %s | 929 %s |
| 929 | 930 |
| 930 Issue Description: | 931 Issue Description: |
| 931 %s | 932 %s |
| 932 | 933 |
| 933 Sincerely, | 934 Sincerely, |
| 934 | 935 |
| 935 Your friendly code review daemon (%s). | 936 Your friendly code review daemon (%s). |
| 936 """ | 937 """ |
| 937 | 938 |
| 938 @issue_required | 939 @issue_required |
| 939 @login_required | 940 @login_required |
| 940 def publish(request): | 941 def publish(request): |
| 941 """ /<issue>/publish - Publish draft comments and send mail.""" | 942 """ /<issue>/publish - Publish draft comments and send mail.""" |
| 942 issue = request.issue | 943 issue = request.issue |
| 943 if request.user == issue.owner: | 944 if request.user == issue.owner: |
| 944 form_class = PublishForm | 945 form_class = PublishForm |
| 945 else: | 946 else: |
| 946 form_class = MiniPublishForm | 947 form_class = MiniPublishForm |
| 947 if request.method != 'POST': | 948 if request.method != 'POST': |
| 948 reviewers = issue.reviewers[:] | 949 if request.user != issue.owner and (not request.user.email() |
| 949 if request.user != issue.owner and (request.user.email() | 950 in issue.reviewers): |
|
GvR
2008/05/11 19:20:49
You can use 'not in' for the latter test.
| |
| 950 not in issue.reviewers): | 951 issue.reviewers.append(request.user.email()) |
|
GvR
2008/05/11 19:20:49
I wouldn't assign to issue.reviewers here, since y
| |
| 951 reviewers.append(request.user.email()) | |
| 952 form = form_class(initial={'subject': issue.subject, | 952 form = form_class(initial={'subject': issue.subject, |
| 953 'reviewers': ', '.join(reviewers), | 953 'reviewers': ', '.join(issue.reviewers), |
| 954 'send_mail': True, | 954 'send_mail': True, |
| 955 }) | 955 }) |
| 956 return respond(request, 'publish.html', {'form': form, 'issue': issue}) | 956 return respond(request, 'publish.html', {'form': form, 'issue': issue}) |
| 957 | 957 |
| 958 form = form_class(request.POST) | 958 form = form_class(request.POST) |
| 959 if form.is_valid(): | 959 if form.is_valid(): |
| 960 reviewers = _get_reviewers(form) | 960 reviewers = _get_reviewers(form) |
| 961 if not form.is_valid(): | 961 if not form.is_valid(): |
| 962 return respond(request, 'publish.html', {'form': form, 'issue': issue}) | 962 return respond(request, 'publish.html', {'form': form, 'issue': issue}) |
| 963 tbd = [] # List of things to put() after all is said and done | 963 tbd = [] # List of things to put() after all is said and done |
| 964 if request.user == issue.owner: | 964 if request.user == issue.owner: |
| 965 subject = form.cleaned_data['subject'] | 965 subject = form.cleaned_data['subject'] |
| 966 issue.subject = subject | 966 issue.subject = subject |
| 967 issue.reviewers = reviewers | 967 issue.reviewers = reviewers |
| 968 else: | 968 else: |
| 969 subject = issue.subject | 969 subject = issue.subject |
| 970 issue.reviewers = reviewers | 970 issue.reviewers = reviewers |
| 971 tbd.append(issue) # To update the last modified time | 971 tbd.append(issue) # To update the last modified time |
| 972 message = form.cleaned_data['message'].replace('\r\n', '\n') | 972 message = form.cleaned_data['message'].replace('\r\n', '\n') |
| 973 send_mail = form.cleaned_data['send_mail'] | 973 send_mail = form.cleaned_data['send_mail'] |
| 974 comments = [] | 974 comments = [] |
| 975 | 975 |
| 976 # XXX Should request all drafts for this issue once, now we can. | 976 # XXX Should request all drafts for this issue once, now we can. |
| 977 for patchset in issue.patchset_set.order('created'): | 977 for patchset in issue.patchset_set.order('created'): |
| 978 ## ps_comments = list(models.Comment.gql( | 978 ## ps_comments = list(models.Comment.gql( |
| 979 ## 'WHERE ANCESTOR IS :1 AND author = :2 AND draft = TRUE', | 979 ## 'WHERE ANCESTOR IS :1 AND author = :2 AND draft = TRUE', |
| 980 ## patchset, request.user)) | 980 ## patchset, request.user)) |
| 981 # XXX Somehow the index broke, do without it | 981 # XXX Somehow the index broke, do without it |
| 982 ps_comments = [c for c in | 982 ps_comments = [c for c in |
| 983 models.Comment.gql('WHERE ANCESTOR IS :1', patchset) | 983 models.Comment.gql('WHERE ANCESTOR IS :1', patchset) |
| 984 if c.draft and c.author == request.user] | 984 if c.draft and c.author == request.user] |
| 985 # XXX End | 985 # XXX End |
| 986 if ps_comments: | 986 if ps_comments: |
| 987 patches = dict((p.key(), p) for p in patchset.patch_set) | 987 patches = dict((p.key(), p) for p in patchset.patch_set) |
| 988 for p in patches.itervalues(): | 988 for p in patches.itervalues(): |
| 989 p.patchset = patchset | 989 p.patchset = patchset |
| 990 for c in ps_comments: | 990 for c in ps_comments: |
| 991 c.draft = False | 991 c.draft = False |
| 992 # XXX Using internal knowledge about db package: the key for | 992 # XXX Using internal knowledge about db package: the key for |
| 993 # reference property foo is stored as _foo. | 993 # reference property foo is stored as _foo. |
| 994 pkey = getattr(c, '_patch', None) | 994 pkey = getattr(c, '_patch', None) |
| 995 if pkey in patches: | 995 if pkey in patches: |
| 996 patch = patches[pkey] | 996 patch = patches[pkey] |
| 997 c.patch = patch | 997 c.patch = patch |
| 998 tbd.append(ps_comments) | 998 tbd.append(ps_comments) |
| 999 ps_comments.sort(key=lambda c: (c.patch.filename, not c.left, | 999 ps_comments.sort(key=lambda c: (c.patch.filename, not c.left, |
| 1000 c.lineno, c.date)) | 1000 c.lineno, c.date)) |
| 1001 comments += ps_comments | 1001 comments += ps_comments |
| 1002 | 1002 |
| 1003 if comments: | 1003 if comments: |
| 1004 logging.warn('Publishing %d comments', len(comments)) | 1004 logging.warn('Publishing %d comments', len(comments)) |
| 1005 # Decide who should receive mail | 1005 # Decide who should receive mail |
| 1006 my_email = db.Email(request.user.email()) | 1006 my_email = db.Email(request.user.email()) |
| 1007 addressees = [db.Email(issue.owner.email())] + issue.reviewers | 1007 addressees = [db.Email(issue.owner.email())] + issue.reviewers |
| 1008 if my_email in addressees: | 1008 if my_email in addressees: |
| 1009 everyone = addressees[:] | 1009 everyone = addressees[:] |
| 1010 if len(addressees) > 1: # Keep it if sending only to yourself | 1010 if len(addressees) > 1: # Keep it if sending only to yourself |
| 1011 addressees.remove(my_email) | 1011 addressees.remove(my_email) |
| 1012 else: | 1012 else: |
| 1013 everyone = addressees + [my_email] | 1013 everyone = addressees + [my_email] |
|
GvR
2008/05/11 23:27:41
What would you think of *not* adding my_email in t
jiayao
2008/05/12 09:36:35
Make sense. It would be confusing if this person r
| |
| 1014 details = _get_draft_details(request, comments) | 1014 details = _get_draft_details(request, comments) |
| 1015 text = ((message.strip() + '\n\n' + details.strip())).strip() | 1015 text = ((message.strip() + '\n\n' + details.strip())).strip() |
| 1016 msg = models.Message(issue=issue, | 1016 msg = models.Message(issue=issue, |
| 1017 subject=issue.subject, | 1017 subject=issue.subject, |
| 1018 sender=my_email, | 1018 sender=my_email, |
| 1019 recipients=everyone, | 1019 recipients=everyone, |
| 1020 text=db.Text(text), | 1020 text=db.Text(text), |
| 1021 parent=issue) | 1021 parent=issue) |
| 1022 tbd.append(msg) | 1022 tbd.append(msg) |
| 1023 | 1023 | <