Follow up inheritable modal fixes

contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeOptions.vue

@@ -282,9 +282,11 @@
         });
       },
       moveNode(target) {
-        return this.moveContentNodes({ id__in: [this.nodeId], parent: target }).then(
-          this.$refs.moveModal.moveComplete
-        );
+        return this.moveContentNodes({
+          id__in: [this.nodeId],
+          parent: target,
+          inherit: this.node.parent !== target,
+        }).then(this.$refs.moveModal.moveComplete);
       },
       getRemoveNodeRedirect() {
         // Returns a callback to do appropriate post-removal navigation
@@ -322,7 +324,7 @@
       removeNode: withChangeTracker(function(id__in, changeTracker) {
         this.trackAction('Delete');
         const redirect = this.getRemoveNodeRedirect();
-        return this.moveContentNodes({ id__in, parent: this.trashId }).then(() => {
+        return this.moveContentNodes({ id__in, parent: this.trashId, inherit: false }).then(() => {
           redirect();
           this.showSnackbar({
             text: this.$tr('removedItems'),

contentcuration/contentcuration/frontend/channelEdit/components/edit/InheritAncestorMetadataModal.vue

@@ -10,10 +10,10 @@
     @cancel="closed = true"
   >
     <div>
-      <p v-if="parentHasInheritableMetadata">
-        {{ $tr('inheritMetadataDescription') }}
-      </p>
       <div>
+        <p v-if="parentHasNonLanguageMetadata">
+          {{ $tr('inheritMetadataDescription') }}
+        </p>
         <KCheckbox
           v-if="!!inheritableMetadataItems.categories"
           key="categories"
@@ -104,9 +104,9 @@
         // as to be inherited or not by a previous interaction with the modal.
         return Boolean(
           this.parent &&
-            this.parent?.extra_fields?.inherit_metadata &&
+            this.parent?.extra_fields?.inherited_metadata &&
             Object.keys(this.inheritableMetadataItems).every(
-              field => !isUndefined(this.parent.extra_fields.inherit_metadata[field])
+              field => !isUndefined(this.parent.extra_fields.inherited_metadata[field])
             )
         );
       },
@@ -141,6 +141,11 @@
       fieldsToInherit() {
         return Object.keys(this.inheritableMetadataItems).filter(field => this.checks[field]);
       },
+      parentHasNonLanguageMetadata() {
+        return (
+          !isEmpty(this.categories) || !isEmpty(this.grade_levels) || !isEmpty(this.learner_needs)
+        );
+      },
       parentHasInheritableMetadata() {
         return !isEmpty(this.inheritableMetadataItems);
       },
@@ -198,10 +203,10 @@
           ContentNode.getAncestors(this.parent.id).then(ancestors => {
             for (const field of inheritableFields) {
               if (
-                this.parent.extra_fields.inherit_metadata &&
-                this.parent.extra_fields.inherit_metadata[field]
+                this.parent.extra_fields.inherited_metadata &&
+                this.parent.extra_fields.inherited_metadata[field]
               ) {
-                this.checks[field] = this.parent.extra_fields.inherit_metadata[field];
+                this.checks[field] = this.parent.extra_fields.inherited_metadata[field];
               }
             }
             this.categories = ancestors.reduce((acc, ancestor) => {
@@ -253,19 +258,19 @@
           // but just in case, return
           return;
         }
-        const inherit_metadata = {
-          ...(this.parent?.extra_fields.inherit_metadata || {}),
+        const inherited_metadata = {
+          ...(this.parent?.extra_fields.inherited_metadata || {}),
         };
         for (const field of inheritableFields) {
           if (this.inheritableMetadataItems[field]) {
             // Only store preferences for fields that have been shown to the user as inheritable
-            inherit_metadata[field] = this.checks[field];
+            inherited_metadata[field] = this.checks[field];
           }
         }
         this.updateContentNode({
           id: this.parent.id,
           extra_fields: {
-            inherit_metadata,
+            inherited_metadata,
           },
         });
       },

contentcuration/contentcuration/frontend/channelEdit/components/move/MoveModal.vue

@@ -135,7 +135,7 @@
 </template>
 <script>
 
-  import { mapGetters, mapActions, mapMutations } from 'vuex';
+  import { mapGetters, mapActions } from 'vuex';
   import ResourceDrawer from '../ResourceDrawer';
   import { RouteNames } from '../../constants';
   import NewTopicModal from './NewTopicModal';
@@ -248,7 +248,6 @@
     },
     methods: {
       ...mapActions('contentNode', ['createContentNode', 'loadChildren']),
-      ...mapMutations('contentNode', ['ADD_INHERITING_NODE']),
       isDisabled(node) {
         return this.moveNodeIds.includes(node.id);
       },
@@ -301,9 +300,6 @@
           actionText: this.$tr('goToLocationButton'),
           actionCallback: this.goToLocation,
         });
-        for (const nodeId of this.moveNodeIds) {
-          this.ADD_INHERITING_NODE(this.getContentNode(nodeId));
-        }
         this.moveNodesInProgress = false;
       },
     },

contentcuration/contentcuration/frontend/channelEdit/views/CurrentTopicView.vue

@@ -214,6 +214,7 @@
       </ResourceDrawer>
     </VLayout>
     <InheritAncestorMetadataModal
+      ref="inheritModal"
       :parent="inheritanceParent"
       @inherit="inheritMetadata"
     />
@@ -587,7 +588,7 @@
         'setQuickEditModal',
         'updateContentNode',
       ]),
-      ...mapMutations('contentNode', ['ADD_INHERITING_NODE', 'CLEAR_INHERITING_NODES']),
+      ...mapMutations('contentNode', ['CLEAR_INHERITING_NODES']),
       ...mapActions('clipboard', ['copyAll']),
       clearSelections() {
         this.selected = [];
@@ -613,6 +614,7 @@
           id__in: nodeIdsToMove,
           target: targetNode,
           position: targetPosition,
+          inherit: false,
         });
       },
 
@@ -785,14 +787,7 @@
           return this.moveContentNodes({
             ...payload,
             id__in: data.sources.map(s => s.metadata.id),
-          }).then(ids => {
-            if (this.topicId !== payload.target) {
-              // If we are not moving within the same parent
-              // trigger the node inheritance behaviour.
-              for (const id of ids) {
-                this.ADD_INHERITING_NODE(this.getContentNode(id));
-              }
-            }
+            inherit: !data.sources.some(s => s.metadata.parent === payload.target),
           });
         }
       },
@@ -807,10 +802,13 @@
           : RELATIVE_TREE_POSITIONS.RIGHT;
       },
       moveNodes(target) {
-        return this.moveContentNodes({ id__in: this.selected, parent: target }).then(() => {
-          this.clearSelections();
-          this.$refs.moveModal.moveComplete();
-        });
+        // Always inherit here, as the move modal doesn't allow moving to the same parent.
+        return this.moveContentNodes({ id__in: this.selected, parent: target, inherit: true }).then(
+          () => {
+            this.clearSelections();
+            this.$refs.moveModal.moveComplete();
+          }
+        );
       },
       removeNodes: withChangeTracker(function(id__in, changeTracker) {
         this.trackClickEvent('Delete');
@@ -819,7 +817,7 @@
         if (id__in.includes(this.$route.params.detailNodeId)) {
           nextRoute = { params: { detailNodeId: null } };
         }
-        return this.moveContentNodes({ id__in, parent: this.trashId }).then(() => {
+        return this.moveContentNodes({ id__in, parent: this.trashId, inherit: false }).then(() => {
           this.clearSelections();
           if (nextRoute) {
             this.$router.replace(nextRoute);
@@ -928,6 +926,12 @@
           this.updateContentNode({ id: nodeId, ...metadata, mergeMapFields: true });
         }
         this.CLEAR_INHERITING_NODES(nodeIds);
+        this.$nextTick(() => {
+          // Once the inheritance is complete, reset the modal closed state.
+          if (!this.inheritingNodes || this.inheritingNodes.length === 0) {
+            this.$refs.inheritModal?.resetClosed();
+          }
+        });
       },
     },
     $trs: {

contentcuration/contentcuration/frontend/channelEdit/views/trash/TrashModal.vue

@@ -235,7 +235,11 @@
         'loadAncestors',
       ]),
       moveNodes(target) {
-        return this.moveContentNodes({ id__in: this.selected, parent: target }).then(() => {
+        return this.moveContentNodes({
+          id__in: this.selected,
+          parent: target,
+          inherit: false,
+        }).then(() => {
           this.reset();
           this.$refs.moveModal && this.$refs.moveModal.moveComplete();
         });

contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js

@@ -322,8 +322,8 @@ function generateContentNodeData({
     if (extra_fields.suggested_duration_type) {
       contentNodeData.extra_fields.suggested_duration_type = extra_fields.suggested_duration_type;
     }
-    if (extra_fields.inherit_metadata) {
-      contentNodeData.extra_fields.inherit_metadata = extra_fields.inherit_metadata;
+    if (extra_fields.inherited_metadata) {
+      contentNodeData.extra_fields.inherited_metadata = { ...extra_fields.inherited_metadata };
     }
   }
   if (prerequisite !== NOVALUE) {
@@ -369,11 +369,11 @@ export function updateContentNode(context, { id, mergeMapFields, ...payload } =
       };
     }
 
-    if (contentNodeData.extra_fields.inherit_metadata) {
-      // Don't set inherit_metadata on non-topic nodes
+    if (contentNodeData.extra_fields.inherited_metadata) {
+      // Don't set inherited_metadata on non-topic nodes
       // as they cannot have children to bequeath metadata to
       if (node.kind !== ContentKindsNames.TOPIC) {
-        delete contentNodeData.extra_fields.inherit_metadata;
+        delete contentNodeData.extra_fields.inherited_metadata;
       }
     }
 
@@ -561,7 +561,7 @@ export function copyContentNodes(
 const PARENT_POSITIONS = [RELATIVE_TREE_POSITIONS.FIRST_CHILD, RELATIVE_TREE_POSITIONS.LAST_CHILD];
 export function moveContentNodes(
   context,
-  { id__in, parent, target = null, position = RELATIVE_TREE_POSITIONS.LAST_CHILD }
+  { id__in, parent, target = null, position = RELATIVE_TREE_POSITIONS.LAST_CHILD, inherit = true }
 ) {
   // Make sure use of parent vs target matches position param
   if (parent && !(PARENT_POSITIONS.indexOf(position) >= 0)) {
@@ -576,6 +576,9 @@ export function moveContentNodes(
     id__in.map(id => {
       return ContentNode.move(id, target, position).then(node => {
         context.commit('ADD_CONTENTNODE', node);
+        if (inherit) {
+          context.commit('ADD_INHERITING_NODE', node);
+        }
         return id;
       });
     })