API V3: clean version when deactivated and build version when activated (#10308)

Closes #10221.

---------

Co-authored-by: Benjamin Balder Bach <[email protected]>

Author: stsewd

docs/user/api/v3.rst

@@ -580,6 +580,11 @@ Version update
 
     Update a version.
 
+    When a version is deactivated, its documentation is removed,
+    and when it's activated, a new build is triggered.
+
+    Updates to a version also invalidates its CDN cache.
+
     **Example request**:
 
     .. tabs::

readthedocs/api/v3/tests/test_versions.py

@@ -1,3 +1,5 @@
+from unittest import mock
+
 import django_dynamic_fixture as fixture
 from django.test import override_settings
 from django.urls import reverse
@@ -139,6 +141,9 @@ def test_projects_versions_detail_unique(self):
         )
         self.assertEqual(response.status_code, 200)
 
+    @mock.patch(
+        "readthedocs.projects.tasks.utils.clean_project_resources", new=mock.MagicMock
+    )
     def test_projects_versions_partial_update(self):
         self.assertTrue(self.version.active)
         self.assertFalse(self.version.hidden)
@@ -166,7 +171,7 @@ def test_projects_versions_partial_update(self):
         self.assertEqual(self.version.identifier, 'a1b2c3')
         self.assertFalse(self.version.active)
         self.assertTrue(self.version.hidden)
-        self.assertTrue(self.version.built)
+        self.assertFalse(self.version.built)
         self.assertEqual(self.version.type, TAG)
 
     def test_projects_versions_partial_update_privacy_levels_disabled(self):
@@ -227,6 +232,54 @@ def test_projects_versions_partial_update_invalid_privacy_levels(self):
         self.assertEqual(response.status_code, 400)
         self.assertEqual(self.version.privacy_level, "public")
 
+    @mock.patch("readthedocs.builds.models.trigger_build")
+    @mock.patch("readthedocs.projects.tasks.utils.clean_project_resources")
+    def test_activate_version(self, clean_project_resources, trigger_build):
+        self.version.active = False
+        self.version.save()
+        self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
+        self.assertFalse(self.version.active)
+        data = {"active": True}
+        response = self.client.patch(
+            reverse(
+                "projects-versions-detail",
+                kwargs={
+                    "parent_lookup_project__slug": self.project.slug,
+                    "version_slug": self.version.slug,
+                },
+            ),
+            data,
+        )
+        self.assertEqual(response.status_code, 204)
+        self.version.refresh_from_db()
+        self.assertTrue(self.version.active)
+        clean_project_resources.assert_not_called()
+        trigger_build.assert_called_once()
+
+    @mock.patch("readthedocs.builds.models.trigger_build")
+    @mock.patch("readthedocs.projects.tasks.utils.clean_project_resources")
+    def test_deactivate_version(self, clean_project_resources, trigger_build):
+        self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
+        data = {"active": False}
+        self.assertTrue(self.version.active)
+        self.assertTrue(self.version.built)
+        response = self.client.patch(
+            reverse(
+                "projects-versions-detail",
+                kwargs={
+                    "parent_lookup_project__slug": self.project.slug,
+                    "version_slug": self.version.slug,
+                },
+            ),
+            data,
+        )
+        self.assertEqual(response.status_code, 204)
+        self.version.refresh_from_db()
+        self.assertFalse(self.version.active)
+        self.assertFalse(self.version.built)
+        clean_project_resources.assert_called_once()
+        trigger_build.assert_not_called()
+
     def test_projects_version_external(self):
         self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
         self.version.type = EXTERNAL

readthedocs/api/v3/views.py

@@ -297,6 +297,17 @@ def get_serializer_class(self):
             return VersionSerializer
         return VersionUpdateSerializer
 
+    def update(self, request, *args, **kwargs):
+        """Overridden to call ``post_save`` method on the updated version."""
+        # Get the current value before updating.
+        version = self.get_object()
+        was_active = version.active
+        result = super().update(request, *args, **kwargs)
+        # Get the updated version.
+        version = self.get_object()
+        version.post_save(was_active=was_active)
+        return result
+
 
 class BuildsViewSet(APIv3Settings, NestedViewSetMixin, ProjectQuerySetMixin,
                     FlexFieldsMixin, ReadOnlyModelViewSet):

readthedocs/builds/forms.py

@@ -22,8 +22,6 @@
     Version,
     VersionAutomationRule,
 )
