From 2f0f13d63ed74cefdde33460f8564e89a2db2720 Mon Sep 17 00:00:00 2001 From: ozer550 Date: Tue, 21 Mar 2023 22:01:45 +0530 Subject: [PATCH 01/10] add tests for deploy change event --- .../contentcuration/tests/viewsets/base.py | 7 ++ .../tests/viewsets/test_channel.py | 72 +++++++++++++++++++ .../contentcuration/viewsets/channel.py | 48 +++++++++++++ .../contentcuration/viewsets/sync/base.py | 2 + .../viewsets/sync/constants.py | 2 + .../contentcuration/viewsets/sync/utils.py | 6 ++ 6 files changed, 137 insertions(+) diff --git a/contentcuration/contentcuration/tests/viewsets/base.py b/contentcuration/contentcuration/tests/viewsets/base.py index 48ddd1c995..7dd55c9622 100644 --- a/contentcuration/contentcuration/tests/viewsets/base.py +++ b/contentcuration/contentcuration/tests/viewsets/base.py @@ -11,6 +11,7 @@ from contentcuration.viewsets.sync.utils import generate_copy_event as base_generate_copy_event from contentcuration.viewsets.sync.utils import generate_create_event as base_generate_create_event from contentcuration.viewsets.sync.utils import generate_delete_event as base_generate_delete_event +from contentcuration.viewsets.sync.utils import generate_deploy_event as base_generate_deploy_event from contentcuration.viewsets.sync.utils import generate_update_event as base_generate_update_event @@ -48,6 +49,12 @@ def generate_sync_channel_event(channel_id, attributes, tags, files, assessment_ return event +def generate_deploy_channel_event(channel_id, user_id): + event = base_generate_deploy_event(channel_id, user_id=user_id) + event["rev"] = random.randint(1, 10000000) + return event + + class SyncTestMixin(object): celery_task_always_eager = None diff --git a/contentcuration/contentcuration/tests/viewsets/test_channel.py b/contentcuration/contentcuration/tests/viewsets/test_channel.py index b88f54ad98..02e8b6571f 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_channel.py +++ b/contentcuration/contentcuration/tests/viewsets/test_channel.py @@ -4,12 +4,15 @@ import mock from django.urls import reverse +from le_utils.constants import content_kinds from contentcuration import models +from contentcuration import models as cc from contentcuration.tests import testdata from contentcuration.tests.base import StudioAPITestCase from contentcuration.tests.viewsets.base import generate_create_event from contentcuration.tests.viewsets.base import generate_delete_event +from contentcuration.tests.viewsets.base import generate_deploy_channel_event from contentcuration.tests.viewsets.base import generate_sync_channel_event from contentcuration.tests.viewsets.base import generate_update_event from contentcuration.tests.viewsets.base import SyncTestMixin @@ -299,6 +302,75 @@ def test_sync_channel_called_correctly(self, sync_channel_mock): self.assertEqual(sync_channel_mock.call_args.args[i], True) sync_channel_mock.assert_called_once() + def test_deploy_channel_event(self): + channel = testdata.channel() + user = testdata.user() + channel.editors.add(user) + self.client.force_authenticate( + user + ) # This will skip all authentication checks + channel.main_tree.refresh_from_db() + + channel.staging_tree = cc.ContentNode( + kind_id=content_kinds.TOPIC, title="test", node_id="aaa" + ) + channel.staging_tree.save() + channel.previous_tree = cc.ContentNode( + kind_id=content_kinds.TOPIC, title="test", node_id="bbb" + ) + channel.previous_tree.save() + channel.chef_tree = cc.ContentNode( + kind_id=content_kinds.TOPIC, title="test", node_id="ccc" + ) + channel.chef_tree.save() + channel.save() + + self.contentnode = cc.ContentNode.objects.create(kind_id="video") + + response = self.sync_changes( + [ + generate_deploy_channel_event(channel.id, user.id) + ] + ) + + self.assertEqual(response.status_code, 200) + modified_channel = models.Channel.objects.get(id=channel.id) + self.assertEqual(modified_channel.main_tree, channel.staging_tree) + self.assertEqual(modified_channel.staging_tree, None) + self.assertEqual(modified_channel.previous_tree, channel.main_tree) + + def test_deploy_with_staging_tree_None(self): + channel = testdata.channel() + user = testdata.user() + channel.editors.add(user) + self.client.force_authenticate( + user + ) # This will skip all authentication checks + channel.main_tree.refresh_from_db() + + channel.staging_tree = None + channel.previous_tree = cc.ContentNode( + kind_id=content_kinds.TOPIC, title="test", node_id="bbb" + ) + channel.previous_tree.save() + channel.chef_tree = cc.ContentNode( + kind_id=content_kinds.TOPIC, title="test", node_id="ccc" + ) + channel.chef_tree.save() + channel.save() + + self.contentnode = cc.ContentNode.objects.create(kind_id="video") + response = self.sync_changes( + [ + generate_deploy_channel_event(channel.id, user.id) + ] + ) + # Should raise validation error as staging tree was set to NONE + self.assertEqual(len(response.json()["errors"]), 1, response.content) + modified_channel = models.Channel.objects.get(id=channel.id) + self.assertNotEqual(modified_channel.main_tree, channel.staging_tree) + self.assertNotEqual(modified_channel.previous_tree, channel.main_tree) + class CRUDTestCase(StudioAPITestCase): @property diff --git a/contentcuration/contentcuration/viewsets/channel.py b/contentcuration/contentcuration/viewsets/channel.py index 773bc0b740..bd416acafa 100644 --- a/contentcuration/contentcuration/viewsets/channel.py +++ b/contentcuration/contentcuration/viewsets/channel.py @@ -28,6 +28,7 @@ from search.models import ContentNodeFullTextSearch from search.utils import get_fts_search_query +import contentcuration.models as models from contentcuration.decorators import cache_no_user_data from contentcuration.models import Change from contentcuration.models import Channel @@ -36,6 +37,7 @@ from contentcuration.models import generate_storage_url from contentcuration.models import SecretToken from contentcuration.models import User +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 publish_channel @@ -558,6 +560,52 @@ def sync(self, pk, attributes=False, tags=False, files=False, assessment_items=F progress_tracker=progress_tracker, ) + def deploy_from_changes(self, changes): + errors = [] + for deploy in changes: + try: + self.deploy(self.request.user, deploy["key"]) + except Exception as e: + log_sync_exception(e, user=self.request.user, change=deploy) + deploy["errors"] = [str(e)] + errors.append(deploy) + return 1 + + def deploy(self, user, pk): + + channel = self.get_edit_queryset().get(pk=pk) + + if channel.staging_tree is None: + raise ValidationError("Cannot deploy a channel without staging tree") + + user.check_channel_space(channel) + + if channel.previous_tree and channel.previous_tree != channel.main_tree: + # IMPORTANT: Do not remove this block, MPTT updating the deleted chefs block could hang the server + with models.ContentNode.objects.disable_mptt_updates(): + garbage_node = get_deleted_chefs_root() + channel.previous_tree.parent = garbage_node + channel.previous_tree.title = "Previous tree for channel {}".format(channel.pk) + channel.previous_tree.save() + + channel.previous_tree = channel.main_tree + channel.main_tree = channel.staging_tree + channel.staging_tree = None + channel.save() + + user.staged_files.all().delete() + user.set_space_used() + + models.Change.create_change(generate_update_event( + channel.id, + CHANNEL, + { + "root_id": channel.main_tree.id, + "staging_root_id": None + }, + channel_id=channel.id, + ), applied=True, created_by_id=user.id) + @method_decorator( cache_page( diff --git a/contentcuration/contentcuration/viewsets/sync/base.py b/contentcuration/contentcuration/viewsets/sync/base.py index 455a97e4aa..f11a8f4729 100644 --- a/contentcuration/contentcuration/viewsets/sync/base.py +++ b/contentcuration/contentcuration/viewsets/sync/base.py @@ -22,6 +22,7 @@ from contentcuration.viewsets.sync.constants import COPIED from contentcuration.viewsets.sync.constants import CREATED from contentcuration.viewsets.sync.constants import DELETED +from contentcuration.viewsets.sync.constants import DEPLOYED from contentcuration.viewsets.sync.constants import EDITOR_M2M from contentcuration.viewsets.sync.constants import FILE from contentcuration.viewsets.sync.constants import INVITATION @@ -92,6 +93,7 @@ def get_change_type(obj): COPIED: "copy_from_changes", PUBLISHED: "publish_from_changes", SYNCED: "sync_from_changes", + DEPLOYED: "deploy_from_changes", } diff --git a/contentcuration/contentcuration/viewsets/sync/constants.py b/contentcuration/contentcuration/viewsets/sync/constants.py index 6e553b6ccd..84c2b5aad7 100644 --- a/contentcuration/contentcuration/viewsets/sync/constants.py +++ b/contentcuration/contentcuration/viewsets/sync/constants.py @@ -6,6 +6,7 @@ COPIED = 5 PUBLISHED = 6 SYNCED = 7 +DEPLOYED = 8 ALL_CHANGES = set([ @@ -16,6 +17,7 @@ COPIED, PUBLISHED, SYNCED, + DEPLOYED, ]) # Client-side table constants diff --git a/contentcuration/contentcuration/viewsets/sync/utils.py b/contentcuration/contentcuration/viewsets/sync/utils.py index 1d3718b5e2..47b5a17e54 100644 --- a/contentcuration/contentcuration/viewsets/sync/utils.py +++ b/contentcuration/contentcuration/viewsets/sync/utils.py @@ -6,6 +6,7 @@ from contentcuration.viewsets.sync.constants import COPIED from contentcuration.viewsets.sync.constants import CREATED from contentcuration.viewsets.sync.constants import DELETED +from contentcuration.viewsets.sync.constants import DEPLOYED from contentcuration.viewsets.sync.constants import MOVED from contentcuration.viewsets.sync.constants import PUBLISHED from contentcuration.viewsets.sync.constants import UPDATED @@ -74,6 +75,11 @@ def generate_publish_event( return event +def generate_deploy_event(key, user_id): + event = _generate_event(key, CHANNEL, DEPLOYED, channel_id=key, user_id=user_id) + return event + + def log_sync_exception(e, user=None, change=None, changes=None): # Capture exception and report, but allow sync # to complete properly. From 36c10752a7773b5b321fd6b486a5ccfff0f210eb Mon Sep 17 00:00:00 2001 From: Vivek Agrawal Date: Wed, 22 Mar 2023 16:16:52 +0530 Subject: [PATCH 02/10] Put deploy event into indexedDB --- .../channelEdit/pages/StagingTreePage/index.vue | 2 +- .../channelEdit/vuex/currentChannel/actions.js | 10 +--------- .../frontend/shared/data/constants.js | 1 + .../frontend/shared/data/resources.js | 13 +++++++++++++ 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue index 21f5fd82b0..dcf0cb6e50 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue @@ -509,7 +509,7 @@ async onDeployChannelClick() { this.submitDisabled = true; try { - await this.deployCurrentChannel(); + this.deployCurrentChannel(); } catch (e) { this.submitDisabled = false; throw e; diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/currentChannel/actions.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/currentChannel/actions.js index dd5bb6139a..9dca09a0a2 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/currentChannel/actions.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/currentChannel/actions.js @@ -48,15 +48,7 @@ export function reloadCurrentChannelStagingDiff(context) { } export function deployCurrentChannel(context) { - let payload = { - channel_id: context.state.currentChannelId, - }; - return client.post(window.Urls.activate_channel(), payload).catch(e => { - // If response is 'Bad request', channel must already be activated - if (e.response && e.response.status === 400) { - return Promise.resolve(); - } - }); + return Channel.deploy(context.state.currentChannelId); } export function publishChannel(context, version_notes) { diff --git a/contentcuration/contentcuration/frontend/shared/data/constants.js b/contentcuration/contentcuration/frontend/shared/data/constants.js index 8e658c0ecb..bbc35582a3 100644 --- a/contentcuration/contentcuration/frontend/shared/data/constants.js +++ b/contentcuration/contentcuration/frontend/shared/data/constants.js @@ -6,6 +6,7 @@ export const CHANGE_TYPES = { COPIED: 5, PUBLISHED: 6, SYNCED: 7, + DEPLOYED: 8, }; /** * An array of change types that directly result in the creation of nodes diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index ac51f15330..2fb4405443 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -1085,6 +1085,19 @@ export const Channel = new Resource({ }); }, + deploy(id) { + const change = { + key: id, + source: CLIENTID, + table: this.tableName, + type: CHANGE_TYPES.DEPLOYED, + channel_id: id, + }; + return this.transaction({ mode: 'rw', source: IGNORED_SOURCE }, CHANGES_TABLE, () => { + return db[CHANGES_TABLE].put(change); + }); + }, + sync(id, { attributes = false, tags = false, files = false, assessment_items = false } = {}) { const change = { key: id, From d9bcc228a416759462d4ba8bbd91a3c9e58383a3 Mon Sep 17 00:00:00 2001 From: Vivek Agrawal Date: Mon, 3 Apr 2023 12:39:29 +0530 Subject: [PATCH 03/10] Added DEPLOYED to frontend change event --- .../contentcuration/frontend/shared/data/applyRemoteChanges.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/applyRemoteChanges.js b/contentcuration/contentcuration/frontend/shared/data/applyRemoteChanges.js index 3e5b8a79d9..4788f302d6 100644 --- a/contentcuration/contentcuration/frontend/shared/data/applyRemoteChanges.js +++ b/contentcuration/contentcuration/frontend/shared/data/applyRemoteChanges.js @@ -4,7 +4,7 @@ import { CHANGE_TYPES, IGNORED_SOURCE, TABLE_NAMES } from './constants'; import db from './db'; import { INDEXEDDB_RESOURCES } from './registry'; -const { CREATED, DELETED, UPDATED, MOVED, PUBLISHED, SYNCED } = CHANGE_TYPES; +const { CREATED, DELETED, UPDATED, MOVED, PUBLISHED, SYNCED, DEPLOYED } = CHANGE_TYPES; export function applyMods(obj, mods) { for (let keyPath in mods) { @@ -28,6 +28,7 @@ export function collectChanges(changes) { [MOVED]: [], [PUBLISHED]: [], [SYNCED]: [], + [DEPLOYED]: [], }; } collectedChanges[change.table][change.type].push(change); From 3e208e2f6c6410ad22692b0388e208e0fc78729e Mon Sep 17 00:00:00 2001 From: Vivek Agrawal Date: Tue, 4 Apr 2023 02:18:12 +0530 Subject: [PATCH 04/10] Add DEPLOYED to changeMap --- .../contentcuration/frontend/shared/data/serverSync.js | 1 + 1 file changed, 1 insertion(+) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index fa7e9b5aa3..7131e2ee06 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -54,6 +54,7 @@ const ChangeTypeMapFields = { ]), [CHANGE_TYPES.PUBLISHED]: commonFields.concat(['version_notes', 'language']), [CHANGE_TYPES.SYNCED]: commonFields.concat(['attributes', 'tags', 'files', 'assessment_items']), + [CHANGE_TYPES.DEPLOYED]: commonFields, }; function isSyncableChange(change) { From 1195a67886824cf8de28cd3859d52c7b3c65dd22 Mon Sep 17 00:00:00 2001 From: Vivek Agrawal Date: Wed, 5 Apr 2023 16:06:02 +0530 Subject: [PATCH 05/10] Loading spinner while deploying --- .../pages/StagingTreePage/index.vue | 38 ++++++++++--------- .../frontend/shared/data/resources.js | 25 ++++++++++++ 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue index dcf0cb6e50..aed0e4e316 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue @@ -21,7 +21,7 @@ {{ $tr('reviewMode') }} - + @@ -204,8 +204,8 @@ data-test="deploy-dialog" :title="$tr('deployChannel')" :submitText="$tr('confirmDeployBtn')" - :submitDisabled="submitDisabled" - :cancelDisabled="submitDisabled" + :submitDisabled="isDeploying" + :cancelDisabled="isDeploying" :cancelText="$tr('cancelDeployBtn')" @submit="onDeployChannelClick" @cancel="displayDeployDialog = false" @@ -258,6 +258,7 @@ import ToolBar from 'shared/views/ToolBar'; import MainNavigationDrawer from 'shared/views/MainNavigationDrawer'; import OfflineText from 'shared/views/OfflineText'; + import { Channel } from 'shared/data/resources'; export default { name: 'StagingTreePage', @@ -295,7 +296,7 @@ displayDeployDialog: false, drawer: false, elevated: false, - submitDisabled: false, + isDeploying: false, }; }, computed: { @@ -432,7 +433,6 @@ }, methods: { ...mapActions(['showSnackbar', 'addViewModeOverride', 'removeViewModeOverride']), - ...mapActions('channel', ['loadChannel']), ...mapActions('currentChannel', [ 'loadCurrentChannelStagingDiff', 'deployCurrentChannel', @@ -506,20 +506,22 @@ scroll(e) { this.elevated = e.target.scrollTop > 0; }, - async onDeployChannelClick() { - this.submitDisabled = true; - try { - this.deployCurrentChannel(); - } catch (e) { - this.submitDisabled = false; - throw e; - } - await this.loadChannel(this.currentChannel.id); - - this.$router.push(this.rootTreeRoute); + onDeployChannelClick() { + this.displayDeployDialog = false; + this.deployCurrentChannel(); + this.isDeploying = true; - this.showSnackbar({ - text: this.$tr('channelDeployed'), + Channel.waitForDeploying(this.currentChannel.id).then(rootId => { + this.isDeploying = false; + this.$router.push({ + name: RouteNames.TREE_VIEW, + params: { + nodeId: rootId, + }, + }); + this.showSnackbar({ + text: this.$tr('channelDeployed'), + }); }); }, }, diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 2fb4405443..2523b86623 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -1098,6 +1098,31 @@ export const Channel = new Resource({ }); }, + waitForDeploying(id) { + const observable = Dexie.liveQuery(() => { + return this.table + .where('id') + .equals(id) + .filter(channel => channel['staging_root_id'] === null) + .toArray(); + }); + + return new Promise((resolve, reject) => { + const subscription = observable.subscribe({ + next(result) { + if (result.length === 1 && result[0].staging_root_id === null) { + subscription.unsubscribe(); + resolve(result[0].root_id); + } + }, + error() { + subscription.unsubscribe(); + reject(); + }, + }); + }); + }, + sync(id, { attributes = false, tags = false, files = false, assessment_items = false } = {}) { const change = { key: id, From c7d751b0024b0bf50e799528e6af6b3f546de1f7 Mon Sep 17 00:00:00 2001 From: Vivek Agrawal Date: Thu, 6 Apr 2023 18:25:51 +0530 Subject: [PATCH 06/10] Fix staging_id being watched unnecessarily --- .../channelEdit/pages/StagingTreePage/index.vue | 11 +++++++---- .../contentcuration/frontend/shared/data/resources.js | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue index aed0e4e316..a1ccf15c8e 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue @@ -404,9 +404,11 @@ }); }, stagingId() { - this.$router.push({ - name: RouteNames.STAGING_TREE_VIEW_REDIRECT, - }); + if (this.hasStagingTree) { + this.$router.push({ + name: RouteNames.STAGING_TREE_VIEW_REDIRECT, + }); + } }, }, created() { @@ -508,7 +510,6 @@ }, onDeployChannelClick() { this.displayDeployDialog = false; - this.deployCurrentChannel(); this.isDeploying = true; Channel.waitForDeploying(this.currentChannel.id).then(rootId => { @@ -523,6 +524,8 @@ text: this.$tr('channelDeployed'), }); }); + + this.deployCurrentChannel(); }, }, $trs: { diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 2523b86623..e33a6860a7 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -1103,14 +1103,14 @@ export const Channel = new Resource({ return this.table .where('id') .equals(id) - .filter(channel => channel['staging_root_id'] === null) + .filter(channel => !channel['staging_root_id'] || channel['staging_root_id'] === null) .toArray(); }); return new Promise((resolve, reject) => { const subscription = observable.subscribe({ next(result) { - if (result.length === 1 && result[0].staging_root_id === null) { + if (result.length === 1) { subscription.unsubscribe(); resolve(result[0].root_id); } From 11fcf218204ec8fb37996c9456b2e793a8e666b4 Mon Sep 17 00:00:00 2001 From: Vivek Agrawal Date: Thu, 6 Apr 2023 18:38:24 +0530 Subject: [PATCH 07/10] Remove stagingId condition --- .../frontend/channelEdit/pages/StagingTreePage/index.vue | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue index a1ccf15c8e..ad29caeb54 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue @@ -404,11 +404,9 @@ }); }, stagingId() { - if (this.hasStagingTree) { - this.$router.push({ - name: RouteNames.STAGING_TREE_VIEW_REDIRECT, - }); - } + this.$router.push({ + name: RouteNames.STAGING_TREE_VIEW_REDIRECT, + }); }, }, created() { From a2520e3b275d9b39b3034718450a2449e305cd30 Mon Sep 17 00:00:00 2001 From: Vivek Agrawal Date: Wed, 12 Apr 2023 15:49:28 +0530 Subject: [PATCH 08/10] Fix root tree redirect tests --- .../pages/StagingTreePage/index.spec.js | 17 ++++++++++++----- .../channelEdit/pages/StagingTreePage/index.vue | 8 +++++--- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.spec.js b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.spec.js index f0d8d37c1c..a37faea02b 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.spec.js +++ b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.spec.js @@ -2,12 +2,12 @@ import { mount, createLocalVue } from '@vue/test-utils'; import Vuex from 'vuex'; import VueRouter from 'vue-router'; import cloneDeep from 'lodash/cloneDeep'; -import flushPromises from 'flush-promises'; import { RouteNames } from '../../constants'; import StagingTreePage from './index'; import { createStore } from 'shared/vuex/draggablePlugin/test/setup'; import { ContentKindsNames } from 'shared/leUtils/ContentKinds'; +import { Channel } from 'shared/data/resources'; const localVue = createLocalVue(); localVue.use(Vuex); @@ -217,9 +217,14 @@ describe('StagingTreePage', () => { ]; }; - mockDeployCurrentChannel = jest.fn().mockResolvedValue(''); + mockDeployCurrentChannel = jest.fn(); actions.currentChannel.deployCurrentChannel = mockDeployCurrentChannel; - //actions.channel.loadChannel = jest.fn().mockResolvedValue('') + + Channel.waitForDeploying = () => { + return new Promise(resolve => { + return resolve(ROOT_ID); + }); + }; wrapper = initWrapper({ getters, actions }); // make sure router is reset before each test @@ -414,9 +419,11 @@ describe('StagingTreePage', () => { }); it('redirects to a root tree page after deploy channel button click', async () => { - getDeployDialog(wrapper).vm.$emit('submit'); - await flushPromises(); + let waitForDeployingSpy = jest.spyOn(Channel, 'waitForDeploying'); + + await getDeployDialog(wrapper).vm.$emit('submit'); + expect(waitForDeployingSpy).toHaveBeenCalledTimes(1); expect(wrapper.vm.$router.currentRoute.name).toBe(RouteNames.TREE_VIEW); expect(wrapper.vm.$router.currentRoute.params).toEqual({ nodeId: ROOT_ID, diff --git a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue index ad29caeb54..a1ccf15c8e 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue @@ -404,9 +404,11 @@ }); }, stagingId() { - this.$router.push({ - name: RouteNames.STAGING_TREE_VIEW_REDIRECT, - }); + if (this.hasStagingTree) { + this.$router.push({ + name: RouteNames.STAGING_TREE_VIEW_REDIRECT, + }); + } }, }, created() { From 84b02dc76dbb4735deabeffce04ecb74f25c05cc Mon Sep 17 00:00:00 2001 From: ozer550 Date: Wed, 12 Apr 2023 23:38:50 +0530 Subject: [PATCH 09/10] Clean up existing code --- contentcuration/contentcuration/api.py | 33 ----------------- .../tests/utils/test_garbage_collect.py | 30 --------------- .../tests/views/test_views_base.py | 37 ------------------- .../tests/views/test_views_internal.py | 17 --------- contentcuration/contentcuration/urls.py | 2 - contentcuration/contentcuration/views/base.py | 20 ---------- .../contentcuration/views/internal.py | 20 +--------- docs/api_endpoints.md | 30 --------------- 8 files changed, 1 insertion(+), 188 deletions(-) diff --git a/contentcuration/contentcuration/api.py b/contentcuration/contentcuration/api.py index 33c9692cbc..b297ffaba6 100644 --- a/contentcuration/contentcuration/api.py +++ b/contentcuration/contentcuration/api.py @@ -10,9 +10,6 @@ from django.core.files.storage import default_storage import contentcuration.models as models -from contentcuration.utils.garbage_collect import get_deleted_chefs_root -from contentcuration.viewsets.sync.constants import CHANNEL -from contentcuration.viewsets.sync.utils import generate_update_event def write_file_to_storage(fobj, check_valid=False, name=None): @@ -68,33 +65,3 @@ def get_hash(fobj): md5.update(chunk) fobj.seek(0) return md5.hexdigest() - - -def activate_channel(channel, user): - user.check_channel_space(channel) - - if channel.previous_tree and channel.previous_tree != channel.main_tree: - # IMPORTANT: Do not remove this block, MPTT updating the deleted chefs block could hang the server - with models.ContentNode.objects.disable_mptt_updates(): - garbage_node = get_deleted_chefs_root() - channel.previous_tree.parent = garbage_node - channel.previous_tree.title = "Previous tree for channel {}".format(channel.pk) - channel.previous_tree.save() - - channel.previous_tree = channel.main_tree - channel.main_tree = channel.staging_tree - channel.staging_tree = None - channel.save() - - user.staged_files.all().delete() - user.set_space_used() - - models.Change.create_change(generate_update_event( - channel.id, - CHANNEL, - { - "root_id": channel.main_tree.id, - "staging_root_id": None - }, - channel_id=channel.id, - ), applied=True, created_by_id=user.id) diff --git a/contentcuration/contentcuration/tests/utils/test_garbage_collect.py b/contentcuration/contentcuration/tests/utils/test_garbage_collect.py index e4c9941df4..a718a408c2 100644 --- a/contentcuration/contentcuration/tests/utils/test_garbage_collect.py +++ b/contentcuration/contentcuration/tests/utils/test_garbage_collect.py @@ -16,7 +16,6 @@ from le_utils.constants import format_presets from contentcuration import models as cc -from contentcuration.api import activate_channel from contentcuration.constants import user_history from contentcuration.models import ContentNode from contentcuration.models import File @@ -146,35 +145,6 @@ def test_old_staging_tree(self): self.assertFalse(cc.ContentNode.objects.filter(parent=garbage_node).exists()) self.assertFalse(cc.ContentNode.objects.filter(pk=child_pk).exists()) - def test_activate_channel(self): - previous_tree = self.channel.previous_tree - tree(parent=previous_tree) - garbage_node = get_deleted_chefs_root() - - # Previous tree shouldn't be in garbage tree until activate_channel is called - self.assertFalse( - garbage_node.get_descendants().filter(pk=previous_tree.pk).exists() - ) - activate_channel(self.channel, self.user) - garbage_node.refresh_from_db() - previous_tree.refresh_from_db() - self.channel.refresh_from_db() - - # We can't use MPTT methods on the deleted chefs tree because we are not running the sort code - # for performance reasons, so just do a parent test instead. - self.assertTrue(previous_tree.parent == garbage_node) - - # New previous tree should not be in garbage tree - self.assertFalse(self.channel.previous_tree.parent) - self.assertNotEqual(garbage_node.tree_id, self.channel.previous_tree.tree_id) - - child_pk = previous_tree.children.first().pk - - clean_up_deleted_chefs() - - self.assertFalse(cc.ContentNode.objects.filter(parent=garbage_node).exists()) - self.assertFalse(cc.ContentNode.objects.filter(pk=child_pk).exists()) - THREE_MONTHS_AGO = datetime.now() - timedelta(days=93) diff --git a/contentcuration/contentcuration/tests/views/test_views_base.py b/contentcuration/contentcuration/tests/views/test_views_base.py index 201058d289..74d0633971 100644 --- a/contentcuration/contentcuration/tests/views/test_views_base.py +++ b/contentcuration/contentcuration/tests/views/test_views_base.py @@ -5,47 +5,10 @@ from django.urls import reverse_lazy from ..base import BaseAPITestCase -from contentcuration.models import Channel from contentcuration.models import TaskResult from contentcuration.utils.db_tools import TreeBuilder -class APIActivateChannelEndpointTestCase(BaseAPITestCase): - def test_200_post(self): - main_tree = TreeBuilder(user=self.user) - staging_tree = TreeBuilder(user=self.user) - self.channel.main_tree = main_tree.root - self.channel.staging_tree = staging_tree.root - self.channel.save() - response = self.post( - reverse_lazy("activate_channel"), {"channel_id": self.channel.id} - ) - self.assertEqual(response.status_code, 200) - - def test_404_no_permission(self): - new_channel = Channel.objects.create() - staging_tree = TreeBuilder(user=self.user, levels=1) - new_channel.staging_tree = staging_tree.root - new_channel.save() - response = self.post( - reverse_lazy("activate_channel"), {"channel_id": new_channel.id} - ) - self.assertEqual(response.status_code, 404) - - def test_200_no_change_in_space(self): - main_tree = TreeBuilder(user=self.user) - staging_tree = TreeBuilder(user=self.user) - self.channel.main_tree = main_tree.root - self.channel.staging_tree = staging_tree.root - self.channel.save() - self.user.disk_space = self.user.get_space_used(active_files=self.user.get_user_active_files()) - self.user.save() - response = self.post( - reverse_lazy("activate_channel"), {"channel_id": self.channel.id} - ) - self.assertEqual(response.status_code, 200) - - class PublishingStatusEndpointTestCase(BaseAPITestCase): def test_200_get(self): self.user.is_admin = True diff --git a/contentcuration/contentcuration/tests/views/test_views_internal.py b/contentcuration/contentcuration/tests/views/test_views_internal.py index 999cc9a7b6..0a40ab2136 100644 --- a/contentcuration/contentcuration/tests/views/test_views_internal.py +++ b/contentcuration/contentcuration/tests/views/test_views_internal.py @@ -613,23 +613,6 @@ def test_404_no_permission(self): self.assertEqual(response.status_code, 404) -class APIActivateChannelEndpointTestCase(BaseAPITestCase): - def test_200_post(self): - self.channel.staging_tree = self.channel.main_tree - self.channel.save() - response = self.post( - reverse_lazy("activate_channel_internal"), {"channel_id": self.channel.id} - ) - self.assertEqual(response.status_code, 200) - - def test_404_no_permission(self): - new_channel = Channel.objects.create() - response = self.post( - reverse_lazy("activate_channel_internal"), {"channel_id": new_channel.id} - ) - self.assertEqual(response.status_code, 404) - - class CheckUserIsEditorEndpointTestCase(BaseAPITestCase): def test_200_post(self): response = self.post( diff --git a/contentcuration/contentcuration/urls.py b/contentcuration/contentcuration/urls.py index fbf007d5e7..bb03f3876e 100644 --- a/contentcuration/contentcuration/urls.py +++ b/contentcuration/contentcuration/urls.py @@ -71,7 +71,6 @@ def get_redirect_url(self, *args, **kwargs): urlpatterns = [ re_path(r'^api/', include(router.urls)), re_path(r'^serviceWorker.js$', pwa.ServiceWorkerView.as_view(), name="service_worker"), - re_path(r'^api/activate_channel$', views.activate_channel_endpoint, name='activate_channel'), re_path(r'^healthz$', views.health, name='health'), re_path(r'^stealthz$', views.stealth, name='stealth'), re_path(r'^api/search/', include('search.urls'), name='search'), @@ -124,7 +123,6 @@ def get_redirect_url(self, *args, **kwargs): re_path(r'^api/internal/file_diff$', internal_views.file_diff, name="file_diff"), re_path(r'^api/internal/file_upload$', internal_views.api_file_upload, name="api_file_upload"), re_path(r'^api/internal/publish_channel$', internal_views.api_publish_channel, name="api_publish_channel"), - re_path(r'^api/internal/activate_channel_internal$', internal_views.activate_channel_internal, name='activate_channel_internal'), re_path(r'^api/internal/check_user_is_editor$', internal_views.check_user_is_editor, name='check_user_is_editor'), re_path(r'^api/internal/get_tree_data$', internal_views.get_tree_data, name='get_tree_data'), re_path(r'^api/internal/get_node_tree_data$', internal_views.get_node_tree_data, name='get_node_tree_data'), diff --git a/contentcuration/contentcuration/views/base.py b/contentcuration/contentcuration/views/base.py index 158ab54068..1f0eb999d4 100644 --- a/contentcuration/contentcuration/views/base.py +++ b/contentcuration/contentcuration/views/base.py @@ -40,7 +40,6 @@ from .json_dump import json_for_parse_from_data from .json_dump import json_for_parse_from_serializer -from contentcuration.api import activate_channel from contentcuration.constants import channel_history from contentcuration.decorators import browser_is_supported from contentcuration.models import Change @@ -359,25 +358,6 @@ class SQCountDistinct(Subquery): output_field = IntegerField() -@api_view(['POST']) -@authentication_classes((SessionAuthentication,)) -@permission_classes((IsAuthenticated,)) -def activate_channel_endpoint(request): - data = request.data - try: - channel = Channel.filter_edit_queryset(Channel.objects.all(), request.user).get(pk=data["channel_id"]) - except Channel.DoesNotExist: - return HttpResponseNotFound("Channel not found") - if channel.staging_tree is None: - return HttpResponseBadRequest('Channel is not staged') - try: - activate_channel(channel, request.user) - except PermissionDenied as e: - return HttpResponseForbidden(str(e)) - - return HttpResponse(json.dumps({"success": True})) - - @require_POST # flake8: noqa: C901 def set_language(request): diff --git a/contentcuration/contentcuration/views/internal.py b/contentcuration/contentcuration/views/internal.py index ccbc358e5e..acb3578ce0 100644 --- a/contentcuration/contentcuration/views/internal.py +++ b/contentcuration/contentcuration/views/internal.py @@ -2,8 +2,8 @@ import logging from builtins import str from collections import namedtuple -from distutils.version import LooseVersion +from distutils.version import LooseVersion from django.core.exceptions import ObjectDoesNotExist from django.core.exceptions import PermissionDenied from django.core.exceptions import SuspiciousOperation @@ -34,7 +34,6 @@ from sentry_sdk import capture_exception from contentcuration import ricecooker_versions as rc -from contentcuration.api import activate_channel from contentcuration.api import write_file_to_storage from contentcuration.constants import completion_criteria from contentcuration.decorators import delay_user_storage_calculation @@ -340,23 +339,6 @@ def api_publish_channel(request): return HttpResponseServerError(content=str(e), reason=str(e)) -@api_view(['POST']) -@authentication_classes((TokenAuthentication,)) -@permission_classes((IsAuthenticated,)) -def activate_channel_internal(request): - try: - data = json.loads(request.body) - channel_id = data['channel_id'] - channel = Channel.get_editable(request.user, channel_id) - activate_channel(channel, request.user) - return Response({"success": True}) - except Channel.DoesNotExist: - return HttpResponseNotFound("No channel matching: {}".format(channel_id)) - except Exception as e: - handle_server_error(e, request) - return HttpResponseServerError(content=str(e), reason=str(e)) - - @api_view(["POST"]) @authentication_classes((TokenAuthentication, SessionAuthentication,)) @permission_classes((IsAuthenticated,)) diff --git a/docs/api_endpoints.md b/docs/api_endpoints.md index 094a018f5f..9d2de4a9f4 100644 --- a/docs/api_endpoints.md +++ b/docs/api_endpoints.md @@ -199,24 +199,6 @@ Response: } - -### api/internal/activate_channel_internal -_Method: contentcuration.views.internal.activate_channel_internal_ -Deploys a staged channel to the live channel - - POST api/internal/activate_channel_internal - Header: --- - Body: - {"channel_id": "{uuid.hex}"} - -Response: - - { - "success": True - } - - - ### api/internal/get_tree_data _Method: contentcuration.views.internal.get_tree_data_ Returns the complete tree hierarchy information (for tree specified in `tree`). @@ -309,18 +291,6 @@ Response: Channel endpoints -------------------------- -### api/activate_channel -_Method: contentcuration.views.base.activate_channel_endpoint_ -Moves the channel's staging tree to the main tree - - POST api/activate_channel - Body: {"channel_id": "{uuid.hex}"} - Response: - - { - "success": true - } - ### api/get_staged_diff_endpoint _Method: contentcuration.views.base.get_staged_diff_endpoint_ From 84cb5118863f9f5e694b2af07ce464ddb098d0be Mon Sep 17 00:00:00 2001 From: ozer550 Date: Thu, 13 Apr 2023 20:48:29 +0530 Subject: [PATCH 10/10] fix return statement --- contentcuration/contentcuration/viewsets/channel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/viewsets/channel.py b/contentcuration/contentcuration/viewsets/channel.py index bd416acafa..5c3b1fb2c1 100644 --- a/contentcuration/contentcuration/viewsets/channel.py +++ b/contentcuration/contentcuration/viewsets/channel.py @@ -569,7 +569,7 @@ def deploy_from_changes(self, changes): log_sync_exception(e, user=self.request.user, change=deploy) deploy["errors"] = [str(e)] errors.append(deploy) - return 1 + return errors def deploy(self, user, pk):