Allow users to change version slug (#11930)

Basically what I had in my old design doc
#6273

- When changing the slug, we check that it's a valid one (lowercase,
only number, etc), in case the slug is not valid, we suggest a valid
slug based on the invalid one. Why not just transform the value to a
valid slug? Changing the slug is a breaking operation, we want to avoid
surprises for the user, if they set a slug to "foo/bar", and then the
actual slug is "foo-bar", that's unexpected.
- `project` is used as s hidden field, following the same pattern we
already have for other forms (this way all normal validations like the
unique constraint work instead of getting a 500).
- Editing the slug is disabled for machine created versions (stable,
latest).
- I'm not restricting the usage of latest or stable, since those will be
non-machine created, similar to what a user does already if they name a
version stable/latest.
- If a version was active and its slug was changed, the resources
attached to the old slug are removed before triggering a new build.
- Cache is always purged after deleting resources from the version.

I was playing with using the "pattern" attribute for the input, so it
validates the slug before submitting the form.


![vlcsnap-2025-01-28-12h30m33s792](https://github.com/user-attachments/assets/1b89a0fc-74e4-4f89-ab37-7498b55708f4)


But I think showing the error with the suggested slug works better,
maybe we can port that to JS instead as a future improvement.

![Screenshot 2025-01-28 at 12-30-10 rynoh-pdf - test-builds - Read the
Docs](https://github.com/user-attachments/assets/a38bb6e8-295c-457c-9a77-702b574d082d)


### Decisions

- Expose this as a feature flag first, or just document it right away?

### TODO

- Allow changing the slug using the API.
- Suggest a redirect from the old slug to the new one.

The idea implemented here comes from this comment:
readthedocs/meta#147 (reply in thread)

ref readthedocs/meta#147

readthedocs/builds/forms.py

@@ -22,19 +22,26 @@
     Version,
     VersionAutomationRule,
 )
+from readthedocs.builds.version_slug import generate_version_slug
+from readthedocs.projects.models import Feature
 
 
 class VersionForm(forms.ModelForm):
+    project = forms.CharField(widget=forms.HiddenInput(), required=False)
+
     class Meta:
         model = Version
         states_fields = ["active", "hidden"]
         privacy_fields = ["privacy_level"]
         fields = (
+            "project",
+            "slug",
             *states_fields,
             *privacy_fields,
         )
 
     def __init__(self, *args, **kwargs):
+        self.project = kwargs.pop("project")
         super().__init__(*args, **kwargs)
 
         field_sets = [
@@ -66,11 +73,20 @@ def __init__(self, *args, **kwargs):
             )
         )
 
+        # Don't allow changing the slug of machine created versions
+        # (stable/latest), as we rely on the slug to identify them.
+        if self.instance and self.instance.machine:
+            self.fields["slug"].disabled = True
+
+        if not self.project.has_feature(Feature.ALLOW_CHANGING_VERSION_SLUG):
+            self.fields.pop("slug")
+
         self.helper = FormHelper()
         self.helper.layout = Layout(*field_sets)
         # We need to know if the version was active before the update.
         # We use this value in the save method.
         self._was_active = self.instance.active if self.instance else False
+        self._previous_slug = self.instance.slug if self.instance else None
 
     def clean_active(self):
         active = self.cleaned_data["active"]
@@ -88,7 +104,41 @@ def _is_default_version(self):
         project = self.instance.project
         return project.default_version == self.instance.slug
 
+    def clean_slug(self):
+        slug = self.cleaned_data["slug"]
+        validated_slug = generate_version_slug(slug)
+        if slug != validated_slug:
+            msg = _(
+                "The slug can contain lowercase letters, numbers, dots, dashes or underscores, "
+                f"and it must start with a lowercase letter or a number. Consider using '{validated_slug}'."
+            )
+            raise forms.ValidationError(msg)
+
+        # NOTE: Django already checks for unique slugs and raises a ValidationError,
+        # but that message is attached to the whole form instead of the the slug field.
+        # So we do the check here to provide a better error message.
+        if (
+            self.project.versions.filter(slug=slug)
+            .exclude(pk=self.instance.pk)
+            .exists()
+        ):
+            raise forms.ValidationError(_("A version with that slug already exists."))
+        return slug
+
+    def clean_project(self):
+        return self.project
+
     def save(self, commit=True):
+        # If the slug was changed, and the version was active,
+        # we need to delete all the resources, since the old slug is used in several places.
+        # NOTE: we call clean_resources with the previous slug,
+        # as all resources are associated with that slug.
+        if "slug" in self.changed_data and self._was_active:
+            self.instance.clean_resources(version_slug=self._previous_slug)
+            # We need to set the flag to False,
+            # so the post_save method triggers a new build.
+            self._was_active = False
+
         obj = super().save(commit=commit)
         obj.post_save(was_active=self._was_active)
         return obj

readthedocs/builds/models.py

@@ -401,7 +401,7 @@ def delete(self, *args, **kwargs):
         clean_project_resources(self.project, self)
         super().delete(*args, **kwargs)
 
-    def clean_resources(self):
+    def clean_resources(self, version_slug=None):
         """
         Remove all resources from this version.
 
@@ -410,6 +410,11 @@ def clean_resources(self):
         - Files from storage
         - Search index
         - Imported files
+
+        :param version_slug: The version slug to use.
+         Version resources are stored using the version's slug,
+         since slugs can change, we need to be able to provide a different slug
+         sometimes to clean old resources.
         """
         from readthedocs.projects.tasks.utils import clean_project_resources
 