-from readthedocs.builds.signals import version_changed
-from readthedocs.core.utils import trigger_build
 
 
 class VersionForm(forms.ModelForm):
@@ -67,6 +65,9 @@ def __init__(self, *args, **kwargs):
 
         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
 
     def clean_active(self):
         active = self.cleaned_data['active']
@@ -86,10 +87,7 @@ def _is_default_version(self):
 
     def save(self, commit=True):
         obj = super().save(commit=commit)
-        if obj.active and not obj.built and not obj.uploaded:
-            trigger_build(project=obj.project, version=obj)
-        if self.has_changed():
-            version_changed.send(sender=self.__class__, version=obj)
+        obj.post_save(was_active=self._was_active)
         return obj
 
 

readthedocs/builds/models.py

@@ -1,5 +1,4 @@
 """Models for the builds app."""
-
 import datetime
 import os.path
 import re
@@ -52,6 +51,7 @@
     RelatedBuildQuerySet,
     VersionQuerySet,
 )
+from readthedocs.builds.signals import version_changed
 from readthedocs.builds.utils import (
     external_version_name,
     get_bitbucket_username_repo,
@@ -61,6 +61,7 @@
 )
 from readthedocs.builds.version_slug import VersionSlugField
 from readthedocs.config import LATEST_CONFIGURATION_VERSION
+from readthedocs.core.utils import trigger_build
 from readthedocs.projects.constants import (
     BITBUCKET_COMMIT_URL,
     BITBUCKET_URL,
@@ -381,6 +382,50 @@ def delete(self, *args, **kwargs):  # pylint: disable=arguments-differ
         clean_project_resources(self.project, self)
         super().delete(*args, **kwargs)
 
+    def clean_resources(self):
+        """
+        Remove all resources from this version.
+
+        This includes removing files from storage,
+        and removing its search index.
+        """
+        from readthedocs.projects.tasks.utils import clean_project_resources
+
+        log.info(
+            "Removing files for version.",
+            project_slug=self.project.slug,
+            version_slug=self.slug,
+        )
+        clean_project_resources(project=self.project, version=self)
+        self.built = False
+        self.save()
+
+    def post_save(self, was_active=False):
+        """
+        Run extra steps after updating a version.
+
+        This method isn't called automatically by a signal but is called explicitly
+        from other processes.
+
+        Useful to run after the version has been saved/updated
+        by the user, like from a form or API.
+
+        - When a version is deactivated, we need to clean up its
+          files from storage, and search index.
+        - When a version is activated, we need to trigger a build.
+        - We also need to purge the cache from the CDN,
+          since the version could have been activated/deactivated,
+          or its privacy level could have changed.
+        """
+        # If the version is deactivated, we need to clean up the files.
+        if was_active and not self.active:
+            self.clean_resources()
+        # 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)
+
     @property
     def identifier_friendly(self):
         """Return display friendly identifier."""

readthedocs/projects/views/private.py

@@ -215,19 +215,7 @@ def get_form(self, data=None, files=None, **kwargs):
         return self.get_form_class()(data, files, **kwargs)
 
     def form_valid(self, form):
-        version = form.save()
-        if form.has_changed():
-            if 'active' in form.changed_data and version.active is False:
-                log.info(
-                    'Removing files for version.',
-                    version_slug=version.slug,
-                )
-                clean_project_resources(
-                    version.project,
-                    version,
-                )
-                version.built = False
-                version.save()
+        form.save()
         return HttpResponseRedirect(self.get_success_url())
 
 

readthedocs/rtd_tests/tests/test_build_forms.py

@@ -90,9 +90,11 @@ def test_can_update_privacy_level(self):
         self.assertTrue(form.is_valid())
         self.assertEqual(version.privacy_level, PRIVATE)
 
-    @mock.patch('readthedocs.builds.forms.trigger_build', mock.MagicMock())
-    @mock.patch('readthedocs.projects.views.private.clean_project_resources')
-    def test_resources_are_deleted_when_version_is_inactive(self, clean_project_resources):
+    @mock.patch("readthedocs.builds.models.trigger_build", mock.MagicMock())
+    @mock.patch("readthedocs.projects.tasks.utils.clean_project_resources")
+    def test_resources_are_deleted_when_version_is_inactive(
+        self, clean_project_resources
+    ):
         version = get(
             Version,
             project=self.project,