diff --git a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue index 6c7240e25f..612a870f7a 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue @@ -552,7 +552,7 @@ ...mapActions('currentChannel', [ 'loadCurrentChannelStagingDiff', 'deployCurrentChannel', - 'publishDraftChannel', + 'publishStagingChannel', 'reloadCurrentChannelStagingDiff', ]), ...mapActions('currentChannel', { loadCurrentChannel: 'loadChannel' }), @@ -646,7 +646,7 @@ this.displayPublishDraftDialog = false; this.isPublishingDraft = true; - this.publishDraftChannel() + this.publishStagingChannel() .then(() => { this.isPublishingDraft = false; this.showSnackbar({ diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/currentChannel/actions.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/currentChannel/actions.js index df5f528cd4..33e084e453 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/currentChannel/actions.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/currentChannel/actions.js @@ -56,7 +56,11 @@ export function publishChannel(context, version_notes) { } export function publishDraftChannel(context) { - return Channel.publishDraft(context.state.currentChannelId); + return Channel.publishDraft(context.state.currentChannelId, {use_staging_tree: false}); +} + +export function publishStagingChannel(context) { + return Channel.publishDraft(context.state.currentChannelId, { use_staging_tree: true }); } export function channelLanguageExistsInResources(context) { diff --git a/contentcuration/contentcuration/frontend/shared/data/__tests__/changes.spec.js b/contentcuration/contentcuration/frontend/shared/data/__tests__/changes.spec.js index 51b69efe8b..76a9264276 100644 --- a/contentcuration/contentcuration/frontend/shared/data/__tests__/changes.spec.js +++ b/contentcuration/contentcuration/frontend/shared/data/__tests__/changes.spec.js @@ -257,6 +257,7 @@ describe('Change Types', () => { const change = new PublishedNextChange({ key: '1', table: TABLE_NAMES.CHANNEL, + use_staging_tree: false, source: CLIENTID, }); const rev = await change.saveChange(); @@ -264,7 +265,7 @@ describe('Change Types', () => { expect(persistedChange).toEqual({ rev, channel_id: change.key, - ...pick(change, ['type', 'key', 'table', 'source']), + ...pick(change, ['type', 'key', 'table', 'use_staging_tree', 'source']), }); }); diff --git a/contentcuration/contentcuration/frontend/shared/data/changes.js b/contentcuration/contentcuration/frontend/shared/data/changes.js index 8c7f02d49a..acb9f963e2 100644 --- a/contentcuration/contentcuration/frontend/shared/data/changes.js +++ b/contentcuration/contentcuration/frontend/shared/data/changes.js @@ -439,7 +439,7 @@ export class PublishedChange extends Change { } export class PublishedNextChange extends Change { - constructor(fields) { + constructor({ use_staging_tree, ...fields }) { fields.type = CHANGE_TYPES.PUBLISHED_NEXT; super(fields); if (this.table !== TABLE_NAMES.CHANNEL) { @@ -447,6 +447,7 @@ export class PublishedNextChange extends Change { `${this.changeType} is only supported by ${TABLE_NAMES.CHANNEL} table but ${this.table} was passed instead`, ); } + this.setAndValidateBoolean(use_staging_tree, 'use_staging_tree'); this.setChannelAndUserId({ id: this.key }); } } diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 17e3431cb7..cb7a3dc73b 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -1234,9 +1234,14 @@ export const Channel = new CreateModelResource({ }); }, - publishDraft(id) { + publishDraft(id, opts={}) { + const { + use_staging_tree = false, + } = opts; + const change = new PublishedNextChange({ key: id, + use_staging_tree, table: this.tableName, source: CLIENTID, }); diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index 72c8c60e4c..29efce4c16 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -36,7 +36,7 @@ const ChangeTypeMapFields = { 'excluded_descendants', ]), [CHANGE_TYPES.PUBLISHED]: commonFields.concat(['version_notes', 'language']), - [CHANGE_TYPES.PUBLISHED_NEXT]: commonFields, + [CHANGE_TYPES.PUBLISHED_NEXT]: commonFields.concat(['use_staging_tree']), [CHANGE_TYPES.SYNCED]: commonFields.concat([ 'titles_and_descriptions', 'resource_details', diff --git a/contentcuration/contentcuration/tests/test_exportchannel.py b/contentcuration/contentcuration/tests/test_exportchannel.py index 95d75e7886..f8dd4cf08a 100644 --- a/contentcuration/contentcuration/tests/test_exportchannel.py +++ b/contentcuration/contentcuration/tests/test_exportchannel.py @@ -879,12 +879,11 @@ def run_publish_channel(self): publish_channel( self.admin_user.id, self.content_channel.id, - version_notes="", force=False, force_exercises=False, send_email=False, progress_tracker=None, - language="fr", + is_draft_version=True, use_staging_tree=True, ) @@ -894,11 +893,9 @@ def test_none_staging_tree(self): with self.assertRaises(NoneContentNodeTreeError): self.run_publish_channel() - def test_staging_tree_published(self): - self.assertFalse(self.content_channel.staging_tree.published) + def test_staging_tree_not_published_for_draft(self): self.run_publish_channel() - self.content_channel.refresh_from_db() - self.assertTrue(self.content_channel.staging_tree.published) + self.assertFalse(self.content_channel.staging_tree.published) def test_next_version_exported(self): self.run_publish_channel() @@ -928,6 +925,7 @@ def test_staging_tree_used_for_publish(self): self.admin_user.id, True, progress_tracker=None, + is_draft_version=True, use_staging_tree=True, ) set_active_content_database(self.tempdb) @@ -944,3 +942,106 @@ def test_staging_tree_used_for_publish(self): set_active_content_database(None) if os.path.exists(self.tempdb): os.remove(self.tempdb) + +class PublishDraftUsingMainTreeTestCase(StudioTestCase): + @classmethod + def setUpClass(cls): + super(PublishDraftUsingMainTreeTestCase, cls).setUpClass() + cls.patch_copy_db = patch("contentcuration.utils.publish.save_export_database") + cls.mock_save_export = cls.patch_copy_db.start() + + @classmethod + def tearDownClass(cls): + super(PublishDraftUsingMainTreeTestCase, cls).tearDownClass() + cls.patch_copy_db.stop() + + def setUp(self): + super(PublishDraftUsingMainTreeTestCase, self).setUp() + + self.channel_version = 3 + self.incomplete_video_in_main = "Incomplete video in main tree" + self.complete_video_in_main = "Complete video in main tree" + + self.content_channel = channel() + self.content_channel.version = self.channel_version + self.content_channel.save() + + # Incomplete node in main_tree should be excluded. + new_node = create_node( + {"kind_id": "video", "title": self.incomplete_video_in_main, "children": []} + ) + new_node.complete = False + new_node.parent = self.content_channel.main_tree + new_node.published = False + new_node.save() + + # Complete node in main_tree should be included. + new_node = create_node( + {"kind_id": "video", "title": self.complete_video_in_main, "children": []} + ) + new_node.complete = True + new_node.parent = self.content_channel.main_tree + new_node.published = False + new_node.save() + + def run_publish_channel(self): + publish_channel( + self.admin_user.id, + self.content_channel.id, + force=False, + force_exercises=False, + send_email=False, + progress_tracker=None, + is_draft_version=True, + use_staging_tree=False, + ) + + def test_next_version_exported(self): + self.run_publish_channel() + self.mock_save_export.assert_called_with( + self.content_channel.id, + "next", + True, + ) + + def test_main_tree_not_impacted(self): + self.assertFalse(self.content_channel.main_tree.published) + self.run_publish_channel() + self.content_channel.refresh_from_db() + self.assertFalse(self.content_channel.main_tree.published) + + def test_channel_version_not_incremented(self): + self.assertEqual(self.content_channel.version, self.channel_version) + self.run_publish_channel() + self.content_channel.refresh_from_db() + self.assertEqual(self.content_channel.version, self.channel_version) + + def test_main_tree_used_for_publish(self): + set_channel_icon_encoding(self.content_channel) + self.tempdb = create_content_database( + self.content_channel, + True, + self.admin_user.id, + True, + progress_tracker=None, + is_draft_version=True, + use_staging_tree=False, + ) + set_active_content_database(self.tempdb) + + nodes = kolibri_models.ContentNode.objects.all() + self.assertEqual(nodes.filter(title=self.incomplete_video_in_main).count(), 0) + self.assertEqual(nodes.filter(title=self.complete_video_in_main).count(), 1) + + cleanup_content_database_connection(self.tempdb) + set_active_content_database(None) + if os.path.exists(self.tempdb): + os.remove(self.tempdb) + + def test_only_next_file_created(self): + self.mock_save_export.reset_mock() + self.run_publish_channel() + self.assertEqual(self.mock_save_export.call_count, 1) + call_args = self.mock_save_export.call_args + self.assertEqual(call_args[0][1], "next") + self.assertEqual(call_args[0][2], True) diff --git a/contentcuration/contentcuration/tests/viewsets/base.py b/contentcuration/contentcuration/tests/viewsets/base.py index 617d23bb26..23cea48f53 100644 --- a/contentcuration/contentcuration/tests/viewsets/base.py +++ b/contentcuration/contentcuration/tests/viewsets/base.py @@ -94,8 +94,8 @@ def generate_publish_channel_event(channel_id): return event -def generate_publish_next_event(channel_id): - event = base_generate_publish_next_event(channel_id) +def generate_publish_next_event(channel_id, use_staging_tree=False): + event = base_generate_publish_next_event(channel_id, use_staging_tree=use_staging_tree) event["rev"] = random.randint(1, 10000000) return event diff --git a/contentcuration/contentcuration/tests/viewsets/test_channel.py b/contentcuration/contentcuration/tests/viewsets/test_channel.py index d0e1dd4fca..cf2081446a 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_channel.py +++ b/contentcuration/contentcuration/tests/viewsets/test_channel.py @@ -500,7 +500,7 @@ def test_publish_next(self): self.assertEqual(response.status_code, 200) modified_channel = models.Channel.objects.get(id=channel.id) - self.assertEqual(modified_channel.staging_tree.published, True) + self.assertEqual(modified_channel.staging_tree.published, False) def test_publish_next_with_incomplete_staging_tree(self): channel = testdata.channel() @@ -515,7 +515,7 @@ def test_publish_next_with_incomplete_staging_tree(self): channel.save() self.assertEqual(channel.staging_tree.published, False) - response = self.sync_changes([generate_publish_next_event(channel.id)]) + response = self.sync_changes([generate_publish_next_event(channel.id, use_staging_tree=True)]) self.assertEqual(response.status_code, 200) self.assertTrue( diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py index 841f440134..a04a05d1e5 100644 --- a/contentcuration/contentcuration/utils/publish.py +++ b/contentcuration/contentcuration/utils/publish.py @@ -146,18 +146,22 @@ def create_content_database( user_id, force_exercises, progress_tracker=None, + is_draft_version=False, use_staging_tree=False, ): """ :type progress_tracker: contentcuration.utils.celery.ProgressTracker|None """ + if not is_draft_version and use_staging_tree: + raise ValueError("Staging tree is only supported for draft versions") + # increment the channel version - if not use_staging_tree and not force: + if not is_draft_version and not force: raise_if_nodes_are_all_unchanged(channel) fh, tempdb = tempfile.mkstemp(suffix=".sqlite3") with using_content_database(tempdb): - if not use_staging_tree and not channel.main_tree.publishing: + if not is_draft_version and not channel.main_tree.publishing: channel.mark_publishing(user_id) call_command( @@ -183,11 +187,11 @@ def create_content_database( progress_tracker.track(90) map_prerequisites(base_tree) # Need to save as version being published, not current version - version = "next" if use_staging_tree else channel.version + 1 + version = "next" if is_draft_version else channel.version + 1 save_export_database( channel.pk, version, - use_staging_tree, + is_draft_version, ) if channel.public: mapper = ChannelMapper(kolibri_channel) @@ -1127,14 +1131,14 @@ def mark_all_nodes_as_published(tree): logging.info("Marked all nodes as published.") -def save_export_database(channel_id, version, use_staging_tree=False): +def save_export_database(channel_id, version, is_draft_version=False): logging.debug("Saving export database") current_export_db_location = get_active_content_database() target_paths = [ os.path.join(settings.DB_ROOT, "{}-{}.sqlite3".format(channel_id, version)) ] - # Only create non-version path if not using the staging tree - if not use_staging_tree: + # Only create non-version path if not is_draft_version + if not is_draft_version: target_paths.append( os.path.join(settings.DB_ROOT, "{id}.sqlite3".format(id=channel_id)) ) @@ -1297,6 +1301,7 @@ def publish_channel( # noqa: C901 send_email=False, progress_tracker=None, language=settings.LANGUAGE_CODE, + is_draft_version=False, use_staging_tree=False, ): """ @@ -1317,23 +1322,23 @@ def publish_channel( # noqa: C901 user_id, force_exercises, progress_tracker=progress_tracker, - use_staging_tree=use_staging_tree, + is_draft_version=is_draft_version, + use_staging_tree=use_staging_tree, ) add_tokens_to_channel(channel) - if not use_staging_tree: + if not is_draft_version: increment_channel_version(channel) sync_contentnode_and_channel_tsvectors(channel_id=channel.id) mark_all_nodes_as_published(base_tree) fill_published_fields(channel, version_notes) - - # Attributes not getting set for some reason, so just save it here - base_tree.publishing = False - base_tree.changed = False - base_tree.published = True - base_tree.save() + base_tree.publishing = False + base_tree.changed = False + base_tree.published = True + base_tree.save() + # Delete public channel cache. - if not use_staging_tree and channel.public: + if not is_draft_version and channel.public: delete_public_channel_cache_keys() if send_email: @@ -1355,8 +1360,9 @@ def publish_channel( # noqa: C901 finally: if kolibri_temp_db and os.path.exists(kolibri_temp_db): os.remove(kolibri_temp_db) - base_tree.publishing = False - base_tree.save() + if not is_draft_version: + base_tree.publishing = False + base_tree.save() elapsed = time.time() - start diff --git a/contentcuration/contentcuration/viewsets/channel.py b/contentcuration/contentcuration/viewsets/channel.py index d18f58bb99..be7f26e799 100644 --- a/contentcuration/contentcuration/viewsets/channel.py +++ b/contentcuration/contentcuration/viewsets/channel.py @@ -614,28 +614,27 @@ def publish(self, pk, version_notes="", language=None): raise def publish_next_from_changes(self, changes): + errors = [] for publish in changes: try: - self.publish_next(publish["key"]) + self.publish_next( + publish["key"], + use_staging_tree=publish.get("use_staging_tree", False), + ) except Exception as e: log_sync_exception(e, user=self.request.user, change=publish) publish["errors"] = [str(e)] errors.append(publish) return errors - def publish_next(self, pk): + def publish_next(self, pk, use_staging_tree=False): logging.debug("Entering the publish staging channel endpoint") channel = self.get_edit_queryset().get(pk=pk) if channel.deleted: raise ValidationError("Cannot publish a deleted channel") - elif channel.staging_tree.publishing: - raise ValidationError("Channel staging tree is already publishing") - - channel.staging_tree.publishing = True - channel.staging_tree.save() with create_change_tracker( pk, CHANNEL, channel.id, self.request.user, "export-channel-staging-tree" @@ -645,7 +644,8 @@ def publish_next(self, pk): self.request.user.pk, channel.id, progress_tracker=progress_tracker, - use_staging_tree=True, + is_draft_version=True, + use_staging_tree=use_staging_tree, ) Change.create_changes( [ @@ -661,12 +661,8 @@ def publish_next(self, pk): applied=True, ) except ChannelIncompleteError: - channel.staging_tree.publishing = False - channel.staging_tree.save() raise ValidationError("Channel is not ready to be published") except Exception: - channel.staging_tree.publishing = False - channel.staging_tree.save() raise def sync_from_changes(self, changes): diff --git a/contentcuration/contentcuration/viewsets/sync/utils.py b/contentcuration/contentcuration/viewsets/sync/utils.py index e7df210926..8f5ce98a05 100644 --- a/contentcuration/contentcuration/viewsets/sync/utils.py +++ b/contentcuration/contentcuration/viewsets/sync/utils.py @@ -98,10 +98,9 @@ def generate_update_descendants_event(key, mods, channel_id=None, user_id=None): return event -def generate_publish_next_event(key, version_notes="", language=None): +def generate_publish_next_event(key, use_staging_tree=False): event = _generate_event(key, CHANNEL, PUBLISHED_NEXT, key, None) - event["version_notes"] = version_notes - event["language"] = language + event["use_staging_tree"] = use_staging_tree return event