Use todolist slugs for all URLs

Signed-off-by: Dan McGee <dan@archlinux.org>

4 files changed, 45 insertions, 28 deletions

diff --git a/templates/todolists/view.html b/templates/todolists/view.html
index 0f3475a2..66bbec6f 100644
--- a/templates/todolists/view.html
+++ b/templates/todolists/view.html
@@ -11,11 +11,11 @@
 
     <ul class="admin-actions">
         {% if perms.todolists.delete_todolist %}
-        <li><a href="/todo/delete/{{list.id}}/"
+        <li><a href="/todo/{{ list.slug }}/delete/"
             title="Delete this todo list">Delete Todo List</a></li>
         {% endif %}
         {% if perms.todolists.change_todolist %}
-        <li><a href="/todo/edit/{{list.id}}/"
+        <li><a href="/todo/{{ list.slug }}/edit/"
             title="Edit this todo list">Edit Todo List</a></li>
         {% endif %}
     </ul>
@@ -80,7 +80,7 @@
                 <td>{{ pkg.maintainers|join:', ' }}</td>
                 <td>
                     {% if perms.todolist.change_todolistpackage %}
-                    <a href="/todo/flag/{{ list.id }}/{{ pkg.id }}/"
+                    <a href="/todo/{{ list.slug }}/flag/{{ pkg.id }}/"
                         class="status-link {{ pkg.status_css_class }}" title="Toggle completion status">{{ pkg.get_status_display }}</a>
                     {% else %}
                     <span class="{{ pkg.status_css_class }}">{{ pkg.get_status_display }}</span>
diff --git a/todolists/models.py b/todolists/models.py
index a6dda2ab..76af0d35 100644
--- a/todolists/models.py
+++ b/todolists/models.py
@@ -35,7 +35,7 @@ class Todolist(models.Model):
         return self.name
 
     def get_absolute_url(self):
-        return '/todo/%i/' % self.old_id
+        return '/todo/%s/' % self.slug
 
     def get_full_url(self, proto='https'):
         '''get a URL suitable for things like email including the domain'''
diff --git a/todolists/urls.py b/todolists/urls.py
index 81ac11f5..cbc9547e 100644
--- a/todolists/urls.py
+++ b/todolists/urls.py
@@ -1,17 +1,26 @@
 from django.conf.urls import patterns
-from django.contrib.auth.decorators import permission_required
+from django.contrib.auth.decorators import login_required, permission_required
 
-from .views import DeleteTodolist
+from .views import (view_redirect, view, todolist_list, add, edit, flag,
+        list_pkgbases, DeleteTodolist)
 
