diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index 2a9f99633f..86a400931c 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -1971,6 +1971,9 @@ def copy_to( def copy(self): return self.copy_to() + def is_publishable(self): + return self.complete and self.get_descendants(include_self=True).exclude(kind_id=content_kinds.TOPIC).exists() + class Meta: verbose_name = "Topic" verbose_name_plural = "Topics" diff --git a/contentcuration/contentcuration/tests/test_exportchannel.py b/contentcuration/contentcuration/tests/test_exportchannel.py index 68e5c3c502..36e331c713 100644 --- a/contentcuration/contentcuration/tests/test_exportchannel.py +++ b/contentcuration/contentcuration/tests/test_exportchannel.py @@ -9,6 +9,7 @@ from django.core.management import call_command from django.db import connections from kolibri_content import models as kolibri_models +from kolibri_content.router import cleanup_content_database_connection from kolibri_content.router import get_active_content_database from kolibri_content.router import set_active_content_database from le_utils.constants import exercises @@ -28,6 +29,7 @@ from .testdata import slideshow from .testdata import thumbnail_bytes from contentcuration import models as cc +from contentcuration.utils.publish import ChannelIncompleteError from contentcuration.utils.publish import convert_channel_thumbnail from contentcuration.utils.publish import create_content_database from contentcuration.utils.publish import create_slideshow_manifest @@ -216,8 +218,7 @@ def setUp(self): def tearDown(self): # Clean up datbase connection after the test - connections[self.tempdb].close() - del connections.databases[self.tempdb] + cleanup_content_database_connection(self.tempdb) super(ExportChannelTestCase, self).tearDown() set_active_content_database(None) if os.path.exists(self.tempdb): @@ -429,6 +430,29 @@ def test_publish_no_modify_legacy_exercise_extra_fields(self): }) +class EmptyChannelTestCase(StudioTestCase): + + @classmethod + def setUpClass(cls): + super(EmptyChannelTestCase, cls).setUpClass() + cls.patch_copy_db = patch('contentcuration.utils.publish.save_export_database') + cls.patch_copy_db.start() + + @classmethod + def tearDownClass(cls): + super(EmptyChannelTestCase, cls).tearDownClass() + cls.patch_copy_db.stop() + + def test_publish_empty_channel(self): + content_channel = channel() + set_channel_icon_encoding(content_channel) + content_channel.main_tree.complete = True + content_channel.main_tree.save() + content_channel.main_tree.get_descendants().exclude(kind_id="topic").delete() + with self.assertRaises(ChannelIncompleteError): + create_content_database(content_channel, True, self.admin_user.id, True) + + class ChannelExportUtilityFunctionTestCase(StudioTestCase): @classmethod def setUpClass(cls): diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py index 90d3ef3ccc..5bfddf4b7b 100644 --- a/contentcuration/contentcuration/utils/publish.py +++ b/contentcuration/contentcuration/utils/publish.py @@ -74,6 +74,10 @@ class NoNodesChangedError(Exception): pass +class ChannelIncompleteError(Exception): + pass + + class SlowPublishError(Exception): """ Used to track slow Publishing operations. We don't raise this error, @@ -195,8 +199,8 @@ def __init__( force_exercises=False, progress_tracker=None, ): - if not root_node.complete: - raise ValueError("Attempted to publish a channel with an incomplete root node") + if not root_node.is_publishable(): + raise ChannelIncompleteError("Attempted to publish a channel with an incomplete root node or no resources") self.root_node = root_node task_percent_total = 80.0 @@ -220,7 +224,7 @@ def recurse_nodes(self, node, inherited_fields): # noqa C901 logging.debug("Mapping node with id {id}".format(id=node.pk)) # Only process nodes that are either non-topics or have non-topic descendants - if node.get_descendants(include_self=True).exclude(kind_id=content_kinds.TOPIC).exists() and node.complete: + if node.is_publishable(): # early validation to make sure we don't have any exercises without mastery models # which should be unlikely when the node is complete, but just in case if node.kind_id == content_kinds.EXERCISE: diff --git a/contentcuration/contentcuration/viewsets/channel.py b/contentcuration/contentcuration/viewsets/channel.py index 737269acf1..6715cfd3e4 100644 --- a/contentcuration/contentcuration/viewsets/channel.py +++ b/contentcuration/contentcuration/viewsets/channel.py @@ -40,6 +40,7 @@ from contentcuration.utils.garbage_collect import get_deleted_chefs_root from contentcuration.utils.pagination import CachedListPagination from contentcuration.utils.pagination import ValuesViewsetPageNumberPagination +from contentcuration.utils.publish import ChannelIncompleteError from contentcuration.utils.publish import publish_channel from contentcuration.utils.sync import sync_channel from contentcuration.viewsets.base import BulkListSerializer @@ -503,6 +504,13 @@ def publish(self, pk, version_notes="", language=None): }, channel_id=channel.id ), ], applied=True) + except ChannelIncompleteError: + Change.create_changes([ + generate_update_event( + channel.id, CHANNEL, {"publishing": False}, channel_id=channel.id + ), + ], applied=True) + raise ValidationError("Channel is not ready to be published") except Exception: Change.create_changes([ generate_update_event( diff --git a/contentcuration/kolibri_content/router.py b/contentcuration/kolibri_content/router.py index b7b2e69b70..9a991a9df1 100644 --- a/contentcuration/kolibri_content/router.py +++ b/contentcuration/kolibri_content/router.py @@ -72,6 +72,16 @@ def get_content_database_connection(alias=None): return connections[alias].connection +def cleanup_content_database_connection(alias): + try: + connection = connections[alias] + connection.close() + del connections.databases[alias] + except (ConnectionDoesNotExist, KeyError): + # Already cleaned up, nothing to do here! + pass + + class ContentDBRouter(object): """A router that decides what content database to read from based on a thread-local variable.""" @@ -158,6 +168,7 @@ def __enter__(self): def __exit__(self, exc_type, exc_value, traceback): set_active_content_database(self.previous_alias) + cleanup_content_database_connection(self.alias) def __call__(self, querying_func): # allow using the context manager as a decorator diff --git a/contentcuration/kolibri_public/tests/test_mapper.py b/contentcuration/kolibri_public/tests/test_mapper.py index a10ef159e1..4b56813464 100644 --- a/contentcuration/kolibri_public/tests/test_mapper.py +++ b/contentcuration/kolibri_public/tests/test_mapper.py @@ -2,9 +2,9 @@ import tempfile from django.core.management import call_command -from django.db import connections from django.test import TestCase from kolibri_content import models as kolibri_content_models +from kolibri_content.router import cleanup_content_database_connection from kolibri_content.router import get_active_content_database from kolibri_content.router import using_content_database from kolibri_public import models as kolibri_public_models @@ -116,8 +116,7 @@ def test_map_replace(self): @classmethod def tearDownClass(cls): # Clean up datbase connection after the test - connections[cls.tempdb].close() - del connections.databases[cls.tempdb] + cleanup_content_database_connection(cls.tempdb) super(ChannelMapperTest, cls).tearDownClass() if os.path.exists(cls.tempdb): os.remove(cls.tempdb)