From 7c1e5ae8e7ce531bc01c85274846a46418a8207f Mon Sep 17 00:00:00 2001
From: Serghei Mihai <smihai@entrouvert.com>
Date: Fri, 20 Jan 2017 16:42:31 +0100
Subject: [PATCH] use unique category slugs in urls (#13791)

---
 corbo/forms.py                              | 13 ++++++++++++-
 corbo/manage_urls.py                        |  8 ++++----
 corbo/migrations/0009_auto_20170120_1533.py | 27 +++++++++++++++++++++++++++
 corbo/models.py                             |  2 +-
 corbo/templates/corbo/announce_form.html    |  4 ++--
 corbo/templates/corbo/category_detail.html  |  8 ++++----
 corbo/templates/corbo/manage.html           |  2 +-
 corbo/views.py                              |  8 ++++----
 tests/test_emailing.py                      |  5 +++--
 9 files changed, 58 insertions(+), 19 deletions(-)
 create mode 100644 corbo/migrations/0009_auto_20170120_1533.py

diff --git a/corbo/forms.py b/corbo/forms.py
index 4bd7f89..2fe8010 100644
--- a/corbo/forms.py
+++ b/corbo/forms.py
@@ -1,6 +1,7 @@
 from django import forms
 from django.utils.translation import ugettext_lazy as _
 from django.utils.text import slugify
+from django.core.exceptions import ObjectDoesNotExist
 
 from .models import Announce, Category, Broadcast, channel_choices
 
@@ -31,6 +32,16 @@ class CategoryForm(forms.ModelForm):
         model = Category
 
     def save(self, commit=True):
+        slug = slugify(self.instance.name)
+        base_slug = slug
         if not self.instance.slug:
-            self.instance.slug = slugify(self.instance.name)
+            i = 1
+            while True:
+                try:
+                    c = Category.objects.get(slug=slug)
+                except ObjectDoesNotExist:
+                    break
+                i += 1
+                slug = '%s-%s' % (base_slug, i)
+            self.instance.slug = slug
         return super(CategoryForm, self).save(commit=commit)
diff --git a/corbo/manage_urls.py b/corbo/manage_urls.py
index 49cb86b..f4c519b 100644
--- a/corbo/manage_urls.py
+++ b/corbo/manage_urls.py
@@ -6,19 +6,19 @@ from .views import add_announce, edit_announce, delete_announce, \
 
 urlpatterns = patterns('',
             url(r'^$', manage, name='manage'),
-            url(r'^category/(?P<pk>\d+)/announce/$', add_announce,
+            url(r'^category/(?P<slug>[\w-]+)/announce/$', add_announce,
                 name='add_announce'),
             url(r'^announce/edit/(?P<pk>\d+)$', edit_announce,
                 name='edit_announce'),
             url(r'^announce/delete/(?P<pk>\d+)$', delete_announce,
                 name='delete_announce'),
-            url(r'^category/(?P<pk>\d+)/$', view_category,
+            url(r'^category/(?P<slug>[\w-]+)/$', view_category,
                 name='view_category'),
             url(r'^category/add$', add_category,
                 name='add_category'),
-            url(r'^category/edit/(?P<pk>\d+)$', edit_category,
+            url(r'^category/edit/(?P<slug>[\w-]+)$', edit_category,
                 name='edit_category'),
-            url(r'^category/delete/(?P<pk>\d+)$', delete_category,
+            url(r'^category/delete/(?P<slug>[\w-]+)$', delete_category,
                 name='delete_category'),
             url(r'^menu.json$', menu_json),
 )
diff --git a/corbo/migrations/0009_auto_20170120_1533.py b/corbo/migrations/0009_auto_20170120_1533.py
new file mode 100644
index 0000000..1d58903
--- /dev/null
+++ b/corbo/migrations/0009_auto_20170120_1533.py
@@ -0,0 +1,27 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+from django.utils.text import slugify
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('corbo', '0008_category_slug'),
+    ]
+
+    def make_slug_unique(apps, schema_editor):
+        Category = apps.get_model('corbo', 'Category')
+        for category in Category.objects.all():
+            category.slug = slugify(category.name)
+            category.save()
+
+    operations = [
+        migrations.RunPython(make_slug_unique),
+        migrations.AlterField(
+            model_name='category',
+            name='slug',
+            field=models.SlugField(unique=True, verbose_name='Slug'),
+        ),
+    ]
diff --git a/corbo/models.py b/corbo/models.py
index b22642d..ff30ad3 100644
--- a/corbo/models.py
+++ b/corbo/models.py
@@ -43,7 +43,7 @@ def transform_image_src(src, **kwargs):
 
 class Category(models.Model):
     name = models.CharField(_('Name'), max_length=64, blank=False, null=False)
-    slug = models.SlugField(_('Slug'))
+    slug = models.SlugField(_('Slug'), unique=True)
     rss_feed_url = models.URLField(_('Feed URL'), blank=True, null=True,
                 help_text=_('if defined, announces will be automatically created from rss items'))
     ctime = models.DateTimeField(auto_now_add=True)
diff --git a/corbo/templates/corbo/announce_form.html b/corbo/templates/corbo/announce_form.html
index 9596f57..ece7a8e 100644
--- a/corbo/templates/corbo/announce_form.html
+++ b/corbo/templates/corbo/announce_form.html
@@ -4,7 +4,7 @@
 {% block breadcrumb %}
 {{ block.super }}
 {% if category %}
-<a href='{% url "view_category" pk=category.pk %}'>{{ category }}</a>
+<a href='{% url "view_category" slug=category.slug %}'>{{ category }}</a>
 {% endif %}
 {% endblock %}
 
@@ -26,7 +26,7 @@
   {{ form.as_p }}
   <div class="buttons">
     <button>{% trans "Save" %}</button>
-    <a class="cancel" href="{% url 'view_category' pk=category.pk %}">{% trans 'Cancel' %}</a>
+    <a class="cancel" href="{% url 'view_category' slug=category.slug %}">{% trans 'Cancel' %}</a>
   </div>
   <script type="text/javascript">
     $(function() {
diff --git a/corbo/templates/corbo/category_detail.html b/corbo/templates/corbo/category_detail.html
index 8fc7dce..df31de9 100644
--- a/corbo/templates/corbo/category_detail.html
+++ b/corbo/templates/corbo/category_detail.html
@@ -4,15 +4,15 @@
 {% block breadcrumb %}
 {{ block.super }}
 {% if category %}
-<a href='{% url "view_category" pk=category.pk %}'>{{ category }}</a>
+<a href='{% url "view_category" slug=category.slug %}'>{{ category }}</a>
 {% endif %}
 {% endblock %}
 
 {% block appbar %}
 <h2>{{ object.name }}</h2>
-<a href="{% url 'delete_category' object.id %}" rel="popup">{% trans 'Delete' %}</a>
-<a href="{% url 'edit_category' object.id %}" rel="popup">{% trans 'Edit' %}</a>
-<a href="{% url 'add_announce' pk=object.pk %}">{% trans 'New announce' %}</a>
+<a href="{% url 'delete_category' slug=object.slug %}" rel="popup">{% trans 'Delete' %}</a>
+<a href="{% url 'edit_category' slug=object.slug %}" rel="popup">{% trans 'Edit' %}</a>
+<a href="{% url 'add_announce' slug=object.slug %}">{% trans 'New announce' %}</a>
 {% endblock %}
 
 {% block content %}
diff --git a/corbo/templates/corbo/manage.html b/corbo/templates/corbo/manage.html
index 2c70f68..6c537b4 100644
--- a/corbo/templates/corbo/manage.html
+++ b/corbo/templates/corbo/manage.html
@@ -11,7 +11,7 @@
   <ul class='objects-list single-links'>
     {% for obj in object_list %}
     <li class='category'>
-        <a href="{% url 'view_category' pk=obj.id %}">{{ obj.name }} /
+        <a href="{% url 'view_category' slug=obj.slug %}">{{ obj.name }} /
           {% blocktrans count announces_number=obj.get_announces_count %}
           {{ announces_number }} announce
           {% plural %}
diff --git a/corbo/views.py b/corbo/views.py
index b5326a6..09c9c90 100644
--- a/corbo/views.py
+++ b/corbo/views.py
@@ -58,11 +58,11 @@ class AnnounceCreateView(CreateView):
     template_name = 'corbo/announce_form.html'
 
     def get_success_url(self):
-        return reverse('view_category', kwargs={'pk': self.object.category.pk})
+        return reverse('view_category', kwargs={'slug': self.object.category.slug})
 
     def get_initial(self):
         initial = super(AnnounceCreateView, self).get_initial()
-        initial['category'] = models.Category.objects.get(pk=self.kwargs['pk'])
+        initial['category'] = models.Category.objects.get(slug=self.kwargs['slug'])
         return initial
 
     def get_context_data(self, **kwargs):
@@ -95,7 +95,7 @@ class AnnounceDeleteView(DeleteView):
     model = models.Announce
 
     def get_success_url(self):
-        return reverse('view_category', kwargs={'pk': self.object.category.pk})
+        return reverse('view_category', kwargs={'slug': self.object.category.slug})
 
 
 delete_announce = AnnounceDeleteView.as_view()
@@ -121,7 +121,7 @@ class CategoryEditView(UpdateView):
     model = models.Category
 
     def get_success_url(self):
-        return reverse('view_category', kwargs={'pk': self.object.pk})
+        return reverse('view_category', kwargs={'slug': self.object.slug})
 
 
 edit_category = CategoryEditView.as_view()
diff --git a/tests/test_emailing.py b/tests/test_emailing.py
index 036e3eb..cda9351 100644
--- a/tests/test_emailing.py
+++ b/tests/test_emailing.py
@@ -9,19 +9,20 @@ from django.core.urlresolvers import reverse
 from django.core import mail, signing
 from django.utils import timezone
 from django.core.files.storage import DefaultStorage
+from django.utils.text import slugify
 
 from corbo.models import Category, Announce, Subscription, Broadcast, transform_image_src
 
 pytestmark = pytest.mark.django_db
 
-CATEGORIES = ('Alerts', 'News')
+CATEGORIES = (u'Alerts', u'News')
 
 
 @pytest.fixture
 def categories():
     categories = []
     for category in CATEGORIES:
-        c, created = Category.objects.get_or_create(name=category)
+        c, created = Category.objects.get_or_create(name=category, slug=slugify(category))
         categories.append(c)
     return categories
 
-- 
2.11.0