rf#220 Fix tag name (#221)

* #220  Fix tag slug autocreation for long names

* rf#220  Increase tag name max length

* #220  Refactor slug uniqueness processing methods

* #220  Change Tag.slug uniqueness

catalog/models.py

@@ -15,9 +15,13 @@
 from django.utils.translation import ugettext_lazy as _
 
 
-def randomize_slug(slug: str) -> str:
+SLUG_MAX_LENGTH = 50
+
+
+def randomize_slug(slug: str, hash_size: int) -> str:
+    hash_size = hash_size
     slug_hash = ''.join(
-        random.choices(string.ascii_lowercase, k=settings.SLUG_HASH_SIZE)
+        random.choices(string.ascii_lowercase, k=hash_size)
     )
     return f'{slug}_{slug_hash}'
 
@@ -359,38 +363,65 @@ def parsed(self, raw):
 
 class Tag(models.Model):
 
+    SLUG_HASH_SIZE = 5
+
     class Meta:
         abstract = True
-        unique_together = ('name', 'group')
+        unique_together = [('name', 'group'), ('slug', 'group')]
 
     objects = TagManager()
 
     uuid = models.UUIDField(default=uuid4, editable=False)
     name = models.CharField(
-        max_length=100, db_index=True, verbose_name=_('name'))
+        max_length=1000, db_index=True, verbose_name=_('name'))
     position = models.PositiveSmallIntegerField(
         default=0, blank=True, db_index=True, verbose_name=_('position'),
     )
 
-    slug = models.SlugField(default='', unique=True)
+    slug = models.SlugField(
+        blank=False, unique=False, max_length=SLUG_MAX_LENGTH
+    )
 
     def __str__(self):
         return self.name
 
-    def save(self, *args, **kwargs):
-        if not self.slug:
-            # same slugify code used in PageMixin object
-            self.slug = slugify(
-                unidecode(self.name.replace('.', '-').replace('+', '-'))
+    def _generate_short_slug(self, max_length=SLUG_MAX_LENGTH) -> None:
+        """
+        Generate slug with limited length.
+
+        Slug is autogenerated from name. But name can be 2000 symbols in length.
+        This method regenerate slug to keep it length less then SLUG_MAX_LENGTH.
+        """
+        slug = slugify(
+            unidecode(self.name.replace('.', '-').replace('+', '-'))
+        )
+        if len(slug) < max_length:
+            self.slug = slug
+        else:
+            slug_length = max_length - self.SLUG_HASH_SIZE - 1
+            self.slug = randomize_slug(
+                slug=slug[:slug_length],
+                hash_size=self.SLUG_HASH_SIZE
             )
+
+    def _make_slug_unique(self) -> None:
+        """Force slug name to be unique."""
         tag_is_doubled = (
             self.__class__.objects
             .filter(slug=self.slug)
             .exclude(group=self.group)
             .exists()
         )
         if tag_is_doubled:
-            self.slug = randomize_slug(self.slug)
+            self.slug = randomize_slug(
+                self.slug, hash_size=self.SLUG_HASH_SIZE
+            )
+
+    def save(self, *args, **kwargs):
+        if not self.slug:
+            # same slugify code used in PageMixin object
+            self._generate_short_slug()
+        self._make_slug_unique()
         super(Tag, self).save(*args, **kwargs)
 
     # @todo #168:15m Move `Tags.parse_url_tags` Tags context.

tests/catalog/test_models.py

@@ -1,10 +1,12 @@
 """Defines tests for models in Catalog app."""
 import unittest
 
+from django.db import DataError
 from django.test import TestCase
 
 from pages.models import CustomPage
 
+import catalog
 from tests.catalog import models as catalog_models
 
 
@@ -276,3 +278,18 @@ def test_tag_doubled_save_slug_postfix(self):
         self.assertEqual(tag.slug, '12-v')
         tag.save()
         self.assertEqual(tag.slug, '12-v')
+
+    def test_long_name(self):
+        """
+        Tag should accept long names.
+
+        Slug length has limited limited size `catalog.models.MAX_SLUG_LENGTH`.
+        It may create problems for tag with long name.
+        """
+        name = 'Имя ' * 50
+        group = catalog_models.MockTagGroup.objects.first()
+        try:
+            tag = catalog_models.MockTag.objects.create(group=group, name=name)
+            self.assertLessEqual(len(tag.slug), catalog.models.SLUG_MAX_LENGTH)
+        except DataError as e:
+            self.assertTrue(False, f'Tag has too long name. {e}')

tests/migrations/0003_buff_tag_name.py

@@ -0,0 +1,35 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.10 on 2018-12-11 05:59
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('tests', '0002_add_tags'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='mockcategory',
+            name='name',
+            field=models.CharField(db_index=True, max_length=1000, verbose_name='name'),
+        ),
+        migrations.AlterField(
+            model_name='mockcategorywithdefaultpage',
+            name='name',
+            field=models.CharField(db_index=True, max_length=1000, verbose_name='name'),
+        ),
+        migrations.AlterField(
+            model_name='mockecommercecategory',
+            name='name',
+            field=models.CharField(db_index=True, max_length=1000, verbose_name='name'),
+        ),
+        migrations.AlterField(
+            model_name='mocktag',
+            name='name',
+            field=models.CharField(db_index=True, max_length=1000, verbose_name='name'),
+        ),
+    ]

tests/migrations/0004_fix_tag_slug_uniqueness.py

@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.10 on 2018-12-13 01:56
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('tests', '0003_buff_tag_name'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='mocktag',
+            name='slug',
+            field=models.SlugField(),
+        ),
+        migrations.AlterUniqueTogether(
+            name='mocktag',
+            unique_together=set([('slug', 'group'), ('name', 'group')]),
+        ),
+    ]