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

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: Created 3 months, 2 weeks ago
Right Patch Set: Minor changes 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: 578 if reviewer and reviewer not in reviewers:
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())
948 form = form_class(initial={'subject': issue.subject, 952 form = form_class(initial={'subject': issue.subject,
949 'reviewers': ', '.join(issue.reviewers), 953 'reviewers': ', '.join(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
950 'send_mail': True, 954 'send_mail': True,
951 }) 955 })
952 return respond(request, 'publish.html', {'form': form, 'issue': issue}) 956 return respond(request, 'publish.html', {'form': form, 'issue': issue})
953 957
954 form = form_class(request.POST) 958 form = form_class(request.POST)
955 if form.is_valid(): 959 if form.is_valid():
956 reviewers = _get_reviewers(form) 960 reviewers = _get_reviewers(form)
957 if not form.is_valid(): 961 if not form.is_valid():
958 return respond(request, 'publish.html', {'form': form, 'issue': issue}) 962 return respond(request, 'publish.html', {'form': form, 'issue': issue})
959 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
960 if request.user == issue.owner: 964 if request.user == issue.owner:
961 subject = form.cleaned_data['subject'] 965 subject = form.cleaned_data['subject']
962 issue.subject = subject 966 issue.subject = subject
963 issue.reviewers = reviewers 967 issue.reviewers = reviewers
964 else: 968 else:
969 subject = issue.subject
965 issue.reviewers = reviewers 970 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
966 subject = issue.subject
967 tbd.append(issue) # To update the last modified time 971 tbd.append(issue) # To update the last modified time
968 message = form.cleaned_data['message'].replace('\r\n', '\n') 972 message = form.cleaned_data['message'].replace('\r\n', '\n')
969 send_mail = form.cleaned_data['send_mail'] 973 send_mail = form.cleaned_data['send_mail']
970 comments = [] 974 comments = []
971 975
972 # 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.
973 for patchset in issue.patchset_set.order('created'): 977 for patchset in issue.patchset_set.order('created'):
974 ## ps_comments = list(models.Comment.gql( 978 ## ps_comments = list(models.Comment.gql(
975 ## 'WHERE ANCESTOR IS :1 AND author = :2 AND draft = TRUE', 979 ## 'WHERE ANCESTOR IS :1 AND author = :2 AND draft = TRUE',
976 ## patchset, request.user)) 980 ## patchset, request.user))
977 # XXX Somehow the index broke, do without it 981 # XXX Somehow the index broke, do without it
978 ps_comments = [c for c in 982 ps_comments = [c for c in
979 models.Comment.gql('WHERE ANCESTOR IS :1', patchset) 983 models.Comment.gql('WHERE ANCESTOR IS :1', patchset)
980 if c.draft and c.author == request.user] 984 if c.draft and c.author == request.user]
981 # XXX End 985 # XXX End
982 if ps_comments: 986 if ps_comments:
983 patches = dict((p.key(), p) for p in patchset.patch_set) 987 patches = dict((p.key(), p) for p in patchset.patch_set)
984 for p in patches.itervalues(): 988 for p in patches.itervalues():
985 p.patchset = patchset 989 p.patchset = patchset
986 for c in ps_comments: 990 for c in ps_comments:
987 c.draft = False 991 c.draft = False
988 # XXX Using internal knowledge about db package: the key for 992 # XXX Using internal knowledge about db package: the key for
989 # reference property foo is stored as _foo. 993 # reference property foo is stored as _foo.
990 pkey = getattr(c, '_patch', None) 994 pkey = getattr(c, '_patch', None)
991 if pkey in patches: 995 if pkey in patches:
992 patch = patches[pkey] 996 patch = patches[pkey]
993 c.patch = patch 997 c.patch = patch
994 tbd.append(ps_comments) 998 tbd.append(ps_comments)
995 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,
996 c.lineno, c.date)) 1000 c.lineno, c.date))
997 comments += ps_comments 1001 comments += ps_comments
998 1002
999 if comments: 1003 if comments:
1000 logging.warn('Publishing %d comments', len(comments)) 1004 logging.warn('Publishing %d comments', len(comments))
1001 # Decide who should receive mail 1005 # Decide who should receive mail
1002 my_email = db.Email(request.user.email()) 1006 my_email = db.Email(request.user.email())
1003 addressees = [db.Email(issue.owner.email())] + issue.reviewers 1007 addressees = [db.Email(issue.owner.email())] + issue.reviewers
1004 if my_email in addressees: 1008 if my_email in addressees:
1005 everyone = addressees[:] 1009 everyone = addressees[:]
1006 if len(addressees) > 1: # Keep it if sending only to yourself 1010 if len(addressees) > 1: # Keep it if sending only to yourself
1007 addressees.remove(my_email) 1011 addressees.remove(my_email)
1008 else: 1012 else:
1009 everyone = addressees + [my_email] 1013 everyone = addressees + [my_email]
1010 details = _get_draft_details(request, comments) 1014 details = _get_draft_details(request, comments)
1011 text = ((message.strip() + '\n\n' + details.strip())).strip() 1015 text = ((message.strip() + '\n\n' + details.strip())).strip()
1012 msg = models.Message(issue=issue, 1016 msg = models.Message(issue=issue,
1013 subject=issue.subject, 1017 subject=issue.subject,
1014 sender=my_email, 1018 sender=my_email,
1015 recipients=everyone, 1019 recipients=everyone,
1016 text=db.Text(text), 1020 text=db.Text(text),
(...skipping 183 matching lines...) Show 10 above Show 10 below
1200 return respond(request, 'branch_edit.html', 1204 return respond(request, 'branch_edit.html',
1201 {'branch': branch, 'form': form}) 1205 {'branch': branch, 'form': form})
1202 branch.put() 1206 branch.put()
1203 return HttpResponseRedirect('/repos') 1207 return HttpResponseRedirect('/repos')
1204 1208
1205 1209
1206 @login_required 1210 @login_required
1207 def branch_delete(request, branch_id): 1211 def branch_delete(request, branch_id):
1208 """/branch_delete/<branch> - Delete a Branch record.""" 1212 """/branch_delete/<branch> - Delete a Branch record."""
1209 branch = models.Branch.get_by_id(int(branch_id)) 1213 branch = models.Branch.get_by_id(int(branch_id))
1210 if branch.owner != request.user: 1214 if branch.owner != request.user:
1211 return HttpResponseForbidden('You do not own this branch') 1215 return HttpResponseForbidden('You do not own this branch')
1212 repo = branch.repo 1216 repo = branch.repo
1213 branch.delete() 1217 branch.delete()
1214 num_branches = models.Branch.gql('WHERE repo = :1', repo).count() 1218 num_branches = models.Branch.gql('WHERE repo = :1', repo).count()
1215 if not num_branches: 1219 if not num_branches:
1216 # Even if we don't own the repository? Yes, I think so! Empty 1220 # Even if we don't own the repository? Yes, I think so! Empty
1217 # repositories have no representation on screen. 1221 # repositories have no representation on screen.
1218 repo.delete() 1222 repo.delete()
1219 return HttpResponseRedirect('/repos') 1223 return HttpResponseRedirect('/repos')
1220 1224
1221 1225
1222 ### User Profiles ### 1226 ### User Profiles ###
1223 1227
1224 @login_required 1228 @login_required
1225 def settings(request): 1229 def settings(request):
1226 account = models.Account.get_account_for_user(request.user) 1230 account = models.Account.get_account_for_user(request.user)
1227 if request.method != 'POST': 1231 if request.method != 'POST':
1228 nickname = account.nickname 1232 nickname = account.nickname
1229 form = SettingsForm(initial={'nickname': nickname}) 1233 form = SettingsForm(initial={'nickname': nickname})
1230 return respond(request, 'settings.html', {'form': form}) 1234 return respond(request, 'settings.html', {'form': form})
1231 form = SettingsForm(request.POST) 1235 form = SettingsForm(request.POST)
1232 if form.is_valid(): 1236 if form.is_valid():
1233 nickname = form.cleaned_data['nickname'].strip() 1237 nickname = form.cleaned_data['nickname'].strip()
1234 if not nickname: 1238 if not nickname:
1235 form.errors['nickname'] = ['Your nickname cannot be empty.'] 1239 form.errors['nickname'] = ['Your nickname cannot be empty.']
1236 elif '@' in nickname: 1240 elif '@' in nickname:
1237 form.errors['nickname'] = ['Your nickname cannot contain "@".'] 1241 form.errors['nickname'] = ['Your nickname cannot contain "@".']
1238 elif ',' in nickname: 1242 elif ',' in nickname:
1239 form.errors['nickname'] = ['Your nickname cannot contain ",".'] 1243 form.errors['nickname'] = ['Your nickname cannot contain ",".']
1240 else: 1244 else:
1241 accounts = models.Account.get_accounts_for_nickname(nickname) 1245 accounts = models.Account.get_accounts_for_nickname(nickname)
1242 if nickname != account.nickname and accounts: 1246 if nickname != account.nickname and accounts:
1243 form.errors['nickname'] = ['This nickname is already in use.'] 1247 form.errors['nickname'] = ['This nickname is already in use.']
1244 else: 1248 else:
1245 account.nickname = nickname 1249 account.nickname = nickname
1246 account.put() 1250 account.put()
1247 if not form.is_valid(): 1251 if not form.is_valid():
1248 return respond(request, 'settings.html', {'form': form}) 1252 return respond(request, 'settings.html', {'form': form})
1249 return HttpResponseRedirect('/settings') 1253 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