From 6622878f70c114a8acbdd1fc9650d5e1e8bee5da Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 3 Sep 2020 16:10:57 -0700 Subject: [PATCH 01/28] Make syncing more frequent. --- .../contentcuration/frontend/shared/data/serverSync.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index 6ca93c2645..c52b7af0ae 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -22,7 +22,7 @@ const SYNC_BUFFER = 1000; // When this many seconds pass without a syncable // change being registered, sync changes! -const SYNC_IF_NO_CHANGES_FOR = 10; +const SYNC_IF_NO_CHANGES_FOR = 2; // In order to listen to messages being sent // by all windows, including this one, for requests From 83fd2ad0888b2a5350969d5d32f37dc3a91a24c0 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 3 Sep 2020 16:12:51 -0700 Subject: [PATCH 02/28] Prompt syncing from changes in the changes table and lock deletion. --- .../frontend/shared/data/serverSync.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index c52b7af0ae..4214d62e24 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -250,17 +250,21 @@ function handleChanges(changes) { const { rev, ...filteredChange } = change; // eslint-disable-line no-unused-vars return filteredChange; }); - const lockChanges = changes.filter(change => change.table === CHANGE_LOCKS_TABLE); + const lockChanges = changes.find( + change => change.table === CHANGE_LOCKS_TABLE && change.type === CHANGE_TYPES.DELETED + ); + const newChangeChanges = changes.find( + change => change.table === CHANGES_TABLE && change.type !== CHANGE_TYPES.DELETED + ); if (syncableChanges.length) { // Flatten any changes before we store them in the changes table - db[CHANGES_TABLE].bulkPut(mergeAllChanges(syncableChanges, true)).then(() => { - debouncedSyncChanges(); - }); + db[CHANGES_TABLE].bulkPut(mergeAllChanges(syncableChanges, true)); } - // If we detect locks were removed, then we'll trigger sync - if (lockChanges.length && lockChanges.find(change => change.type === CHANGE_TYPES.DELETED)) { + // If we detect locks were removed, or changes were written to the changes table + // then we'll trigger sync + if (lockChanges || newChangeChanges) { debouncedSyncChanges(); } } From 36d92a8737f74e5194e4bb551b6262a481328264 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 3 Sep 2020 16:26:04 -0700 Subject: [PATCH 03/28] Update dev server sync forcing. --- .../contentcuration/frontend/shared/data/serverSync.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index 4214d62e24..2630e08e25 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -241,7 +241,10 @@ const debouncedSyncChanges = debounce(() => { }, SYNC_IF_NO_CHANGES_FOR * 1000); if (process.env.NODE_ENV !== 'production') { - window.forceServerSync = debouncedSyncChanges; + window.forceServerSync = function() { + debouncedSyncChanges(); + debouncedSyncChanges.flush(); + }; } function handleChanges(changes) { From d93d0e6e8e2d935326c9d760c2c7e895993b8569 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 3 Sep 2020 16:26:36 -0700 Subject: [PATCH 04/28] Automatically clear locks once they have expired. --- .../contentcuration/frontend/shared/data/changes.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/changes.js b/contentcuration/contentcuration/frontend/shared/data/changes.js index d2f509a533..767a2c1b08 100644 --- a/contentcuration/contentcuration/frontend/shared/data/changes.js +++ b/contentcuration/contentcuration/frontend/shared/data/changes.js @@ -172,7 +172,7 @@ export class ChangeTracker extends EventEmitter { */ stop() { this._isTracking = false; - + this._timeout = setTimeout(this.dismiss.bind(this), this.expiry); // This sets the expiry, from 0 to an actual expiry. The clock is ticking... return this.renew(this.expiry); } @@ -217,6 +217,12 @@ export class ChangeTracker extends EventEmitter { throw new Error('Unable to revert changes without locks'); } + if (this._timeout) { + // Clear the timeout for dismissing the lock, as we will dismiss + // it once we are done reverting. + clearTimeout(this._timeout); + } + this._isReverted = true; if (Dexie.currentTransaction) { From 26dd8d018c3c09c971def2e53b5de705417d2626 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 3 Sep 2020 16:29:04 -0700 Subject: [PATCH 05/28] Add logging messages in dev mode to better trace async calls. --- .../contentcuration/frontend/shared/data/constants.js | 1 + .../contentcuration/frontend/shared/data/resources.js | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/contentcuration/contentcuration/frontend/shared/data/constants.js b/contentcuration/contentcuration/frontend/shared/data/constants.js index 2989b4382b..2b4fc27435 100644 --- a/contentcuration/contentcuration/frontend/shared/data/constants.js +++ b/contentcuration/contentcuration/frontend/shared/data/constants.js @@ -21,6 +21,7 @@ export const TABLE_NAMES = { EDITOR_M2M: 'editor_m2m', VIEWER_M2M: 'viewer_m2m', SAVEDSEARCH: 'savedsearch', + CLIPBOARD: 'clipboard', }; export const MESSAGES = { diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 8d5a568d66..af49122c7f 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -516,6 +516,11 @@ class Resource extends mix(APIResource, IndexedDBResource) { } where(params = {}) { + if (process.env.NODE_ENV !== 'production') { + console.groupCollapsed(`Getting data for ${this.tableName} table with params: `, params); + console.trace(); + console.groupEnd(); + } return super.where(params).then(objs => { if (!objs.length) { return this.requestCollection(params); @@ -562,6 +567,11 @@ class Resource extends mix(APIResource, IndexedDBResource) { } get(id) { + if (process.env.NODE_ENV !== 'production') { + console.groupCollapsed(`Getting instance for ${this.tableName} table with id: ${id}`); + console.trace(); + console.groupEnd(); + } return this.table.get(id).then(obj => { if (obj) { return obj; From df3087a43cf3776920bafa155d515bb1a5b13139 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 3 Sep 2020 16:32:52 -0700 Subject: [PATCH 06/28] Remove tree resource, replace with clipboard resource. --- .../frontend/shared/data/resources.js | 710 ++++++++---------- 1 file changed, 329 insertions(+), 381 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index af49122c7f..8f33551009 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -6,7 +6,6 @@ import isFunction from 'lodash/isFunction'; import matches from 'lodash/matches'; import overEvery from 'lodash/overEvery'; import pick from 'lodash/pick'; -import uniq from 'lodash/uniq'; import uuidv4 from 'uuid/v4'; import channel from './broadcastChannel'; @@ -680,19 +679,6 @@ class Resource extends mix(APIResource, IndexedDBResource) { } } -/** - * @param {string} mode - * @param {Function} callback - * @return {Promise} - */ -function treeTransaction(mode, callback) { - const tables = [TABLE_NAMES.CONTENTNODE, TABLE_NAMES.TREE, CHANGES_TABLE]; - return db.transaction(mode, ...tables, () => { - Dexie.currentTransaction.source = CLIENTID; - return callback(); - }); -} - export const Channel = new Resource({ tableName: TABLE_NAMES.CHANNEL, urlName: 'channel', @@ -744,8 +730,17 @@ export const Channel = new Resource({ export const ContentNode = new Resource({ tableName: TABLE_NAMES.CONTENTNODE, urlName: 'contentnode', - indexFields: ['title', 'language'], - transaction: treeTransaction, + indexFields: [ + 'title', + 'language', + 'parent', + 'channel_id', + 'node_id', + 'root_id', + 'lft', + '[root_id+parent]', + '[node_id+channel_id]', + ], /** * @param {string} id The ID of the node to treeCopy * @param {string} target The ID of the target node used for positioning @@ -758,274 +753,119 @@ export const ContentNode = new Resource({ throw new TypeError(`${position} is not a valid position`); } - // Override the change type, since we'll be issuing the COPY change, so all we - // need is the update to position, aka MOVED - const changeType = CHANGE_TYPES.MOVED; - return Tree.copy(id, target, position, deep, changeType).then(treeNodes => { - return promiseChunk(treeNodes, 50, treeNodes => { - return Tree.table - .where('parent') - .anyOf(uniq(treeNodes.map(n => n.parent))) - .toArray() - .then(siblings => { - // Count all nodes with the same source that are siblings so we - // can determine how many copies of the source element there are - const countMap = siblings.reduce((countMap, sibling) => { - if (!(sibling.parent in countMap)) { - countMap[sibling.parent] = {}; - } + const changeType = CHANGE_TYPES.COPIED; + if (!validPositions.has(position)) { + throw new TypeError(`${position} is not a valid position`); + } - if (!(sibling.source_id in countMap[sibling.parent])) { - countMap[sibling.parent][sibling.source_id] = 0; - } + return this.transaction('rw', CHANGES_TABLE, () => { + // Ignore changes from this operation except for the + // explicit copy change we generate. + Dexie.currentTransaction.source = IGNORED_SOURCE; + return this.tableCopy(id, target, position, changeType); + }).then(data => { + if (!deep) { + return [data]; + } - countMap[sibling.parent][sibling.source_id]++; - return countMap; - }, {}); - - const sourceNodeMap = treeNodes.reduce((sourceNodeMap, treeNode) => { - sourceNodeMap[treeNode.source_id] = treeNode; - return sourceNodeMap; - }, {}); - const sourceNodeIds = treeNodes.map(node => node.source_id); - - return promiseChunk(Object.keys(sourceNodeMap), 10, chunk => { - // All of these should be loaded already, but we'll chunk anyway - return Promise.all(chunk.map(id => Tree.get(id))).then(sourceTreeNodes => { - return this.bulkCopy({ id__in: sourceNodeIds }, sourceNode => { - const treeNode = sourceNodeMap[sourceNode.id]; - const sourceTreeNode = sourceTreeNodes.find( - sourceTreeNode => sourceTreeNode.id === treeNode.source_id - ); - let title = sourceNode.title; - - // When we've made a copy as a sibling of source, update the title - if (treeNode.parent === sourceTreeNode.parent) { - title = - countMap[treeNode.parent][treeNode.source_id] <= 2 - ? constantStrings.$tr('firstCopy', { title }) - : constantStrings.$tr('nthCopy', { - title, - n: countMap[treeNode.parent][treeNode.source_id], - }); - } - - return { - id: treeNode.id, - channel_id: treeNode.channel_id, - source_channel_id: sourceTreeNode.channel_id, - title, - - // Should be removing these soon - files: [], - assessment_items: [], - prerequisite: [], - }; - }); - }); - }); + return this.table + .where({ parent: id }) + .sortBy('lft') + .then(children => { + // Chunk children, and call `copy` again for each, merging all results together + return promiseChunk(children, 50, children => { + return Promise.all( + children.map(child => { + // Recurse for all children of the node we just copied + return this.copy(child.id, data.id, RELATIVE_TREE_POSITIONS.LAST_CHILD, deep); + }) + ); }); - }).then(nodes => { - return { - treeNodes, - nodes, - }; - }); + }) + .then(results => { + return results.reduce((all, subset) => all.concat(subset), []); + }) + .then(results => { + // Be sure to add our first node's result to the beginning of our results + results.unshift(data); + return results; + }); }); }, -}); - -export const ChannelSet = new Resource({ - tableName: TABLE_NAMES.CHANNELSET, - urlName: 'channelset', -}); - -export const Invitation = new Resource({ - tableName: TABLE_NAMES.INVITATION, - urlName: 'invitation', -}); -export const SavedSearch = new Resource({ - tableName: TABLE_NAMES.SAVEDSEARCH, - urlName: 'savedsearch', -}); + tableCopy(id, target, position, changeType) { + if (!validPositions.has(position)) { + return Promise.reject(); + } -export const User = new Resource({ - tableName: TABLE_NAMES.USER, - urlName: 'user', - uuid: false, -}); + return this.getNewParentAndSiblings(target, position).then(({ parent, siblings }) => { + let lft = 1; -export const EditorM2M = new IndexedDBResource({ - tableName: TABLE_NAMES.EDITOR_M2M, - indexFields: ['channel'], - idField: '[user+channel]', - uuid: false, - put(channel, user) { - return this.transaction('rw', CHANGES_TABLE, () => { - return this.table.put({ user, channel }).then(() => { - return db[CHANGES_TABLE].put({ - obj: { - user, - channel, - }, - source: CLIENTID, - table: this.tableName, - type: CHANGE_TYPES.CREATED_RELATION, - }); - }); - }); - }, -}); + if (siblings.length) { + lft = this.getNewSortOrder(id, target, position, siblings); + } else { + // if there are no siblings, overwrite + target = parent; + position = RELATIVE_TREE_POSITIONS.LAST_CHILD; + } -export const ViewerM2M = new IndexedDBResource({ - tableName: TABLE_NAMES.VIEWER_M2M, - indexFields: ['channel'], - idField: '[user+channel]', - uuid: false, - delete(channel, user) { - return this.transaction('rw', CHANGES_TABLE, () => { - return this.table.delete([user, channel]).then(() => { - return db[CHANGES_TABLE].put({ - obj: { - user, - channel, + // Get source node and parent so we can reference some specifics + const nodePromise = this.table.get(id); + const parentNodePromise = this.table.get(parent); + + // Next, we'll add the new node immediately + return Promise.all([nodePromise, parentNodePromise]).then(([node, parentNode]) => { + let title = node.title; + + // When we've made a copy as a sibling of source, update the title + if (node.parent === parentNode.id) { + // Count all nodes with the same source that are siblings so we + // can determine how many copies of the source element there are + const totalSiblingCopies = siblings.filter(s => s.source_node_id === node.source_node_id) + .length; + title = + totalSiblingCopies <= 2 + ? constantStrings.$tr('firstCopy', { title }) + : constantStrings.$tr('nthCopy', { + title, + n: totalSiblingCopies, + }); + } + const data = { + ...node, + id: uuid4(), + original_source_node_id: node.original_source_node_id || node.node_id, + lft, + title, + source_channel_id: node.channel_id, + source_node_id: node.node_id, + root_id: parentNode.root_id, + parent: parentNode.id, + level: parentNode.level + 1, + }; + // Manually put our changes into the tree changes for syncing table + db[CHANGES_TABLE].put({ + key: data.id, + from_key: id, + mods: { + target, + position, + source_channel_id: node.channel_id, + title, }, source: CLIENTID, + oldObj: null, table: this.tableName, - type: CHANGE_TYPES.DELETED_RELATION, - }); - }); - }); - }, -}); - -export const ChannelUser = new APIResource({ - urlName: 'channeluser', - makeEditor(channel, user) { - return ViewerM2M.delete(channel, user).then(() => { - return EditorM2M.put(channel, user); - }); - }, - removeViewer(channel, user) { - return ViewerM2M.delete(channel, user); - }, - fetchCollection(params) { - return client.get(this.collectionUrl(), { params }).then(response => { - const now = Date.now(); - const itemData = response.data; - const userData = []; - const editorM2M = []; - const viewerM2M = []; - for (let datum of itemData) { - const userDatum = { - ...datum, - }; - userDatum[LAST_FETCHED] = now; - delete userDatum.can_edit; - delete userDatum.can_view; - userData.push(userDatum); - const m2mDatum = { - [LAST_FETCHED]: now, - user: datum.id, - channel: params.channel, - }; - if (datum.can_edit) { - editorM2M.push(m2mDatum); - } else if (datum.can_view) { - viewerM2M.push(m2mDatum); - } - } - - return db.transaction('rw', User.tableName, EditorM2M.tableName, ViewerM2M.tableName, () => { - // Explicitly set the source of this as a fetch - // from the server, to prevent us from trying - // to sync these changes back to the server! - Dexie.currentTransaction.source = IGNORED_SOURCE; - return Promise.all([ - EditorM2M.table.bulkPut(editorM2M), - ViewerM2M.table.bulkPut(viewerM2M), - User.table.bulkPut(userData), - ]).then(() => { - return itemData; + type: changeType, }); + return this.table.put(data).then(() => ({ + // Return the id along with the data for further processing + source_id: id, + ...data, + })); }); }); }, - where({ channel }) { - if (!channel) { - throw TypeError('Not a valid channelId'); - } - const params = { - channel, - }; - const editorCollection = EditorM2M.table.where(params); - const viewerCollection = ViewerM2M.table.where(params); - return Promise.all([editorCollection.toArray(), viewerCollection.toArray()]).then( - ([editors, viewers]) => { - if (!editors.length && !viewers.length) { - return this.requestCollection(params); - } - if (objectsAreStale(editors) || objectsAreStale(viewers)) { - // Do a synchronous refresh instead of background refresh here. - return this.requestCollection(params); - } - const editorSet = new Set(editors.map(editor => editor.user)); - const viewerSet = new Set(viewers.map(viewer => viewer.user)); - // Directly query indexeddb here, to avoid triggering - // an additional request if the user data is stale but the M2M table data is not. - return User.table - .where('id') - .anyOf(...editorSet.values(), ...viewerSet.values()) - .toArray(users => { - return users.map(user => { - const can_edit = editorSet.has(user.id); - const can_view = viewerSet.has(user.id); - return { - ...user, - can_edit, - can_view, - }; - }); - }); - } - ); - }, -}); - -export const AssessmentItem = new Resource({ - tableName: TABLE_NAMES.ASSESSMENTITEM, - urlName: 'assessmentitem', - idField: 'assessment_id', - indexFields: ['contentnode'], -}); - -export const File = new Resource({ - tableName: TABLE_NAMES.FILE, - urlName: 'file', - indexFields: ['contentnode'], -}); - -export const Tree = new Resource({ - tableName: TABLE_NAMES.TREE, - urlName: 'tree', - indexFields: [ - 'channel_id', - 'parent', - 'source_id', - 'tree_id', - 'lft', - '[tree_id+parent]', - '[channel_id+parent]', - ], - // Any changes made to the tree table should only be propagated to the - // backend via moves, so that we can make local tree changes in the frontend - // and have them replicated in the backend on the global tree state. - syncable: false, - // ids have to exactly correlate with content node ids, so don't auto - // set uuids on this table. - uuid: false, - transaction: treeTransaction, /** * @param {string} target @@ -1131,7 +971,10 @@ export const Tree = new Resource({ if (!validPositions.has(position)) { throw new TypeError(`${position} is not a valid position`); } - return this.transaction('rw', () => { + return this.transaction('rw', CHANGES_TABLE, () => { + // Ignore changes from this operation except for the + // explicit move change we generate. + Dexie.currentTransaction.source = IGNORED_SOURCE; return this.tableMove(id, target, position); }); }, @@ -1172,9 +1015,7 @@ export const Tree = new Resource({ id, parent, lft, - tree_id: parentNode.tree_id, - channel_id: parentNode.channel_id, - source_id: null, + root_id: parentNode.root_id, }; return this.table.put(data).then(() => data); }); @@ -1195,131 +1036,238 @@ export const Tree = new Resource({ }); }, - /** - * @param {string} id - The ID of the node to treeCopy - * @param {string} target - The ID of the target node used for positioning - * @param {string} [position] - The position relative to `target` - * @param {boolean} [deep] - Whether or not to treeCopy all descendants - * @param {number} [changeType] - Override the change type - * @return {Promise} - */ - copy( - id, - target, - position = RELATIVE_TREE_POSITIONS.LAST_CHILD, - deep = false, - changeType = CHANGE_TYPES.COPIED - ) { - if (!validPositions.has(position)) { - throw new TypeError(`${position} is not a valid position`); - } - - return this.transaction('rw', () => { - return this.tableCopy(id, target, position, changeType); - }) - .then(data => { - if (!deep) { - return [data]; - } + getAncestors(id) { + return this.table.get(id).then(node => { + if (node) { + if (node.parent) { + return this.getAncestors(node.parent).then(nodes => { + nodes.push(node); - return this.table - .where({ parent: id }) - .sortBy('lft') - .then(children => { - // Chunk children, and call `copy` again for each, merging all results together - return promiseChunk(children, 50, children => { - return Promise.all( - children.map(child => { - // Recurse for all children of the node we just copied - return this.copy( - child.id, - data.id, - RELATIVE_TREE_POSITIONS.LAST_CHILD, - deep, - changeType - ); - }) - ); - }); - }) - .then(results => { - return results.reduce((all, subset) => all.concat(subset), []); - }) - .then(results => { - // Be sure to add our first node's result to the beginning of our results - results.unshift(data); - return results; + return nodes; }); - }) - .then(results => { - if (changeType === CHANGE_TYPES.COPIED) { - return results; } + return [node]; + } + return this.requestCollection({ ancestors_of: id }); + }); + }, +}); + +export const ChannelSet = new Resource({ + tableName: TABLE_NAMES.CHANNELSET, + urlName: 'channelset', +}); + +export const Invitation = new Resource({ + tableName: TABLE_NAMES.INVITATION, + urlName: 'invitation', +}); + +export const SavedSearch = new Resource({ + tableName: TABLE_NAMES.SAVEDSEARCH, + urlName: 'savedsearch', +}); - return this.transaction('rw', () => { - // Change source to ignored so we avoid tracking the changes. This is because - // the creation of tree node here is creating a bare content node, so we'll put the - // ID in IndexedDB so later updates sync correctly - Dexie.currentTransaction.source = IGNORED_SOURCE; - return ContentNode.table.bulkPut(results.map(result => ({ id: result.id }))); - }).then(() => results); +export const User = new Resource({ + tableName: TABLE_NAMES.USER, + urlName: 'user', + uuid: false, +}); + +export const EditorM2M = new IndexedDBResource({ + tableName: TABLE_NAMES.EDITOR_M2M, + indexFields: ['channel'], + idField: '[user+channel]', + uuid: false, + put(channel, user) { + return this.transaction('rw', CHANGES_TABLE, () => { + return this.table.put({ user, channel }).then(() => { + return db[CHANGES_TABLE].put({ + obj: { + user, + channel, + }, + source: CLIENTID, + table: this.tableName, + type: CHANGE_TYPES.CREATED_RELATION, + }); }); + }); }, +}); - tableCopy(id, target, position, changeType) { - if (!validPositions.has(position)) { - return Promise.reject(); +export const ViewerM2M = new IndexedDBResource({ + tableName: TABLE_NAMES.VIEWER_M2M, + indexFields: ['channel'], + idField: '[user+channel]', + uuid: false, + delete(channel, user) { + return this.transaction('rw', CHANGES_TABLE, () => { + return this.table.delete([user, channel]).then(() => { + return db[CHANGES_TABLE].put({ + obj: { + user, + channel, + }, + source: CLIENTID, + table: this.tableName, + type: CHANGE_TYPES.DELETED_RELATION, + }); + }); + }); + }, +}); + +export const ChannelUser = new APIResource({ + urlName: 'channeluser', + makeEditor(channel, user) { + return ViewerM2M.delete(channel, user).then(() => { + return EditorM2M.put(channel, user); + }); + }, + removeViewer(channel, user) { + return ViewerM2M.delete(channel, user); + }, + fetchCollection(params) { + return client.get(this.collectionUrl(), { params }).then(response => { + const now = Date.now(); + const itemData = response.data; + const userData = []; + const editorM2M = []; + const viewerM2M = []; + for (let datum of itemData) { + const userDatum = { + ...datum, + }; + userDatum[LAST_FETCHED] = now; + delete userDatum.can_edit; + delete userDatum.can_view; + userData.push(userDatum); + const m2mDatum = { + [LAST_FETCHED]: now, + user: datum.id, + channel: params.channel, + }; + if (datum.can_edit) { + editorM2M.push(m2mDatum); + } else if (datum.can_view) { + viewerM2M.push(m2mDatum); + } + } + + return db.transaction('rw', User.tableName, EditorM2M.tableName, ViewerM2M.tableName, () => { + // Explicitly set the source of this as a fetch + // from the server, to prevent us from trying + // to sync these changes back to the server! + Dexie.currentTransaction.source = IGNORED_SOURCE; + return Promise.all([ + EditorM2M.table.bulkPut(editorM2M), + ViewerM2M.table.bulkPut(viewerM2M), + User.table.bulkPut(userData), + ]).then(() => { + return itemData; + }); + }); + }); + }, + where({ channel }) { + if (!channel) { + throw TypeError('Not a valid channelId'); } + const params = { + channel, + }; + const editorCollection = EditorM2M.table.where(params); + const viewerCollection = ViewerM2M.table.where(params); + return Promise.all([editorCollection.toArray(), viewerCollection.toArray()]).then( + ([editors, viewers]) => { + if (!editors.length && !viewers.length) { + return this.requestCollection(params); + } + if (objectsAreStale(editors) || objectsAreStale(viewers)) { + // Do a synchronous refresh instead of background refresh here. + return this.requestCollection(params); + } + const editorSet = new Set(editors.map(editor => editor.user)); + const viewerSet = new Set(viewers.map(viewer => viewer.user)); + // Directly query indexeddb here, to avoid triggering + // an additional request if the user data is stale but the M2M table data is not. + return User.table + .where('id') + .anyOf(...editorSet.values(), ...viewerSet.values()) + .toArray(users => { + return users.map(user => { + const can_edit = editorSet.has(user.id); + const can_view = viewerSet.has(user.id); + return { + ...user, + can_edit, + can_view, + }; + }); + }); + } + ); + }, +}); + +export const AssessmentItem = new Resource({ + tableName: TABLE_NAMES.ASSESSMENTITEM, + urlName: 'assessmentitem', + idField: 'assessment_id', + indexFields: ['contentnode'], +}); - return this.getNewParentAndSiblings(target, position) - .then(({ parent, siblings }) => { +export const File = new Resource({ + tableName: TABLE_NAMES.FILE, + urlName: 'file', + indexFields: ['contentnode'], +}); + +export const Clipboard = new Resource({ + tableName: TABLE_NAMES.CLIPBOARD, + urlName: 'clipboard', + indexFields: ['parent'], + copy(node_id, channel_id, clipboardRootId, parent = null) { + return this.transaction('rw', TABLE_NAMES.CONTENTNODE, () => { + return this.tableCopy(node_id, channel_id, clipboardRootId, parent); + }); + }, + + tableCopy(node_id, channel_id, clipboardRootId, parent = null) { + parent = parent || clipboardRootId; + return this.table + .where({ parent }) + .sortBy('lft') + .then(siblings => { let lft = 1; if (siblings.length) { - lft = this.getNewSortOrder(id, target, position, siblings); - } else { - // if there are no siblings, overwrite - target = parent; - position = RELATIVE_TREE_POSITIONS.LAST_CHILD; + lft = siblings.slice(-1)[0].lft + 1; } - const data = { - id: uuid4(), - source_id: id, - lft, - }; - - // Get source node and parent so we can reference some specifics - const node = this.table.get(id); - const parentNode = this.table.get(parent); + // Get source node so we can reference some specifics + const nodePromise = ContentNode.table.get({ + '[node_id+channel_id]': [node_id, channel_id], + }); - // Next, we'll add the new tree node immediately - return Promise.all([node, parentNode]).then(([node, parentNode]) => ({ - ...data, - tree_id: parentNode.tree_id, - channel_id: node.channel_id, - parent: parentNode.id, - level: parentNode.level + 1, - })); - }) - .then(data => this.table.put(data).then(() => data)) - .then(data => { - // Manually put our changes into the tree changes for syncing table - const { id: key, lft, channel_id } = data; - return db[CHANGES_TABLE].put({ - key, - from_key: id, - mods: { - target, - position, + // Next, we'll add the new node immediately + return nodePromise.then(node => { + const data = { + id: uuid4(), lft, - channel_id, - }, - source: CLIENTID, - oldObj: null, - table: this.tableName, - type: changeType, - }).then(() => data); + source_channel_id: channel_id, + source_node_id: node_id, + root_id: clipboardRootId, + kind: node.kind, + parent, + }; + return this.table.put(data).then(() => ({ + // Return the id along with the data for further processing + source_id: node.id, + ...data, + })); + }); }); }, }); From 8b2cdd729854d36594ea57b1a0952ddff7f9b9b7 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 3 Sep 2020 16:33:05 -0700 Subject: [PATCH 07/28] Do caching based on specific requests, rather than indexedDB state. Massively reduce cache timeouts. --- .../contentcuration/frontend/shared/client.js | 26 +++++++++++++++--- .../frontend/shared/data/resources.js | 27 ++++++++++++++----- 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/client.js b/contentcuration/contentcuration/frontend/shared/client.js index 61cac422f1..05885bb813 100644 --- a/contentcuration/contentcuration/frontend/shared/client.js +++ b/contentcuration/contentcuration/frontend/shared/client.js @@ -1,13 +1,31 @@ import axios from 'axios'; import qs from 'qs'; +export function paramsSerializer(params) { + // Do custom querystring stingifying to comma separate array params + return qs.stringify(params, { + arrayFormat: 'comma', + encoder: function(str, defaultEncoder, charset, type) { + if (type === 'key') { + // Handle params for queries to joint indexes + // of the form [index1+index2] + // Turn into parameter: + // _index1_index2_ + // This is mostly an implementation detail caused by the + // fact that filters are defined in Python with bare variables names + // and the []+ characters would violate Python variable naming rules + return defaultEncoder(str.replace(/[[\]+]/g, '_')); + } else if (type === 'value') { + return defaultEncoder(str); + } + }, + }); +} + const client = axios.create({ xsrfCookieName: 'csrftoken', xsrfHeaderName: 'X-CSRFToken', - paramsSerializer: function(params) { - // Do custom querystring stingifying to comma separate array params - return qs.stringify(params, { arrayFormat: 'comma' }); - }, + paramsSerializer, }); client.interceptors.response.use( diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 8f33551009..7de3738bb9 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -23,12 +23,12 @@ import mergeAllChanges from './mergeChanges'; import db, { CLIENTID, Collection } from './db'; import { API_RESOURCES, INDEXEDDB_RESOURCES } from './registry'; import { NEW_OBJECT } from 'shared/constants'; -import client from 'shared/client'; +import client, { paramsSerializer } from 'shared/client'; import { constantStrings } from 'shared/mixins'; import { promiseChunk } from 'shared/utils'; // Number of seconds after which data is considered stale. -const REFRESH_INTERVAL = 60; +const REFRESH_INTERVAL = 5; const LAST_FETCHED = '__last_fetch'; @@ -436,10 +436,22 @@ class Resource extends mix(APIResource, IndexedDBResource) { API_RESOURCES[urlName] = this; // Overwrite the false default for IndexedDBResource this.syncable = syncable; + // A map of stringified request params to a last fetched time and a promise + this._requests = {}; } fetchCollection(params) { - return client.get(this.collectionUrl(), { params }).then(response => { + const now = Date.now(); + const queryString = paramsSerializer(params); + if ( + this._requests[queryString] && + this._requests[queryString][LAST_FETCHED] && + this._requests[queryString][LAST_FETCHED] + REFRESH_INTERVAL * 1000 > now && + this._requests[queryString].promise + ) { + return this._requests[queryString].promise; + } + const promise = client.get(this.collectionUrl(), { params }).then(response => { const now = Date.now(); let itemData; let pageData; @@ -512,6 +524,11 @@ class Resource extends mix(APIResource, IndexedDBResource) { }); }); }); + this._requests[queryString] = { + [LAST_FETCHED]: now, + promise, + }; + return promise; } where(params = {}) { @@ -524,9 +541,7 @@ class Resource extends mix(APIResource, IndexedDBResource) { if (!objs.length) { return this.requestCollection(params); } - if (objectsAreStale(objs)) { - this.requestCollection(params); - } + this.requestCollection(params); return objs; }); } From 45c6e9e3dcea6cd53d72e2348ba7c8eb3c446024 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 3 Sep 2020 16:55:09 -0700 Subject: [PATCH 08/28] Remove tree representation from vuex. --- .../contentNode/__tests__/actions.spec.js | 29 ++--- .../contentNode/__tests__/getters.spec.js | 65 +++------- .../channelEdit/vuex/contentNode/actions.js | 115 ++++-------------- .../channelEdit/vuex/contentNode/getters.js | 66 ++++------ .../channelEdit/vuex/contentNode/index.js | 6 - .../channelEdit/vuex/contentNode/mutations.js | 24 ---- 6 files changed, 72 insertions(+), 233 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/__tests__/actions.spec.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/__tests__/actions.spec.js index f4608df1ff..fca506447f 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/__tests__/actions.spec.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/__tests__/actions.spec.js @@ -1,7 +1,7 @@ import contentNode from '../index'; import currentChannel from '../../currentChannel/index'; import file from 'shared/vuex/file'; -import { ContentNode, Tree } from 'shared/data/resources'; +import { ContentNode } from 'shared/data/resources'; import storeFactory from 'shared/vuex/baseStore'; jest.mock('../../currentChannel/index'); @@ -13,27 +13,20 @@ const parentId = '000000000000000000000000000000000000'; describe('contentNode actions', () => { let store; let id; - const contentNodeDatum = { title: 'test', parent: parentId }; + const contentNodeDatum = { title: 'test', parent: parentId, lft: 1 }; beforeEach(() => { return ContentNode.put(contentNodeDatum).then(newId => { id = newId; contentNodeDatum.id = newId; - return ContentNode.put({ title: 'notatest', parent: newId }).then(childId => { - return Tree.table - .bulkPut([ - { id: childId, parent: newId, lft: 2 }, - { id: newId, parent: null, lft: 1 }, - ]) - .then(() => { - store = storeFactory({ - modules: { - contentNode, - currentChannel, - file, - }, - }); - store.state.session.currentUser.id = userId; - }); + return ContentNode.put({ title: 'notatest', parent: newId, lft: 2 }).then(() => { + store = storeFactory({ + modules: { + contentNode, + currentChannel, + file, + }, + }); + store.state.session.currentUser.id = userId; }); }); }); diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/__tests__/getters.spec.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/__tests__/getters.spec.js index 8a532b3cf5..5e88c4be44 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/__tests__/getters.spec.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/__tests__/getters.spec.js @@ -14,35 +14,21 @@ describe('contentNode getters', () => { beforeEach(() => { state = { // English -> Elementary -> Literacy -> Reading - treeNodesMap: { - 'id-elementary': { - id: 'id-elementary', - parent: 'id-english', - }, - 'id-literacy': { - id: 'id-literacy', - parent: 'id-elementary', - }, - 'id-reading': { - id: 'id-reading', - parent: 'id-reading', - }, - 'id-english': { - id: 'id-english', - }, - }, contentNodesMap: { 'id-elementary': { id: 'id-elementary', title: 'Elementary', + parent: 'id-english', }, 'id-literacy': { id: 'id-literacy', title: 'Literacy', + parent: 'id-elementary', }, 'id-reading': { id: 'id-reading', title: 'Reading', + parent: 'id-literacy', }, 'id-english': { id: 'id-english', @@ -68,11 +54,13 @@ describe('contentNode getters', () => { id: 'id-elementary', thumbnail_encoding: {}, title: 'Elementary', + parent: 'id-english', }, { id: 'id-literacy', thumbnail_encoding: {}, title: 'Literacy', + parent: 'id-elementary', }, ]); }); @@ -89,6 +77,7 @@ describe('contentNode getters', () => { id: 'id-elementary', thumbnail_encoding: {}, title: 'Elementary', + parent: 'id-english', }, ]); }); @@ -99,80 +88,54 @@ describe('contentNode getters', () => { beforeEach(() => { state = { - treeNodesMap: { - 'id-science': { - id: 'id-science', - parent: 'id-channel', - }, - 'id-literacy': { - id: 'id-literacy', - parent: 'id-channel', - }, - 'id-alphabet': { - id: 'id-alphabet', - parent: 'id-literacy', - }, - 'id-reading': { - id: 'id-reading', - parent: 'id-literacy', - }, - 'id-counting': { - id: 'id-counting', - parent: 'id-science', - }, - 'id-integrals': { - id: 'id-integrals', - parent: 'id-science', - }, - 'id-geography': { - id: 'id-geography', - parent: 'id-science', - }, - 'id-philosophy': { - id: 'id-philosophy', - parent: 'id-arts', - }, - }, contentNodesMap: { 'id-science': { id: 'id-science', title: 'Science', kind: 'topic', + parent: 'id-channel', }, 'id-literacy': { id: 'id-literacy', title: 'Literacy', kind: 'topic', + parent: 'id-channel', }, 'id-alphabet': { id: 'id-alphabet', title: 'Alphabet', kind: 'document', + parent: 'id-literacy', }, 'id-reading': { id: 'id-reading', title: 'Reading', kind: 'document', + parent: 'id-literacy', }, 'id-counting': { id: 'id-counting', title: 'Counting', kind: 'video', + parent: 'id-science', }, 'id-integrals': { id: 'id-integrals', title: 'Integrals', kind: 'document', + parent: 'id-science', }, 'id-geography': { id: 'id-geography', title: 'Geography', kind: 'html5', + parent: 'id-science', }, 'id-philosophy': { id: 'id-philosophy', title: 'Philosophy', kind: 'document', + parent: 'id-arts', }, }, nextStepsMap: [ diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js index 98f48985d0..bddb229b35 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js @@ -3,7 +3,7 @@ import union from 'lodash/union'; import { NOVALUE } from 'shared/constants'; import client from 'shared/client'; import { RELATIVE_TREE_POSITIONS } from 'shared/data/constants'; -import { ContentNode, Tree } from 'shared/data/resources'; +import { ContentNode } from 'shared/data/resources'; import { promiseChunk } from 'shared/utils'; export function loadContentNodes(context, params = {}) { @@ -24,75 +24,14 @@ export function loadContentNode(context, id) { }); } -export function loadTree(context, params) { - return Tree.where(params).then(nodes => { - context.commit('ADD_TREENODES', nodes); - return nodes; - }); -} - -export function loadTreeNode(context, id) { - return Tree.get(id).then(node => { - context.commit('ADD_TREENODE', node); - return node; - }); -} - -export function loadChannelTree(context, channel_id) { - return context.dispatch('loadTree', { channel_id }); -} - -export function loadTrashTree(context, tree_id) { - return context.dispatch('loadTree', { tree_id }); -} - -export function loadClipboardTree(context) { - const tree_id = context.rootGetters['clipboardRootId']; - return client.get(window.Urls.get_clipboard_channels()).then(response => { - if (response.data && response.data.length) { - return promiseChunk(response.data, 1, ids => - context.dispatch('loadTree', { tree_id, channel_id: ids[0] }) - ); - } else { - // If response comes back as [] then we still want to load the tree, - // but just don't have to worry about channels - return context.dispatch('loadTree', { tree_id }).then(nodes => Promise.resolve(nodes)); - } - }); -} - -export function loadChildren(context, { parent, tree_id }) { - return Tree.where({ parent, tree_id }).then(nodes => { - if (!nodes || !nodes.length) { - return Promise.resolve([]); - } - context.commit('ADD_TREENODES', nodes); - return loadContentNodes(context, { id__in: nodes.map(node => node.id) }); - }); +export function loadChildren(context, { parent }) { + return loadContentNodes(context, { parent }); } -export function loadAncestors(context, { id, includeSelf = false }) { - return loadTreeNodeAncestors(context, { id, includeSelf }).then(nodes => { - if (!nodes || !nodes.length) { - return Promise.resolve(); - } - return loadContentNodes(context, { id__in: nodes.map(node => node.id) }); - }); -} - -export function loadTreeNodeAncestors(context, { id, includeSelf = false }) { - return loadTreeNode(context, id).then(node => { - if (node.parent) { - return loadTreeNodeAncestors(context, { id: node.parent, includeSelf: true }).then(nodes => { - if (includeSelf) { - nodes.push(node); - } - - return nodes; - }); - } - - return includeSelf ? [node] : []; +export function loadAncestors(context, { id }) { + return ContentNode.getAncestors(id).then(contentNodes => { + context.commit('ADD_CONTENTNODES', contentNodes); + return contentNodes; }); } @@ -246,6 +185,7 @@ export function createContentNode(context, { parent, kind = 'topic', ...payload extra_fields: {}, isNew: true, language: session.preferences ? session.preferences.language : session.currentLanguage, + parent, ...context.rootGetters['currentChannel/currentChannel'].content_defaults, ...payload, }; @@ -255,16 +195,11 @@ export function createContentNode(context, { parent, kind = 'topic', ...payload contentNodeData.files.forEach(file => { context.dispatch('file/updateFile', { id: file, contentnode: id }, { root: true }); }); - - return Tree.move(id, parent, RELATIVE_TREE_POSITIONS.LAST_CHILD).then(treeNode => { - context.commit('ADD_CONTENTNODE', { - id, - ...contentNodeData, - }); - - context.commit('ADD_TREENODE', treeNode); - return id; + context.commit('ADD_CONTENTNODE', { + id, + ...contentNodeData, }); + return id; }); } @@ -378,10 +313,7 @@ export function removeTags(context, { ids, tags }) { export function deleteContentNode(context, contentNodeId) { return ContentNode.delete(contentNodeId).then(() => { - return Tree.delete(contentNodeId).then(() => { - context.commit('REMOVE_CONTENTNODE', { id: contentNodeId }); - context.commit('REMOVE_TREENODE', { id: contentNodeId }); - }); + context.commit('REMOVE_CONTENTNODE', { id: contentNodeId }); }); } @@ -399,29 +331,26 @@ export function copyContentNode( ) { // First, this will parse the tree and create the copy the local tree nodes, // with a `source_id` of the source node then create the content node copies - return ContentNode.copy(id, target, position, deep).then(results => { - const { treeNodes, nodes } = results; + return ContentNode.copy(id, target, position, deep).then(nodes => { context.commit('ADD_CONTENTNODES', nodes); - context.commit('ADD_TREENODES', treeNodes); return promiseChunk(nodes, 10, chunk => { // create a map of the source ID to the our new ID // this will help orchestrate file and assessessmentItem copying - const treeNodeMap = chunk.reduce((treeNodeMap, node) => { - const treeNode = context.getters.getTreeNode(node.id); - if (treeNode.source_id in treeNodeMap) { + const nodeMap = chunk.reduce((nodeMap, node) => { + if (node.source_id in nodeMap) { // I don't think this should happen throw new Error('Not implemented'); } - treeNodeMap[treeNode.source_id] = treeNode; - return treeNodeMap; + nodeMap[node.source_id] = node; + return nodeMap; }, {}); - const contentnode__in = Object.keys(treeNodeMap); + const contentnode__in = Object.keys(nodeMap); const updater = fileOrAssessment => { return { - contentnode: treeNodeMap[fileOrAssessment.contentnode].id, + contentnode: nodeMap[fileOrAssessment.contentnode].id, }; }; @@ -463,8 +392,8 @@ export function copyContentNodes(context, { id__in, target, deep = false }) { export function moveContentNodes(context, { id__in, parent: target }) { return Promise.all( id__in.map(id => { - return Tree.move(id, target, RELATIVE_TREE_POSITIONS.LAST_CHILD).then(treeNode => { - context.commit('UPDATE_TREENODE', treeNode); + return ContentNode.move(id, target, RELATIVE_TREE_POSITIONS.LAST_CHILD).then(node => { + context.commit('UPDATE_CONTENTNODE', node); return id; }); }) diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/getters.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/getters.js index 2419533484..0e3c5541d4 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/getters.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/getters.js @@ -24,37 +24,10 @@ export function getContentNode(state) { }; } -export function getTreeNode(state) { - return function(treeNodeId) { - return state.treeNodesMap[treeNodeId]; - }; -} - -export function getTreeNodeAncestors(state) { - return function(id, includeSelf = false) { - let node = state.treeNodesMap[id]; - - if (!node || !node.parent) { - return [node].filter(Boolean); - } - - const self = includeSelf ? [node] : []; - return getTreeNodeAncestors(state)(node.parent, true).concat(self); - }; -} - -export function getTreeNodeChildren(state) { - return function(treeNodeId) { - return sorted( - Object.values(state.treeNodesMap).filter(contentNode => contentNode.parent === treeNodeId) - ); - }; -} - -export function getTreeNodeDescendants(state, getters) { - return function(treeNodeId) { +export function getContentNodeDescendants(state, getters) { + return function(contentNodeId) { // First find the immediate children of the target tree node - return getters.getTreeNodeChildren(treeNodeId).reduce((descendants, treeNode) => { + return getters.getContentNodeChildren(contentNodeId).reduce((descendants, contentNode) => { // Then recursively call ourselves again for each child, so for this structure: // (target) // > (child-1) @@ -62,7 +35,7 @@ export function getTreeNodeDescendants(state, getters) { // > (child-2) // // it should map out to: [(child-1), (grandchild-1), (child-2)] - descendants.push(treeNode, ...getters.getTreeNodeDescendants(treeNode.id)); + descendants.push(contentNode, ...getters.getContentNodeDescendants(contentNode.id)); return descendants; }, []); }; @@ -70,13 +43,13 @@ export function getTreeNodeDescendants(state, getters) { export function hasChildren(state) { return function(id) { - return !!Object.values(state.treeNodesMap).find(contentNode => contentNode.parent === id); + return getContentNode(state)(id).total_count > 0; }; } -export function countTreeNodeDescendants(state, getters) { - return function(treeNodeId) { - return getters.getTreeNodeDescendants(treeNodeId).length; +export function countContentNodeDescendants(state, getters) { + return function(contentNodeId) { + return getters.getContentNodeDescendants(contentNodeId).length; }; } @@ -104,16 +77,27 @@ export function getTopicAndResourceCounts(state) { }; } -export function getContentNodeChildren(state, getters) { +export function getContentNodeChildren(state) { return function(contentNodeId) { - return getters.getContentNodes(getters.getTreeNodeChildren(contentNodeId).map(node => node.id)); + return sorted( + Object.values(state.contentNodesMap) + .filter(contentNode => contentNode.parent === contentNodeId) + .map(node => getContentNode(state)(node.id)) + .filter(Boolean) + ); }; } export function getContentNodeAncestors(state) { - return function(contentNodeId, includeSelf = false) { - const nodes = getTreeNodeAncestors(state)(contentNodeId, includeSelf); - return nodes.length ? getContentNodes(state)(nodes.map(n => n.id)) : []; + return function(id, includeSelf = false) { + let node = getContentNode(state)(id); + + if (!node || !node.parent) { + return [node].filter(Boolean); + } + + const self = includeSelf ? [node] : []; + return getContentNodeAncestors(state)(node.parent, true).concat(self); }; } @@ -173,7 +157,7 @@ function getStepDetail(state, contentNodeId) { stepDetail.title = node.title; stepDetail.kind = node.kind; - const parentNodeId = state.treeNodesMap[contentNodeId].parent; + const parentNodeId = state.contentNodesMap[contentNodeId].parent; if (parentNodeId) { const parentNode = getContentNode(state)(parentNodeId); diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/index.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/index.js index 215eea7ada..0260c85959 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/index.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/index.js @@ -13,7 +13,6 @@ export default { // } return { contentNodesMap: {}, - treeNodesMap: {}, expandedNodes, /* @@ -48,10 +47,5 @@ export default { [CHANGE_TYPES.UPDATED]: 'UPDATE_CONTENTNODE', [CHANGE_TYPES.DELETED]: 'REMOVE_CONTENTNODE', }, - [TABLE_NAMES.TREE]: { - [CHANGE_TYPES.CREATED]: 'ADD_TREENODE', - [CHANGE_TYPES.UPDATED]: 'UPDATE_TREENODE', - [CHANGE_TYPES.DELETED]: 'REMOVE_TREENODE', - }, }, }; diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/mutations.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/mutations.js index 46f4375725..fa7439e971 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/mutations.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/mutations.js @@ -73,30 +73,6 @@ export function SET_MOVE_NODES(state, ids) { state.moveNodes = ids; } -export function ADD_TREENODE(state, treeNode) { - state.treeNodesMap = mergeMapItem(state.treeNodesMap, treeNode); -} - -export function ADD_TREENODES(state, treeNodes = []) { - state.treeNodesMap = treeNodes.reduce((treeNodesMap, treeNode) => { - return mergeMapItem(treeNodesMap, treeNode); - }, state.treeNodesMap); -} - -export function REMOVE_TREENODE(state, treeNode) { - Vue.delete(state.treeNodesMap, treeNode.id); -} - -export function UPDATE_TREENODE(state, { id, ...payload } = {}) { - if (!id) { - throw ReferenceError('id must be defined to update a tree node'); - } - state.treeNodesMap[id] = { - ...state.treeNodesMap[id], - ...payload, - }; -} - /** * Saves the complete chain of previous/next steps (pre/post-requisites) * of a node to next steps map. From 5bd8638ba7b1b3d3472be62cb7efd7c93f2d025e Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 3 Sep 2020 16:55:28 -0700 Subject: [PATCH 09/28] Don't display additional traceback info in travis tests. --- .../contentcuration/frontend/shared/data/resources.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 7de3738bb9..da42558ea0 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -532,7 +532,7 @@ class Resource extends mix(APIResource, IndexedDBResource) { } where(params = {}) { - if (process.env.NODE_ENV !== 'production') { + if (process.env.NODE_ENV !== 'production' && !process.env.TRAVIS) { console.groupCollapsed(`Getting data for ${this.tableName} table with params: `, params); console.trace(); console.groupEnd(); @@ -581,7 +581,7 @@ class Resource extends mix(APIResource, IndexedDBResource) { } get(id) { - if (process.env.NODE_ENV !== 'production') { + if (process.env.NODE_ENV !== 'production' && !process.env.TRAVIS) { console.groupCollapsed(`Getting instance for ${this.tableName} table with id: ${id}`); console.trace(); console.groupEnd(); From 4146cb0af9a6640d3867f79e4ee572d955c4743d Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 3 Sep 2020 17:02:57 -0700 Subject: [PATCH 10/28] Remove all references to tree state in components. --- .../components/ContentNodeOptions.vue | 7 ++--- .../components/NodeTreeNavigation.vue | 6 ----- .../components/StudioTree/StudioTree.spec.js | 6 ++--- .../components/StudioTree/StudioTree.vue | 23 +++++++--------- .../channelEdit/components/move/MoveModal.vue | 7 +++-- .../pages/StagingTreePage/index.spec.js | 12 +++------ .../pages/StagingTreePage/index.vue | 21 +++++---------- .../frontend/channelEdit/router.js | 17 +++--------- .../channelEdit/views/CurrentTopicView.vue | 18 +++++-------- .../ImportFromChannels/ContentTreeList.vue | 22 ++++++--------- .../frontend/channelEdit/views/NodePanel.vue | 4 +-- .../channelEdit/views/TreeView/index.vue | 27 ++++++++++++++----- .../channelEdit/views/trash/TrashModal.vue | 10 +++---- 13 files changed, 72 insertions(+), 108 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeOptions.vue b/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeOptions.vue index e7f01c8450..e5e92b3a7e 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeOptions.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeOptions.vue @@ -48,7 +48,7 @@ }, computed: { ...mapGetters('currentChannel', ['canEdit', 'trashId']), - ...mapGetters('contentNode', ['getContentNode', 'getTreeNode']), + ...mapGetters('contentNode', ['getContentNode']), node() { return this.getContentNode(this.nodeId); }, @@ -73,9 +73,6 @@ }, }; }, - treeNode() { - return this.getTreeNode(this.nodeId); - }, }, methods: { ...mapActions(['showSnackbar']), @@ -130,7 +127,7 @@ actionText: this.$tr('cancel'), actionCallback: () => changeTracker.revert(), }); - const target = this.treeNode.parent; + const target = this.node.parent; return this.copyContentNode({ id: this.nodeId, target, deep: true }).then(() => { return this.showSnackbar({ text: this.$tr('copiedSnackbar'), diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/NodeTreeNavigation.vue b/contentcuration/contentcuration/frontend/channelEdit/components/NodeTreeNavigation.vue index b7686c46ee..27499f4527 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/NodeTreeNavigation.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/NodeTreeNavigation.vue @@ -45,10 +45,6 @@ event: 'updateSelectedNodeId', }, props: { - treeId: { - type: String, - required: true, - }, selectedNodeId: { type: String, required: true, @@ -107,7 +103,6 @@ this.loadContentNode(nodeId), this.loadAncestors({ id: nodeId, - includeSelf: true, }), ]; @@ -115,7 +110,6 @@ promises.push( this.loadChildren({ parent: nodeId, - tree_id: this.treeId, }) ); } diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/StudioTree/StudioTree.spec.js b/contentcuration/contentcuration/frontend/channelEdit/components/StudioTree/StudioTree.spec.js index 27bd13914f..cf4f356933 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/StudioTree/StudioTree.spec.js +++ b/contentcuration/contentcuration/frontend/channelEdit/components/StudioTree/StudioTree.spec.js @@ -7,7 +7,7 @@ import { ContentKindsNames } from 'shared/leUtils/ContentKinds'; const localVue = createLocalVue(); localVue.use(Vuex); -const TREE_ID = 'tree-id'; +const ROOT_ID = 'tree-id'; const NODE_ID = 'node-id'; const initWrapper = ({ getters = {}, mutations = {}, actions = {}, propsData = {} } = {}) => { @@ -46,7 +46,7 @@ const initWrapper = ({ getters = {}, mutations = {}, actions = {}, propsData = { return mount(StudioTree, { propsData: { - treeId: TREE_ID, + treeId: ROOT_ID, nodeId: NODE_ID, onNodeClick: jest.fn(), ...propsData, @@ -172,7 +172,7 @@ describe('StudioTree', () => { }); expect(mockLoadChildren).toHaveBeenCalledTimes(1); - expect(mockLoadChildren.mock.calls[0][1]).toEqual({ parent: NODE_ID, tree_id: TREE_ID }); + expect(mockLoadChildren.mock.calls[0][1]).toEqual({ parent: NODE_ID, root_id: ROOT_ID }); }); it("doesn't dispatch load children action if node has no resources", () => { diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/StudioTree/StudioTree.vue b/contentcuration/contentcuration/frontend/channelEdit/components/StudioTree/StudioTree.vue index 12b5d5f710..99c0b8de86 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/StudioTree/StudioTree.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/StudioTree/StudioTree.vue @@ -85,7 +85,6 @@ { + data() { return { ContentKindsNames, loading: false, - loaded: false, + loaded: this.dataPreloaded, }; }, computed: { @@ -153,12 +152,11 @@ node() { return this.getContentNode(this.nodeId); }, + children() { + return this.getContentNodeChildren(this.nodeId) || []; + }, subtopics() { - const children = this.getContentNodeChildren(this.nodeId); - if (!children) { - return []; - } - return children.filter(child => child.kind === this.ContentKindsNames.TOPIC); + return this.children.filter(child => child.kind === this.ContentKindsNames.TOPIC); }, showExpansion() { return this.node && this.node.total_count > this.node.resource_count; @@ -198,7 +196,6 @@ this.loading = true; return this.loadChildren({ parent: this.nodeId, - tree_id: this.treeId, }).then(() => { this.loading = false; this.loaded = true; diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/move/MoveModal.vue b/contentcuration/contentcuration/frontend/channelEdit/components/move/MoveModal.vue index d2443793aa..5695d97515 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/move/MoveModal.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/move/MoveModal.vue @@ -166,7 +166,6 @@ ...mapGetters('contentNode', [ 'getContentNode', 'getContentNodeChildren', - 'getTreeNode', 'getContentNodeAncestors', 'getTopicAndResourceCounts', ]), @@ -184,8 +183,8 @@ return this.$tr('moveItems', this.getTopicAndResourceCounts(this.moveNodeIds)); }, currentLocationId() { - let treeNode = this.getTreeNode(this.moveNodeIds[0]); - return treeNode && treeNode.parent; + const contentNode = this.getContentNode(this.moveNodeIds[0]); + return contentNode && contentNode.parent; }, currentNode() { return this.getContentNode(this.targetNodeId); @@ -234,7 +233,7 @@ this.loading = true; return this.loadChildren({ parent: this.targetNodeId, - tree_id: this.rootId, + root_id: this.rootId, }).then(() => { this.loading = false; }); diff --git a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.spec.js b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.spec.js index 8e469dfdb5..df4c6b5d4d 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.spec.js +++ b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.spec.js @@ -13,21 +13,20 @@ localVue.use(Vuex); localVue.use(VueRouter); const NODE_ID = 'id-reading'; -const ROOT_TREE_ID = 'channel-root-tree'; +const ROOT_ID = 'channel-root-tree'; const GETTERS = { global: { isCompactViewMode: jest.fn(), }, currentChannel: { - rootId: () => ROOT_TREE_ID, + rootId: () => ROOT_ID, currentChannel: () => jest.fn(), stagingId: jest.fn(), hasStagingTree: jest.fn(), getCurrentChannelStagingDiff: jest.fn(), }, contentNode: { - getTreeNodeChildren: () => jest.fn(), getContentNodeChildren: () => jest.fn(), getContentNodeAncestors: () => jest.fn(), getContentNode: () => jest.fn(), @@ -174,7 +173,7 @@ describe('StagingTreePage', () => { const wrapper = initWrapper(); const link = wrapper.find({ name: 'ToolBar' }).find('[data-test="root-tree-link"]'); - expect(link.attributes().href).toBe(`#/${ROOT_TREE_ID}`); + expect(link.attributes().href).toBe(`#/${ROOT_ID}`); }); it('renders no resources found message if a channel has no staging tree', () => { @@ -211,9 +210,6 @@ describe('StagingTreePage', () => { }, }; }; - getters.contentNode.getTreeNodeChildren = () => () => { - return [{ id: 'id-topic' }, { id: 'id-document' }, { id: 'id-exercise' }]; - }; getters.contentNode.getContentNodeChildren = () => () => { return [ { id: 'id-topic', title: 'Topic', kind: ContentKindsNames.TOPIC }, @@ -425,7 +421,7 @@ describe('StagingTreePage', () => { expect(wrapper.vm.$router.currentRoute.name).toBe(RouterNames.TREE_VIEW); expect(wrapper.vm.$router.currentRoute.params).toEqual({ - nodeId: ROOT_TREE_ID, + nodeId: ROOT_ID, }); }); }); diff --git a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue index 226aa3c6e5..88363458ee 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue @@ -323,13 +323,9 @@ 'hasStagingTree', 'getCurrentChannelStagingDiff', ]), - ...mapGetters('contentNode', [ - 'getTreeNodeChildren', - 'getContentNodeChildren', - 'getContentNodeAncestors', - ]), + ...mapGetters('contentNode', ['getContentNodeChildren', 'getContentNodeAncestors']), isEmpty() { - return !this.hasStagingTree || !this.getTreeNodeChildren(this.stagingId); + return !this.hasStagingTree || !this.getContentNodeChildren(this.stagingId); }, rootTreeRoute() { return { @@ -405,8 +401,8 @@ }, watch: { nodeId(newNodeId) { - this.loadAncestors({ id: newNodeId, includeSelf: true }); - this.loadChildren({ parent: newNodeId, tree_id: this.stagingId }); + this.loadAncestors({ id: newNodeId }); + this.loadChildren({ parent: newNodeId, root_id: this.stagingId }); }, detailNodeId(newDetailNodeId) { if (!newDetailNodeId) { @@ -424,18 +420,13 @@ }, created() { return this.loadCurrentChannel({ staging: true }) - .then(channel => { - if (channel.staging_root_id) { - return this.loadTree({ tree_id: channel.staging_root_id }); - } - }) .then(() => { if (!this.hasStagingTree) { return; } Promise.all([ - this.loadAncestors({ id: this.nodeId, includeSelf: true }), - this.loadChildren({ parent: this.nodeId, tree_id: this.stagingId }), + this.loadAncestors({ id: this.nodeId }), + this.loadChildren({ parent: this.nodeId, root_id: this.stagingId }), ]).then(() => { this.isLoading = false; this.loadCurrentChannelStagingDiff(); diff --git a/contentcuration/contentcuration/frontend/channelEdit/router.js b/contentcuration/contentcuration/frontend/channelEdit/router.js index f6f5b48abe..1604fb755f 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/router.js +++ b/contentcuration/contentcuration/frontend/channelEdit/router.js @@ -23,13 +23,9 @@ const router = new VueRouter({ component: Sandbox, beforeEnter: (to, from, next) => { const channelPromise = store.dispatch('currentChannel/loadChannel'); - const treePromise = store.dispatch( - 'contentNode/loadTree', - store.state.currentChannel.currentChannelId - ); const nodePromise = store.dispatch('contentNode/loadContentNode', to.params.nodeId); // api call to get ancestors if nodeId is a child descendant??? - return Promise.all([channelPromise, treePromise, nodePromise]) + return Promise.all([channelPromise, nodePromise]) .then(() => next()) .catch(() => {}); }, @@ -80,10 +76,9 @@ const router = new VueRouter({ props: true, component: AddPreviousStepsPage, beforeEnter: (to, from, next) => { - const { currentChannelId } = store.state.currentChannel; const { targetNodeId } = to.params; const promises = [ - store.dispatch('channel/loadChannel', currentChannelId), + store.dispatch('currentChannel/loadChannel'), store.dispatch('contentNode/loadRelatedResources', targetNodeId), ]; @@ -100,10 +95,9 @@ const router = new VueRouter({ props: true, component: AddNextStepsPage, beforeEnter: (to, from, next) => { - const { currentChannelId } = store.state.currentChannel; const { targetNodeId } = to.params; const promises = [ - store.dispatch('channel/loadChannel', currentChannelId), + store.dispatch('currentChannel/loadChannel'), store.dispatch('contentNode/loadRelatedResources', targetNodeId), ]; @@ -218,13 +212,8 @@ const router = new VueRouter({ props: true, component: TreeView, beforeEnter: (to, from, next) => { - const { currentChannelId } = store.state.currentChannel; - return store .dispatch('currentChannel/loadChannel') - .then(() => { - return store.dispatch('contentNode/loadChannelTree', currentChannelId); - }) .catch(error => { throw new Error(error); }) diff --git a/contentcuration/contentcuration/frontend/channelEdit/views/CurrentTopicView.vue b/contentcuration/contentcuration/frontend/channelEdit/views/CurrentTopicView.vue index a14438e939..33dc5b7c19 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/views/CurrentTopicView.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/views/CurrentTopicView.vue @@ -228,16 +228,16 @@ ...mapGetters('contentNode', [ 'getContentNode', 'getContentNodeAncestors', - 'getTreeNodeChildren', 'getTopicAndResourceCounts', + 'getContentNodeChildren', ]), selectAll: { get() { - return this.selected.length === this.treeChildren.length; + return this.selected.length === this.children.length; }, set(value) { if (value) { - this.selected = this.treeChildren.map(node => node.id); + this.selected = this.children.map(node => node.id); } else { this.selected = []; } @@ -256,8 +256,8 @@ }; }); }, - treeChildren() { - return this.getTreeNodeChildren(this.topicId); + children() { + return this.getContentNodeChildren(this.topicId); }, uploadFilesLink() { return { name: RouterNames.UPLOAD_FILES }; @@ -288,7 +288,7 @@ this.selected = []; this.loadingAncestors = true; - this.loadAncestors({ id: this.topicId, includeSelf: true }).then(() => { + this.loadAncestors({ id: this.topicId }).then(() => { this.loadingAncestors = false; }); }, @@ -305,12 +305,6 @@ } }, }, - created() { - this.loadingAncestors = true; - this.loadAncestors({ id: this.topicId, includeSelf: true }).then(() => { - this.loadingAncestors = false; - }); - }, methods: { ...mapActions(['showSnackbar']), ...mapActions(['setViewMode', 'addViewModeOverride', 'removeViewModeOverride']), diff --git a/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/ContentTreeList.vue b/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/ContentTreeList.vue index ff9545f60a..9f11e0b6c4 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/ContentTreeList.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/ContentTreeList.vue @@ -85,7 +85,7 @@ }; }, computed: { - ...mapGetters('contentNode', ['getContentNodeAncestors', 'getTreeNode']), + ...mapGetters('contentNode', ['getContentNodeAncestors']), selectAll: { get() { return this.ancestorIsSelected || !differenceBy(this.nodes, this.selected, 'id').length; @@ -94,9 +94,6 @@ this.$emit('change_selected', { isSelected, nodes: this.nodes }); }, }, - topicNode() { - return this.getTreeNode(this.topicId); - }, isSelected() { return function(node) { if (this.ancestorIsSelected) { @@ -141,7 +138,6 @@ this.loading = true; return this.loadChildren({ parent, - tree_id: this.topicNode.tree_id, }).then(nodes => { this.nodes = nodes; this.loading = false; @@ -150,18 +146,16 @@ }, mounted() { this.loading = true; - return this.loadChannelTree(this.$route.params.channelId).then(nodes => { - return Promise.all([ - this.loadChildren({ parent: this.topicId, tree_id: nodes[0].tree_id }), - this.loadAncestors({ id: this.topicId, includeSelf: true }), - ]).then(([nodes]) => { - this.nodes = nodes; - this.loading = false; - }); + return Promise.all([ + this.loadChildren({ parent: this.topicId }), + this.loadAncestors({ id: this.topicId }), + ]).then(([nodes]) => { + this.nodes = nodes; + this.loading = false; }); }, methods: { - ...mapActions('contentNode', ['loadChildren', 'loadAncestors', 'loadChannelTree']), + ...mapActions('contentNode', ['loadChildren', 'loadAncestors']), // @public scrollToNode(nodeId) { const ref = this.$refs[nodeId]; diff --git a/contentcuration/contentcuration/frontend/channelEdit/views/NodePanel.vue b/contentcuration/contentcuration/frontend/channelEdit/views/NodePanel.vue index 217313167a..210404663e 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/views/NodePanel.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/views/NodePanel.vue @@ -94,10 +94,10 @@ return this.rootId === this.parentId; }, }, - mounted() { + created() { if (this.node && this.node.total_count && !this.children.length) { this.loading = true; - this.loadChildren({ parent: this.parentId, tree_id: this.rootId }).then(() => { + this.loadChildren({ parent: this.parentId }).then(() => { this.loading = false; }); } diff --git a/contentcuration/contentcuration/frontend/channelEdit/views/TreeView/index.vue b/contentcuration/contentcuration/frontend/channelEdit/views/TreeView/index.vue index 4728e3ae99..82c0165c25 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/views/TreeView/index.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/views/TreeView/index.vue @@ -54,19 +54,23 @@ />
+
- + + @@ -78,13 +82,14 @@ diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/mixins.js b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/mixins.js index 9b1c3c304b..1bee31bf83 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/mixins.js +++ b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/mixins.js @@ -1,5 +1,4 @@ import { mapActions, mapGetters } from 'vuex'; -import uniq from 'lodash/uniq'; import { SelectionFlags } from 'frontend/channelEdit/vuex/clipboard/constants'; export default { @@ -8,25 +7,19 @@ export default { type: String, required: true, }, - sourceId: { - type: String, - }, }, computed: { - ...mapGetters('contentNode', ['getContentNode', 'getTreeNode']), ...mapGetters('clipboard', [ + 'getClipboardNodeForRender', 'currentSelectionState', 'getNextSelectionState', 'getClipboardChildren', ]), - treeNode() { - return this.getTreeNode(this.nodeId); - }, contentNode() { - return this.sourceId ? this.getContentNode(this.sourceId) : null; + return this.nodeId ? this.getClipboardNodeForRender(this.nodeId) : null; }, indentPadding() { - const level = this.treeNode.level || 0; + const level = this.contentNode ? this.contentNode.level : 0; return `${level * 32}px`; }, selectionState() { @@ -52,32 +45,12 @@ export default { export const parentMixin = { computed: { - ...mapGetters('contentNode', [ - 'hasChildren', - 'getTreeNodeChildren', - 'countTreeNodeDescendants', - ]), - ...mapGetters('clipboard', ['channelIds', 'getClipboardChildren']), - treeChildren() { + ...mapGetters('clipboard', ['channelIds', 'getClipboardChildren', 'hasClipboardChildren']), + children() { return this.getClipboardChildren(this.nodeId); }, - childrenSourceIds() { - return this.treeChildren.map(child => child.source_id); - }, - descendantCount() { - return this.countTreeNodeDescendants(this.nodeId); - }, - }, - mounted() { - const id__in = uniq(this.childrenSourceIds); - - // Prefetch content node data. Since we're using `lazy` with the - // nested VListGroup, this prefetches one level at a time! - if (id__in.length) { - this.$nextTick(() => this.loadContentNodes({ id__in })); - } }, methods: { - ...mapActions('contentNode', ['loadContentNodes']), + ...mapActions('clipboard', ['loadClipboardNodes']), }, }; diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeOptions.vue b/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeOptions.vue index e5e92b3a7e..0d527eb73f 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeOptions.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeOptions.vue @@ -112,12 +112,14 @@ actionCallback: () => changeTracker.revert(), }); - return this.copy({ id: this.nodeId }).then(() => { - return this.showSnackbar({ - text: this.$tr('copiedToClipboardSnackbar'), - actionText: this.$tr('undo'), - actionCallback: () => changeTracker.revert(), - }); + return this.copy({ node_id: this.node.node_id, channel_id: this.node.channel_id }).then( + () => { + return this.showSnackbar({ + text: this.$tr('copiedToClipboardSnackbar'), + actionText: this.$tr('undo'), + actionCallback: () => changeTracker.revert(), + } + ); }); }), duplicateNode: withChangeTracker(function(changeTracker) { diff --git a/contentcuration/contentcuration/frontend/channelEdit/views/CurrentTopicView.vue b/contentcuration/contentcuration/frontend/channelEdit/views/CurrentTopicView.vue index 33dc5b7c19..a3ab429865 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/views/CurrentTopicView.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/views/CurrentTopicView.vue @@ -373,7 +373,9 @@ }); }); }), - copyToClipboard: withChangeTracker(function(id__in, changeTracker) { + copyToClipboard: withChangeTracker(function(ids, changeTracker) { + const nodes = this.getContentNodes(ids); + const count = nodes.length; this.showSnackbar({ duration: null, text: this.$tr('creatingClipboardCopies'), @@ -381,7 +383,7 @@ actionCallback: () => changeTracker.revert(), }); - return this.copyAll({ id__in, deep: true }).then(() => { + return this.copyAll({ nodes }).then(() => { this.selectAll = false; return this.showSnackbar({ text: this.$tr('copiedItemsToClipboard'), diff --git a/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/SearchOrBrowseWindow.vue b/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/SearchOrBrowseWindow.vue index 50b69f0eb7..2014cf597d 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/SearchOrBrowseWindow.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/SearchOrBrowseWindow.vue @@ -155,7 +155,7 @@ actionText: this.$tr('cancel'), actionCallback: () => changeTracker.revert(), }); - return this.copy({ id: this.copyNode.id }) + return this.copy({ node_id: this.copyNode.node_id, channel_id: this.copyNode.channel_id }) .then(() => { return this.$store.dispatch('showSnackbar', { text: this.$tr('copiedToClipboard'), diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js index 6ea3170568..8889c238c0 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js @@ -1,43 +1,38 @@ import uniq from 'lodash/uniq'; +import uniqBy from 'lodash/uniqBy'; import * as Vibrant from 'node-vibrant'; import { SelectionFlags } from './constants'; -import { Tree } from 'shared/data/resources'; import { promiseChunk } from 'shared/utils'; -import { RELATIVE_TREE_POSITIONS } from 'shared/data/constants'; +import { Clipboard } from 'shared/data/resources'; export function loadChannels(context) { - return context.dispatch('contentNode/loadClipboardTree', null, { root: true }).then(treeNodes => { - return context.dispatch('loadTreeNodes', treeNodes); - }); -} - -export function loadTreeNodes(context, treeNodes) { - const clipboardTreeId = context.rootGetters['clipboardRootId']; - const rootNodes = treeNodes.filter(node => node.parent === clipboardTreeId); - - if (!rootNodes.length) { - return []; - } + const clipboardRootId = context.rootGetters['clipboardRootId']; + return Clipboard.where({ parent: clipboardRootId }).then(clipboardNodes => { + if (!clipboardNodes.length) { + return []; + } - const root = true; + const root = true; - // Find the channels we need to load - const channelIds = uniq(treeNodes.map(node => node.channel_id).filter(Boolean)); - return promiseChunk(channelIds, 50, id__in => { - // Load up the source channels - return context.dispatch('channel/loadChannelList', { id__in }, { root }); - }) - .then(channels => { - // We need the channel tree for each source, which we need for copying, - // although we could probably delay this. - // - return promiseChunk(channelIds, 5, ids => { - return Promise.all( - ids.map(id => context.dispatch('contentNode/loadChannelTree', id, { root })) - ); - }).then(() => channels); - }) - .then(channels => { + // Find the channels we need to load + const channelIds = uniq( + clipboardNodes.map(clipboardNode => clipboardNode.source_channel_id).filter(Boolean) + ); + const nodeIdChannelIdPairs = uniqBy(clipboardNodes, [ + 'source_channel_id', + 'source_node_id', + ]).map(c => [c.source_node_id, c.source_channel_id]); + return Promise.all([ + promiseChunk(channelIds, 50, id__in => { + // Load up the source channels + return context.dispatch('channel/loadChannelList', { id__in }, { root }); + }), + context.dispatch( + 'contentNode/loadContentNodes', + { '[node_id+channel_id]__in': nodeIdChannelIdPairs }, + { root } + ), + ]).then(([channels]) => { // Add the channel to the selected state, it acts like a node channels.forEach(channel => { if (!(channel.id in context.state.selected)) { @@ -49,7 +44,7 @@ export function loadTreeNodes(context, treeNodes) { }); // Be sure to put these in after the channels! - treeNodes.forEach(node => { + clipboardNodes.forEach(node => { if (!(node.id in context.state.selected)) { context.commit('UPDATE_SELECTION_STATE', { id: node.id, @@ -60,8 +55,67 @@ export function loadTreeNodes(context, treeNodes) { // Do not return dispatch, let this operate async context.dispatch('loadChannelColors'); - return treeNodes; + context.commit('ADD_CLIPBOARD_NODES', clipboardNodes); + return clipboardNodes; + }); + }); +} + +export function loadClipboardNodes(context, parent) { + const parentNode = context.state.clipboardNodesMap[parent]; + const root = true; + if (parentNode.total_resources) { + return Clipboard.where({ parent }).then(clipboardNodes => { + if (!clipboardNodes.length) { + return []; + } + + const nodeIdChannelIdPairs = uniqBy(clipboardNodes, [ + 'source_channel_id', + 'source_node_id', + ]).map(c => [c.source_node_id, c.source_channel_id]); + return context + .dispatch( + 'contentNode/loadContentNodes', + { '[node_id+channel_id]__in': nodeIdChannelIdPairs }, + { root } + ) + .then(() => { + // Be sure to put these in after the channels! + clipboardNodes.forEach(node => { + if (!(node.id in context.state.selected)) { + context.commit('UPDATE_SELECTION_STATE', { + id: node.id, + selectionState: SelectionFlags.NONE, + }); + } + }); + + context.commit('ADD_CLIPBOARD_NODES', clipboardNodes); + return clipboardNodes; + }); }); + } else { + // Has no child resources, so fetch children of associated contentnode instead + const contentNode = context.getters.getClipboardNodeForRender(parent); + if (contentNode) { + return context + .dispatch('contentNode/loadContentNodes', { parent: contentNode.id }, { root }) + .then(contentNodes => { + // Be sure to put these in after the channels! + contentNodes.forEach(node => { + if (!(node.id in context.state.selected)) { + context.commit('UPDATE_SELECTION_STATE', { + id: node.id, + selectionState: SelectionFlags.NONE, + }); + } + }); + return []; + }); + } + } + return []; } // Here are the swatches Vibrant gives, and the order we'll check them for colors we can use. @@ -116,72 +170,57 @@ export function loadChannelColors(context) { /** * @param context - * @param {string} id + * @param {string} node_id + * @param {string} channel_id * @param {string|null} [target] * @param {boolean} [deep] * @param children * @return {*} */ -export function copy(context, { id, target = null, deep = false, children = [] }) { - if (deep && children.length) { - throw new Error('Both children and deep flag cannot be set'); - } - - // On the client-side, tree ID's are also the root node ID - const clipboardTreeId = context.rootGetters['clipboardRootId']; - target = target || clipboardTreeId; +export function copy(context, { node_id, channel_id, children = [], parent = null }) { + const clipboardRootId = context.rootGetters['clipboardRootId']; - return Tree.get(id).then(source => { - // If the copy source is in the clipboard, "redirect" to real source and try again - if (source.tree_id === clipboardTreeId) { - return context.dispatch('copy', { id: source.source_id, target, deep, children }); + // This copies a "bare" copy, if you want a full content node copy, + // go to the contentNode state actions + return Clipboard.copy(node_id, channel_id, clipboardRootId, parent).then(node => { + if (!children.length) { + // Refresh our channel list following the copy + context.dispatch('loadChannels'); + return [node]; } - // This copies a "bare" copy, if you want a full content node copy, - // go to the contentNode state actions - return Tree.copy(id, target, RELATIVE_TREE_POSITIONS.LAST_CHILD, deep) - .then(treeNodes => { - return context.dispatch('loadTreeNodes', treeNodes); + return promiseChunk(children, 1, ([child]) => { + return context.dispatch( + 'copy', + Object.assign(child, { + parent: node.id, + }) + ); + }) + .then(otherNodes => { + // Add parent to beginning + otherNodes.unshift(node); + // Refresh our channel list following the copy + context.dispatch('loadChannels'); + return otherNodes; }) - .then(treeNodes => { - if (!children.length) { - return treeNodes; - } - - // If we have children to add, then copy only returned one node - const [parentNode] = treeNodes; - return promiseChunk(children, 1, ([child]) => { - return context.dispatch( - 'copy', - Object.assign(child, { - target: parentNode.id, - }) - ); - }).then(otherTreeNodes => { - // Add parent to beginning - otherTreeNodes.unshift(parentNode); - return otherTreeNodes; - }); + .then(nodes => { + context.commit('contentNode/ADD_CONTENTNODES', nodes, { root: true }); }); }); } -export function copyAll(context, { id__in, deep = false }) { - const clipboardNode = context.rootGetters['contentNode/getContentNode']( - window.user.clipboard_root_id - ); - const copyPromise = promiseChunk(id__in, 20, idChunk => { - return Promise.all(idChunk.map(id => context.dispatch('copy', { id, deep }))); +/* + * For convenience, this function takes an array of nodes as the argument, + * to consolidate any uniquefying. + */ +export function copyAll(context, { nodes }) { + const sources = uniqBy(nodes, ['node_id', 'channel_id']) + .map(n => [n.node_id, n.channel_id]) + .filter(s => s[0] && s[1]); + return promiseChunk(sources, 20, sourcesChunk => { + return Promise.all(sourcesChunk.map(source => context.dispatch('copy', source))); }); - - // We need to check if the clipboard tree is initialized before we try to copy. - if (!clipboardNode) { - return context - .dispatch('contentNode/loadClipboardTree', null, { root: true }) - .then(() => copyPromise); - } else { - return copyPromise; - } } /** diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/getters.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/getters.js index e900cd4040..afbf38afc4 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/getters.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/getters.js @@ -22,7 +22,14 @@ import { SelectionFlags } from './constants'; */ export function clipboardChildren(state, getters, rootState, rootGetters) { const rootId = rootGetters['clipboardRootId']; - return rootGetters['contentNode/getTreeNodeChildren'](rootId); + return Object.values(state.clipboardNodesMap).filter(c => c.parent === rootId); +} + +export function hasClipboardChildren(state, getters) { + return function(id) { + const node = getters.getClipboardNodeForRender(id); + return node && node.total_count > 0; + }; } export function getClipboardChildren(state, getters, rootState, rootGetters) { @@ -39,9 +46,25 @@ export function getClipboardChildren(state, getters, rootState, rootGetters) { return getters.channelIds.map(id => ({ id, channel_id: id })); } - return getters.channelIds.includes(id) - ? getters.clipboardChildren.filter(child => child.channel_id === id) - : rootGetters['contentNode/getTreeNodeChildren'](id); + if (getters.channelIds.includes(id)) { + // This is a channel level node, so return the channels + return sortBy( + getters.clipboardChildren.filter(child => child.source_channel_id === id), + 'lft' + ); + } + if (state.clipboardNodesMap[id] && state.clipboardNodesMap[id].total_count) { + // We have a clipboard node that has children return its children directly + return sortBy( + Object.values(state.clipboardNodesMap).filter(c => c.parent === id), + 'lft' + ); + } + const contentNode = getters.getClipboardNodeForRender(id); + if (contentNode && contentNode.total_count) { + return rootGetters['contentNode/getContentNodeChildren'](contentNode.id); + } + return []; }; } @@ -63,13 +86,60 @@ export function getClipboardParentId(state, getters, rootState, rootGetters) { return rootId; } - const treeNode = rootGetters['contentNode/getTreeNode'](id); + const contentNode = rootGetters['contentNode/getContentNode'](id); - if (!treeNode) { + if (!contentNode) { return null; } - return treeNode.parent === rootId ? treeNode.channel_id : treeNode.parent; + return contentNode.parent === rootId ? contentNode.source_channel_id : contentNode.parent; + }; +} + +export function getClipboardNodeForRender(state, getters, rootState, rootGetters) { + /** + * Get a ContentNode for the clipboard based on the clipboard node id + * this then will look up the content node matching that node_id and channel_id + * + * @param {string} id + */ + return function(id) { + const rootId = rootGetters['clipboardRootId']; + + if (id === rootId) { + // Don't need to fetch a contentnode for the root id + return null; + } + + const clipboardNode = state.clipboardNodesMap[id]; + + if (!clipboardNode) { + // No record of this node locally try a content node + const contentNode = rootGetters['contentNode/getContentNode'](id); + + if (contentNode) { + return contentNode; + } + return null; + } + + // First try to look up by source_node_id and source_channel_id + const sourceNode = Object.values(rootState.contentNode.contentNodesMap).find( + node => + node.node_id === clipboardNode.source_node_id && + node.channel_id === clipboardNode.source_channel_id + ); + + if (sourceNode) { + const contentNode = rootGetters['contentNode/getContentNode'](sourceNode.id); + return { + ...contentNode, + resource_count: clipboardNode.resource_count || contentNode.resource_count, + total_count: clipboardNode.total_count || contentNode.total_count, + }; + } + + return null; }; } @@ -77,7 +147,7 @@ export function getClipboardParentId(state, getters, rootState, rootGetters) { * List of distinct source channel ID's containing a node on the clipboard */ export function channelIds(state, getters) { - return uniq(getters.clipboardChildren.map(n => n.channel_id)).filter(Boolean); + return uniq(getters.clipboardChildren.map(n => n.source_channel_id)).filter(Boolean); } /** @@ -110,7 +180,7 @@ export function filterSelectionIds(state) { export function selectedNodes(state, getters, rootState, rootGetters) { return getters .filterSelectionIds(SelectionFlags.SELECTED) - .map(id => rootGetters['contentNode/getTreeNode'](id)) + .map(id => rootGetters['contentNode/getContentNode'](id)) .filter(Boolean); } @@ -119,7 +189,7 @@ export function selectedNodes(state, getters, rootState, rootGetters) { */ export function selectedChannels(state, getters, rootState, rootGetters) { return getters.selectedNodes - .map(node => node.channel_id) + .map(node => node.source_channel_id) .filter(Boolean) .reduce((channelIds, channelId) => { if (!channelIds.includes(channelId)) { @@ -323,7 +393,8 @@ export function getCopyTrees(state, getters) { const updates = sortBy(selectedNodes, 'lft').reduce((updates, selectedNode) => { const selectionState = getters.currentSelectionState(selectedNode.id); const update = { - id: selectedNode.source_id, + node_id: selectedNode.source_node_id, + channel_id: selectedNode.source_channel_id, target, deep: Boolean(selectionState & SelectionFlags.ALL_DESCENDANTS), children: [], diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/index.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/index.js index 4cc4480151..0e60779af8 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/index.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/index.js @@ -28,6 +28,11 @@ export default { * in the selection state management */ channelMap: {}, + /** + * A map of clipboard node ID => clipboard node + * + */ + clipboardNodesMap: {}, }), getters, actions, diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/mutations.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/mutations.js index 080e6ec1e7..6fc6ba7c2f 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/mutations.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/mutations.js @@ -11,3 +11,13 @@ export function ADD_CHANNEL_COLOR(state, { id, color }) { export function UPDATE_SELECTION_STATE(state, { id, selectionState }) { Vue.set(state.selected, id, selectionState); } + +export function ADD_CLIPBOARD_NODE(state, clipboardNode) { + Vue.set(state.clipboardNodesMap, clipboardNode.id, clipboardNode); +} + +export function ADD_CLIPBOARD_NODES(state, clipboardNodes) { + for (let clipboardNode of clipboardNodes) { + ADD_CLIPBOARD_NODE(state, clipboardNode); + } +} From b8721ee54e02ed3af85ff95b584b7adf93dfc437 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 3 Sep 2020 17:15:04 -0700 Subject: [PATCH 12/28] Add channel_id to contentnode viewset. Consolidate all contentnode operations onto contentnode viewset. --- .../contentcuration/viewsets/base.py | 40 +++ .../contentcuration/viewsets/contentnode.py | 262 ++++++++++-------- .../search/viewsets/contentnode.py | 12 +- 3 files changed, 197 insertions(+), 117 deletions(-) diff --git a/contentcuration/contentcuration/viewsets/base.py b/contentcuration/contentcuration/viewsets/base.py index dbeac59f62..5e12380eee 100644 --- a/contentcuration/contentcuration/viewsets/base.py +++ b/contentcuration/contentcuration/viewsets/base.py @@ -22,6 +22,9 @@ from contentcuration.viewsets.common import MissingRequiredParamsException +_valid_positions = {"first-child", "last-child", "left", "right"} + + class SimpleReprMixin(object): def __repr__(self): """ @@ -626,6 +629,43 @@ def copy_from_changes(self, changes): return errors, changes_to_return +class MoveMixin(object): + def validate_targeting_args(self, target, position): + if target is None: + raise ValidationError("A target must be specified") + try: + target = self.get_edit_queryset().get(pk=target) + except ObjectDoesNotExist: + raise ValidationError("Target: {} does not exist".format(target)) + except ValueError: + raise ValidationError("Invalid target specified: {}".format(target)) + if position not in _valid_positions: + raise ValidationError( + "Invalid position specified, must be one of {}".format( + ", ".join(_valid_positions) + ) + ) + return target, position + + def move_from_changes(self, changes): + errors = [] + changes = [] + for move in changes: + # Move change will have key, must also have target property + # optionally can include the desired position. + target = move["mods"].get("target") + position = move["mods"].get("position") + move_error, move_change = self.move( + move["key"], target=target, position=position + ) + if move_error: + move.update({"errors": [move_error]}) + errors.append(move) + if move_change: + changes.append(move_change) + return errors, changes + + class RelationMixin(object): def create_relation_from_changes(self, changes): errors = [] diff --git a/contentcuration/contentcuration/viewsets/contentnode.py b/contentcuration/contentcuration/viewsets/contentnode.py index 53febd4574..eb22705c98 100644 --- a/contentcuration/contentcuration/viewsets/contentnode.py +++ b/contentcuration/contentcuration/viewsets/contentnode.py @@ -1,6 +1,4 @@ -import copy import json -import uuid from django.conf import settings from django.db import transaction @@ -11,10 +9,10 @@ from django.db.models import Subquery from django_filters.rest_framework import CharFilter from django_filters.rest_framework import DjangoFilterBackend +from django_filters.rest_framework import UUIDFilter from le_utils.constants import content_kinds from le_utils.constants import roles from rest_framework.permissions import IsAuthenticated -from rest_framework.serializers import empty from rest_framework.serializers import PrimaryKeyRelatedField from rest_framework.serializers import ValidationError @@ -29,6 +27,7 @@ from contentcuration.viewsets.base import BulkModelSerializer from contentcuration.viewsets.base import BulkUpdateMixin from contentcuration.viewsets.base import CopyMixin +from contentcuration.viewsets.base import MoveMixin from contentcuration.viewsets.base import RequiredFilterSet from contentcuration.viewsets.base import ValuesViewset from contentcuration.viewsets.common import NotNullArrayAgg @@ -36,29 +35,58 @@ from contentcuration.viewsets.common import UUIDInFilter from contentcuration.viewsets.sync.constants import CONTENTNODE from contentcuration.viewsets.sync.constants import DELETED -from contentcuration.viewsets.sync.constants import UPDATED orphan_tree_id_subquery = ContentNode.objects.filter( pk=settings.ORPHANAGE_ROOT_ID ).values_list("tree_id", flat=True)[:1] +channel_query = Channel.objects.filter(main_tree__tree_id=OuterRef("tree_id")) + class ContentNodeFilter(RequiredFilterSet): id__in = UUIDInFilter(name="id") - channel_root = CharFilter(method="filter_channel_root") + root_id = UUIDFilter(method="filter_root_id") + ancestors_of = UUIDFilter(method="filter_ancestors_of") + parent__in = UUIDInFilter(name="parent") + _node_id_channel_id___in = CharFilter(method="filter__node_id_channel_id") class Meta: model = ContentNode - fields = ("parent", "id__in", "kind", "channel_root") + fields = ( + "parent", + "parent__in", + "id__in", + "kind", + "root_id", + "ancestors_of", + "_node_id_channel_id___in", + ) - def filter_channel_root(self, queryset, name, value): + def filter_root_id(self, queryset, name, value): return queryset.filter( parent=Channel.objects.filter(pk=value).values_list( "main_tree__id", flat=True ) ) + def filter_ancestors_of(self, queryset, name, value): + # For simplicity include the target node in the query + target_node_query = ContentNode.objects.filter(pk=value) + return queryset.filter( + tree_id=target_node_query.values_list("tree_id", flat=True)[:1], + lft__lte=target_node_query.values_list("lft", flat=True)[:1], + rght__gte=target_node_query.values_list("rght", flat=True)[:1], + ) + + def filter__node_id_channel_id(self, queryset, name, value): + query = Q() + values = value.split(",") + num_pairs = len(values) // 2 + for i in range(0, num_pairs): + query |= Q(node_id=values[i * 2], channel_id=values[i * 2 + 1]) + return queryset.filter(query) + class ContentNodeListSerializer(BulkListSerializer): def gather_prerequisites(self, validated_data, add_empty=True): @@ -147,6 +175,7 @@ class Meta: "provider", "extra_fields", "thumbnail_encoding", + "parent", ) list_serializer_class = ContentNodeListSerializer @@ -159,6 +188,14 @@ def create(self, validated_data): return super(ContentNodeSerializer, self).create(validated_data) + def update(self, instance, validated_data): + if "parent" in validated_data: + raise ValidationError( + {"parent": "This field should only be changed by a move operation"} + ) + + return super(ContentNodeSerializer, self).update(validated_data) + def post_save_create(self, instance, many_to_many=None): prerequisite_ids = getattr(self, "prerequisite_ids", []) super(ContentNodeSerializer, self).post_save_create( @@ -254,7 +291,7 @@ def copy_tags(from_node, to_channel_id, to_node): # Apply mixin first to override ValuesViewset -class ContentNodeViewSet(BulkUpdateMixin, CopyMixin, ValuesViewset): +class ContentNodeViewSet(BulkUpdateMixin, CopyMixin, MoveMixin, ValuesViewset): queryset = ContentNode.objects.all() serializer_class = ContentNodeSerializer permission_classes = [IsAuthenticated] @@ -279,6 +316,8 @@ class ContentNodeViewSet(BulkUpdateMixin, CopyMixin, ValuesViewset): "copyright_holder", "extra_fields", "node_id", + "root_id", + "channel_id", "original_source_node_id", "original_channel_id", "original_channel_name", @@ -296,6 +335,8 @@ class ContentNodeViewSet(BulkUpdateMixin, CopyMixin, ValuesViewset): "has_children", "parent_id", "complete", + "level", + "lft", ) field_map = { @@ -307,6 +348,7 @@ class ContentNodeViewSet(BulkUpdateMixin, CopyMixin, ValuesViewset): "assessment_items": "assessment_items_ids", "thumbnail_src": retrieve_thumbail_src, "title": get_title, + "parent": "parent_id", } def get_queryset(self): @@ -321,7 +363,10 @@ def get_queryset(self): public=True, main_tree__tree_id=OuterRef("tree_id") ) ), + # Annotate channel id + channel_id=Subquery(channel_query.values_list("id", flat=True)[:1]), ) + queryset = queryset.filter( Q(view=True) | Q(edit=True) @@ -337,6 +382,8 @@ def get_edit_queryset(self): queryset = ContentNode.objects.annotate( edit=Exists(user_queryset.filter(edit_filter)), + # Annotate channel id + channel_id=Subquery(channel_query.values_list("id", flat=True)[:1]), ) queryset = queryset.filter(Q(edit=True) | Q(tree_id=orphan_tree_id_subquery)) @@ -346,30 +393,15 @@ def get_edit_queryset(self): def annotate_queryset(self, queryset): queryset = queryset.annotate(total_count=(F("rght") - F("lft") - 1) / 2) - descendant_resources = ( - ContentNode.objects.filter( - tree_id=OuterRef("tree_id"), - lft__gt=OuterRef("lft"), - rght__lt=OuterRef("rght"), - ) - .exclude(kind_id=content_kinds.TOPIC) - .order_by("id") - .distinct("id") - .values_list("id", flat=True) - ) + descendant_resources = ContentNode.objects.filter( + tree_id=OuterRef("tree_id"), + lft__gt=OuterRef("lft"), + rght__lt=OuterRef("rght"), + ).exclude(kind_id=content_kinds.TOPIC) # Get count of descendant nodes with errors - descendant_errors = ( - ContentNode.objects.filter( - tree_id=OuterRef("tree_id"), - lft__gt=OuterRef("lft"), - rght__lt=OuterRef("rght"), - ) - .filter(complete=False) - .order_by("id") - .distinct("id") - .values_list("id", flat=True) - ) + descendant_errors = descendant_resources.filter(complete=False) + thumbnails = File.objects.filter( contentnode=OuterRef("id"), preset__thumbnail=True ) @@ -380,11 +412,15 @@ def annotate_queryset(self, queryset): original_node = ContentNode.objects.filter( node_id=OuterRef("original_source_node_id") ).filter(node_id=F("original_source_node_id")) + + root_id = ContentNode.objects.filter( + tree_id=OuterRef("tree_id"), parent__isnull=True + ).values_list("id", flat=True)[:1] + queryset = queryset.annotate( resource_count=SQCount(descendant_resources, field="id"), coach_count=SQCount( - descendant_resources.filter(role_visibility=roles.COACH), - field="id", + descendant_resources.filter(role_visibility=roles.COACH), field="id", ), error_count=SQCount(descendant_errors, field="id"), thumbnail_checksum=Subquery(thumbnails.values("checksum")[:1]), @@ -395,6 +431,7 @@ def annotate_queryset(self, queryset): original_parent_id=Subquery(original_node.values("parent_id")[:1]), original_node_id=Subquery(original_node.values("pk")[:1]), has_children=Exists(ContentNode.objects.filter(parent=OuterRef("id"))), + root_id=Subquery(root_id), ) queryset = queryset.annotate(content_tags=NotNullArrayAgg("tags__tag_name")) queryset = queryset.annotate(file_ids=NotNullArrayAgg("files__id")) @@ -407,88 +444,97 @@ def annotate_queryset(self, queryset): return queryset - def copy(self, pk, from_key=None, **mods): - delete_response = [ - dict(key=pk, table=CONTENTNODE, type=DELETED,), - ] + def move(self, pk, target=None, position="first-child"): + try: + contentnode = self.get_edit_queryset().get(pk=pk) + except ContentNode.DoesNotExist: + error = ValidationError("Specified node does not exist") + return str(error), None try: - with transaction.atomic(): - try: - source = ContentNode.objects.get(pk=from_key) - except ContentNode.DoesNotExist: - error = ValidationError("Copy source node does not exist") - return str(error), delete_response - - if ContentNode.objects.filter(pk=pk).exists(): - raise ValidationError("Copy pk already exists") - - # clone the model (in-memory) and update the fields on the cloned model - new_node = copy.copy(source) - new_node.pk = pk - new_node.published = False - new_node.changed = True - new_node.cloned_source = source - new_node.node_id = uuid.uuid4().hex - new_node.source_node_id = source.node_id - new_node.freeze_authoring_data = not Channel.objects.filter( - pk=source.original_channel_id, editors=self.request.user - ).exists() - - # Creating a new node, by default put it in the orphanage on initial creation. - new_node.parent_id = settings.ORPHANAGE_ROOT_ID - - # There might be some legacy nodes that don't have these, so ensure they are added - if ( - not new_node.original_channel_id - or not new_node.original_source_node_id - ): - original_node = source.get_original_node() - original_channel = original_node.get_channel() - new_node.original_channel_id = ( - original_channel.id if original_channel else None - ) - new_node.original_source_node_id = original_node.node_id + target, position = self.validate_targeting_args(target, position) + try: + contentnode.move_to(target, position) + except ValueError: + raise ValidationError( + "Invalid position argument specified: {}".format(position) + ) - new_node.source_channel_id = mods.pop("source_channel_id", None) - if not new_node.source_channel_id: - source_channel = source.get_channel() - new_node.source_channel_id = ( - source_channel.id if source_channel else None - ) + return ( + None, + None, + ) + except ValidationError as e: + return str(e), None + + def copy(self, pk, from_key=None, **mods): - new_node.save(force_insert=True) + target = mods.pop("target") + position = mods.pop("position") + source_channel_id = mods.pop("source_channel_id") - # because we don't know the tree yet, and tag data model currently uses channel, - # we can't copy them unless we were given the new channel - channel_id = mods.pop("channel_id", None) - if channel_id: - copy_tags(source, channel_id, new_node) + try: + target, position = self.validate_targeting_args(target, position) + except ValidationError as e: + return str(e), None + + try: + source = self.get_queryset().get(pk=from_key) + except ContentNode.DoesNotExist: + error = ValidationError("Copy source node does not exist") + return str(error), [dict(key=pk, table=CONTENTNODE, type=DELETED)] + + if ContentNode.objects.filter(pk=pk).exists(): + error = ValidationError("Copy pk already exists") + return str(error), None + + with transaction.atomic(): + # create a very basic copy + + new_node_data = { + "id": pk, + "content_id": source.content_id, + "kind": source.kind, + "title": source.title, + "description": source.description, + "cloned_source": source, + "source_channel_id": source_channel_id, + "source_node_id": source.node_id, + "freeze_authoring_data": not Channel.objects.filter( + pk=source.original_channel_id, editors=self.request.user + ).exists(), + "changed": True, + "published": False, + } - # Remove these because we do not want to define any mod operations on them during copy - def clean_copy_data(data): - return { - key: empty if key in copy_ignore_fields else value - for key, value in data.items() - } + # Add any additional modifications sent from the frontend + new_node_data.update(mods) - serializer = ContentNodeSerializer( - instance=new_node, data=clean_copy_data(mods), partial=True + # There might be some legacy nodes that don't have these, so ensure they are added + if source.original_channel_id: + new_node_data["original_channel_id"] = source.original_channel_id + else: + original_node = source.get_original_node() + original_channel = original_node.get_channel() + new_node_data["original_channel_id"] = ( + original_channel.id if original_channel else None ) + + serializer = ContentNodeSerializer(data=new_node_data, partial=True) + try: + serializer.is_valid(raise_exception=True) - node = serializer.save() - node.save() - - return ( - None, - [ - dict( - key=pk, - table=CONTENTNODE, - type=UPDATED, - mods=clean_copy_data(serializer.validated_data), - ), - ], - ) - except ValidationError as e: - return e.detail, None + + except ValidationError as e: + return e.detail, None + + new_node = ContentNode(**new_node_data) + + new_node.insert_at(target, position, save=False, allow_existing_pk=True) + + new_node.save(force_insert=True) + + return ( + None, + None, + ) diff --git a/contentcuration/search/viewsets/contentnode.py b/contentcuration/search/viewsets/contentnode.py index 417594a5a9..5d32a1e58d 100644 --- a/contentcuration/search/viewsets/contentnode.py +++ b/contentcuration/search/viewsets/contentnode.py @@ -83,7 +83,7 @@ def filter_languages(self, queryset, name, value): return queryset.filter(language__lang_code__in=value.split(",")) def filter_licenses(self, queryset, name, value): - licenses = [int(l) for l in value.split(",")] + licenses = [int(li) for li in value.split(",")] return queryset.filter(license__in=licenses) def filter_kinds(self, queryset, name, value): @@ -123,17 +123,11 @@ class Meta: class SearchContentNodeViewSet(ContentNodeViewSet): filter_class = ContentNodeFilter pagination_class = ListPagination - values = ContentNodeViewSet.values + ( - "channel_id", - "location_ids", - "location_channel_ids", - ) + values = ContentNodeViewSet.values + ("location_ids", "location_channel_ids",) def get_accessible_nodes_queryset(self): # jayoshih: May the force be with you, optimizations team... user_id = not self.request.user.is_anonymous() and self.request.user.id - # Annotate channel id - channel_query = Channel.objects.filter(main_tree__tree_id=OuterRef("tree_id")) # Filter by channel type channel_type = self.request.query_params.get("channel_list", "public") @@ -157,7 +151,7 @@ def get_accessible_nodes_queryset(self): .exclude(pk=self.request.query_params.get("exclude_channel", "")) .values_list("main_tree__tree_id", flat=True) .distinct() - ).annotate(channel_id=Subquery(channel_query.values("id")[:1]),) + ) def get_queryset(self): return self.get_accessible_nodes_queryset() From 8b571354e97eb66096f25ea6735a9f3069c61ed2 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 3 Sep 2020 17:17:05 -0700 Subject: [PATCH 13/28] Add clipboard endpoint, cleanup unused endpoint. --- contentcuration/contentcuration/urls.py | 3 +- contentcuration/contentcuration/views/base.py | 12 +- .../contentcuration/viewsets/clipboard.py | 114 ++++++++++++++++++ .../viewsets/sync/constants.py | 13 +- .../contentcuration/viewsets/sync/endpoint.py | 3 + 5 files changed, 132 insertions(+), 13 deletions(-) create mode 100644 contentcuration/contentcuration/viewsets/clipboard.py diff --git a/contentcuration/contentcuration/urls.py b/contentcuration/contentcuration/urls.py index 11f2935031..5603d1f6bd 100644 --- a/contentcuration/contentcuration/urls.py +++ b/contentcuration/contentcuration/urls.py @@ -49,6 +49,7 @@ from contentcuration.viewsets.channel import CatalogViewSet from contentcuration.viewsets.channel import ChannelViewSet from contentcuration.viewsets.channelset import ChannelSetViewSet +from contentcuration.viewsets.clipboard import ClipboardViewSet from contentcuration.viewsets.contentnode import ContentNodeViewSet from contentcuration.viewsets.file import FileViewSet from contentcuration.viewsets.invitation import InvitationViewSet @@ -168,6 +169,7 @@ def get_queryset(self): router.register(r'assessmentitem', AssessmentItemViewSet) router.register(r'tree', TreeViewSet, base_name='tree') router.register(r'admin-users', AdminUserViewSet, base_name='admin-users') +router.register(r'clipboard', ClipboardViewSet, base_name='clipboard') urlpatterns = [ url(r'^$', views.base, name='base'), @@ -188,7 +190,6 @@ def get_queryset(self): url(r'^api/download_channel_content_csv/(?P[^/]{32})$', views.download_channel_content_csv, name='download_channel_content_csv'), url(r'^api/probers/get_prober_channel', views.get_prober_channel, name='get_prober_channel'), url(r'^api/sync/$', sync, name="sync"), - url(r'^api/get_clipboard_channels/$', views.get_clipboard_channels, name="get_clipboard_channels"), ] # if activated, turn on django prometheus urls diff --git a/contentcuration/contentcuration/views/base.py b/contentcuration/contentcuration/views/base.py index c3bc59d4bf..61fbf17a72 100644 --- a/contentcuration/contentcuration/views/base.py +++ b/contentcuration/contentcuration/views/base.py @@ -158,7 +158,7 @@ def channel_list(request): PREFERENCES: json_for_parse_from_data(preferences), MESSAGES: json_for_parse_from_data(get_messages()), "LIBRARY_MODE": settings.LIBRARY_MODE, - 'public_languages': json_for_parse_from_data({l['lang_code']: l['count'] for l in public_lang_query}), + 'public_languages': json_for_parse_from_data({lang['lang_code']: lang['count'] for lang in public_lang_query}), 'public_collections': json_for_parse_from_serializer(PublicChannelSetSerializer(public_channelset_query, many=True)) }, ) @@ -450,13 +450,3 @@ def get_context_data(self, **kwargs): } ) return kwargs - - -@api_view(["GET"]) -@authentication_classes((TokenAuthentication, SessionAuthentication)) -@permission_classes((IsAuthenticated,)) -def get_clipboard_channels(request): - if not request.user: - return Response([]) - channel_ids = request.user.clipboard_tree.get_descendants().order_by('original_channel_id').values_list('original_channel_id', flat=True).distinct() - return Response(channel_ids) diff --git a/contentcuration/contentcuration/viewsets/clipboard.py b/contentcuration/contentcuration/viewsets/clipboard.py new file mode 100644 index 0000000000..214f6fb0c4 --- /dev/null +++ b/contentcuration/contentcuration/viewsets/clipboard.py @@ -0,0 +1,114 @@ +import re + +from django.db.models import F +from django.db.models import OuterRef +from django.db.models import Subquery +from django_filters.rest_framework import DjangoFilterBackend +from le_utils.constants import content_kinds +from rest_framework.permissions import IsAuthenticated +from rest_framework.serializers import RegexField +from rest_framework.serializers import ValidationError + +from contentcuration.models import Channel +from contentcuration.models import ContentNode +from contentcuration.models import User +from contentcuration.viewsets.base import BulkListSerializer +from contentcuration.viewsets.base import BulkModelSerializer +from contentcuration.viewsets.base import RequiredFilterSet +from contentcuration.viewsets.base import ValuesViewset +from contentcuration.viewsets.common import SQCount + + +class ClipboardFilter(RequiredFilterSet): + class Meta: + model = ContentNode + fields = ("parent",) + + +uuidregex = re.compile("^[0-9a-f]{32}$") + + +class ClipboardSerializer(BulkModelSerializer): + """ + This is a write only serializer - we leverage it to do create and update + operations, but read operations are handled by the Viewset. + """ + + # These are set as editable=False on the model so by default DRF does not allow us + # to set them. + source_channel_id = RegexField(uuidregex) + source_node_id = RegexField(uuidregex) + + class Meta: + model = ContentNode + fields = ( + "id", + "kind", + "parent", + "source_node_id", + "source_channel_id", + ) + list_serializer_class = BulkListSerializer + + def create(self, validated_data): + # Creating a new node, by default put it in the clipboard root. + if "parent" not in validated_data: + if "request" in self.context and self.context["request"].user: + validated_data["parent_id"] = self.context[ + "request" + ].user.clipboard_tree_id + else: + raise ValidationError( + "Trying to create a clipboard node when there is no user" + ) + try: + channel_query = Channel.objects.filter( + main_tree__tree_id=OuterRef("tree_id") + ) + ContentNode.objects.all().annotate( + # Annotate channel id + channel_id=Subquery(channel_query.values_list("id", flat=True)[:1]) + ).get( + node_id=validated_data["source_node_id"], + channel_id=validated_data["source_channel_id"], + ) + except ContentNode.DoesNotExist: + raise ValidationError("ContentNode does not exist") + return super(ClipboardSerializer, self).create(validated_data) + + +class ClipboardViewSet(ValuesViewset): + permission_classes = [IsAuthenticated] + filter_backends = (DjangoFilterBackend,) + filter_class = ClipboardFilter + serializer_class = ClipboardSerializer + values = ( + "id", + "source_node_id", + "source_channel_id", + "parent", + "lft", + "total_count", + "resource_count", + ) + + def get_queryset(self): + user_id = not self.request.user.is_anonymous() and self.request.user.id + user_queryset = User.objects.filter(id=user_id) + + clipboard_tree_id_query = ContentNode.objects.filter( + pk=user_queryset.values_list("clipboard_tree_id", flat=True)[:1] + ).values_list("tree_id", flat=True)[:1] + + return ContentNode.objects.filter(tree_id=clipboard_tree_id_query) + + def annotate_queryset(self, queryset): + descendant_resources = ContentNode.objects.filter( + tree_id=OuterRef("tree_id"), + lft__gt=OuterRef("lft"), + rght__lt=OuterRef("rght"), + ).exclude(kind_id=content_kinds.TOPIC) + return queryset.annotate( + total_count=(F("rght") - F("lft") - 1) / 2, + resource_count=SQCount(descendant_resources, field="content_id"), + ) diff --git a/contentcuration/contentcuration/viewsets/sync/constants.py b/contentcuration/contentcuration/viewsets/sync/constants.py index 4bc6eb7d8c..6d48cf24ec 100644 --- a/contentcuration/contentcuration/viewsets/sync/constants.py +++ b/contentcuration/contentcuration/viewsets/sync/constants.py @@ -19,10 +19,21 @@ EDITOR_M2M = "editor_m2m" VIEWER_M2M = "viewer_m2m" SAVEDSEARCH = "savedsearch" +CLIPBOARD = "clipboard" ALL_TABLES = set( - [CHANNEL, CONTENTNODE, ASSESSMENTITEM, CHANNELSET, FILE, TREE, INVITATION, USER, SAVEDSEARCH] + [ + CHANNEL, + CONTENTNODE, + ASSESSMENTITEM, + CHANNELSET, + FILE, + TREE, + INVITATION, + USER, + SAVEDSEARCH, + ] ) diff --git a/contentcuration/contentcuration/viewsets/sync/endpoint.py b/contentcuration/contentcuration/viewsets/sync/endpoint.py index 6f4cf01205..edeb0536d6 100644 --- a/contentcuration/contentcuration/viewsets/sync/endpoint.py +++ b/contentcuration/contentcuration/viewsets/sync/endpoint.py @@ -24,12 +24,14 @@ from contentcuration.viewsets.assessmentitem import AssessmentItemViewSet from contentcuration.viewsets.channel import ChannelViewSet from contentcuration.viewsets.channelset import ChannelSetViewSet +from contentcuration.viewsets.clipboard import ClipboardViewSet from contentcuration.viewsets.contentnode import ContentNodeViewSet from contentcuration.viewsets.file import FileViewSet from contentcuration.viewsets.invitation import InvitationViewSet from contentcuration.viewsets.sync.constants import ASSESSMENTITEM from contentcuration.viewsets.sync.constants import CHANNEL from contentcuration.viewsets.sync.constants import CHANNELSET +from contentcuration.viewsets.sync.constants import CLIPBOARD from contentcuration.viewsets.sync.constants import CONTENTNODE from contentcuration.viewsets.sync.constants import COPIED from contentcuration.viewsets.sync.constants import CREATED @@ -84,6 +86,7 @@ def __init__(self, change_type, time, changes): # Tree operations require content nodes to exist, and any new assessment items # need to point to an existing content node (CONTENTNODE, ContentNodeViewSet), + (CLIPBOARD, ClipboardViewSet), # The exact order of these three is not important. (ASSESSMENTITEM, AssessmentItemViewSet), (CHANNELSET, ChannelSetViewSet), From 9afc9f6a46223a2e12ffa7855cf77db142b34f13 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 3 Sep 2020 17:18:44 -0700 Subject: [PATCH 14/28] Fell the tree endpoint. --- contentcuration/contentcuration/urls.py | 2 - .../viewsets/sync/constants.py | 3 +- .../contentcuration/viewsets/sync/endpoint.py | 3 - .../contentcuration/viewsets/tree.py | 281 ------------------ 4 files changed, 1 insertion(+), 288 deletions(-) delete mode 100644 contentcuration/contentcuration/viewsets/tree.py diff --git a/contentcuration/contentcuration/urls.py b/contentcuration/contentcuration/urls.py index 5603d1f6bd..330d253589 100644 --- a/contentcuration/contentcuration/urls.py +++ b/contentcuration/contentcuration/urls.py @@ -54,7 +54,6 @@ from contentcuration.viewsets.file import FileViewSet from contentcuration.viewsets.invitation import InvitationViewSet from contentcuration.viewsets.sync.endpoint import sync -from contentcuration.viewsets.tree import TreeViewSet from contentcuration.viewsets.user import AdminUserViewSet from contentcuration.viewsets.user import ChannelUserViewSet from contentcuration.viewsets.user import UserViewSet @@ -167,7 +166,6 @@ def get_queryset(self): router.register(r'invitation', InvitationViewSet) router.register(r'contentnode', ContentNodeViewSet) router.register(r'assessmentitem', AssessmentItemViewSet) -router.register(r'tree', TreeViewSet, base_name='tree') router.register(r'admin-users', AdminUserViewSet, base_name='admin-users') router.register(r'clipboard', ClipboardViewSet, base_name='clipboard') diff --git a/contentcuration/contentcuration/viewsets/sync/constants.py b/contentcuration/contentcuration/viewsets/sync/constants.py index 6d48cf24ec..fd8c3a3107 100644 --- a/contentcuration/contentcuration/viewsets/sync/constants.py +++ b/contentcuration/contentcuration/viewsets/sync/constants.py @@ -14,7 +14,6 @@ ASSESSMENTITEM = "assessmentitem" FILE = "file" INVITATION = "invitation" -TREE = "tree" USER = "user" EDITOR_M2M = "editor_m2m" VIEWER_M2M = "viewer_m2m" @@ -25,11 +24,11 @@ ALL_TABLES = set( [ CHANNEL, + CLIPBOARD, CONTENTNODE, ASSESSMENTITEM, CHANNELSET, FILE, - TREE, INVITATION, USER, SAVEDSEARCH, diff --git a/contentcuration/contentcuration/viewsets/sync/endpoint.py b/contentcuration/contentcuration/viewsets/sync/endpoint.py index edeb0536d6..458732d712 100644 --- a/contentcuration/contentcuration/viewsets/sync/endpoint.py +++ b/contentcuration/contentcuration/viewsets/sync/endpoint.py @@ -43,12 +43,10 @@ from contentcuration.viewsets.sync.constants import INVITATION from contentcuration.viewsets.sync.constants import MOVED from contentcuration.viewsets.sync.constants import SAVEDSEARCH -from contentcuration.viewsets.sync.constants import TREE from contentcuration.viewsets.sync.constants import UPDATED from contentcuration.viewsets.sync.constants import USER from contentcuration.viewsets.sync.constants import VIEWER_M2M from contentcuration.viewsets.sync.utils import get_and_clear_user_events -from contentcuration.viewsets.tree import TreeViewSet from contentcuration.viewsets.user import ChannelUserViewSet from contentcuration.viewsets.user import UserViewSet @@ -90,7 +88,6 @@ def __init__(self, change_type, time, changes): # The exact order of these three is not important. (ASSESSMENTITEM, AssessmentItemViewSet), (CHANNELSET, ChannelSetViewSet), - (TREE, TreeViewSet), (FILE, FileViewSet), (EDITOR_M2M, ChannelUserViewSet), (VIEWER_M2M, ChannelUserViewSet), diff --git a/contentcuration/contentcuration/viewsets/tree.py b/contentcuration/contentcuration/viewsets/tree.py deleted file mode 100644 index 84b9794a26..0000000000 --- a/contentcuration/contentcuration/viewsets/tree.py +++ /dev/null @@ -1,281 +0,0 @@ -import uuid - -from django.db import transaction -from django.db.models import CharField -from django.db.models.functions import Coalesce -from django.db.models.sql.constants import LOUTER -from django.shortcuts import get_object_or_404 -from django_cte import With -from django_filters.rest_framework import DjangoFilterBackend -from django_filters.rest_framework import FilterSet -from rest_framework.permissions import IsAuthenticated -from rest_framework.response import Response -from rest_framework.serializers import Serializer -from rest_framework.serializers import ValidationError -from rest_framework.viewsets import GenericViewSet - -from contentcuration.models import Channel -from contentcuration.models import ContentNode -from contentcuration.viewsets.base import CopyMixin -from contentcuration.viewsets.common import MissingRequiredParamsException -from contentcuration.viewsets.sync.constants import CONTENTNODE -from contentcuration.viewsets.sync.constants import DELETED -from contentcuration.viewsets.sync.constants import TREE -from contentcuration.viewsets.sync.constants import UPDATED - - -_valid_positions = {"first-child", "last-child", "left", "right"} - - -class TreeFilter(FilterSet): - class Meta: - model = ContentNode - fields = ( - "id", - "parent", - ) - - -def validate_targeting_args(target, position): - if target is None: - raise ValidationError("A target content node must be specified") - try: - target = ContentNode.objects.get(pk=target) - except ContentNode.DoesNotExist: - raise ValidationError("Target content node: {} does not exist".format(target)) - except ValueError: - raise ValidationError( - "Invalid target content node specified: {}".format(target) - ) - if position not in _valid_positions: - raise ValidationError( - "Invalid node position specified, must be one of {}".format( - ", ".join(_valid_positions) - ) - ) - return target, position - - -def map_source_id(item): - return item.pop("real_source_id") - - -def map_channel_id(item): - return item.pop("source_channel_id", item.pop("original_channel_id", None)) - - -class Mapper(object): - def __init__(self, field_map, channel_id=None, tree_id=None): - self.field_map = field_map - self.channel_id = channel_id - self.tree_id = tree_id - - def __call__(self, item): - item.update({key: mapping(item) for key, mapping in self.field_map.items()}) - - item["channel_id"] = self.channel_id or item["channel_id"] - item["tree_id"] = self.tree_id - return item - - -class TreeViewSet(GenericViewSet, CopyMixin): - permission_classes = [IsAuthenticated] - filter_backends = (DjangoFilterBackend,) - filter_class = TreeFilter - serializer_class = Serializer - values = ( - "id", - "tree_id", - "real_source_id", - "source_channel_id", - "original_channel_id", - "parent", - "level", - "lft", - ) - - field_map = {"source_id": map_source_id, "channel_id": map_channel_id} - - def add_source_cte(self, queryset, field): - cte = With( - ContentNode.objects.filter(node_id__in=queryset.values(field)).values( - "id", "node_id" - ), - name="{}_cte".format(field), - ) - - queryset = cte.join( - queryset, _join_type=LOUTER, **{field: cte.col.node_id} - ).with_cte(cte) - return queryset, cte - - def annotate_queryset(self, queryset): - queryset, source_node_cte = self.add_source_cte(queryset, "source_node_id") - queryset, original_node_cte = self.add_source_cte( - queryset, "original_source_node_id" - ) - - real_source_id = Coalesce( - source_node_cte.col.id, original_node_cte.col.id, output_field=CharField() - ) - return queryset.annotate(real_source_id=real_source_id) - - @classmethod - def id_attr(cls): - return None - - def list(self, request, *args, **kwargs): - channel_id = request.query_params.get("channel_id") - tree_id = request.query_params.get("tree_id") - - if channel_id is None and tree_id is None: - raise MissingRequiredParamsException( - "tree_id or channel_id query parameter is required but was missing from the request" - ) - - root_filter = dict() - if not tree_id: - root_filter.update(channel_main=channel_id) - else: - root_filter.update(pk=tree_id) - - root = get_object_or_404(ContentNode, **root_filter) - - # Temporary hack: filter by channel_id if both tree_id and channel_id are provided - if tree_id and channel_id: - queryset = self.filter_queryset(root.get_descendants(include_self=True).filter(original_channel_id=channel_id)) - else: - queryset = self.filter_queryset(root.get_descendants(include_self=True)) - - if tree_id is None: - tree_id = root.pk - - map_data = Mapper(self.field_map, channel_id=channel_id, tree_id=tree_id) - - tree = map(map_data, self.annotate_queryset(queryset).values(*self.values)) - return Response(tree) - - def map_model(self, node): - tree_id = node.get_root_id() - channel_id = ( - Channel.objects.filter(main_tree_id=tree_id) - .values_list("pk", flat=True) - .first() - ) - - mapper = Mapper(self.field_map, channel_id=channel_id, tree_id=tree_id) - queryset = self.annotate_queryset(ContentNode.objects.filter(pk=node.pk)) - return next(map(mapper, queryset.values(*self.values))) - - def move(self, pk, target=None, position="first-child"): - try: - contentnode = ContentNode.objects.get(pk=pk) - except ContentNode.DoesNotExist: - error = ValidationError("Specified node does not exist") - return str(error), None - - try: - target, position = validate_targeting_args(target, position) - try: - contentnode.move_to(target, position) - except ValueError: - raise ValidationError( - "Invalid position argument specified: {}".format(position) - ) - - contentnode.refresh_from_db() - return ( - None, - dict( - key=pk, table=TREE, type=UPDATED, mods=self.map_model(contentnode) - ), - ) - except ValidationError as e: - return str(e), None - - def move_from_changes(self, changes): - errors = [] - changes = [] - for move in changes: - # Move change will have key, must also have target property - # optionally can include the desired position. - target = move["mods"].get("target") - position = move["mods"].get("position") - move_error, move_change = self.move( - move["key"], target=target, position=position - ) - if move_error: - move.update({"errors": [move_error]}) - errors.append(move) - if move_change: - changes.append(move_change) - return errors, changes - - def copy(self, pk, from_key=None, **mods): - """ - Creates a minimal copy, primarily for the clipboard - """ - target = mods.pop("target") - position = mods.pop("position") - sort_order = mods.pop("lft") - channel_id = mods.pop("channel_id") - - delete_response = [ - dict(key=pk, table=TREE, type=DELETED,), - dict(key=pk, table=CONTENTNODE, type=DELETED,), - ] - - try: - target, position = validate_targeting_args(target, position) - except ValidationError as e: - return str(e), None - - try: - source = ContentNode.objects.get(pk=from_key) - except ContentNode.DoesNotExist: - error = ValidationError("Copy source node does not exist") - return str(error), delete_response - - if ContentNode.objects.filter(pk=pk).exists(): - error = ValidationError("Copy pk already exists") - return str(error), None - - with transaction.atomic(): - # create a very basic copy - new_node = ContentNode( - content_id=source.content_id, - kind=source.kind, - title=source.title, - description=source.description, - sort_order=sort_order, - cloned_source=source, - source_channel_id=channel_id, - node_id=uuid.uuid4().hex, - source_node_id=source.node_id, - freeze_authoring_data=True, - changed=True, - published=False, - ) - new_node.id = pk - - # There might be some legacy nodes that don't have these, so ensure they are added - if not new_node.original_channel_id or not new_node.original_source_node_id: - original_node = source.get_original_node() - original_channel = original_node.get_channel() - new_node.original_channel_id = ( - original_channel.id if original_channel else None - ) - new_node.original_source_node_id = original_node.node_id - - new_node.insert_at(target, position, save=False, allow_existing_pk=True) - new_node.save(force_insert=True) - new_node.refresh_from_db() - - return ( - None, - [ - dict( - key=pk, table=TREE, type=UPDATED, mods=self.map_model(new_node) - ), - ], - ) From 31b4b5ec1fe063294c6fd5af16677b084611b5cc Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Fri, 4 Sep 2020 09:08:37 -0700 Subject: [PATCH 15/28] Fix tests and simplify required copy args for contentnode. --- .../frontend/shared/data/resources.js | 1 - .../tests/viewsets/test_contentnode.py | 52 ++++++++++++++++--- .../contentcuration/viewsets/contentnode.py | 9 ++-- 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index da42558ea0..08c78b3e43 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -865,7 +865,6 @@ export const ContentNode = new Resource({ mods: { target, position, - source_channel_id: node.channel_id, title, }, source: CLIENTID, diff --git a/contentcuration/contentcuration/tests/viewsets/test_contentnode.py b/contentcuration/contentcuration/tests/viewsets/test_contentnode.py index 3a91889db6..e68d12389c 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_contentnode.py +++ b/contentcuration/contentcuration/tests/viewsets/test_contentnode.py @@ -7,13 +7,13 @@ from le_utils.constants import content_kinds from contentcuration import models -from contentcuration.tests.base import StudioAPITestCase from contentcuration.tests import testdata +from contentcuration.tests.base import StudioAPITestCase from contentcuration.viewsets.sync.constants import CONTENTNODE from contentcuration.viewsets.sync.utils import generate_copy_event from contentcuration.viewsets.sync.utils import generate_create_event -from contentcuration.viewsets.sync.utils import generate_update_event from contentcuration.viewsets.sync.utils import generate_delete_event +from contentcuration.viewsets.sync.utils import generate_update_event class SyncTestCase(StudioAPITestCase): @@ -55,6 +55,26 @@ def test_create_contentnode(self): except models.ContentNode.DoesNotExist: self.fail("ContentNode was not created") + def test_create_contentnode_with_parent(self): + channel = testdata.channel() + user = testdata.user() + channel.editors.add(user) + self.client.force_authenticate(user=user) + contentnode = self.contentnode_metadata + contentnode["parent"] = channel.main_tree_id + response = self.client.post( + self.sync_url, + [generate_create_event(contentnode["id"], CONTENTNODE, contentnode)], + format="json", + ) + self.assertEqual(response.status_code, 200, response.content) + try: + new_node = models.ContentNode.objects.get(id=contentnode["id"]) + except models.ContentNode.DoesNotExist: + self.fail("ContentNode was not created") + + self.assertEqual(new_node.parent_id, channel.main_tree_id) + def test_create_contentnodes(self): user = testdata.user() self.client.force_authenticate(user=user) @@ -168,13 +188,22 @@ def test_delete_contentnodes(self): pass def test_copy_contentnode(self): + channel = testdata.channel() user = testdata.user() + channel.editors.add(user) contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata) new_node_id = uuid.uuid4().hex self.client.force_authenticate(user=user) response = self.client.post( self.sync_url, - [generate_copy_event(new_node_id, CONTENTNODE, contentnode.id, {})], + [ + generate_copy_event( + new_node_id, + CONTENTNODE, + contentnode.id, + {"target": channel.main_tree_id}, + ) + ], format="json", ) self.assertEqual(response.status_code, 200, response.content) @@ -184,7 +213,7 @@ def test_copy_contentnode(self): except models.ContentNode.DoesNotExist: self.fail("ContentNode was not copied") - self.assertEqual(new_node.parent_id, settings.ORPHANAGE_ROOT_ID) + self.assertEqual(new_node.parent_id, channel.main_tree_id) def test_create_contentnode_moveable(self): """ @@ -223,13 +252,22 @@ def test_copy_contentnode_moveable(self): Regression test to ensure that nodes created here are able to be moved to other MPTT trees without invalidating data. """ + channel = testdata.channel() user = testdata.user() + channel.editors.add(user) contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata) new_node_id = uuid.uuid4().hex self.client.force_authenticate(user=user) response = self.client.post( self.sync_url, - [generate_copy_event(new_node_id, CONTENTNODE, contentnode.id, {})], + [ + generate_copy_event( + new_node_id, + CONTENTNODE, + contentnode.id, + {"target": channel.main_tree_id}, + ) + ], format="json", ) self.assertEqual(response.status_code, 200, response.content) @@ -274,7 +312,7 @@ def test_update_orphanage_root(self): def test_delete_orphanage_root(self): user = testdata.user() - contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata) + models.ContentNode.objects.create(**self.contentnode_db_metadata) self.client.force_authenticate(user=user) response = self.client.post( @@ -401,7 +439,7 @@ def test_update_orphanage_root(self): def test_delete_orphanage_root(self): user = testdata.user() - contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata) + models.ContentNode.objects.create(**self.contentnode_db_metadata) self.client.force_authenticate(user=user) response = self.client.delete( diff --git a/contentcuration/contentcuration/viewsets/contentnode.py b/contentcuration/contentcuration/viewsets/contentnode.py index eb22705c98..af78326b03 100644 --- a/contentcuration/contentcuration/viewsets/contentnode.py +++ b/contentcuration/contentcuration/viewsets/contentnode.py @@ -194,7 +194,7 @@ def update(self, instance, validated_data): {"parent": "This field should only be changed by a move operation"} ) - return super(ContentNodeSerializer, self).update(validated_data) + return super(ContentNodeSerializer, self).update(instance, validated_data) def post_save_create(self, instance, many_to_many=None): prerequisite_ids = getattr(self, "prerequisite_ids", []) @@ -444,7 +444,7 @@ def annotate_queryset(self, queryset): return queryset - def move(self, pk, target=None, position="first-child"): + def move(self, pk, target=None, position="last-child"): try: contentnode = self.get_edit_queryset().get(pk=pk) except ContentNode.DoesNotExist: @@ -470,8 +470,7 @@ def move(self, pk, target=None, position="first-child"): def copy(self, pk, from_key=None, **mods): target = mods.pop("target") - position = mods.pop("position") - source_channel_id = mods.pop("source_channel_id") + position = mods.pop("position", "last-child") try: target, position = self.validate_targeting_args(target, position) @@ -498,7 +497,7 @@ def copy(self, pk, from_key=None, **mods): "title": source.title, "description": source.description, "cloned_source": source, - "source_channel_id": source_channel_id, + "source_channel_id": source.channel_id, "source_node_id": source.node_id, "freeze_authoring_data": not Channel.objects.filter( pk=source.original_channel_id, editors=self.request.user From 205a320f3963d9e033bd753478898def7038a5c9 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Fri, 4 Sep 2020 10:42:46 -0700 Subject: [PATCH 16/28] Fix tests. Fix copying from current topic view to clipboard. --- .../channelEdit/components/StudioTree/StudioTree.spec.js | 2 +- .../frontend/channelEdit/views/CurrentTopicView.vue | 2 +- .../frontend/channelEdit/views/TreeView/index.spec.js | 9 ++++++--- .../frontend/channelEdit/vuex/clipboard/actions.js | 4 ++-- .../contentcuration/frontend/shared/data/resources.js | 6 ++++++ 5 files changed, 16 insertions(+), 7 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/StudioTree/StudioTree.spec.js b/contentcuration/contentcuration/frontend/channelEdit/components/StudioTree/StudioTree.spec.js index cf4f356933..aa77ed91e7 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/StudioTree/StudioTree.spec.js +++ b/contentcuration/contentcuration/frontend/channelEdit/components/StudioTree/StudioTree.spec.js @@ -172,7 +172,7 @@ describe('StudioTree', () => { }); expect(mockLoadChildren).toHaveBeenCalledTimes(1); - expect(mockLoadChildren.mock.calls[0][1]).toEqual({ parent: NODE_ID, root_id: ROOT_ID }); + expect(mockLoadChildren.mock.calls[0][1]).toEqual({ parent: NODE_ID }); }); it("doesn't dispatch load children action if node has no resources", () => { diff --git a/contentcuration/contentcuration/frontend/channelEdit/views/CurrentTopicView.vue b/contentcuration/contentcuration/frontend/channelEdit/views/CurrentTopicView.vue index a3ab429865..49357510d5 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/views/CurrentTopicView.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/views/CurrentTopicView.vue @@ -227,6 +227,7 @@ ...mapGetters('currentChannel', ['canEdit', 'currentChannel', 'trashId']), ...mapGetters('contentNode', [ 'getContentNode', + 'getContentNodes', 'getContentNodeAncestors', 'getTopicAndResourceCounts', 'getContentNodeChildren', @@ -375,7 +376,6 @@ }), copyToClipboard: withChangeTracker(function(ids, changeTracker) { const nodes = this.getContentNodes(ids); - const count = nodes.length; this.showSnackbar({ duration: null, text: this.$tr('creatingClipboardCopies'), diff --git a/contentcuration/contentcuration/frontend/channelEdit/views/TreeView/index.spec.js b/contentcuration/contentcuration/frontend/channelEdit/views/TreeView/index.spec.js index b6e50dc81f..a941b6f6a9 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/views/TreeView/index.spec.js +++ b/contentcuration/contentcuration/frontend/channelEdit/views/TreeView/index.spec.js @@ -18,11 +18,13 @@ const GETTERS = { hasStagingTree: jest.fn(), stagingId: jest.fn(), rootId: jest.fn(), + canEdit: jest.fn(() => true), }, contentNode: { - getContentNodeChildren: () => jest.fn(), - getContentNodeAncestors: () => jest.fn(), - getContentNode: () => jest.fn(), + getContentNodeChildren: () => jest.fn(() => []), + getContentNodeAncestors: () => jest.fn(() => []), + getContentNode: () => jest.fn(() => ({})), + getTopicAndResourceCounts: () => jest.fn(() => ({ topicCount: 0, resourceCount: 0 })), }, }; @@ -30,6 +32,7 @@ const ACTIONS = { contentNode: { loadAncestors: jest.fn(), loadContentNode: jest.fn(), + loadContentNodes: jest.fn(), }, }; diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js index 8889c238c0..04c449e29c 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js @@ -216,8 +216,8 @@ export function copy(context, { node_id, channel_id, children = [], parent = nul */ export function copyAll(context, { nodes }) { const sources = uniqBy(nodes, ['node_id', 'channel_id']) - .map(n => [n.node_id, n.channel_id]) - .filter(s => s[0] && s[1]); + .map(n => ({ node_id: n.node_id, channel_id: n.channel_id })) + .filter(n => n.node_id && n.channel_id); return promiseChunk(sources, 20, sourcesChunk => { return Promise.all(sourcesChunk.map(source => context.dispatch('copy', source))); }); diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 08c78b3e43..189350f3b4 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -768,6 +768,12 @@ export const ContentNode = new Resource({ throw new TypeError(`${position} is not a valid position`); } + if (process.env.NODE_ENV !== 'production' && !process.env.TRAVIS) { + console.groupCollapsed(`Copying contentnode from ${id} with target ${target}`); + console.trace(); + console.groupEnd(); + } + const changeType = CHANGE_TYPES.COPIED; if (!validPositions.has(position)) { throw new TypeError(`${position} is not a valid position`); From 2d953b748bcab3c6f769f95494ed9e058c9d5b09 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Fri, 4 Sep 2020 11:44:53 -0700 Subject: [PATCH 17/28] JS linting. --- .../frontend/channelEdit/pages/StagingTreePage/index.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue index 88363458ee..bc223703e6 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue @@ -441,7 +441,7 @@ ...mapActions('channel', ['loadChannel']), ...mapActions('currentChannel', ['loadCurrentChannelStagingDiff', 'deployCurrentChannel']), ...mapActions('currentChannel', { loadCurrentChannel: 'loadChannel' }), - ...mapActions('contentNode', ['loadAncestors', 'loadChildren', 'loadTree']), + ...mapActions('contentNode', ['loadAncestors', 'loadChildren']), ...mapMutations('contentNode', { collapseAll: 'COLLAPSE_ALL_EXPANDED', setExpanded: 'SET_EXPANSION', From 55e446106bb8dc8f9ef7c194658764d76ba000a6 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Fri, 4 Sep 2020 12:07:38 -0700 Subject: [PATCH 18/28] Just one more thing... --- .../frontend/channelEdit/components/ContentNodeOptions.vue | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeOptions.vue b/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeOptions.vue index 0d527eb73f..e1369137d7 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeOptions.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeOptions.vue @@ -118,9 +118,9 @@ text: this.$tr('copiedToClipboardSnackbar'), actionText: this.$tr('undo'), actionCallback: () => changeTracker.revert(), - } - ); - }); + }); + } + ); }), duplicateNode: withChangeTracker(function(changeTracker) { this.showSnackbar({ From c0598a819f8452be557333103c63626f3863ba13 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Fri, 4 Sep 2020 12:46:12 -0700 Subject: [PATCH 19/28] Couple clipboard node vuex state to indexeddb. --- .../frontend/channelEdit/vuex/clipboard/index.js | 7 +++++++ .../frontend/channelEdit/vuex/clipboard/mutations.js | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/index.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/index.js index 0e60779af8..6f86b0651f 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/index.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/index.js @@ -2,6 +2,7 @@ import * as getters from './getters'; import * as actions from './actions'; import * as mutations from './mutations'; import persistFactory from 'shared/vuex/persistFactory'; +import { TABLE_NAMES, CHANGE_TYPES } from 'shared/data'; export default { namespaced: true, @@ -38,4 +39,10 @@ export default { actions, mutations, plugins: [persistFactory('clipboard', ['ADD_CHANNEL_COLOR'])], + listeners: { + [TABLE_NAMES.CLIPBOARD]: { + [CHANGE_TYPES.CREATED]: 'ADD_CLIPBOARD_NODE', + [CHANGE_TYPES.DELETED]: 'REMOVE_CLIPBOARD_NODE', + }, + }, }; diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/mutations.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/mutations.js index 6fc6ba7c2f..bbf1b2a6d6 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/mutations.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/mutations.js @@ -21,3 +21,7 @@ export function ADD_CLIPBOARD_NODES(state, clipboardNodes) { ADD_CLIPBOARD_NODE(state, clipboardNode); } } + +export function REMOVE_CLIPBOARD_NODE(state, clipboardNode) { + Vue.delete(state.clipboardNodesMap, clipboardNode.id); +} From ea56e68a800c5c7460a39559461f5f0c4fabd55d Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Fri, 4 Sep 2020 12:46:29 -0700 Subject: [PATCH 20/28] Add clipboard nodes to the right place after copying. --- .../frontend/channelEdit/vuex/clipboard/actions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js index 04c449e29c..bc1c7d4db4 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js @@ -205,7 +205,7 @@ export function copy(context, { node_id, channel_id, children = [], parent = nul return otherNodes; }) .then(nodes => { - context.commit('contentNode/ADD_CONTENTNODES', nodes, { root: true }); + context.commit('ADD_CLIPBOARD_NODES', nodes); }); }); } From c7daf34ee5af0b734f51b465424812f05becb875 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Fri, 4 Sep 2020 12:46:58 -0700 Subject: [PATCH 21/28] Properly delete clipboard nodes. --- .../channelEdit/components/Clipboard/ContentNodeOptions.vue | 5 ++--- .../frontend/channelEdit/vuex/clipboard/actions.js | 6 ++++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/ContentNodeOptions.vue b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/ContentNodeOptions.vue index 674900b218..999c45fa3a 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/ContentNodeOptions.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/ContentNodeOptions.vue @@ -55,8 +55,7 @@ }, methods: { ...mapActions(['showSnackbar']), - ...mapActions('clipboard', ['copy']), - ...mapActions('contentNode', ['deleteContentNodes']), + ...mapActions('clipboard', ['copy', 'deleteClipboardNode']), ...mapMutations('contentNode', { setMoveNodes: 'SET_MOVE_NODES' }), removeNode: withChangeTracker(function(changeTracker) { this.showSnackbar({ @@ -66,7 +65,7 @@ actionCallback: () => changeTracker.revert(), }); - return this.deleteContentNodes([this.nodeId]).then(() => { + return this.deleteClipboardNode(this.nodeId).then(() => { return this.showSnackbar({ text: this.$tr('removedFromClipboard'), actionText: this.$tr('undo'), diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js index bc1c7d4db4..f6ef38a2fc 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js @@ -282,3 +282,9 @@ export function resetSelectionState(context) { deep: true, }); } + +export function deleteClipboardNode(context, clipboardNodeId) { + return Clipboard.delete(clipboardNodeId).then(() => { + context.commit('REMOVE_CLIPBOARD_NODE', { id: clipboardNodeId }); + }); +} From 17698ba10d159b327cc468789589eeffd4e47cd0 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Fri, 4 Sep 2020 16:28:22 -0700 Subject: [PATCH 22/28] Code cleanup based on PR review. --- .../frontend/shared/data/changes.js | 22 +++++++++++++------ .../frontend/shared/data/resources.js | 11 +++++----- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/changes.js b/contentcuration/contentcuration/frontend/shared/data/changes.js index 767a2c1b08..75e99a7a0e 100644 --- a/contentcuration/contentcuration/frontend/shared/data/changes.js +++ b/contentcuration/contentcuration/frontend/shared/data/changes.js @@ -172,11 +172,23 @@ export class ChangeTracker extends EventEmitter { */ stop() { this._isTracking = false; - this._timeout = setTimeout(this.dismiss.bind(this), this.expiry); // This sets the expiry, from 0 to an actual expiry. The clock is ticking... return this.renew(this.expiry); } + clearTimeout() { + if (this._timeout) { + // Clear the timeout for dismissing the lock, as we will dismiss + // it once we are done reverting. + clearTimeout(this._timeout); + } + } + + setTimeout() { + this.clearTimeout(); + this._timeout = setTimeout(this.dismiss.bind(this), this.expiry); + } + /** * Extends the current safety delay by `milliseconds` * @@ -198,7 +210,7 @@ export class ChangeTracker extends EventEmitter { lock.expiry += milliseconds; return lock; }) - ); + ).then(() => this.setTimeout); }); }); } @@ -217,11 +229,7 @@ export class ChangeTracker extends EventEmitter { throw new Error('Unable to revert changes without locks'); } - if (this._timeout) { - // Clear the timeout for dismissing the lock, as we will dismiss - // it once we are done reverting. - clearTimeout(this._timeout); - } + this.clearTimeout(); this._isReverted = true; diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 189350f3b4..5dabcffdd8 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -443,13 +443,14 @@ class Resource extends mix(APIResource, IndexedDBResource) { fetchCollection(params) { const now = Date.now(); const queryString = paramsSerializer(params); + const cachedRequest = this._requests[queryString]; if ( - this._requests[queryString] && - this._requests[queryString][LAST_FETCHED] && - this._requests[queryString][LAST_FETCHED] + REFRESH_INTERVAL * 1000 > now && - this._requests[queryString].promise + cachedRequest && + cachedRequest[LAST_FETCHED] && + cachedRequest[LAST_FETCHED] + REFRESH_INTERVAL * 1000 > now && + cachedRequest.promise ) { - return this._requests[queryString].promise; + return cachedRequest.promise; } const promise = client.get(this.collectionUrl(), { params }).then(response => { const now = Date.now(); From 9cdf94052869cfc83fa4859be17c215dc72a9709 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Sat, 5 Sep 2020 22:27:05 -0700 Subject: [PATCH 23/28] Fix basic bulk clipboard deletion. --- .../frontend/channelEdit/components/Clipboard/index.vue | 5 ++--- .../frontend/channelEdit/vuex/clipboard/actions.js | 8 ++++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/index.vue b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/index.vue index 53103acbcc..7352445e56 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/index.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/index.vue @@ -167,8 +167,7 @@ methods: { ...mapActions(['showSnackbar']), ...mapMutations('contentNode', { setMoveNodes: 'SET_MOVE_NODES' }), - ...mapActions('clipboard', ['loadChannels', 'copy']), - ...mapActions('contentNode', ['deleteContentNodes']), + ...mapActions('clipboard', ['loadChannels', 'copy', 'deleteClipboardNodes']), refresh() { if (this.refreshing) { return; @@ -226,7 +225,7 @@ actionCallback: () => changeTracker.revert(), }); - return this.deleteContentNodes(id__in).then(() => { + return this.deleteClipboardNodes(id__in).then(() => { return this.showSnackbar({ text: this.$tr('removedFromClipboard'), actionText: this.$tr('undo'), diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js index f6ef38a2fc..38d9097038 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js @@ -288,3 +288,11 @@ export function deleteClipboardNode(context, clipboardNodeId) { context.commit('REMOVE_CLIPBOARD_NODE', { id: clipboardNodeId }); }); } + +export function deleteClipboardNodes(context, clipboardNodeIds) { + return Promise.all( + clipboardNodeIds.map(id => { + return deleteClipboardNode(context, id); + }) + ); +} From b40b0e01cf4e1e7573ac0cb2dcc79369d63aef3c Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Tue, 8 Sep 2020 08:51:18 -0700 Subject: [PATCH 24/28] Add level information to clipboard. --- .../channelEdit/components/Clipboard/Channel.vue | 2 ++ .../channelEdit/components/Clipboard/TopicNode.vue | 3 +++ .../channelEdit/components/Clipboard/mixins.js | 10 ++++++++++ .../contentcuration/viewsets/contentnode.py | 1 - 4 files changed, 15 insertions(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/Channel.vue b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/Channel.vue index 966b58f4ed..318ecaf90a 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/Channel.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/Channel.vue @@ -43,11 +43,13 @@ v-if="hasClipboardChildren(child.id)" :key="child.id" :nodeId="child.id" + :level="level + 1" /> diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/TopicNode.vue b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/TopicNode.vue index 57a4f860a0..7a67141c01 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/TopicNode.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/TopicNode.vue @@ -10,6 +10,7 @@ diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/mixins.js b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/mixins.js index 1bee31bf83..702d9f3b67 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/mixins.js +++ b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/mixins.js @@ -7,6 +7,10 @@ export default { type: String, required: true, }, + level: { + type: Number, + default: 0, + }, }, computed: { ...mapGetters('clipboard', [ @@ -44,6 +48,12 @@ export default { }; export const parentMixin = { + props: { + level: { + type: Number, + default: 0, + }, + }, computed: { ...mapGetters('clipboard', ['channelIds', 'getClipboardChildren', 'hasClipboardChildren']), children() { diff --git a/contentcuration/contentcuration/viewsets/contentnode.py b/contentcuration/contentcuration/viewsets/contentnode.py index af78326b03..bd5969d23a 100644 --- a/contentcuration/contentcuration/viewsets/contentnode.py +++ b/contentcuration/contentcuration/viewsets/contentnode.py @@ -335,7 +335,6 @@ class ContentNodeViewSet(BulkUpdateMixin, CopyMixin, MoveMixin, ValuesViewset): "has_children", "parent_id", "complete", - "level", "lft", ) From 7b19c066957a29898d0e7e2b94325ca4dac30deb Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Tue, 8 Sep 2020 08:54:24 -0700 Subject: [PATCH 25/28] Use ancestor id to differentiate duplicate clipboard entries. --- .../components/Clipboard/ContentNode.vue | 4 +- .../Clipboard/ContentNodeOptions.vue | 9 ++- .../components/Clipboard/TopicNode.vue | 17 ++++- .../components/Clipboard/mixins.js | 36 +++++++-- .../channelEdit/vuex/clipboard/actions.js | 76 +++++++++++++------ .../channelEdit/vuex/clipboard/getters.js | 58 +++++++++++--- .../channelEdit/vuex/clipboard/mutations.js | 5 +- .../channelEdit/vuex/clipboard/utils.js | 3 + .../contentcuration/viewsets/clipboard.py | 2 + 9 files changed, 160 insertions(+), 50 deletions(-) create mode 100644 contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/utils.js diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/ContentNode.vue b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/ContentNode.vue index b54716dc7d..d580aacf9c 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/ContentNode.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/ContentNode.vue @@ -49,12 +49,12 @@ - + diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/ContentNodeOptions.vue b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/ContentNodeOptions.vue index 999c45fa3a..17b456a3f8 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/ContentNodeOptions.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/ContentNodeOptions.vue @@ -30,6 +30,10 @@ type: String, required: true, }, + ancestorId: { + type: String, + default: null, + }, }, computed: { ...mapGetters('channel', ['getChannel']), @@ -65,7 +69,10 @@ actionCallback: () => changeTracker.revert(), }); - return this.deleteClipboardNode(this.nodeId).then(() => { + return this.deleteClipboardNode({ + clipboardNodeId: this.nodeId, + ancestorId: this.ancestorId, + }).then(() => { return this.showSnackbar({ text: this.$tr('removedFromClipboard'), actionText: this.$tr('undo'), diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/TopicNode.vue b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/TopicNode.vue index 7a67141c01..2139965c33 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/TopicNode.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/TopicNode.vue @@ -36,16 +36,18 @@ @@ -64,16 +66,27 @@ ContentNode, }, mixins: [clipboardMixin, parentMixin], + props: { + ancestorId: { + type: String, + default: null, + }, + }, data() { return { open: false, }; }, + computed: { + childAncestorId() { + return this.isClipboardNode(this.nodeId) ? this.nodeId : this.ancestorId; + }, + }, mounted() { // Prefetch content node data. Since we're using `lazy` with the // nested VListGroup, this prefetches one level at a time! if (this.contentNode.total_count > 0) { - this.loadClipboardNodes(this.nodeId); + this.loadClipboardNodes({ parent: this.nodeId, ancestorId: this.childAncestorId }); } }, }; diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/mixins.js b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/mixins.js index 702d9f3b67..5a4d5d3b32 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/mixins.js +++ b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/mixins.js @@ -7,6 +7,10 @@ export default { type: String, required: true, }, + ancestorId: { + type: String, + default: null, + }, level: { type: Number, default: 0, @@ -15,7 +19,7 @@ export default { computed: { ...mapGetters('clipboard', [ 'getClipboardNodeForRender', - 'currentSelectionState', + 'getSelectionState', 'getNextSelectionState', 'getClipboardChildren', ]), @@ -23,14 +27,13 @@ export default { return this.nodeId ? this.getClipboardNodeForRender(this.nodeId) : null; }, indentPadding() { - const level = this.contentNode ? this.contentNode.level : 0; - return `${level * 32}px`; + return `${this.level * 32}px`; }, selectionState() { - return this.currentSelectionState(this.nodeId); + return this.getSelectionState(this.nodeId, this.ancestorId); }, nextSelectionState() { - return this.getNextSelectionState(this.nodeId); + return this.getNextSelectionState(this.nodeId, this.ancestorId); }, selected() { return Boolean(this.selectionState & SelectionFlags.SELECTED); @@ -42,22 +45,39 @@ export default { methods: { ...mapActions('clipboard', ['setSelectionState', 'resetSelectionState']), goNextSelectionState() { - this.setSelectionState({ id: this.nodeId, selectionState: this.nextSelectionState }); + this.setSelectionState({ + id: this.nodeId, + selectionState: this.nextSelectionState, + ancestorId: this.ancestorId, + }); }, }, }; export const parentMixin = { props: { + nodeId: { + type: String, + required: true, + }, + ancestorId: { + type: String, + default: null, + }, level: { type: Number, default: 0, }, }, computed: { - ...mapGetters('clipboard', ['channelIds', 'getClipboardChildren', 'hasClipboardChildren']), + ...mapGetters('clipboard', [ + 'channelIds', + 'getClipboardChildren', + 'hasClipboardChildren', + 'isClipboardNode', + ]), children() { - return this.getClipboardChildren(this.nodeId); + return this.getClipboardChildren(this.nodeId, this.ancestorId); }, }, methods: { diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js index 38d9097038..1f08dc402a 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js @@ -2,6 +2,7 @@ import uniq from 'lodash/uniq'; import uniqBy from 'lodash/uniqBy'; import * as Vibrant from 'node-vibrant'; import { SelectionFlags } from './constants'; +import { selectionId } from './utils'; import { promiseChunk } from 'shared/utils'; import { Clipboard } from 'shared/data/resources'; @@ -18,10 +19,10 @@ export function loadChannels(context) { const channelIds = uniq( clipboardNodes.map(clipboardNode => clipboardNode.source_channel_id).filter(Boolean) ); - const nodeIdChannelIdPairs = uniqBy(clipboardNodes, [ - 'source_channel_id', - 'source_node_id', - ]).map(c => [c.source_node_id, c.source_channel_id]); + const nodeIdChannelIdPairs = uniqBy( + clipboardNodes, + c => c.source_node_id + c.source_channel_id + ).map(c => [c.source_node_id, c.source_channel_id]); return Promise.all([ promiseChunk(channelIds, 50, id__in => { // Load up the source channels @@ -61,19 +62,19 @@ export function loadChannels(context) { }); } -export function loadClipboardNodes(context, parent) { +export function loadClipboardNodes(context, { parent, ancestorId }) { const parentNode = context.state.clipboardNodesMap[parent]; const root = true; - if (parentNode.total_resources) { + if (parentNode && parentNode.total_resources) { return Clipboard.where({ parent }).then(clipboardNodes => { if (!clipboardNodes.length) { return []; } - const nodeIdChannelIdPairs = uniqBy(clipboardNodes, [ - 'source_channel_id', - 'source_node_id', - ]).map(c => [c.source_node_id, c.source_channel_id]); + const nodeIdChannelIdPairs = uniqBy( + clipboardNodes, + c => c.source_node_id + c.source_channel_id + ).map(c => [c.source_node_id, c.source_channel_id]); return context .dispatch( 'contentNode/loadContentNodes', @@ -108,6 +109,7 @@ export function loadClipboardNodes(context, parent) { context.commit('UPDATE_SELECTION_STATE', { id: node.id, selectionState: SelectionFlags.NONE, + ancestorId, }); } }); @@ -215,7 +217,7 @@ export function copy(context, { node_id, channel_id, children = [], parent = nul * to consolidate any uniquefying. */ export function copyAll(context, { nodes }) { - const sources = uniqBy(nodes, ['node_id', 'channel_id']) + const sources = uniqBy(nodes, c => c.node_id + c.channel_id) .map(n => ({ node_id: n.node_id, channel_id: n.channel_id })) .filter(n => n.node_id && n.channel_id); return promiseChunk(sources, 20, sourcesChunk => { @@ -226,12 +228,15 @@ export function copyAll(context, { nodes }) { /** * Recursive function to set selection state for a node, and possibly it's ancestors and descendants */ -export function setSelectionState(context, { id, selectionState, deep = true, parents = true }) { - const currentState = context.state.selected[id]; +export function setSelectionState( + context, + { id, selectionState, deep = true, parents = true, ancestorId = null } +) { + const currentState = context.state.selected[selectionId(id, ancestorId)]; // Well, we should only bother if it needs updating if (currentState !== selectionState) { - context.commit('UPDATE_SELECTION_STATE', { id, selectionState }); + context.commit('UPDATE_SELECTION_STATE', { id, selectionState, ancestorId }); } else if (!deep && !parents) { return; } @@ -246,27 +251,33 @@ export function setSelectionState(context, { id, selectionState, deep = true, pa const children = id === clipboardTreeId ? context.getters.channelIds - : context.getters.getClipboardChildren(id).map(c => c.id); - + : context.getters.getClipboardChildren(id, ancestorId).map(c => c.id); // Update descendants - children.forEach(id => { + children.forEach(cid => { context.dispatch('setSelectionState', { - id, + id: cid, selectionState: selectionState === ALL ? ALL : SelectionFlags.NONE, parents: false, + ancestorId: context.getters.isClipboardNode(cid) + ? null + : context.getters.isClipboardNode(id) + ? id + : ancestorId, }); }); } - const parentId = context.getters.getClipboardParentId(id); + const parentId = context.getters.getClipboardParentId(id, ancestorId); if (parentId && parents) { // Updating the parents at this point is fairly easy, we just recurse upwards, recomputing // the new state for the parent + const parentAncestorId = ancestorId === parentId ? null : ancestorId; context.dispatch('setSelectionState', { id: parentId, - selectionState: context.getters.getSelectionState(parentId), + selectionState: context.getters.getSelectionState(parentId, parentAncestorId), deep: false, + ancestorId: parentAncestorId, }); } } @@ -283,9 +294,28 @@ export function resetSelectionState(context) { }); } -export function deleteClipboardNode(context, clipboardNodeId) { - return Clipboard.delete(clipboardNodeId).then(() => { - context.commit('REMOVE_CLIPBOARD_NODE', { id: clipboardNodeId }); +export function deleteClipboardNode(context, { clipboardNodeId, ancestorId = null }) { + if (context.getters.isClipboardNode(clipboardNodeId)) { + return Clipboard.delete(clipboardNodeId).then(() => { + context.commit('REMOVE_CLIPBOARD_NODE', { id: clipboardNodeId }); + }); + } + const ancestor = context.state.clipboardNodesMap[ancestorId]; + const excluded_descendants = { + ...(ancestor.extra_fields.excluded_descendants || {}), + [clipboardNodeId]: true, + }; + const update = { + extra_fields: { + excluded_descendants, + }, + }; + // Otherwise we have a non clipboard node + return Clipboard.update(ancestorId).then(() => { + context.commit('ADD_CLIPBOARD_NODE', { + ...ancestor, + ...update, + }); }); } diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/getters.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/getters.js index afbf38afc4..018d43af8d 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/getters.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/getters.js @@ -2,6 +2,7 @@ import isFunction from 'lodash/isFunction'; import uniq from 'lodash/uniq'; import sortBy from 'lodash/sortBy'; import { SelectionFlags } from './constants'; +import { selectionId } from './utils'; /** * Several of these handle the clipboard root and channel ID's because we're @@ -32,6 +33,12 @@ export function hasClipboardChildren(state, getters) { }; } +export function isClipboardNode(state) { + return function(id) { + return Boolean(state.clipboardNodesMap[id]); + }; +} + export function getClipboardChildren(state, getters, rootState, rootGetters) { /** * Get the children of any "node" ID in the clipboard. This ID could @@ -39,7 +46,7 @@ export function getClipboardChildren(state, getters, rootState, rootGetters) { * * @param {string} id */ - return function(id) { + return function(id, ancestorId = null) { const rootId = rootGetters['clipboardRootId']; if (id === rootId) { @@ -62,7 +69,13 @@ export function getClipboardChildren(state, getters, rootState, rootGetters) { } const contentNode = getters.getClipboardNodeForRender(id); if (contentNode && contentNode.total_count) { - return rootGetters['contentNode/getContentNodeChildren'](contentNode.id); + // Last clipboard node ancestor could either be further up the chain or this node + const ancestor = state.clipboardNodesMap[ancestorId] || state.clipboardNodesMap[id]; + const children = rootGetters['contentNode/getContentNodeChildren'](contentNode.id); + if (ancestor) { + return children.filter(c => !ancestor.extra_fields.excluded_descendants[c.id]); + } + return children; } return []; }; @@ -75,7 +88,7 @@ export function getClipboardParentId(state, getters, rootState, rootGetters) { * * @param {string} id */ - return function(id) { + return function(id, ancestorId = null) { const rootId = rootGetters['clipboardRootId']; if (id === rootId) { @@ -86,13 +99,28 @@ export function getClipboardParentId(state, getters, rootState, rootGetters) { return rootId; } + const clipboardNode = state.clipboardNodesMap[id]; + + if (clipboardNode) { + return clipboardNode.parent === rootId + ? clipboardNode.source_channel_id + : clipboardNode.parent; + } + const contentNode = rootGetters['contentNode/getContentNode'](id); if (!contentNode) { return null; } - return contentNode.parent === rootId ? contentNode.source_channel_id : contentNode.parent; + // Check if this contentNode is an immediate child of the clipboard ancestor + if (getters.getClipboardChildren(ancestorId).find(c => c.id === id)) { + return ancestorId; + } + + // Otherwise, this is a contentNode that is somewhere further down the tree, so just return + // its parent + return contentNode.parent; }; } @@ -261,15 +289,21 @@ export function getSelectionState(state, getters, rootState, rootGetters) { * * @param {string} id */ - return function(id) { + return function(id, ancestorId = null) { const rootId = rootGetters['clipboardRootId']; // Start with simply just 0 or 1, selection state - let selectionState = state.selected[id] & SelectionFlags.SELECTED; + let selectionState = state.selected[selectionId(id, ancestorId)] & SelectionFlags.SELECTED; // Compute the children state, if any + const children = getters.getClipboardChildren(id, ancestorId); + // Calculate the ancestor Id of the children, if they are clipboard nodes, + // then they have none, otherwise, either any passed in ancestorId or the id of this node + const childAncestorId = + children.length && getters.isClipboardNode(children[0].id) ? null : ancestorId || id; const childrenState = getters.getSelectionStateFromIds( - getters.getClipboardChildren(id).map(c => c.id) + children.map(c => c.id), + childAncestorId ); // No children state, we're done @@ -303,14 +337,14 @@ export function getSelectionStateFromIds(state, getters) { * * @param {string[]} ids */ - return function(ids) { - const states = ids.map(id => getters.getSelectionState(id)); - + return function(ids, ancestorId = null) { if (!ids.length) { // undefined state return null; } + const states = ids.map(id => getters.getSelectionState(id, ancestorId)); + // If all states are none, then none if (!states.find(s => s > SelectionFlags.NONE)) { return SelectionFlags.NONE; @@ -344,9 +378,9 @@ export function getNextSelectionState(state, getters) { * * @param {string} id */ - return function(id) { + return function(id, ancestorId = null) { // Compute the current state - const current = getters.getSelectionState(id); + const current = getters.getSelectionState(id, ancestorId); // Carefully determine each bit/piece of the state // Be sure to cast to bool because if we compare ===, we want true/false values diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/mutations.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/mutations.js index bbf1b2a6d6..ba0b6f5104 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/mutations.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/mutations.js @@ -1,4 +1,5 @@ import Vue from 'vue'; +import { selectionId } from './utils'; export function ADD_CHANNEL(state, channel) { Vue.set(state.channelMap, channel.id, channel); @@ -8,8 +9,8 @@ export function ADD_CHANNEL_COLOR(state, { id, color }) { Vue.set(state.channelColors, id, color); } -export function UPDATE_SELECTION_STATE(state, { id, selectionState }) { - Vue.set(state.selected, id, selectionState); +export function UPDATE_SELECTION_STATE(state, { id, selectionState, ancestorId = null } = {}) { + Vue.set(state.selected, selectionId(id, ancestorId), selectionState); } export function ADD_CLIPBOARD_NODE(state, clipboardNode) { diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/utils.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/utils.js new file mode 100644 index 0000000000..95379f41bd --- /dev/null +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/utils.js @@ -0,0 +1,3 @@ +export function selectionId(id, ancestorId = null) { + return ancestorId ? `${id}-${ancestorId}` : id; +} diff --git a/contentcuration/contentcuration/viewsets/clipboard.py b/contentcuration/contentcuration/viewsets/clipboard.py index 214f6fb0c4..f194099536 100644 --- a/contentcuration/contentcuration/viewsets/clipboard.py +++ b/contentcuration/contentcuration/viewsets/clipboard.py @@ -47,6 +47,7 @@ class Meta: "parent", "source_node_id", "source_channel_id", + "extra_fields", ) list_serializer_class = BulkListSerializer @@ -86,6 +87,7 @@ class ClipboardViewSet(ValuesViewset): "id", "source_node_id", "source_channel_id", + "extra_fields", "parent", "lft", "total_count", From 22d0bbe8e8e5a8e6d9b9e71bc1a2e0cd3232ea5e Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Tue, 8 Sep 2020 13:22:35 -0700 Subject: [PATCH 26/28] Update duplicate clipboard node logic to handle shallow and deep copies. --- .../components/Clipboard/index.vue | 6 +- .../channelEdit/vuex/clipboard/actions.js | 7 +- .../channelEdit/vuex/clipboard/getters.js | 117 ++++++++++++------ .../channelEdit/vuex/clipboard/utils.js | 4 + .../frontend/shared/data/resources.js | 7 +- 5 files changed, 95 insertions(+), 46 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/index.vue b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/index.vue index 7352445e56..38c4dfa71f 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/index.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/index.vue @@ -139,7 +139,7 @@ 'getCopyTrees', ]), selectedNodeIds() { - return this.selectedNodes.map(n => n.id); + return this.selectedNodes.map(([sid]) => sid); }, canEdit() { return !this.selectedChannels.find(channel => !channel.edit); @@ -184,10 +184,10 @@ return; } - this.setMoveNodes(this.selectedNodes.map(n => n.source_id)); + this.setMoveNodes(this.selectedNodes.map(([, n]) => n.id)); }, duplicateNodes: withChangeTracker(function(changeTracker) { - const trees = this.getCopyTrees(this.clipboardRootId); + const trees = this.getCopyTrees(); if (!trees.length) { return Promise.resolve([]); diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js index 1f08dc402a..3b754ebf91 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/actions.js @@ -179,12 +179,15 @@ export function loadChannelColors(context) { * @param children * @return {*} */ -export function copy(context, { node_id, channel_id, children = [], parent = null }) { +export function copy( + context, + { node_id, channel_id, children = [], parent = null, extra_fields = null } +) { const clipboardRootId = context.rootGetters['clipboardRootId']; // This copies a "bare" copy, if you want a full content node copy, // go to the contentNode state actions - return Clipboard.copy(node_id, channel_id, clipboardRootId, parent).then(node => { + return Clipboard.copy(node_id, channel_id, clipboardRootId, parent, extra_fields).then(node => { if (!children.length) { // Refresh our channel list following the copy context.dispatch('loadChannels'); diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/getters.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/getters.js index 018d43af8d..bf181836e7 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/getters.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/getters.js @@ -1,8 +1,9 @@ +import flatten from 'lodash/flatten'; import isFunction from 'lodash/isFunction'; import uniq from 'lodash/uniq'; import sortBy from 'lodash/sortBy'; import { SelectionFlags } from './constants'; -import { selectionId } from './utils'; +import { selectionId, idFromSelectionId } from './utils'; /** * Several of these handle the clipboard root and channel ID's because we're @@ -205,11 +206,14 @@ export function filterSelectionIds(state) { /** * List of the selected tree nodes on the clipboard */ -export function selectedNodes(state, getters, rootState, rootGetters) { +export function selectedNodes(state, getters) { return getters .filterSelectionIds(SelectionFlags.SELECTED) - .map(id => rootGetters['contentNode/getContentNode'](id)) - .filter(Boolean); + .map(selectionId => [ + selectionId, + getters.getClipboardNodeForRender(idFromSelectionId(selectionId)), + ]) + .filter(p => Boolean(p[1])); } /** @@ -217,7 +221,7 @@ export function selectedNodes(state, getters, rootState, rootGetters) { */ export function selectedChannels(state, getters, rootState, rootGetters) { return getters.selectedNodes - .map(node => node.source_channel_id) + .map(([, node]) => node.channel_id) .filter(Boolean) .reduce((channelIds, channelId) => { if (!channelIds.includes(channelId)) { @@ -300,7 +304,11 @@ export function getSelectionState(state, getters, rootState, rootGetters) { // Calculate the ancestor Id of the children, if they are clipboard nodes, // then they have none, otherwise, either any passed in ancestorId or the id of this node const childAncestorId = - children.length && getters.isClipboardNode(children[0].id) ? null : ancestorId || id; + children.length && getters.isClipboardNode(children[0].id) + ? null + : getters.isClipboardNode(id) + ? id + : ancestorId; const childrenState = getters.getSelectionStateFromIds( children.map(c => c.id), childAncestorId @@ -409,49 +417,82 @@ export function getNextSelectionState(state, getters) { }; } -export function getCopyTrees(state, getters) { +export function getCopyTrees(state, getters, rootState, rootGetters) { /** * Creates an array of "trees" of copy call arguments from the current selection * * @param {string} target * @return {[{ id: Number, deep: Boolean, target: string, children: [] }]} */ - return function(target) { - const selectedNodes = getters.selectedNodes; + return function() { + const rootId = rootGetters['clipboardRootId']; - if (!selectedNodes.length) { - return []; + function recurseForUnselectedIds(id, ancestorId) { + const selectionState = getters.getSelectionState(id, ancestorId); + // Nothing is selected, so return early. + if (selectionState === SelectionFlags.NONE) { + return [id]; + } + return flatten( + getters + .getClipboardChildren(id, ancestorId) + .map(c => recurseForUnselectedIds(c.id, ancestorId)) + ); } - // Ensure we go down in tree order - const updates = sortBy(selectedNodes, 'lft').reduce((updates, selectedNode) => { - const selectionState = getters.currentSelectionState(selectedNode.id); - const update = { - node_id: selectedNode.source_node_id, - channel_id: selectedNode.source_channel_id, - target, - deep: Boolean(selectionState & SelectionFlags.ALL_DESCENDANTS), - children: [], - ignore: false, - }; - - if (selectedNode.parent in updates) { - const parentUpdate = updates[selectedNode.parent]; - - if (!parentUpdate.deep) { - // Target will be set after parent is copied - update.target = null; - parentUpdate.children.push(update); + function recurseforCopy(id, ancestorId = null, target = rootId) { + const selectionState = getters.getSelectionState(id, ancestorId); + // Nothing is selected, so return early. + if (selectionState === SelectionFlags.NONE) { + return []; + } + const children = getters.getClipboardChildren(id, ancestorId); + // Calculate the ancestor Id of the children, if they are clipboard nodes, + // then they have none, otherwise, either any passed in ancestorId or the id of this node + const childrenAreClipboardNodes = children.length && getters.isClipboardNode(children[0].id); + const childAncestorId = childrenAreClipboardNodes + ? null + : getters.isClipboardNode(id) + ? id + : ancestorId; + if (selectionState & SelectionFlags.SELECTED) { + // Node itself is selected, so this can be a starting point in a tree node + const selectedNode = getters.getClipboardNodeForRender(id); + const update = { + node_id: selectedNode.node_id, + channel_id: selectedNode.channel_id, + target, + }; + if (childrenAreClipboardNodes) { + update.children = children + .map(c => recurseforCopy(c.id, childAncestorId, id)) + .filter(Boolean); + } else if (children.length) { + // We have copied the parent as a clipboard node, and the children are content nodes + // can now switch mode to just return ids + const sourceClipboardNode = state.clipboardNodesMap[id]; + const excluded_descendants = {}; + if (sourceClipboardNode) { + for (let key in sourceClipboardNode.extra_fields.excluded_descendants) { + excluded_descendants[key] = true; + } + } + for (let child of children) { + for (let key of recurseForUnselectedIds(child.id)) { + excluded_descendants[key] = true; + } + } + update.extra_fields = { + excluded_descendants, + }; } - - // We'll mark to ignore which doesn't affect the update appended to the children above - update.ignore = true; + return [update]; } + return flatten(children.map(c => recurseforCopy(c.id, childAncestorId, target))).filter( + Boolean + ); + } - updates[selectedNode.id] = update; - return updates; - }, {}); - - return Object.values(updates).filter(update => !update.ignore); + return recurseforCopy(rootId); }; } diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/utils.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/utils.js index 95379f41bd..de7dbdf910 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/utils.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/clipboard/utils.js @@ -1,3 +1,7 @@ export function selectionId(id, ancestorId = null) { return ancestorId ? `${id}-${ancestorId}` : id; } + +export function idFromSelectionId(selectionId) { + return selectionId.split('-')[0]; +} diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 5dabcffdd8..f176037ca3 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -1249,13 +1249,13 @@ export const Clipboard = new Resource({ tableName: TABLE_NAMES.CLIPBOARD, urlName: 'clipboard', indexFields: ['parent'], - copy(node_id, channel_id, clipboardRootId, parent = null) { + copy(node_id, channel_id, clipboardRootId, parent = null, extra_fields = null) { return this.transaction('rw', TABLE_NAMES.CONTENTNODE, () => { - return this.tableCopy(node_id, channel_id, clipboardRootId, parent); + return this.tableCopy(node_id, channel_id, clipboardRootId, parent, extra_fields); }); }, - tableCopy(node_id, channel_id, clipboardRootId, parent = null) { + tableCopy(node_id, channel_id, clipboardRootId, parent = null, extra_fields = null) { parent = parent || clipboardRootId; return this.table .where({ parent }) @@ -1282,6 +1282,7 @@ export const Clipboard = new Resource({ root_id: clipboardRootId, kind: node.kind, parent, + extra_fields, }; return this.table.put(data).then(() => ({ // Return the id along with the data for further processing From 70c76570b0902be0d62d5cf14d3c355e00988c98 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Tue, 8 Sep 2020 17:26:51 -0700 Subject: [PATCH 27/28] Make updates for making full copies to content nodes from a shallow clipboard copy. --- .../Clipboard/ContentNodeOptions.vue | 10 ++++-- .../components/Clipboard/index.vue | 15 +++++---- .../channelEdit/components/move/MoveModal.vue | 32 +++++++++++++++++-- .../views/TreeView/TreeViewBase.vue | 5 ++- .../channelEdit/vuex/clipboard/actions.js | 9 ++++-- .../channelEdit/vuex/clipboard/getters.js | 27 +++++++++------- .../channelEdit/vuex/clipboard/index.js | 2 ++ .../channelEdit/vuex/clipboard/mutations.js | 4 +++ .../channelEdit/vuex/contentNode/actions.js | 18 ++++++++++- 9 files changed, 90 insertions(+), 32 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/ContentNodeOptions.vue b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/ContentNodeOptions.vue index 17b456a3f8..1e984e3d07 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/ContentNodeOptions.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/ContentNodeOptions.vue @@ -7,7 +7,7 @@ {{ $tr('makeACopy') }} - + {{ $tr('moveTo') }} @@ -37,7 +37,7 @@ }, computed: { ...mapGetters('channel', ['getChannel']), - ...mapGetters('clipboard', ['getClipboardNodeForRender']), + ...mapGetters('clipboard', ['getClipboardNodeForRender', 'getCopyTrees']), node() { return this.getClipboardNodeForRender(this.nodeId); }, @@ -60,7 +60,11 @@ methods: { ...mapActions(['showSnackbar']), ...mapActions('clipboard', ['copy', 'deleteClipboardNode']), - ...mapMutations('contentNode', { setMoveNodes: 'SET_MOVE_NODES' }), + ...mapMutations('clipboard', { setCopyNodes: 'SET_CLIPBOARD_MOVE_NODES' }), + moveNode() { + const copyTrees = this.getCopyTrees(this.nodeId, null, this.ancestorId, true); + this.setCopyNodes(copyTrees); + }, removeNode: withChangeTracker(function(changeTracker) { this.showSnackbar({ duration: null, diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/index.vue b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/index.vue index 38c4dfa71f..b260a5b74b 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/index.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/Clipboard/index.vue @@ -166,7 +166,7 @@ }, methods: { ...mapActions(['showSnackbar']), - ...mapMutations('contentNode', { setMoveNodes: 'SET_MOVE_NODES' }), + ...mapMutations('clipboard', { setCopyNodes: 'SET_CLIPBOARD_MOVE_NODES' }), ...mapActions('clipboard', ['loadChannels', 'copy', 'deleteClipboardNodes']), refresh() { if (this.refreshing) { @@ -180,14 +180,15 @@ this.elevated = this.$refs.nodeList.scrollTop > 0; }, moveNodes() { - if (!this.selectedNodeIds.length) { + const trees = this.getCopyTrees(this.clipboardRootId); + if (!trees.length) { return; } - this.setMoveNodes(this.selectedNodes.map(([, n]) => n.id)); + this.setCopyNodes(trees); }, duplicateNodes: withChangeTracker(function(changeTracker) { - const trees = this.getCopyTrees(); + const trees = this.getCopyTrees(this.clipboardRootId, this.clipboardRootId); if (!trees.length) { return Promise.resolve([]); @@ -212,9 +213,9 @@ }); }), removeNodes: withChangeTracker(function(changeTracker) { - const id__in = this.selectedNodeIds; + const selectionIds = this.selectedNodeIds; - if (!id__in.length) { + if (!selectionIds.length) { return Promise.resolve([]); } @@ -225,7 +226,7 @@ actionCallback: () => changeTracker.revert(), }); - return this.deleteClipboardNodes(id__in).then(() => { + return this.deleteClipboardNodes(selectionIds).then(() => { return this.showSnackbar({ text: this.$tr('removedFromClipboard'), actionText: this.$tr('undo'), diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/move/MoveModal.vue b/contentcuration/contentcuration/frontend/channelEdit/components/move/MoveModal.vue index 5695d97515..8b6e2ca9b3 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/move/MoveModal.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/move/MoveModal.vue @@ -162,6 +162,7 @@ }, computed: { ...mapState('contentNode', { moveNodeIds: 'moveNodes' }), + ...mapState('clipboard', { copyNodes: 'clipboardMoveNodes' }), ...mapGetters('currentChannel', ['currentChannel', 'rootId']), ...mapGetters('contentNode', [ 'getContentNode', @@ -171,11 +172,12 @@ ]), dialog: { get() { - return Boolean(this.moveNodeIds.length); + return Boolean(this.moveNodeIds.length || this.copyNodes.length); }, set(value) { if (!value) { this.setMoveNodes([]); + this.setCopyNodes([]); } }, }, @@ -208,8 +210,15 @@ this.targetNodeId = this.currentLocationId || this.rootId; }, methods: { - ...mapActions('contentNode', ['createContentNode', 'loadChildren', 'moveContentNodes']), + ...mapActions('contentNode', [ + 'createContentNode', + 'loadChildren', + 'moveContentNodes', + 'copyContentNode', + ]), + ...mapActions('clipboard', ['deleteClipboardNodes']), ...mapMutations('contentNode', { setMoveNodes: 'SET_MOVE_NODES' }), + ...mapMutations('clipboard', { setCopyNodes: 'SET_CLIPBOARD_MOVE_NODES' }), isDisabled(node) { return this.moveNodeIds.includes(node.id); }, @@ -248,7 +257,24 @@ }); }, moveNodes() { - this.moveContentNodes({ id__in: this.moveNodeIds, parent: this.targetNodeId }).then(() => { + let promise; + if (this.moveNodeIds.length) { + promise = this.moveContentNodes({ id__in: this.moveNodeIds, parent: this.targetNodeId }); + } else if (this.copyNodes.length) { + promise = Promise.all( + this.copyNodes.map(copyNode => + this.copyContentNode({ + id: copyNode.id, + deep: copyNode.deep, + target: this.targetNodeId, + children: copyNode.children, + }) + ) + ).then(() => + this.deleteClipboardNodes(this.copyNodes.map(copyNode => copyNode.selectionId)) + ); + } + promise.then(() => { this.dialog = false; this.$store.dispatch('showSnackbar', { text: this.$tr('movedMessage', { title: this.currentNode.title }), diff --git a/contentcuration/contentcuration/frontend/channelEdit/views/TreeView/TreeViewBase.vue b/contentcuration/contentcuration/frontend/channelEdit/views/TreeView/TreeViewBase.vue index 7d834ae820..af080d901e 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/views/TreeView/TreeViewBase.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/views/TreeView/TreeViewBase.vue @@ -119,7 +119,7 @@ - + @@ -149,7 +149,7 @@