Merge pull request #5233 from Jakoma02/ensure-channel-version-database-exists

Ensure channel version database exists when adding to community library

Author: AlexVelezLl

contentcuration/contentcuration/models.py

@@ -2601,6 +2601,9 @@ class CommunityLibrarySubmission(models.Model):
     internal_notes = models.TextField(blank=True, null=True)
 
     def save(self, *args, **kwargs):
+        # Not a top-level import to avoid circular import issues
+        from contentcuration.tasks import ensure_versioned_database_exists_task
+
         # Validate on save that the submission author is an editor of the channel
         # and that the version is not greater than the current channel version.
         # These cannot be expressed as constraints because traversing
@@ -2623,6 +2626,16 @@ def save(self, *args, **kwargs):
                 code="impossibly_high_channel_version",
             )
 
+        if self.pk is None:
+            # When creating a new submission, ensure the channel has a versioned database
+            # (it might not have if the channel was published before versioned databases
+            # were introduced).
+            ensure_versioned_database_exists_task.fetch_or_enqueue(
+                user=self.author,
+                channel_id=self.channel.id,
+                channel_version=self.channel.version,
+            )
+
         super().save(*args, **kwargs)
 
     def mark_live(self):

contentcuration/contentcuration/tasks.py

@@ -19,6 +19,7 @@
 from contentcuration.utils.csv_writer import write_user_csv
 from contentcuration.utils.nodes import calculate_resource_size
 from contentcuration.utils.nodes import generate_diff
+from contentcuration.utils.publish import ensure_versioned_database_exists
 from contentcuration.viewsets.user import AdminUserFilter
 
 
@@ -159,3 +160,8 @@ def sendcustomemails_task(subject, message, query):
             text,
             settings.DEFAULT_FROM_EMAIL,
         )
+
+
+@app.task(name="ensure_versioned_database_exists_task")
+def ensure_versioned_database_exists_task(channel_id, channel_version):
+    ensure_versioned_database_exists(channel_id, channel_version)

contentcuration/contentcuration/tests/helpers.py

@@ -7,6 +7,7 @@
 from django_celery_results.models import TaskResult
 from search.models import ContentNodeFullTextSearch
 
+from contentcuration.celery import app
 from contentcuration.models import ContentNode
 
 