@@ -418,9 +423,14 @@ def clean_resources(self):
             project_slug=self.project.slug,
             version_slug=self.slug,
         )
-        clean_project_resources(project=self.project, version=self)
+        clean_project_resources(
+            project=self.project,
+            version=self,
+            version_slug=version_slug,
+        )
         self.built = False
         self.save()
+        self.purge_cdn(version_slug=version_slug)
 
     def save(self, *args, **kwargs):
         if not self.slug:
@@ -447,11 +457,25 @@ def post_save(self, was_active=False):
         # If the version is deactivated, we need to clean up the files.
         if was_active and not self.active:
             self.clean_resources()
+            return
         # If the version is activated, we need to trigger a build.
         if not was_active and self.active:
             trigger_build(project=self.project, version=self)
-        # Purge the cache from the CDN.
-        version_changed.send(sender=self.__class__, version=self)
+        # Purge the cache from the CDN for any other changes.
+        self.purge_cdn()
+
+    def purge_cdn(self, version_slug=None):
+        """
+        Purge the cache from the CDN.
+
+        :param version_slug: The version slug to use.
+         Version resources are stored using the version's slug,
+         since slugs can change, we need to be able to provide a different slug
+         sometimes to clean old resources.
+        """
+        version_changed.send(
+            sender=self.__class__, version=self, version_slug=version_slug
+        )
 
     @property
     def identifier_friendly(self):
@@ -516,19 +540,24 @@ def get_conf_py_path(self):
         conf_py_path = os.path.relpath(conf_py_path, checkout_prefix)
         return conf_py_path
 
-    def get_storage_paths(self):
+    def get_storage_paths(self, version_slug=None):
         """
         Return a list of all build artifact storage paths for this version.
 
+        :param version_slug: The version slug to use.
+         Version resources are stored using the version's slug,
+         since slugs can change, we need to be able to provide a different slug
+         sometimes to clean old resources.
         :rtype: list
         """
         paths = []
 
+        slug = version_slug or self.slug
         for type_ in MEDIA_TYPES:
             paths.append(
                 self.project.get_storage_path(
                     type_=type_,
-                    version_slug=self.slug,
+                    version_slug=slug,
                     include_file=False,
                     version_type=self.type,
                 )

readthedocs/projects/models.py

@@ -1910,6 +1910,7 @@ def add_features(sender, **kwargs):
     USE_PROXIED_APIS_WITH_PREFIX = "use_proxied_apis_with_prefix"
     ALLOW_VERSION_WARNING_BANNER = "allow_version_warning_banner"
     DONT_SYNC_WITH_REMOTE_REPO = "dont_sync_with_remote_repo"
+    ALLOW_CHANGING_VERSION_SLUG = "allow_changing_version_slug"
 
     # Versions sync related features
     SKIP_SYNC_TAGS = "skip_sync_tags"
@@ -1961,6 +1962,10 @@ def add_features(sender, **kwargs):
             DONT_SYNC_WITH_REMOTE_REPO,
             _("Remote repository: Don't keep project in sync with remote repository."),
         ),
+        (
+            ALLOW_CHANGING_VERSION_SLUG,
+            _("Dashboard: Allow changing the version slug."),
+        ),
         # Versions sync related features
         (
             SKIP_SYNC_BRANCHES,

readthedocs/projects/tasks/utils.py

@@ -50,7 +50,7 @@ def remove_build_storage_paths(paths):
         build_media_storage.delete_directory(storage_path)
 
 
-def clean_project_resources(project, version=None):
+def clean_project_resources(project, version=None, version_slug=None):
     """
     Delete all extra resources used by `version` of `project`.
 
@@ -61,16 +61,22 @@ def clean_project_resources(project, version=None):
     - Imported files.
 
     :param version: Version instance. If isn't given,
-                    all resources of `project` will be deleted.
+     all resources of `project` will be deleted.
+    :param version_slug: The version slug to use.
+     Version resources are stored using the version's slug,
+     since slugs can change, we need to be able to provide a different slug
+     sometimes to clean old resources.
 
     .. note::
        This function is usually called just before deleting project.
        Make sure to not depend on the project object inside the tasks.
     """
+    version_slug = version_slug or version.slug if version else None
+
     # Remove storage paths
     storage_paths = []
     if version:
-        storage_paths = version.get_storage_paths()
+        storage_paths = version.get_storage_paths(version_slug=version_slug)
     else:
         storage_paths = project.get_storage_paths()
     remove_build_storage_paths.delay(storage_paths)
@@ -80,7 +86,7 @@ def clean_project_resources(project, version=None):
 
     remove_search_indexes.delay(
         project_slug=project.slug,
-        version_slug=version.slug if version else None,
+        version_slug=version_slug,
     )
 
     # Remove imported files

readthedocs/projects/views/private.py

@@ -275,11 +275,6 @@ def get_queryset(self):
             only_active=False,
         )
 
-    def get_form(self, data=None, files=None, **kwargs):
-        # This overrides the method from `ProjectAdminMixin`,
-        # since we don't have a project.
-        return self.get_form_class()(data, files, **kwargs)
-
     def form_valid(self, form):
         form.save()
         return HttpResponseRedirect(self.get_success_url())