Skip to content

Allow users to change version slug #11930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 22 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions readthedocs/builds/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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"]
Expand All @@ -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
Expand Down
41 changes: 35 additions & 6 deletions readthedocs/builds/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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

Expand All @@ -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:
Expand All @@ -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):
Expand Down Expand Up @@ -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,
)
Expand Down
5 changes: 5 additions & 0 deletions readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 10 additions & 4 deletions readthedocs/projects/tasks/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

Expand All @@ -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.
Comment on lines 63 to +64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:param version: Version instance. If isn't given,
all resources of project will be deleted.

We need to adapt it to mention "If neither of version and version_slug are given,"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you always need to pass the version object when passing the version_slug, as the version object has more information about the version like its type that is needed to generate the correct storage paths.

: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)
Expand All @@ -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
Expand Down
5 changes: 0 additions & 5 deletions readthedocs/projects/views/private.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
Loading