@@ -71,3 +72,27 @@ def reverse_with_query(
     if query:
         return f"{url}?{urlencode(query)}"
     return url
+
+
+class EagerTasksTestMixin(object):
+    """
+    Mixin to make Celery tasks run synchronously during the tests.
+    """
+
+    celery_task_always_eager = None
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        # update celery so tasks are always eager for this test, meaning they'll execute synchronously
+        cls.celery_task_always_eager = app.conf.task_always_eager
+        app.conf.update(task_always_eager=True)
+
+    def setUp(self):
+        super().setUp()
+        clear_tasks()
+
+    @classmethod
+    def tearDownClass(cls):
+        super().tearDownClass()
+        app.conf.update(task_always_eager=cls.celery_task_always_eager)

contentcuration/contentcuration/tests/test_models.py

@@ -34,6 +34,7 @@
 from contentcuration.models import UserHistory
 from contentcuration.tests import testdata
 from contentcuration.tests.base import StudioTestCase
+from contentcuration.tests.helpers import EagerTasksTestMixin
 from contentcuration.viewsets.sync.constants import DELETED
 
 
@@ -549,12 +550,18 @@ def test_make_content_id_unique(self):
         self.assertNotEqual(copied_node_old_content_id, copied_node.content_id)
 
 
-class CommunityLibrarySubmissionTestCase(PermissionQuerysetTestCase):
+@mock.patch(
+    "contentcuration.tasks.ensure_versioned_database_exists_task.fetch_or_enqueue",
+    return_value=None,
+)
+class CommunityLibrarySubmissionTestCase(
+    EagerTasksTestMixin, PermissionQuerysetTestCase
+):
     @property
     def base_queryset(self):
         return CommunityLibrarySubmission.objects.all()
 
-    def test_create_submission(self):
+    def test_create_submission(self, mock_ensure_db_exists_task_fetch_or_enqueue):
         # Smoke test
         channel = testdata.channel()
         author = testdata.user()
@@ -577,51 +584,85 @@ def test_create_submission(self):
         submission.full_clean()
         submission.save()
 
-    def test_save__author_not_editor(self):
+    def test_save__author_not_editor(self, mock_ensure_db_exists):
         submission = testdata.community_library_submission()
         user = testdata.user("[email protected]")
         submission.author = user
         with self.assertRaises(ValidationError):
             submission.save()
 
-    def test_save__nonpositive_channel_version(self):
+    def test_save__nonpositive_channel_version(
+        self, mock_ensure_db_exists_task_fetch_or_enqueue
+    ):
         submission = testdata.community_library_submission()
         submission.channel_version = 0
         with self.assertRaises(ValidationError):
             submission.save()
 
-    def test_save__matching_channel_version(self):
+    def test_save__matching_channel_version(
+        self, mock_ensure_db_exists_task_fetch_or_enqueue
+    ):
         submission = testdata.community_library_submission()
         submission.channel.version = 5
         submission.channel.save()
         submission.channel_version = 5
         submission.save()
 
-    def test_save__impossibly_high_channel_version(self):
+    def test_save__impossibly_high_channel_version(
+        self, mock_ensure_db_exists_task_fetch_or_enqueue
+    ):
         submission = testdata.community_library_submission()
         submission.channel.version = 5
         submission.channel.save()
         submission.channel_version = 6
         with self.assertRaises(ValidationError):
             submission.save()
 
-    def test_filter_view_queryset__anonymous(self):
+    def test_save__ensure_versioned_database_exists_on_create(
+        self, mock_ensure_db_exists_task_fetch_or_enqueue
+    ):
+        submission = testdata.community_library_submission()
+
+        mock_ensure_db_exists_task_fetch_or_enqueue.assert_called_once_with(
+            user=submission.author,
+            channel_id=submission.channel.id,
+            channel_version=submission.channel.version,
+        )
+
+    def test_save__dont_ensure_versioned_database_exists_on_update(
+        self, mock_ensure_db_exists_task_fetch_or_enqueue
+    ):
+        submission = testdata.community_library_submission()
+        mock_ensure_db_exists_task_fetch_or_enqueue.reset_mock()
+
+        submission.description = "Updated description"
+        submission.save()
+
+        mock_ensure_db_exists_task_fetch_or_enqueue.assert_not_called()
+
+    def test_filter_view_queryset__anonymous(
+        self, mock_ensure_db_exists_task_fetch_or_enqueue
+    ):
         _ = testdata.community_library_submission()
 
         queryset = CommunityLibrarySubmission.filter_view_queryset(
             self.base_queryset, user=self.anonymous_user
         )
         self.assertFalse(queryset.exists())
 
-    def test_filter_view_queryset__forbidden_user(self):
+    def test_filter_view_queryset__forbidden_user(
+        self, mock_ensure_db_exists_task_fetch_or_enqueue
+    ):
         _ = testdata.community_library_submission()
 
         queryset = CommunityLibrarySubmission.filter_view_queryset(
             self.base_queryset, user=self.forbidden_user
         )
         self.assertFalse(queryset.exists())
 
-    def test_filter_view_queryset__channel_editor(self):
+    def test_filter_view_queryset__channel_editor(
+        self, mock_ensure_db_exists_task_fetch_or_enqueue
+    ):
         submission_a = testdata.community_library_submission()
         submission_b = testdata.community_library_submission()
 
@@ -635,31 +676,39 @@ def test_filter_view_queryset__channel_editor(self):
         self.assertQuerysetContains(queryset, pk=submission_a.id)
         self.assertQuerysetDoesNotContain(queryset, pk=submission_b.id)
 
-    def test_filter_view_queryset__admin(self):
+    def test_filter_view_queryset__admin(
+        self, mock_ensure_db_exists_task_fetch_or_enqueue
+    ):
         submission_a = testdata.community_library_submission()
 
         queryset = CommunityLibrarySubmission.filter_view_queryset(
             self.base_queryset, user=self.admin_user
         )
         self.assertQuerysetContains(queryset, pk=submission_a.id)
 
-    def test_filter_edit_queryset__anonymous(self):
+    def test_filter_edit_queryset__anonymous(
+        self, mock_ensure_db_exists_task_fetch_or_enqueue
+    ):
         _ = testdata.community_library_submission()
 
         queryset = CommunityLibrarySubmission.filter_edit_queryset(
             self.base_queryset, user=self.anonymous_user
         )
         self.assertFalse(queryset.exists())
 
-    def test_filter_edit_queryset__forbidden_user(self):
+    def test_filter_edit_queryset__forbidden_user(
+        self, mock_ensure_db_exists_task_fetch_or_enqueue
+    ):
         _ = testdata.community_library_submission()
 
         queryset = CommunityLibrarySubmission.filter_edit_queryset(
             self.base_queryset, user=self.forbidden_user
         )
         self.assertFalse(queryset.exists())
 
-    def test_filter_edit_queryset__channel_editor(self):
+    def test_filter_edit_queryset__channel_editor(
+        self, mock_ensure_db_exists_task_fetch_or_enqueue
+    ):
         submission = testdata.community_library_submission()
 
         user = testdata.user()
@@ -671,7 +720,9 @@ def test_filter_edit_queryset__channel_editor(self):
         )
         self.assertFalse(queryset.exists())
 