-urlpatterns = patterns('todolists.views',
-    (r'^$',                       'todolist_list'),
-    (r'^(?P<list_id>\d+)/$',      'view'),
-    (r'^(?P<list_id>\d+)/pkgbases/(?P<svn_root>[a-z]+)/$', 'list_pkgbases'),
-    (r'^add/$',                   'add'),
-    (r'^edit/(?P<list_id>\d+)/$', 'edit'),
-    (r'^flag/(\d+)/(\d+)/$',      'flag'),
-    (r'^delete/(?P<pk>\d+)/$',
+urlpatterns = patterns('',
+    (r'^$', login_required(todolist_list)),
+
+    # old todolists URLs, permanent redirect view so we don't break all links
+    (r'^(?P<old_id>\d+)/$', view_redirect),
+
+    (r'^add/$',
+        permission_required('todolists.add_todolist')(add)),
+    (r'^(?P<slug>[-\w]+)/$', login_required(view)),
+    (r'^(?P<slug>[-\w]+)/edit/$',
+        permission_required('todolists.change_todolist')(edit)),
+    (r'^(?P<slug>[-\w]+)/delete/$',
         permission_required('todolists.delete_todolist')(DeleteTodolist.as_view())),
+    (r'^(?P<slug>[-\w]+)/flag/(?P<pkg_id>\d+)/$',
+        permission_required('todolists.change_todolistpackage')(flag)),
+    (r'^(?P<slug>[-\w]+)/pkgbases/(?P<svn_root>[a-z]+)/$',
+        'list_pkgbases'),
 )
 
 # vim: set ts=4 sw=4 et:
diff --git a/todolists/views.py b/todolists/views.py
index ccf65369..29f543c2 100644
--- a/todolists/views.py
+++ b/todolists/views.py
@@ -4,7 +4,6 @@ from django import forms
 from django.http import HttpResponse
 from django.core.mail import send_mail
 from django.shortcuts import get_list_or_404, get_object_or_404, redirect, render
-from django.contrib.auth.decorators import login_required, permission_required
 from django.db import transaction
 from django.views.decorators.cache import never_cache
 from django.views.generic import DeleteView
@@ -32,10 +31,9 @@ class TodoListForm(forms.ModelForm):
         fields = ('name', 'description', 'raw')
 
 
-@permission_required('todolists.change_todolistpackage')
 @never_cache
-def flag(request, list_id, pkg_id):
-    todolist = get_object_or_404(Todolist, old_id=list_id)
+def flag(request, slug, pkg_id):
+    todolist = get_object_or_404(Todolist, slug=slug)
     tlpkg = get_object_or_404(TodolistPackage, id=pkg_id)
     # TODO: none of this; require absolute value on submit
     if tlpkg.status == TodolistPackage.INCOMPLETE:
@@ -51,9 +49,14 @@ def flag(request, list_id, pkg_id):
         return HttpResponse(json.dumps(data), mimetype='application/json')
     return redirect(todolist)
 
-@login_required
-def view(request, list_id):
-    todolist = get_object_or_404(Todolist, old_id=list_id)
+
+def view_redirect(request, old_id):
+    todolist = get_object_or_404(Todolist, old_id=old_id)
+    return redirect(todolist, permanent=True)
+
+
+def view(request, slug):
+    todolist = get_object_or_404(Todolist, slug=slug)
     svn_roots = Repo.objects.values_list(
             'svn_root', flat=True).order_by().distinct()
     # we don't hold onto the result, but the objects are the same here,
@@ -68,22 +71,23 @@ def view(request, list_id):
         'repos': sorted(repos),
     })
 
+
 # really no need for login_required on this one...
-def list_pkgbases(request, list_id, svn_root):
+def list_pkgbases(request, slug, svn_root):
     '''Used to make bulk moves of packages a lot easier.'''
-    todolist = get_object_or_404(Todolist, old_id=list_id)
+    todolist = get_object_or_404(Todolist, slug=slug)
     repos = get_list_or_404(Repo, svn_root=svn_root)
     pkgbases = TodolistPackage.objects.values_list('pkgbase', flat=True).filter(
             todolist=todolist, repo__in=repos).distinct().order_by('pkgbase')
     return HttpResponse('\n'.join(pkgbases),
         mimetype='text/plain')
 
-@login_required
+
 def todolist_list(request):
     lists = get_annotated_todolists()
     return render(request, 'todolists/list.html', {'lists': lists})
 
-@permission_required('todolists.add_todolist')
+
 @never_cache
 def add(request):
     if request.POST:
@@ -103,11 +107,11 @@ def add(request):
     }
     return render(request, 'general_form.html', page_dict)
 
+
 # TODO: this calls for transaction management and async emailing
-@permission_required('todolists.change_todolist')
 @never_cache
-def edit(request, list_id):
-    todo_list = get_object_or_404(Todolist, old_id=list_id)
+def edit(request, slug):
+    todo_list = get_object_or_404(Todolist, slug=slug)
     if request.POST:
         form = TodoListForm(request.POST, instance=todo_list)
         if form.is_valid():
@@ -126,12 +130,14 @@ def edit(request, list_id):
     }
     return render(request, 'general_form.html', page_dict)
 
+
 class DeleteTodolist(DeleteView):
     model = Todolist
     # model in main == assumes name 'main/todolist_confirm_delete.html'
     template_name = 'todolists/todolist_confirm_delete.html'
     success_url = '/todo/'
 
+
 @transaction.commit_on_success
 def create_todolist_packages(form, creator=None):
     packages = form.packages()
@@ -166,6 +172,7 @@ def create_todolist_packages(form, creator=None):
 
     return todo_pkgs
 
+
 def send_todolist_emails(todo_list, new_packages):
     '''Sends emails to package maintainers notifying them that packages have
     been added to a todo list.'''
@@ -193,6 +200,7 @@ def send_todolist_emails(todo_list, new_packages):
                 [maint],
                 fail_silently=True)
 
+
 def public_list(request):
     todo_lists = Todolist.objects.incomplete()
     # total hackjob, but it makes this a lot less query-intensive.