Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(48)

Delta Between Two Patch Sets: rietveld/views.py

Issue 776: Allow reviewers to change the reviewers list (Closed) SVN Base: http://rietveld.googlecode.com/svn/trunk/
Left Patch Set: Minor changes Created 3 months, 2 weeks ago
Right Patch Set: Created 3 months, 2 weeks ago
Use n/p to move between diff chunks; N/P to move between comments. Please Sign in to add in-line comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
LEFTRIGHT
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 30
31 # AppEngine imports 31 # AppEngine imports
32 from google.appengine.api import mail 32 from google.appengine.api import mail
33 from google.appengine.api import users 33 from google.appengine.api import users
34 from google.appengine.api import urlfetch 34 from google.appengine.api import urlfetch
35 from google.appengine.ext import db 35 from google.appengine.ext import db
36 from google.appengine.ext.db import djangoforms 36 from google.appengine.ext.db import djangoforms
37 37
38 # DeadlineExceededError can live in two different places 38 # DeadlineExceededError can live in two different places
39 # TODO(guido): simplify once this is fixed. 39 # TODO(guido): simplify once this is fixed.
40 try: 40 try:
41 # When deployed 41 # When deployed
42 from google.appengine.runtime import DeadlineExceededError 42 from google.appengine.runtime import DeadlineExceededError
43 except ImportError: 43 except ImportError:
44 # In the development server 44 # In the development server
45 from google.appengine.runtime.apiproxy_errors import DeadlineExceededError 45 from google.appengine.runtime.apiproxy_errors import DeadlineExceededError
46 46
47 # Django imports 47 # Django imports
48 # TODO(guido): Don't import classes/functions directly. 48 # TODO(guido): Don't import classes/functions directly.
49 from django import newforms as forms 49 from django import newforms as forms
50 from django.http import HttpResponse, HttpResponseRedirect 50 from django.http import HttpResponse, HttpResponseRedirect
(...skipping 477 matching lines...) Show 10 above Show 10 below
528 return None 528 return None
529 if fetch_result.status_code != 200: 529 if fetch_result.status_code != 200:
530 form.errors['url'] = ['HTTP status code %s' % fetch_result.status_code] 530 form.errors['url'] = ['HTTP status code %s' % fetch_result.status_code]
531 return None 531 return None
532 data = db.Blob(fetch_result.content) 532 data = db.Blob(fetch_result.content)
533 533
534 return data, url 534 return data, url
535 535
536 536
537 @issue_owner_required 537 @issue_owner_required
538 def add(request): 538 def add(request):
539 """/<issue>/add - Add a new PatchSet to an existing Issue.""" 539 """/<issue>/add - Add a new PatchSet to an existing Issue."""
540 issue = request.issue 540 issue = request.issue
541 form = AddForm(request.POST, request.FILES) 541 form = AddForm(request.POST, request.FILES)
542 if not add_patchset_from_form(request, issue, form): 542 if not add_patchset_from_form(request, issue, form):
543 return show(request, issue.key().id(), form) 543 return show(request, issue.key().id(), form)
544 return HttpResponseRedirect('/%s' % issue.key().id()) 544 return HttpResponseRedirect('/%s' % issue.key().id())
545 545
546 546
547 def add_patchset_from_form(request, issue, form, message_key='message'): 547 def add_patchset_from_form(request, issue, form, message_key='message'):
548 """Helper for add() and upload().""" 548 """Helper for add() and upload()."""
549 # TODO(guido): use a transaction like in _make_new(); may be share more code? 549 # TODO(guido): use a transaction like in _make_new(); may be share more code?
550 if form.is_valid(): 550 if form.is_valid():
551 data_url = _get_data_url(form) 551 data_url = _get_data_url(form)
552 if not form.is_valid(): 552 if not form.is_valid():
553 return False 553 return False
554 data, url = data_url 554 data, url = data_url
555 message = form.cleaned_data[message_key] 555 message = form.cleaned_data[message_key]
556 patchset = models.PatchSet(issue=issue, message=message, data=data, url=url, 556 patchset = models.PatchSet(issue=issue, message=message, data=data, url=url,
557 base=issue.base, owner=request.user, parent=issue) 557 base=issue.base, owner=request.user, parent=issue)
558 patchset.put() 558 patchset.put()
559 559
560 patches = engine.ParsePatchSet(patchset) 560 patches = engine.ParsePatchSet(patchset)
561 if not patches: 561 if not patches:
562 patchset.delete() 562 patchset.delete()
563 errkey = url and 'url' or 'data' 563 errkey = url and 'url' or 'data'
564 form.errors[errkey] = ['Patch set contains no recognizable patches'] 564 form.errors[errkey] = ['Patch set contains no recognizable patches']
565 return False 565 return False
566 db.put(patches) 566 db.put(patches)
567 issue.put() # To update last modified time 567 issue.put() # To update last modified time
568 return True 568 return True
569 569
570 570
571 def _get_reviewers(form): 571 def _get_reviewers(form):
572 """Helper to return the list of reviewers, or None for error.""" 572 """Helper to return the list of reviewers, or None for error."""
573 reviewers = [] 573 reviewers = []
574 raw_reviewers = form.cleaned_data.get('reviewers') 574 raw_reviewers = form.cleaned_data.get('reviewers')
575 if raw_reviewers: 575 if raw_reviewers:
576 for reviewer in raw_reviewers.split(','): 576 for reviewer in raw_reviewers.split(','):
577 reviewer = reviewer.strip() 577 reviewer = reviewer.strip()
578 if reviewer and reviewer not in reviewers: 578 if reviewer:
579 try: 579 try:
580 reviewer = db.Email(reviewer) 580 reviewer = db.Email(reviewer)
581 if reviewer.count('@') != 1: 581 if reviewer.count('@') != 1:
582 raise db.BadValueError('Invalid email address: %s' % reviewer) 582 raise db.BadValueError('Invalid email address: %s' % reviewer)
583 head, tail = reviewer.split('@') 583 head, tail = reviewer.split('@')
584 if '.' not in tail: 584 if '.' not in tail:
585 raise db.BadValueError('Invalid email address: %s' % reviewer) 585 raise db.BadValueError('Invalid email address: %s' % reviewer)
586 except db.BadValueError, err: 586 except db.BadValueError, err:
587 form.errors['reviewers'] = [unicode(err)] 587 form.errors['reviewers'] = [unicode(err)]
588 return None 588 return None
589 reviewers.append(reviewer) 589 reviewers.append(reviewer)
590 return reviewers 590 return reviewers
591 591
592 592
593 593
594 @issue_required 594 @issue_required
595 def show(request, form=AddForm()): 595 def show(request, form=AddForm()):
596 """/<issue> - Show an issue.""" 596 """/<issue> - Show an issue."""
597 issue = request.issue 597 issue = request.issue
598 patchsets = list(issue.patchset_set.order('created')) 598 patchsets = list(issue.patchset_set.order('created'))
599 issue.draft_count = 0 599 issue.draft_count = 0
600 issue.comment_count = 0 600 issue.comment_count = 0
601 for patchset in patchsets: 601 for patchset in patchsets:
602 patchset.patches = list(patchset.patch_set.order('filename')) 602 patchset.patches = list(patchset.patch_set.order('filename'))
603 patchset.n_comments = 0 603 patchset.n_comments = 0
604 for patch in patchset.patches: 604 for patch in patchset.patches:
605 patchset.n_comments += patch.num_comments 605 patchset.n_comments += patch.num_comments
606 issue.comment_count += patchset.n_comments 606 issue.comment_count += patchset.n_comments
607 patchset.n_drafts = 0 607 patchset.n_drafts = 0
608 if request.user: 608 if request.user:
609 for patch in patchset.patches: 609 for patch in patchset.patches:
610 patchset.n_drafts += patch.num_drafts 610 patchset.n_drafts += patch.num_drafts
611 issue.draft_count += patchset.n_drafts 611 issue.draft_count += patchset.n_drafts
612 last_patchset = first_patch = None 612 last_patchset = first_patch = None
613 if patchsets: 613 if patchsets:
614 last_patchset = patchsets[-1] 614 last_patchset = patchsets[-1]
615 if last_patchset.patches: 615 if last_patchset.patches:
616 first_patch = last_patchset.patches[0] 616 first_patch = last_patchset.patches[0]
617 messages = list(issue.message_set.order('date')) 617 messages = list(issue.message_set.order('date'))
618 return respond(request, 'issue.html', 618 return respond(request, 'issue.html',
619 {'issue': issue, 'patchsets': patchsets, 619 {'issue': issue, 'patchsets': patchsets,
620 'messages': messages, 'form': form, 620 'messages': messages, 'form': form,
621 'last_patchset': last_patchset, 621 'last_patchset': last_patchset,
622 'first_patch': first_patch}) 622 'first_patch': first_patch})
623 623
624 624
625 @issue_owner_required 625 @issue_owner_required
626 def edit(request): 626 def edit(request):
627 """/<issue>/edit - Edit an issue.""" 627 """/<issue>/edit - Edit an issue."""
628 issue = request.issue 628 issue = request.issue
(...skipping 269 matching lines...) Show 10 above Show 10 below
898 'ORDER BY date', 898 'ORDER BY date',
899 patch=patch, lineno=lineno, left=left) 899 patch=patch, lineno=lineno, left=left)
900 comments = list(c for c in query if not c.draft or c.author == request.user) 900 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: 901 if comment is not None and comment.author is None:
902 # Show anonymous draft even though we don't save it 902 # Show anonymous draft even though we don't save it
903 comments.append(comment) 903 comments.append(comment)
904 if not comments: 904 if not comments:
905 return HttpResponse(' ') 905 return HttpResponse(' ')
906 for c in comments: 906 for c in comments:
907 c.complete(patch) 907 c.complete(patch)
908 return render_to_response('inline_comment.html', 908 return render_to_response('inline_comment.html',
909 {'user': request.user, 909 {'user': request.user,
910 'patch': patch, 910 'patch': patch,
911 'patchset': patchset, 911 'patchset': patchset,
912 'issue': issue, 912 'issue': issue,
913 'comments': comments, 913 'comments': comments,
914 'lineno': lineno, 914 'lineno': lineno,
915 'snapshot': snapshot, 915 'snapshot': snapshot,
916 'side': side}) 916 'side': side})
917 917
918 918
919 PUBLISH_MAIL_TEMPLATE = """Dear %s, 919 PUBLISH_MAIL_TEMPLATE = """Dear %s,
920 920
921 New code review comments by %s have been published. 921 New code review comments by %s have been published.
922 Please go to %s to read them. 922 Please go to %s to read them.
923 923
924 Message: 924 Message:
925 %s 925 %s
926 926
927 Details: 927 Details:
928 %s 928 %s
929 929
930 Issue Description: 930 Issue Description:
931 %s 931 %s
932 932
933 Sincerely, 933 Sincerely,
934 934
935 Your friendly code review daemon (%s). 935 Your friendly code review daemon (%s).
936 """ 936 """
937 937
938 @issue_required 938 @issue_required
939 @login_required 939 @login_required
940 def publish(request): 940 def publish(request):
941 """ /<issue>/publish - Publish draft comments and send mail.""" 941 """ /<issue>/publish - Publish draft comments and send mail."""
942 issue = request.issue 942 issue = request.issue
943 if request.user == issue.owner: 943 if request.user == issue.owner:
944 form_class = PublishForm 944 form_class = PublishForm
945 else: 945 else:
946 form_class = MiniPublishForm 946 form_class = MiniPublishForm
947 if request.method != 'POST': 947 if request.method != 'POST':
948 reviewers = issue.reviewers[:]
949 if request.user != issue.owner and (request.user.email()
950 not in issue.reviewers):
951 reviewers.append(request.user.email())
952 form = form_class(initial={'subject': issue.subject, 948 form = form_class(initial={'subject': issue.subject,
953 'reviewers': ', '.join(reviewers), 949 'reviewers': ', '.join(issue.reviewers),
GvR 2008/05/10 15:44:24 I would suggest one more addition: add the current
jiayao 2008/05/11 11:33:27 Good suggestion, I've added that. Just another tho
954 'send_mail': True, 950 'send_mail': True,
955 }) 951 })
956 return respond(request, 'publish.html', {'form': form, 'issue': issue}) 952 return respond(request, 'publish.html', {'form': form, 'issue': issue})
957 953
958 form = form_class(request.POST) 954 form = form_class(request.POST)
959 if form.is_valid(): 955 if form.is_valid():
960 reviewers = _get_reviewers(form) 956 reviewers = _get_reviewers(form)
961 if not form.is_valid(): 957 if not form.is_valid():
962 return respond(request, 'publish.html', {'form': form, 'issue': issue}) 958 return respond(request, 'publish.html', {'form': form, 'issue': issue})
963 tbd = [] # List of things to put() after all is said and done 959 tbd = [] # List of things to put() after all is said and done
964 if request.user == issue.owner: 960 if request.user == issue.owner:
965 subject = form.cleaned_data['subject'] 961 subject = form.cleaned_data['subject']
966 issue.subject = subject 962 issue.subject = subject
967 issue.reviewers = reviewers 963 issue.reviewers = reviewers
968 else: 964 else:
965 issue.reviewers = reviewers
GvR 2008/05/10 15:44:24 I'd move this one line downfor symmetry with the '
jiayao 2008/05/11 11:33:27 On 2008/05/10 15:44:24, GvR wrote: > I'd move this
969 subject = issue.subject 966 subject = issue.subject
970 issue.reviewers = reviewers
971 tbd.append(issue) # To update the last modified time 967 tbd.append(issue) # To update the last modified time
972 message = form.cleaned_data['message'].replace('\r\n', '\n') 968 message = form.cleaned_data['message'].replace('\r\n', '\n')
973 send_mail = form.cleaned_data['send_mail'] 969 send_mail = form.cleaned_data['send_mail']
974 comments = [] 970 comments = []
975 971
976 # XXX Should request all drafts for this issue once, now we can. 972 # XXX Should request all drafts for this issue once, now we can.
977 for patchset in issue.patchset_set.order('created'): 973 for patchset in issue.patchset_set.order('created'):
978 ## ps_comments = list(models.Comment.gql( 974 ## ps_comments = list(models.Comment.gql(
979 ## 'WHERE ANCESTOR IS :1 AND author = :2 AND draft = TRUE', 975 ## 'WHERE ANCESTOR IS :1 AND author = :2 AND draft = TRUE',
980 ## patchset, request.user)) 976 ## patchset, request.user))
981 # XXX Somehow the index broke, do without it 977 # XXX Somehow the index broke, do without it
982 ps_comments = [c for c in 978 ps_comments = [c for c in
983 models.Comment.gql('WHERE ANCESTOR IS :1', patchset) 979 models.Comment.gql('WHERE ANCESTOR IS :1', patchset)
984 if c.draft and c.author == request.user] 980 if c.draft and c.author == request.user]
985 # XXX End 981 # XXX End
986 if ps_comments: 982 if ps_comments:
987 patches = dict((p.key(), p) for p in patchset.patch_set) 983 patches = dict((p.key(), p) for p in patchset.patch_set)
988 for p in patches.itervalues(): 984 for p in patches.itervalues():
989 p.patchset = patchset 985 p.patchset = patchset
990 for c in ps_comments: 986 for c in ps_comments:
991 c.draft = False 987 c.draft = False
992 # XXX Using internal knowledge about db package: the key for 988 # XXX Using internal knowledge about db package: the key for
993 # reference property foo is stored as _foo. 989 # reference property foo is stored as _foo.
994 pkey = getattr(c, '_patch', None) 990 pkey = getattr(c, '_patch', None)
995 if pkey in patches: 991 if pkey in patches:
996 patch = patches[pkey] 992 patch = patches[pkey]
997 c.patch = patch 993 c.patch = patch
998 tbd.append(ps_comments) 994 tbd.append(ps_comments)
999 ps_comments.sort(key=lambda c: (c.patch.filename, not c.left, 995 ps_comments.sort(key=lambda c: (c.patch.filename, not c.left,
1000 c.lineno, c.date)) 996 c.lineno, c.date))
1001 comments += ps_comments 997 comments += ps_comments
1002 998
1003 if comments: 999 if comments:
1004 logging.warn('Publishing %d comments', len(comments)) 1000 logging.warn('Publishing %d comments', len(comments))
1005 # Decide who should receive mail 1001 # Decide who should receive mail
1006 my_email = db.Email(request.user.email()) 1002 my_email = db.Email(request.user.email())
1007 addressees = [db.Email(issue.owner.email())] + issue.reviewers 1003 addressees = [db.Email(issue.owner.email())] + issue.reviewers
1008 if my_email in addressees: 1004 if my_email in addressees:
1009 everyone = addressees[:] 1005 everyone = addressees[:]
1010 if len(addressees) > 1: # Keep it if sending only to yourself 1006 if len(addressees) > 1: # Keep it if sending only to yourself
1011 addressees.remove(my_email) 1007 addressees.remove(my_email)
1012 else: 1008 else:
1013 everyone = addressees + [my_email] 1009 everyone = addressees + [my_email]
1014 details = _get_draft_details(request, comments) 1010 details = _get_draft_details(request, comments)
1015 text = ((message.strip() + '\n\n' + details.strip())).strip() 1011 text = ((message.strip() + '\n\n' + details.strip())).strip()
1016 msg = models.Message(issue=issue, 1012 msg = models.Message(issue=issue,
1017 subject=issue.subject, 1013 subject=issue.subject,
1018 sender=my_email, 1014 sender=my_email,
1019 recipients=everyone, 1015 recipients=everyone,
1020 text=db.Text(text), 1016 text=db.Text(text),
(...skipping 183 matching lines...) Show 10 above Show 10 below
1204 return respond(request, 'branch_edit.html', 1200 return respond(request, 'branch_edit.html',
1205 {'branch': branch, 'form': form}) 1201 {'branch': branch, 'form': form})
1206 branch.put() 1202 branch.put()
1207 return HttpResponseRedirect('/repos') 1203 return HttpResponseRedirect('/repos')
1208 1204
1209 1205
1210 @login_required 1206 @login_required
1211 def branch_delete(request, branch_id): 1207 def branch_delete(request, branch_id):
1212 """/branch_delete/<branch> - Delete a Branch record.""" 1208 """/branch_delete/<branch> - Delete a Branch record."""
1213 branch = models.Branch.get_by_id(int(branch_id)) 1209 branch = models.Branch.get_by_id(int(branch_id))
1214 if branch.owner != request.user: 1210 if branch.owner != request.user:
1215 return HttpResponseForbidden('You do not own this branch') 1211 return HttpResponseForbidden('You do not own this branch')
1216 repo = branch.repo 1212 repo = branch.repo
1217 branch.delete() 1213 branch.delete()
1218 num_branches = models.Branch.gql('WHERE repo = :1', repo).count() 1214 num_branches = models.Branch.gql('WHERE repo = :1', repo).count()
1219 if not num_branches: 1215 if not num_branches:
1220 # Even if we don't own the repository? Yes, I think so! Empty 1216 # Even if we don't own the repository? Yes, I think so! Empty
1221 # repositories have no representation on screen. 1217 # repositories have no representation on screen.
1222 repo.delete() 1218 repo.delete()
1223 return HttpResponseRedirect('/repos') 1219 return HttpResponseRedirect('/repos')
1224 1220
1225 1221
1226 ### User Profiles ### 1222 ### User Profiles ###
1227 1223
1228 @login_required 1224 @login_required
1229 def settings(request): 1225 def settings(request):
1230 account = models.Account.get_account_for_user(request.user) 1226 account = models.Account.get_account_for_user(request.user)
1231 if request.method != 'POST': 1227 if request.method != 'POST':
1232 nickname = account.nickname 1228 nickname = account.nickname
1233 form = SettingsForm(initial={'nickname': nickname}) 1229 form = SettingsForm(initial={'nickname': nickname})
1234 return respond(request, 'settings.html', {'form': form}) 1230 return respond(request, 'settings.html', {'form': form})
1235 form = SettingsForm(request.POST) 1231 form = SettingsForm(request.POST)
1236 if form.is_valid(): 1232 if form.is_valid():
1237 nickname = form.cleaned_data['nickname'].strip() 1233 nickname = form.cleaned_data['nickname'].strip()
1238 if not nickname: 1234 if not nickname:
1239 form.errors['nickname'] = ['Your nickname cannot be empty.'] 1235 form.errors['nickname'] = ['Your nickname cannot be empty.']
1240 elif '@' in nickname: 1236 elif '@' in nickname:
1241 form.errors['nickname'] = ['Your nickname cannot contain "@".'] 1237 form.errors['nickname'] = ['Your nickname cannot contain "@".']
1242 elif ',' in nickname: 1238 elif ',' in nickname:
1243 form.errors['nickname'] = ['Your nickname cannot contain ",".'] 1239 form.errors['nickname'] = ['Your nickname cannot contain ",".']
1244 else: 1240 else:
1245 accounts = models.Account.get_accounts_for_nickname(nickname) 1241 accounts = models.Account.get_accounts_for_nickname(nickname)
1246 if nickname != account.nickname and accounts: 1242 if nickname != account.nickname and accounts:
1247 form.errors['nickname'] = ['This nickname is already in use.'] 1243 form.errors['nickname'] = ['This nickname is already in use.']
1248 else: 1244 else:
1249 account.nickname = nickname 1245 account.nickname = nickname
1250 account.put() 1246 account.put()
1251 if not form.is_valid(): 1247 if not form.is_valid():
1252 return respond(request, 'settings.html', {'form': form}) 1248 return respond(request, 'settings.html', {'form': form})
1253 return HttpResponseRedirect('/settings') 1249 return HttpResponseRedirect('/settings')
1250
GvR 2008/05/10 15:44:24 Don't add an extra blank line at the end.
jiayao 2008/05/11 11:33:27 On 2008/05/10 15:44:24, GvR wrote: > Don't add an
LEFTRIGHT

Powered by Google App Engine
This is Rietveld r292