Merge pull request #356 from Cadasta/bugfix/#328

Fix #328: Slug length control with custom slugify

cadasta/core/models.py

@@ -1,5 +1,6 @@
 import itertools
-from django.utils.text import slugify
+import math
+from core.util import slugify
 from django.db import models
 
 from .util import random_id, ID_FIELD_LENGTH
@@ -33,15 +34,20 @@ def __init__(self, *args, **kwargs):
         self.__original_slug = self.slug
 
     def save(self, *args, **kwargs):
+        max_length = self._meta.get_field('slug').max_length
         if not self.slug:
-            self.slug = slugify(self.name, allow_unicode=True)
+            self.slug = slugify(
+                self.name, max_length=max_length, allow_unicode=True
+            )
 
         orig = self.slug
 
         if not self.id or self.__original_slug != self.slug:
             for x in itertools.count(1):
                 if not type(self).objects.filter(slug=self.slug).exists():
                     break
+                slug_length = max_length - int(math.log10(x)) - 2
+                orig = self.slug[:slug_length]
                 self.slug = '{}-{}'.format(orig, x)
 
         self.__original_slug = self.slug

cadasta/core/tests/test_models.py

@@ -28,7 +28,7 @@ def test_duplicate_ids(self):
 
 
 class MySlugModel(SlugModel, Model):
-    name = CharField(max_length=50)
+    name = CharField(max_length=100)
     slug = SlugField(max_length=50, unique=True)
 
     class Meta:
@@ -109,3 +109,44 @@ def test_keep_slug(self):
         instance.refresh_from_db()
         assert instance.name == 'Other Name'
         assert instance.slug == 'test-name'
+
+    def test_create_with_long_name(self):
+        instance = MySlugModel()
+        instance.name = ('Very Long Name For The Purposes of '
+                         'Testing That Slug Truncation Functions Correctly')
+        instance.save()
+
+        assert len(instance.slug) == 50
+
+    def test_duplicate_slug_long_name(self):
+        instance1 = MySlugModel()
+        instance1.name = ('Very Long Name For The Purposes of '
+                          'Testing That Slug Truncation Functions Correctly')
+        instance1.save()
+
+        instance2 = MySlugModel()
+        instance2.name = ('Very Long Name For The Purposes of '
+                          'Testing That Slug Truncation Functions Correctly')
+        instance2.save()
+
+        instance1.refresh_from_db()
+        instance2.refresh_from_db()
+        assert instance1.slug != instance2.slug
+        assert instance2.slug[-2:] == '-1'
+        print(instance1.slug)
+        print(instance2.slug)
+
+    def test_duplicate_slug_long_name_ten_times(self):
+        for i in range(0, 100):
+            instance = MySlugModel()
+            instance.name = ('Very Long Name For The Purposes of Testing '
+                             'That Slug Truncation Functions Correctly')
+            instance.save()
+
+        instance = MySlugModel()
+        instance.name = ('Very Long Name For The Purposes of '
+                         'Testing That Slug Truncation Functions Correctly')
+        instance.save()
+
+        assert MySlugModel.objects.count() == 101
+        assert instance.slug[-4:] == '-100'

cadasta/core/util.py

@@ -1,5 +1,6 @@
 import random
 import string
+import django.utils.text as base_utils
 
 
 ID_FIELD_LENGTH = 24
@@ -17,3 +18,10 @@ def byte_to_base32_chr(byte):
 def random_id():
     rand_id = [random.randint(0, 0xFF) for i in range(ID_FIELD_LENGTH)]
     return ''.join(map(byte_to_base32_chr, rand_id))
+
+
+def slugify(text, max_length=None, allow_unicode=False):
+    slug = base_utils.slugify(text, allow_unicode=allow_unicode)
+    if max_length is not None:
+        slug = slug[:max_length]
+    return slug

cadasta/organization/forms.py

@@ -4,7 +4,7 @@
 from django.conf import settings
 from django.contrib.postgres import forms as pg_forms
 from django.contrib.gis import forms as gisforms
-from django.utils.text import slugify
+from core.util import slugify
 from django.utils.translation import ugettext as _
 from django.db import transaction
 

cadasta/organization/serializers.py

@@ -2,7 +2,7 @@
 from django.db.models import Q
 from django.core.mail import send_mail
 from django.core.urlresolvers import reverse
-from django.utils.text import slugify
+from core.util import slugify
 from django.utils.translation import ugettext as _
 from django.template.loader import get_template
 from django.template import Context

cadasta/organization/tests/test_serializers.py

@@ -1,7 +1,7 @@
 import random
 import pytest
 from datetime import datetime
-from django.utils.text import slugify
+from core.util import slugify
 from django.test import TestCase
 from django.utils.translation import gettext as _
 from django.core import mail

cadasta/organization/views/default.py

@@ -4,7 +4,7 @@
 from django.shortcuts import redirect, get_object_or_404
 import django.views.generic as base_generic
 from django.core.urlresolvers import reverse
-from django.utils.text import slugify
+from core.util import slugify
 from django.utils.translation import gettext as _
 from django.contrib import messages