diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js index e5711b9b20..df048f8a5b 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js @@ -12,7 +12,7 @@ import { } from 'shared/data/constants'; import { ContentNode } from 'shared/data/resources'; import { ContentKindsNames } from 'shared/leUtils/ContentKinds'; -import { findLicense } from 'shared/utils/helpers'; +import { findLicense, getMergedMapFields } from 'shared/utils/helpers'; import { RolesNames } from 'shared/leUtils/Roles'; import { isNodeComplete } from 'shared/utils/validation'; import * as publicApi from 'shared/data/public'; @@ -340,16 +340,6 @@ function generateContentNodeData({ return contentNodeData; } -const mapFields = [ - 'accessibility_labels', - 'grade_levels', - 'learner_needs', - 'categories', - 'learning_activities', - 'resource_types', - 'tags', -]; - export function updateContentNode( context, { id, mergeMapFields, checkComplete = false, ...payload } = {} @@ -394,38 +384,10 @@ export function updateContentNode( } if (mergeMapFields) { - for (const mapField of mapFields) { - if (contentNodeData[mapField]) { - if (mapField === 'categories') { - // Reduce categories to the minimal set - const existingCategories = Object.keys(node.categories || {}); - const newCategories = Object.keys(contentNodeData.categories); - const newMap = {}; - for (const category of existingCategories) { - // If any of the new categories are more specific than the existing category, - // omit this. - if (!newCategories.some(newCategory => newCategory.startsWith(category))) { - newMap[category] = true; - } - } - for (const category of newCategories) { - if ( - !existingCategories.some( - existingCategory => - existingCategory.startsWith(category) && category !== existingCategory - ) - ) { - newMap[category] = true; - } - } - } else { - contentNodeData[mapField] = { - ...node[mapField], - ...contentNodeData[mapField], - }; - } - } - } + contentNodeData = { + ...contentNodeData, + ...getMergedMapFields(node, contentNodeData), + }; } if (checkComplete) { @@ -472,11 +434,12 @@ export function updateContentNodeDescendants(context, { id, ...payload } = {}) { const contentNodeData = generateContentNodeData(payload); const descendants = context.getters.getContentNodeDescendants(id); - const contentNodeIds = [id, ...descendants.map(node => node.id)]; + const contentNodes = [node, ...descendants]; - const contentNodesData = contentNodeIds.map(contentNodeId => ({ - id: contentNodeId, + const contentNodesData = contentNodes.map(contentNode => ({ + id: contentNode.id, ...contentNodeData, + ...getMergedMapFields(contentNode, contentNodeData), })); context.commit('ADD_CONTENTNODES', contentNodesData); diff --git a/contentcuration/contentcuration/frontend/shared/data/applyRemoteChanges.js b/contentcuration/contentcuration/frontend/shared/data/applyRemoteChanges.js index eaa03cd6c0..4c3676303d 100644 --- a/contentcuration/contentcuration/frontend/shared/data/applyRemoteChanges.js +++ b/contentcuration/contentcuration/frontend/shared/data/applyRemoteChanges.js @@ -228,12 +228,7 @@ class ReturnedChanges extends ChangeDispatcher { } return transaction(change, TABLE_NAMES.CONTENTNODE, async () => { - const ids = await resource.getLoadedDescendantsIds(change.key); - return db - .table(TABLE_NAMES.CONTENTNODE) - .where(':id') - .anyOf(ids) - .modify(obj => applyMods(obj, change.mods)); + return resource.applyChangesToLoadedDescendants(change.key, change.mods); }); } } diff --git a/contentcuration/contentcuration/frontend/shared/data/changes.js b/contentcuration/contentcuration/frontend/shared/data/changes.js index eca58ee4de..edceabe614 100644 --- a/contentcuration/contentcuration/frontend/shared/data/changes.js +++ b/contentcuration/contentcuration/frontend/shared/data/changes.js @@ -21,12 +21,6 @@ import { COPYING_STATUS, TASK_ID, } from 'shared/data/constants'; -import { - Categories, - ContentLevels, - ResourcesNeededTypes, - ResourcesNeededOptions, -} from 'shared/constants'; import { INDEXEDDB_RESOURCES } from 'shared/data/registry'; /** @@ -489,7 +483,6 @@ export class UpdatedDescendantsChange extends Change { this.validateObj(changes, 'changes'); changes = omitIgnoredSubFields(changes); this.mods = changes; - this.setModsDeletedProperties(); this.setChannelAndUserId(oldObj); } @@ -497,30 +490,6 @@ export class UpdatedDescendantsChange extends Change { return !isEmpty(this.mods); } - /** - * To ensure that the mods properties that are multi valued have set - * to true just the options that are present in the mods object. All - * other options are set to null. - */ - setModsDeletedProperties() { - if (!this.mods) return; - - const multiValueProperties = { - categories: Object.values(Categories), - learner_needs: ResourcesNeededOptions.map(option => ResourcesNeededTypes[option]), - grade_levels: Object.values(ContentLevels), - }; - Object.entries(multiValueProperties).forEach(([key, values]) => { - if (this.mods[key]) { - values.forEach(value => { - if (!this.mods[key][value]) { - this.mods[key][value] = null; - } - }); - } - }); - } - saveChange() { if (!this.changed) { return Promise.resolve(null); diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 25cd88fadf..78d6b4c88d 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -47,6 +47,7 @@ import { currentLanguage } from 'shared/i18n'; import client, { paramsSerializer } from 'shared/client'; import { DELAYED_VALIDATION, fileErrors, NEW_OBJECT } from 'shared/constants'; import { ContentKindsNames } from 'shared/leUtils/ContentKinds'; +import { getMergedMapFields } from 'shared/utils/helpers'; // Number of seconds after which data is considered stale. const REFRESH_INTERVAL = 5; @@ -257,6 +258,10 @@ class IndexedDBResource { inheritedChanges.push( ...parentChanges.map(change => ({ ...change, + mods: { + ...change.mods, + ...getMergedMapFields(item, change.mods), + }, key: item.id, type: CHANGE_TYPES.UPDATED, })) @@ -267,14 +272,24 @@ class IndexedDBResource { return inheritedChanges; } - mergeDescendantsChanges(changes, inheritedChanges) { + mergeDescendantsChanges(changes, inheritedChanges, itemData) { if (inheritedChanges.length) { changes.push(...inheritedChanges); changes = sortBy(changes, 'rev'); } changes .filter(change => change.type === CHANGE_TYPES.UPDATED_DESCENDANTS) - .forEach(change => (change.type = CHANGE_TYPES.UPDATED)); + .forEach(change => { + change.type = CHANGE_TYPES.UPDATED; + const item = itemData.find(i => i.id === change.key); + if (!item) { + return; + } + change.mods = { + ...change.mods, + ...getMergedMapFields(item, change.mods), + }; + }); return changes; } @@ -296,7 +311,7 @@ class IndexedDBResource { return Promise.all([changesPromise, inheritedChangesPromise, currentPromise]).then( ([changes, inheritedChanges, currents]) => { - changes = this.mergeDescendantsChanges(changes, inheritedChanges); + changes = this.mergeDescendantsChanges(changes, inheritedChanges, itemData); changes = mergeAllChanges(changes, true); const collectedChanges = collectChanges(changes)[this.tableName] || {}; for (const changeType of Object.keys(collectedChanges)) { @@ -1882,20 +1897,35 @@ export const ContentNode = new TreeResource({ * @returns {Promise} * */ - async getLoadedDescendantsIds(id) { + async getLoadedDescendants(id) { + const [node] = await this.table.where({ id }).toArray(); + if (!node) { + return []; + } const children = await this.table.where({ parent: id }).toArray(); if (!children.length) { - return [id]; + return [node]; } const descendants = await Promise.all( children.map(child => { if (child.kind === ContentKindsNames.TOPIC) { - return this.getLoadedDescendantsIds(child.id); + return this.getLoadedDescendants(child.id); } - return child.id; + return child; + }) + ); + return [node].concat(flatMap(descendants, d => d)); + }, + async applyChangesToLoadedDescendants(id, changes) { + const descendants = await this.getLoadedDescendants(id); + return Promise.all( + descendants.map(descendant => { + return this.table.update(descendant.id, { + ...changes, + ...getMergedMapFields(descendant, changes), + }); }) ); - return [id].concat(flatMap(descendants, d => d)); }, /** * Update a node and all its descendants that are already loaded in IndexedDB @@ -1907,12 +1937,7 @@ export const ContentNode = new TreeResource({ return this.transaction({ mode: 'rw' }, CHANGES_TABLE, async () => { changes = this._cleanNew(changes); - // Update node descendants that are already loaded - const ids = await this.getLoadedDescendantsIds(id); - await this.table - .where('id') - .anyOf(...ids) - .modify(changes); + await this.applyChangesToLoadedDescendants(id, changes); return this._updateDescendantsChange(id, changes); }); diff --git a/contentcuration/contentcuration/frontend/shared/utils/helpers.js b/contentcuration/contentcuration/frontend/shared/utils/helpers.js index 2e60701048..d39ed44976 100644 --- a/contentcuration/contentcuration/frontend/shared/utils/helpers.js +++ b/contentcuration/contentcuration/frontend/shared/utils/helpers.js @@ -575,3 +575,51 @@ export function hasMultipleFieldValues(array, field) { } return false; } + +export const mapFields = [ + 'accessibility_labels', + 'grade_levels', + 'learner_needs', + 'categories', + 'learning_activities', + 'resource_types', + 'tags', +]; + +export function getMergedMapFields(node, contentNodeData) { + const mergedMapFields = {}; + for (const mapField of mapFields) { + if (contentNodeData[mapField]) { + if (mapField === 'categories') { + // Reduce categories to the minimal set + const existingCategories = Object.keys(node.categories || {}); + const newCategories = Object.keys(contentNodeData.categories); + const newMap = {}; + for (const category of existingCategories) { + // If any of the new categories are more specific than the existing category, + // omit this. + if (!newCategories.some(newCategory => newCategory.startsWith(category))) { + newMap[category] = true; + } + } + for (const category of newCategories) { + if ( + !existingCategories.some( + existingCategory => + existingCategory.startsWith(category) && category !== existingCategory + ) + ) { + newMap[category] = true; + } + } + mergedMapFields[mapField] = newMap; + } else { + mergedMapFields[mapField] = { + ...node[mapField], + ...contentNodeData[mapField], + }; + } + } + } + return mergedMapFields; +} diff --git a/contentcuration/contentcuration/tests/viewsets/test_contentnode.py b/contentcuration/contentcuration/tests/viewsets/test_contentnode.py index 03a7656081..180d8b0e72 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_contentnode.py +++ b/contentcuration/contentcuration/tests/viewsets/test_contentnode.py @@ -580,8 +580,6 @@ def test_update_descendants_contentnode(self): root_node = testdata.tree(parent=self.channel.main_tree) descendants = root_node.get_descendants(include_self=True) - # Fix undefined extra_fields - descendants.exclude(kind_id=content_kinds.TOPIC).update(extra_fields={}) new_language = "es"