From 330dfcc296b047082d9e6d78ad7abc18997e9d74 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 24 Sep 2019 14:46:22 -0500 Subject: [PATCH] Allow users to change version slug Machine created versions aren't allowed to be changed, since we use their harcoded slug to identify them as latest and stable. Ref #5318 --- readthedocs/builds/forms.py | 40 +++++++++++- readthedocs/builds/version_slug.py | 100 ++++++++++++++++++----------- 2 files changed, 99 insertions(+), 41 deletions(-) diff --git a/readthedocs/builds/forms.py b/readthedocs/builds/forms.py index 91aa3ba2d46..0467fe72d09 100644 --- a/readthedocs/builds/forms.py +++ b/readthedocs/builds/forms.py @@ -1,11 +1,14 @@ -# -*- coding: utf-8 -*- - """Django forms for the builds app.""" from django import forms from django.utils.translation import ugettext_lazy as _ from readthedocs.builds.models import Version +from readthedocs.builds.version_slug import ( + VERSION_OK_CHARS, + VERSION_TEST_PATTERN, + VersionSlugify, +) from readthedocs.core.mixins import HideProtectedLevelMixin from readthedocs.core.utils import trigger_build @@ -14,7 +17,38 @@ class VersionForm(HideProtectedLevelMixin, forms.ModelForm): class Meta: model = Version - fields = ['active', 'privacy_level'] + fields = ['slug', 'active', 'privacy_level'] + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.fields['slug'].help_text = 'Warning: changing the slug will break existing URLs' + + if self.instance.pk: + if self.instance.machine: + self.fields['slug'].disabled = True + + def clean_slug(self): + slugifier = VersionSlugify( + ok_chars=VERSION_OK_CHARS, + test_pattern=VERSION_TEST_PATTERN, + ) + original_slug = self.cleaned_data.get('slug') + + slug = slugifier.slugify(original_slug, check_pattern=False) + if not slugifier.is_valid(slug): + msg = _('The slug "{slug}" is not valid.') + raise forms.ValidationError(msg.format(slug=original_slug)) + + duplicated = ( + Version.objects + .filter(project=self.instance.project, slug=slug) + .exclude(pk=self.instance.pk) + .exists() + ) + if duplicated: + msg = _('The slug "{slug}" is already in use.') + raise forms.ValidationError(msg.format(slug=slug)) + return slug def clean_active(self): active = self.cleaned_data['active'] diff --git a/readthedocs/builds/version_slug.py b/readthedocs/builds/version_slug.py index b840ff6e487..5c611a255c8 100644 --- a/readthedocs/builds/version_slug.py +++ b/readthedocs/builds/version_slug.py @@ -1,5 +1,3 @@ -# -*- coding: utf-8 -*- - """ Contains logic for handling version slugs. @@ -50,36 +48,23 @@ def get_fields_with_model(cls): # (?: ... ) -- wrap everything so that the pattern cannot escape when used in # regexes. VERSION_SLUG_REGEX = '(?:[a-z0-9A-Z][-._a-z0-9A-Z]*?)' +VERSION_OK_CHARS = '-._' # dash, dot, underscore +VERSION_TEST_PATTERN = re.compile('^{pattern}$'.format(pattern=VERSION_SLUG_REGEX)) +VERSION_FALLBACK_SLUG = 'unknown' -class VersionSlugField(models.CharField): +class VersionSlugify: """ - Inspired by ``django_extensions.db.fields.AutoSlugField``. + Generates a valid slug for a version. Uses ``unicode-slugify`` to generate the slug. """ - ok_chars = '-._' # dash, dot, underscore - test_pattern = re.compile('^{pattern}$'.format(pattern=VERSION_SLUG_REGEX)) - fallback_slug = 'unknown' - - def __init__(self, *args, **kwargs): - kwargs.setdefault('db_index', True) - - populate_from = kwargs.pop('populate_from', None) - if populate_from is None: - raise ValueError("missing 'populate_from' argument") - else: - self._populate_from = populate_from - super().__init__(*args, **kwargs) - - def get_queryset(self, model_cls, slug_field): - # pylint: disable=protected-access - for field, model in get_fields_with_model(model_cls): - if model and field == slug_field: - return model._default_manager.all() - return model_cls._default_manager.all() + def __init__(self, ok_chars, test_pattern, fallback_slug=''): + self.ok_chars = ok_chars + self.test_pattern = test_pattern + self.fallback_slug = fallback_slug def _normalize(self, content): """ @@ -94,18 +79,23 @@ def _normalize(self, content): """ return re.sub('[/%!?]', '-', content) - def slugify(self, content): + def is_valid(self, content): + return self.test_pattern.match(content) + + def slugify(self, content, check_pattern=True): """ Make ``content`` a valid slug. It uses ``unicode-slugify`` behind the scenes which works properly with Unicode characters. + + If `check_pattern` is `True`, it checks that the final slug is valid. """ if not content: return '' normalized = self._normalize(content) - slugified = unicode_slugify( + slug = unicode_slugify( normalized, only_ascii=True, spaces=False, @@ -114,26 +104,52 @@ def slugify(self, content): space_replacement='-', ) - # Remove first character wile it's an invalid character for the + # Remove first character while it's an invalid character for the # beginning of the slug - slugified = slugified.lstrip(self.ok_chars) + slug = slug.lstrip(self.ok_chars) + slug = slug or self.fallback_slug + + if check_pattern and not self.is_valid(slug): + raise Exception(f'Invalid generated slug: {slug}') + return slug + + +class VersionSlugField(models.CharField): - if not slugified: - return self.fallback_slug - return slugified + """ + Inspired by ``django_extensions.db.fields.AutoSlugField``. + """ + + ok_chars = VERSION_OK_CHARS + test_pattern = VERSION_TEST_PATTERN + fallback_slug = VERSION_OK_CHARS + + def __init__(self, *args, **kwargs): + kwargs.setdefault('db_index', True) + + populate_from = kwargs.pop('populate_from', None) + if populate_from is None: + raise ValueError("missing 'populate_from' argument") + else: + self._populate_from = populate_from + super().__init__(*args, **kwargs) + + def get_queryset(self, model_cls, slug_field): + # pylint: disable=protected-access + for field, model in get_fields_with_model(model_cls): + if model and field == slug_field: + return model._default_manager.all() + return model_cls._default_manager.all() def uniquifying_suffix(self, iteration): """ Create a unique suffix. - This creates a suffix based on the number given as ``iteration``. It will return a value encoded as lowercase ascii letter. So we have an alphabet of 26 letters. The returned suffix will be for example ``_yh`` where ``yh`` is the encoding of ``iteration``. The length of it will be ``math.log(iteration, 26)``. - Examples:: - uniquifying_suffix(0) == '_a' uniquifying_suffix(25) == '_z' uniquifying_suffix(26) == '_ba' @@ -157,10 +173,19 @@ def create_slug(self, model_instance): """Generate a unique slug for a model instance.""" # pylint: disable=protected-access + slugifier = VersionSlugify( + ok_chars=self.ok_chars, + test_pattern=self.test_pattern, + fallback_slug=self.fallback_slug, + ) + # get fields to populate from and slug field to set slug_field = model_instance._meta.get_field(self.attname) - slug = self.slugify(getattr(model_instance, self._populate_from)) + slug = slugifier.slugify( + content=getattr(model_instance, self._populate_from), + check_pattern=False, + ) count = 0 # strip slug depending on max_length attribute of the slug field @@ -196,9 +221,8 @@ def create_slug(self, model_instance): kwargs[self.attname] = slug count += 1 - is_slug_valid = self.test_pattern.match(slug) - if not is_slug_valid: - raise Exception('Invalid generated slug: {slug}'.format(slug=slug)) + if not slugifier.is_valid(slug): + raise Exception(f'Invalid generated slug: {slug}') return slug def pre_save(self, model_instance, add):