Misc fixes to channel editing experience

contentcuration/contentcuration/api.py

@@ -97,4 +97,4 @@ def activate_channel(channel, user):
             "staging_root_id": None
         },
         channel_id=channel.id,
-    ), applied=True)
+    ), applied=True, created_by_id=user.id)

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

@@ -421,8 +421,8 @@
               },
             };
           }
-          this.m = this.value.threshold.m || this.m;
-          this.n = this.value.threshold.n || this.n;
+          this.m = (this.value.threshold ? this.value.threshold.m : null) || this.m;
+          this.n = (this.value.threshold ? this.value.threshold.n : null) || this.n;
           this.handleInput(update);
         },
       },
@@ -450,6 +450,10 @@
       },
       masteryModelItem: {
         get() {
+          if (!this.value.threshold) {
+            return { m: null, n: null };
+          }
+
           if (this.value.threshold.mastery_model !== MasteryModelsNames.M_OF_N) {
             return {
               m: this.value.threshold.m,

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

@@ -248,7 +248,7 @@
     computed: {
       ...mapGetters('contentNode', ['getContentNode', 'getContentNodeIsValid']),
       ...mapGetters('assessmentItem', ['getAssessmentItems']),
-      ...mapGetters('currentChannel', ['canEdit']),
+      ...mapGetters('currentChannel', ['currentChannel', 'canEdit']),
       ...mapGetters('file', ['contentNodesAreUploading', 'getContentNodeFiles']),
       ...mapState({
         online: state => state.connection.online,
@@ -448,6 +448,7 @@
         return this.createContentNode({
           kind,
           parent: this.$route.params.nodeId,
+          channel_id: this.currentChannel.id,
           ...payload,
         }).then(newNodeId => {
           this.$router.push({

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

@@ -16,7 +16,7 @@ describe('contentNode actions', () => {
   let id;
   const contentNodeDatum = { title: 'test', parent: parentId, lft: 1, tags: {} };
   beforeEach(() => {
-    return ContentNode.put(contentNodeDatum).then(newId => {
+    return ContentNode._put(contentNodeDatum).then(newId => {
       id = newId;
       contentNodeDatum.id = newId;
       jest
@@ -25,7 +25,7 @@ describe('contentNode actions', () => {
       jest
         .spyOn(ContentNode, 'fetchModel')
         .mockImplementation(() => Promise.resolve(contentNodeDatum));
-      return ContentNode.put({ title: 'notatest', parent: newId, lft: 2 }).then(() => {
+      return ContentNode._put({ title: 'notatest', parent: newId, lft: 2 }).then(() => {
         store = storeFactory({
           modules: {
             assessmentItem,

contentcuration/contentcuration/frontend/shared/data/__tests__/ContentNodeResource.spec.js

@@ -183,7 +183,10 @@ describe('ContentNode methods', () => {
   });
 
   function mockMethod(name, implementation) {
-    const mock = jest.spyOn(ContentNode, name).mockImplementation(implementation);
+    const path = name.split('.');
+    const prop = path.pop();
+    const mockObj = path.reduce((mockObj, prop) => mockObj[prop], ContentNode);
+    const mock = jest.spyOn(mockObj, prop).mockImplementation(implementation);
     mocks.push(mock);
     return mock;
   }
@@ -270,6 +273,7 @@ describe('ContentNode methods', () => {
       resolveParent,
       treeLock,
       get,
+      tableGet,
       where,
       getNewSortOrder;
     beforeEach(() => {
@@ -284,6 +288,7 @@ describe('ContentNode methods', () => {
       treeLock = mockMethod('treeLock', (id, cb) => cb());
       getNewSortOrder = mockMethod('getNewSortOrder', () => lft);
       get = mockMethod('get', () => Promise.resolve(node));
+      tableGet = mockMethod('table.get', () => Promise.resolve());
       where = mockMethod('where', () => Promise.resolve(siblings));
     });
 
@@ -395,13 +400,13 @@ describe('ContentNode methods', () => {
         ).resolves.toEqual('results');
         expect(resolveParent).toHaveBeenCalledWith('target', 'position');
         expect(treeLock).toHaveBeenCalledWith(parent.root_id, expect.any(Function));
-        expect(get).toHaveBeenCalledWith('abc123', false);
+        expect(tableGet).toHaveBeenCalledWith('abc123');
         expect(where).toHaveBeenCalledWith({ parent: parent.id }, false);
         expect(getNewSortOrder).not.toBeCalled();
         expect(cb).toBeCalled();
         const result = cb.mock.calls[0][0];
         expect(result).toMatchObject({
-          node,
+          node: undefined,
           parent,
           payload: {
             id: expect.not.stringMatching('abc123'),
@@ -434,13 +439,13 @@ describe('ContentNode methods', () => {
         ).resolves.toEqual('results');
         expect(resolveParent).toHaveBeenCalledWith('target', 'position');
         expect(treeLock).toHaveBeenCalledWith(parent.root_id, expect.any(Function));
-        expect(get).toHaveBeenCalledWith('abc123', false);
+        expect(tableGet).toHaveBeenCalledWith('abc123');
         expect(where).toHaveBeenCalledWith({ parent: parent.id }, false);
         expect(getNewSortOrder).toHaveBeenCalledWith(null, 'target', 'position', siblings);
         expect(cb).toBeCalled();
         const result = cb.mock.calls[0][0];
         expect(result).toMatchObject({
-          node,
+          node: undefined,
           parent,
           payload: {
             id: expect.not.stringMatching('abc123'),
@@ -473,7 +478,7 @@ describe('ContentNode methods', () => {
         ).rejects.toThrow('New lft value evaluated to null');
         expect(resolveParent).toHaveBeenCalledWith('target', 'position');
         expect(treeLock).toHaveBeenCalledWith(parent.root_id, expect.any(Function));
-        expect(get).toHaveBeenCalledWith('abc123', false);
+        expect(tableGet).toHaveBeenCalledWith('abc123');
         expect(where).toHaveBeenCalledWith({ parent: parent.id }, false);
         expect(getNewSortOrder).toHaveBeenCalledWith(null, 'target', 'position', siblings);
         expect(cb).not.toBeCalled();

contentcuration/contentcuration/frontend/shared/data/resources.js

@@ -1245,8 +1245,11 @@ export const ContentNode = new TreeResource({
       // Using root_id, we'll keep this locked while we handle this, so no other operations
       // happen while we're potentially waiting for some data we need (siblings, source node)
       return this.treeLock(parent.root_id, () => {
+        // Don't trigger fetch, if this is specified as a creation
+        const getNode = isCreate ? this.table.get(id) : this.get(id, false);
+
         // Preload the ID we're referencing, and get siblings to determine sort order
-        return Promise.all([this.get(id, false), this.where({ parent: parent.id }, false)]).then(
+        return Promise.all([getNode, this.where({ parent: parent.id }, false)]).then(
           ([node, siblings]) => {
             let lft = 1;
             if (siblings.length) {
@@ -1266,6 +1269,7 @@ export const ContentNode = new TreeResource({
             const payload = {
               id: isCreate ? uuid4() : id,
               parent: parent.id,
+              root_id: parent.root_id,
               lft,
               changed: true,
             };
@@ -1295,6 +1299,29 @@ export const ContentNode = new TreeResource({
     });
   },
 
+  // Retain super's put method that does not handle tree insertion
+  _put: TreeResource.prototype.put,
+
+  /**
+   * @param {Object} obj
+   * @return {Promise<string>}
+   */
+  put(obj) {
+    const prepared = this._preparePut(obj);
+
+    return this.resolveTreeInsert(
+      prepared.id,
+      prepared.parent,
+      RELATIVE_TREE_POSITIONS.LAST_CHILD,
+      true,
+      data => {
+        return this.transaction({ mode: 'rw' }, () => {
+          return this.table.put({ ...prepared, ...data.payload });
+        });
+      }
+    );
+  },
+
   move(id, target, position = RELATIVE_TREE_POSITIONS.FIRST_CHILD) {
     return this.resolveTreeInsert(id, target, position, false, data => {
       // Ignore changes from this operation except for the explicit move change we generate.

contentcuration/contentcuration/frontend/shared/utils/validation.js

@@ -1,4 +1,5 @@
 import get from 'lodash/get';
+import CompletionCriteriaModels from 'kolibri-constants/CompletionCriteria';
 import translator from '../translator';
 import { AssessmentItemTypes, ValidationErrors } from '../constants';
 import Licenses from 'shared/leUtils/Licenses';
@@ -52,24 +53,36 @@ export function isNodeComplete({ nodeDetails, assessmentItems, files }) {
   }
 
   if (getNodeDetailsErrors(nodeDetails).length) {
+    if (process.env.NODE_ENV !== 'production') {
+      console.info('Node is incomplete', getNodeDetailsErrors(nodeDetails));
+    }
     return false;
   }
   if (
     nodeDetails.kind !== ContentKindsNames.TOPIC &&
     nodeDetails.kind !== ContentKindsNames.EXERCISE
   ) {
     if (getNodeFilesErrors(files).length) {
+      if (process.env.NODE_ENV !== 'production') {
+        console.info("Node's files are incomplete", getNodeFilesErrors(files));
+      }
       return false;
     }
   }
   if (nodeDetails.kind !== ContentKindsNames.TOPIC) {
     const completionCriteria = get(nodeDetails, 'extra_fields.options.completion_criteria');
     if (completionCriteria && !validateCompletionCriteria(completionCriteria, nodeDetails.kind)) {
+      if (process.env.NODE_ENV !== 'production') {
+        console.info("Node's completion criteria is invalid", validateCompletionCriteria.errors);
+      }
       return false;
     }
   }
   if (nodeDetails.kind === ContentKindsNames.EXERCISE) {
     if (!assessmentItems.length) {
+      if (process.env.NODE_ENV !== 'production') {
+        console.info('Exercise node is missing assessment items');
+      }
       return false;
     }
 
@@ -78,6 +91,12 @@ export function isNodeComplete({ nodeDetails, assessmentItems, files }) {
       return getAssessmentItemErrors(sanitizedAssessmentItem).length;
     };
     if (assessmentItems.some(isInvalid)) {
+      if (process.env.NODE_ENV !== 'production') {
+        console.info(
+          "Exercise node's assessment items are invalid",
+          assessmentItems.some(isInvalid)
+        );
+      }
       return false;
     }
   }
@@ -91,7 +110,11 @@ function _getLicense(node) {
 }
 
 function _getMasteryModel(node) {
-  return node.extra_fields;
+  const criteria = get(node, 'extra_fields.options.completion_criteria', {});
+  if (criteria.model === CompletionCriteriaModels.MASTERY) {
+    return criteria.threshold || {};
+  }
+  return {};
 }
 
 function _getLearningActivity(node) {