From fd4a24607b8d0d083ac8b2406fa6b47ce8da391a Mon Sep 17 00:00:00 2001 From: duker33 Date: Thu, 13 Dec 2018 16:08:39 +0300 Subject: [PATCH] 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 | 53 +++++++++++++++---- tests/catalog/test_models.py | 17 ++++++ tests/migrations/0003_buff_tag_name.py | 35 ++++++++++++ .../0004_fix_tag_slug_uniqueness.py | 24 +++++++++ 4 files changed, 118 insertions(+), 11 deletions(-) create mode 100644 tests/migrations/0003_buff_tag_name.py create mode 100644 tests/migrations/0004_fix_tag_slug_uniqueness.py diff --git a/catalog/models.py b/catalog/models.py index 12b0b17..32b631a 100644 --- a/catalog/models.py +++ b/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,30 +363,49 @@ 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) @@ -390,7 +413,15 @@ def save(self, *args, **kwargs): .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. diff --git a/tests/catalog/test_models.py b/tests/catalog/test_models.py index a4f947d..d66c146 100644 --- a/tests/catalog/test_models.py +++ b/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}') diff --git a/tests/migrations/0003_buff_tag_name.py b/tests/migrations/0003_buff_tag_name.py new file mode 100644 index 0000000..0a52911 --- /dev/null +++ b/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'), + ), + ] diff --git a/tests/migrations/0004_fix_tag_slug_uniqueness.py b/tests/migrations/0004_fix_tag_slug_uniqueness.py new file mode 100644 index 0000000..a3e4aa3 --- /dev/null +++ b/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')]), + ), + ]