Delete imported files when deactivating version

readthedocs/api/v3/tests/test_versions.py

@@ -3,10 +3,11 @@
 import django_dynamic_fixture as fixture
 from django.test import override_settings
 from django.urls import reverse
+from django_dynamic_fixture import get
 
 from readthedocs.builds.constants import EXTERNAL, TAG
 from readthedocs.builds.models import Version
-from readthedocs.projects.models import Project
+from readthedocs.projects.models import HTMLFile, Project
 
 from .mixins import APIEndpointMixin
 
@@ -266,12 +267,33 @@ def test_activate_version(self, clean_project_resources, trigger_build):
         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):
+    @mock.patch("readthedocs.projects.tasks.search.remove_search_indexes")
+    @mock.patch("readthedocs.projects.tasks.utils.remove_build_storage_paths")
+    def test_deactivate_version(
+        self, remove_build_storage_paths, remove_search_indexes, trigger_build
+    ):
+        another_version = get(Version, project=self.project, active=True)
+        get(
+            HTMLFile,
+            project=self.project,
+            version=another_version,
+            name="index.html",
+            path="index.html",
+        )
+        get(
+            HTMLFile,
+            project=self.project,
+            version=self.version,
+            name="index.html",
+            path="index.html",
+        )
+
         self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
         data = {"active": False}
         self.assertTrue(self.version.active)
         self.assertTrue(self.version.built)
+        self.assertTrue(another_version.imported_files.exists())
+        self.assertTrue(self.version.imported_files.exists())
         response = self.client.patch(
             reverse(
                 "projects-versions-detail",
@@ -286,7 +308,10 @@ def test_deactivate_version(self, clean_project_resources, trigger_build):
         self.version.refresh_from_db()
         self.assertFalse(self.version.active)
         self.assertFalse(self.version.built)
-        clean_project_resources.assert_called_once()
+        self.assertFalse(self.version.imported_files.exists())
+        self.assertTrue(another_version.imported_files.exists())
+        remove_build_storage_paths.delay.assert_called_once()
+        remove_search_indexes.delay.assert_called_once()
         trigger_build.assert_not_called()
 
     def test_projects_version_external(self):

readthedocs/builds/models.py

@@ -422,8 +422,11 @@ def clean_resources(self):
         """
         Remove all resources from this version.
 
-        This includes removing files from storage,
-        and removing its search index.
+        This includes:
+
+        - Files from storage
+        - Search index
+        - Imported files
         """
         from readthedocs.projects.tasks.utils import clean_project_resources
 

readthedocs/projects/tasks/utils.py

@@ -67,6 +67,7 @@ def clean_project_resources(project, version=None):
 
     - Artifacts from storage.
     - Search indexes from ES.
+    - Imported files.
 
     :param version: Version instance. If isn't given,
                     all resources of `project` will be deleted.
@@ -90,6 +91,12 @@ def clean_project_resources(project, version=None):
         version_slug=version.slug if version else None,
     )
 
+    # Remove imported files
+    if version:
+        version.imported_files.all().delete()
+    else:
+        project.imported_files.all().delete()
+
 
 @app.task()
 def finish_inactive_builds():

readthedocs/rtd_tests/tests/test_build_forms.py

@@ -8,7 +8,7 @@
 from readthedocs.builds.forms import VersionForm
 from readthedocs.builds.models import Version
 from readthedocs.projects.constants import PRIVATE, PUBLIC
-from readthedocs.projects.models import Project
+from readthedocs.projects.models import HTMLFile, Project
 
 
 class TestVersionForm(TestCase):
@@ -90,15 +90,31 @@ def test_can_update_privacy_level(self):
         self.assertEqual(version.privacy_level, PRIVATE)
 
     @mock.patch("readthedocs.builds.models.trigger_build", mock.MagicMock())
-    @mock.patch("readthedocs.projects.tasks.utils.clean_project_resources")
+    @mock.patch("readthedocs.projects.tasks.search.remove_search_indexes")
+    @mock.patch("readthedocs.projects.tasks.utils.remove_build_storage_paths")
     def test_resources_are_deleted_when_version_is_inactive(
-        self, clean_project_resources
+        self, remove_build_storage_paths, remove_search_indexes
     ):
         version = get(
             Version,
             project=self.project,
             active=True,
         )
+        another_version = get(Version, project=self.project, active=True)
+        get(
+            HTMLFile,
+            project=self.project,
+            version=version,
+            name="index.html",
+            path="index.html",
+        )
+        get(
+            HTMLFile,
+            project=self.project,
+            version=another_version,
+            name="index.html",
+            path="index.html",
+        )
 
         url = reverse(
             "project_version_detail", args=(version.project.slug, version.slug)
@@ -114,7 +130,10 @@ def test_resources_are_deleted_when_version_is_inactive(
             },
         )
         self.assertEqual(r.status_code, 302)
-        clean_project_resources.assert_not_called()
+        remove_build_storage_paths.delay.assert_not_called()
+        remove_search_indexes.delay.assert_not_called()
+        self.assertTrue(version.imported_files.exists())
+        self.assertTrue(another_version.imported_files.exists())
 
         r = self.client.post(
             url,
@@ -124,4 +143,7 @@ def test_resources_are_deleted_when_version_is_inactive(
             },
         )
         self.assertEqual(r.status_code, 302)
-        clean_project_resources.assert_called_once()
+        remove_build_storage_paths.delay.assert_called_once()
+        remove_search_indexes.delay.assert_called_once()
+        self.assertFalse(version.imported_files.exists())
+        self.assertTrue(another_version.imported_files.exists())