Merge pull request #4751 from AlexVelezLl/fix-bulk-descendants-boolea…

…n-maps

Fix bulk descendants boolean maps

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);

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);
     });
   }
 }

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,38 +483,13 @@ export class UpdatedDescendantsChange extends Change {
     this.validateObj(changes, 'changes');
     changes = omitIgnoredSubFields(changes);
     this.mods = changes;
-    this.setModsDeletedProperties();
     this.setChannelAndUserId(oldObj);
   }
 
   get changed() {
     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);

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<string[]>}
    *
    */
-  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);
     });

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;
+}

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"