From 0c368ce4661c91f77aa79f189c4be11de5d94d27 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Thu, 3 Mar 2011 14:40:32 -0600 Subject: Correct some permission decorators Signed-off-by: Dan McGee --- todolists/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'todolists') diff --git a/todolists/views.py b/todolists/views.py index 519fae9d..7d000809 100644 --- a/todolists/views.py +++ b/todolists/views.py @@ -32,7 +32,7 @@ class TodoListForm(forms.Form): return packages -@login_required +@permission_required('main.change_todolistpkg') @never_cache def flag(request, listid, pkgid): list = get_object_or_404(Todolist, id=listid) -- cgit v1.2.3-2-g168b From 3f4570c1995be17fefa9701e943ba9851113cd44 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Fri, 4 Mar 2011 11:11:06 -0600 Subject: Use transactions in todolist creation So we do all of the work at once and don't let things leak out before the list is completely added or updated. Signed-off-by: Dan McGee --- todolists/views.py | 94 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 52 insertions(+), 42 deletions(-) (limited to 'todolists') diff --git a/todolists/views.py b/todolists/views.py index 7d000809..337fd0d6 100644 --- a/todolists/views.py +++ b/todolists/views.py @@ -4,6 +4,7 @@ from django.http import HttpResponse from django.core.mail import send_mail from django.shortcuts import get_object_or_404, redirect from django.contrib.auth.decorators import login_required, permission_required +from django.db import transaction from django.db.models import Count from django.views.decorators.cache import never_cache from django.views.generic.create_update import delete_object @@ -13,11 +14,7 @@ from django.utils import simplejson from main.models import Todolist, TodolistPkg, Package -class TodoListForm(forms.Form): - name = forms.CharField(max_length=255, - widget=forms.TextInput(attrs={'size': '30'})) - description = forms.CharField(required=False, - widget=forms.Textarea(attrs={'rows': '4', 'cols': '60'})) +class TodoListForm(forms.ModelForm): packages = forms.CharField(required=False, help_text='(one per line)', widget=forms.Textarea(attrs={'rows': '20', 'cols': '60'})) @@ -26,11 +23,14 @@ class TodoListForm(forms.Form): package_names = [s.strip() for s in self.cleaned_data['packages'].split("\n")] package_names = set(package_names) - packages = Package.objects.filter( - pkgname__in=package_names).exclude( - repo__testing=True).order_by('arch') + packages = Package.objects.filter(pkgname__in=package_names).exclude( + repo__testing=True).select_related( + 'arch', 'repo').order_by('arch') return packages + class Meta: + model = Todolist + fields = ('name', 'description') @permission_required('main.change_todolistpkg') @never_cache @@ -68,23 +68,17 @@ def list(request): return direct_to_template(request, 'todolists/list.html', {'lists': lists}) -# TODO: this calls for transaction management and async emailing @permission_required('main.add_todolist') @never_cache def add(request): if request.POST: form = TodoListForm(request.POST) if form.is_valid(): - todo = Todolist.objects.create( - creator = request.user, - name = form.cleaned_data['name'], - description = form.cleaned_data['description']) - - for pkg in form.cleaned_data['packages']: - tpkg = TodolistPkg.objects.create(list=todo, pkg=pkg) - send_todolist_email(tpkg) + new_packages = create_todolist_packages(form, creator=request.user) + for new_package in new_packages: + send_newlist_email(new_package) - return redirect('/todo/') + return redirect(form.instance) else: form = TodoListForm() @@ -101,33 +95,18 @@ def add(request): def edit(request, list_id): todo_list = get_object_or_404(Todolist, id=list_id) if request.POST: - form = TodoListForm(request.POST) + form = TodoListForm(request.POST, instance=todo_list) if form.is_valid(): - todo_list.name = form.cleaned_data['name'] - todo_list.description = form.cleaned_data['description'] - todo_list.save() - - packages = [p.pkg for p in todo_list.packages] + new_packages = create_todolist_packages(form) - # first delete any packages not in the new list - for p in todo_list.packages: - if p.pkg not in form.cleaned_data['packages']: - p.delete() - - # now add any packages not in the old list - for pkg in form.cleaned_data['packages']: - if pkg not in packages: - tpkg = TodolistPkg.objects.create( - list=todo_list, pkg=pkg) - send_todolist_email(tpkg) + for new_package in new_packages: + send_todolist_email(new_package) return redirect(todo_list) else: - form = TodoListForm(initial={ - 'name': todo_list.name, - 'description': todo_list.description, - 'packages': '\n'.join(todo_list.package_names), - }) + form = TodoListForm(instance=todo_list, + initial={ 'packages': '\n'.join(todo_list.package_names) }) + page_dict = { 'title': 'Edit Todo List: %s' % todo_list.name, 'form': form, @@ -142,10 +121,41 @@ def delete_todolist(request, object_id): template_name="todolists/todolist_confirm_delete.html", post_delete_redirect='/todo/') + +@transaction.commit_on_success +def create_todolist_packages(form, creator=None): + packages = form.cleaned_data['packages'] + if creator: + # todo list is new + todolist = form.save(commit=False) + todolist.creator = creator + todolist.save() + + old_packages = [] + else: + # todo list already existed + form.save() + todolist = form.instance + # first delete any packages not in the new list + for todo_pkg in todolist.packages: + if todo_pkg.pkg not in packages: + todo_pkg.delete() + + # save the old package list so we know what to add + old_packages = [p.pkg for p in todolist.packages] + + todo_pkgs = [] + for package in packages: + if package not in old_packages: + todo_pkg = TodolistPkg.objects.create(list=todolist, pkg=package) + todo_pkgs.append(todo_pkg) + + return todo_pkgs + def send_todolist_email(todo): '''Sends an e-mail to the maintainer of a package notifying them that the package has been added to a todo list''' - maints = todo.pkg.maintainers + maints = todo.pkg.maintainers.values_list('email', flat=True) if not maints: return @@ -159,7 +169,7 @@ def send_todolist_email(todo): send_mail('arch: Package [%s] added to Todolist' % todo.pkg.pkgname, t.render(c), 'Arch Website Notification ', - [m.email for m in maints], + maints, fail_silently=True) def public_list(request): -- cgit v1.2.3-2-g168b From 65e965c8f76677904f5d98965e13bf89726247d4 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Fri, 4 Mar 2011 12:39:53 -0600 Subject: Send only one email per todolist Customize each email on a per-maintainer basis and list all the relevant packages inside, rather than spamming people. Signed-off-by: Dan McGee --- todolists/views.py | 54 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 26 deletions(-) (limited to 'todolists') diff --git a/todolists/views.py b/todolists/views.py index 337fd0d6..6bd456ae 100644 --- a/todolists/views.py +++ b/todolists/views.py @@ -75,9 +75,7 @@ def add(request): form = TodoListForm(request.POST) if form.is_valid(): new_packages = create_todolist_packages(form, creator=request.user) - for new_package in new_packages: - send_newlist_email(new_package) - + send_todolist_emails(form.instance, new_packages) return redirect(form.instance) else: form = TodoListForm() @@ -98,10 +96,7 @@ def edit(request, list_id): form = TodoListForm(request.POST, instance=todo_list) if form.is_valid(): new_packages = create_todolist_packages(form) - - for new_package in new_packages: - send_todolist_email(new_package) - + send_todolist_emails(todo_list, new_packages) return redirect(todo_list) else: form = TodoListForm(instance=todo_list, @@ -152,25 +147,32 @@ def create_todolist_packages(form, creator=None): return todo_pkgs -def send_todolist_email(todo): - '''Sends an e-mail to the maintainer of a package notifying them that the - package has been added to a todo list''' - maints = todo.pkg.maintainers.values_list('email', flat=True) - if not maints: - return - - page_dict = { - 'pkg': todo.pkg, - 'todolist': todo.list, - 'weburl': todo.pkg.get_full_url() - } - t = loader.get_template('todolists/email_notification.txt') - c = Context(page_dict) - send_mail('arch: Package [%s] added to Todolist' % todo.pkg.pkgname, - t.render(c), - 'Arch Website Notification ', - maints, - fail_silently=True) +def send_todolist_emails(todo_list, new_packages): + '''Sends emails to package maintainers notifying them that packages have + been added to a todo list.''' + # start by flipping the incoming list on its head: we want a list of + # involved maintainers and the packages they need to be notified about. + orphan_packages = [] + maint_packages = {} + for todo_package in new_packages: + maints = todo_package.pkg.maintainers.values_list('email', flat=True) + if not maints: + orphan_packages.append(todo_package) + else: + for maint in maints: + maint_packages.setdefault(maint, []).append(todo_package) + + for maint, packages in maint_packages.iteritems(): + c = Context({ + 'todo_packages': sorted(packages), + 'todolist': todo_list, + }) + t = loader.get_template('todolists/email_notification.txt') + send_mail('Packages added to todo list \'%s\'' % todo_list.name, + t.render(c), + 'Arch Website Notification ', + [maint], + fail_silently=True) def public_list(request): todo_lists = Todolist.objects.incomplete() -- cgit v1.2.3-2-g168b