| 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, |
| (...skipping 168 matching lines...) Show 10 above Show 10 below | |
| 179 max_length=1000, | 179 max_length=1000, |
| 180 widget=forms.TextInput(attrs={'size': 60})) | 180 widget=forms.TextInput(attrs={'size': 60})) |
| 181 send_mail = forms.BooleanField() | 181 send_mail = forms.BooleanField() |
| 182 message = forms.CharField(required=False, | 182 message = forms.CharField(required=False, |
| 183 max_length=10000, | 183 max_length=10000, |
| 184 widget=forms.Textarea(attrs={'cols': 60})) | 184 widget=forms.Textarea(attrs={'cols': 60})) |
| 185 | 185 |
| 186 | 186 |
| 187 class MiniPublishForm(forms.Form): | 187 class MiniPublishForm(forms.Form): |
| 188 | 188 |
| 189 send_mail = forms.BooleanField() | 189 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, | 190 reviewers = forms.CharField(required=False, |
| 191 max_length=1000, | 191 max_length=1000, |
| 192 widget=forms.TextInput(attrs={'size': 60})) | 192 widget=forms.TextInput(attrs={'size': 60})) |
| 193 message = forms.CharField(required=False, | 193 message = forms.CharField(required=False, |
| 194 max_length=10000, | 194 max_length=10000, |
| 195 widget=forms.Textarea(attrs={'cols': 60})) | 195 widget=forms.Textarea(attrs={'cols': 60})) |
| 196 | 196 |
| 197 | 197 |
| 198 class SettingsForm(forms.Form): | 198 class SettingsForm(forms.Form): |
| 199 | 199 |
| (...skipping 368 matching lines...) Show 10 above Show 10 below | |
| 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 |
| (...skipping 349 matching lines...) Show 10 above Show 10 below | |
| 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)) |
| (...skipping 22 matching lines...) Show 10 above Show 10 below | |
| 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] |
|
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) | 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), |
| 1021 parent=issue) | 1017 parent=issue) |
| 1022 tbd.append(msg) | 1018 tbd.append(msg) |
| 1023 | 1019 |
| (...skipping 220 matching lines...) Show 10 above Show 10 below | |
| 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
| |
| LEFT | RIGHT |