-    def test_filter_edit_queryset__author(self):
+    def test_filter_edit_queryset__author(
+        self, mock_ensure_db_exists_task_fetch_or_enqueue
+    ):
         submission_a = testdata.community_library_submission()
         submission_b = testdata.community_library_submission()
 
@@ -681,15 +732,17 @@ def test_filter_edit_queryset__author(self):
         self.assertQuerysetContains(queryset, pk=submission_a.id)
         self.assertQuerysetDoesNotContain(queryset, pk=submission_b.id)
 
-    def test_filter_edit_queryset__admin(self):
+    def test_filter_edit_queryset__admin(
+        self, mock_ensure_db_exists_task_fetch_or_enqueue
+    ):
         submission_a = testdata.community_library_submission()
 
         queryset = CommunityLibrarySubmission.filter_edit_queryset(
             self.base_queryset, user=self.admin_user
         )
         self.assertQuerysetContains(queryset, pk=submission_a.id)
 
-    def test_mark_live(self):
+    def test_mark_live(self, mock_ensure_db_exists_task_fetch_or_enqueue):
         submission_a = testdata.community_library_submission()
         submission_b = testdata.community_library_submission()
 

contentcuration/contentcuration/tests/utils/restricted_filesystemstorage.py

@@ -0,0 +1,32 @@
+from django.core.files.storage import FileSystemStorage
+
+
+class RestrictedFileSystemStorage:
+    """
+    A wrapper around FileSystemStorage that is restricted to more closely match
+    the behavior of S3Storage which is used in production. In particuler,
+    it does not expose the `path` method, and opening files for writing
+    is not allowed.
+    This cannot be solved by just mocking the `path` method, because
+    it is used by the `FileSystemStorage` class internally.
+    """
+
+    def __init__(self, *args, **kwargs):
+        self._inner = FileSystemStorage(*args, **kwargs)
+
+    def __getattr__(self, name):
+        if name == "path":
+            raise NotImplementedError(
+                "The 'path' method is intentionally not available."
+            )
+        return getattr(self._inner, name)
+
+    def open(self, name, mode="rb"):
+        if "w" in mode:
+            raise ValueError(
+                "Opening files for writing will not be available in production."
+            )
+        return self._inner.open(name, mode)
+
+    def __dir__(self):
+        return [x for x in dir(self._inner) if x != "path"]