From ff0cfda526cfe5383e403042c43ed8496ad5521d Mon Sep 17 00:00:00 2001 From: Marcella Maki Date: Thu, 26 Sep 2024 18:14:20 -0400 Subject: [PATCH 1/2] adds a flag to the updateContentNode action to determine whether or not we need to check for completion - required within the EditModal for validations to work properly, but not in other quick edit options. previously was setting to false incorrectly due to assessmentItems validation failing when we hadn't loaded the data --- .../components/ContentNodeEditListItem.vue | 1 + .../components/edit/DetailsTabView.vue | 10 ++++---- .../channelEdit/components/edit/EditModal.vue | 13 +++++++++-- .../channelEdit/vuex/contentNode/actions.js | 23 ++++++++++++++----- 4 files changed, 35 insertions(+), 12 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeEditListItem.vue b/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeEditListItem.vue index 36477d257c..54e46f8799 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeEditListItem.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeEditListItem.vue @@ -258,6 +258,7 @@ retryFailedCopy: withChangeTracker(function(changeTracker) { this.updateContentNode({ id: this.nodeId, + checkComplete: true, [COPYING_STATUS]: COPYING_STATUS_VALUES.COPYING, }); diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue b/contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue index 8c18d1fc70..053a5fd70c 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue @@ -786,10 +786,12 @@ saveFromDiffTracker(id) { if (this.diffTracker[id]) { this.changed = true; - return this.updateContentNode({ id, ...this.diffTracker[id] }).then(() => { - delete this.diffTracker[id]; - return this.changed; - }); + return this.updateContentNode({ id, checkComplete: true, ...this.diffTracker[id] }).then( + () => { + delete this.diffTracker[id]; + return this.changed; + } + ); } return Promise.resolve(this.changed); }, diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/edit/EditModal.vue b/contentcuration/contentcuration/frontend/channelEdit/components/edit/EditModal.vue index de914ef971..3e74a3a986 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/edit/EditModal.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/edit/EditModal.vue @@ -399,7 +399,11 @@ if (completeCheck !== node.complete) { validationPromises.push( - vm.updateContentNode({ id: nodeId, complete: completeCheck }) + vm.updateContentNode({ + id: nodeId, + complete: completeCheck, + checkComplete: true, + }) ); } }); @@ -578,7 +582,12 @@ inheritMetadata(metadata) { const setMetadata = () => { for (const nodeId of this.newNodeIds) { - this.updateContentNode({ id: nodeId, ...metadata, mergeMapFields: true }); + this.updateContentNode({ + id: nodeId, + ...metadata, + mergeMapFields: true, + checkComplete: true, + }); } this.newNodeIds = []; }; diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js index 9b0d8b3e3e..802313c76c 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js @@ -1,5 +1,6 @@ import flatMap from 'lodash/flatMap'; import uniq from 'lodash/uniq'; +import isEmpty from 'lodash/isEmpty'; import { NEW_OBJECT, NOVALUE, DescendantsUpdatableFields } from 'shared/constants'; import client from 'shared/client'; import { @@ -349,10 +350,16 @@ const mapFields = [ 'tags', ]; -export function updateContentNode(context, { id, mergeMapFields, ...payload } = {}) { +export function updateContentNode( + context, + { id, mergeMapFields, checkComplete = false, ...payload } = {} +) { if (!id) { throw ReferenceError('id must be defined to update a contentNode'); } + if (isEmpty(payload)) { + return Promise.resolve(); + } let contentNodeData = generateContentNodeData(payload); const node = context.getters.getContentNode(id); @@ -425,11 +432,15 @@ export function updateContentNode(context, { id, mergeMapFields, ...payload } = ...node, ...contentNodeData, }; - const complete = isNodeComplete({ - nodeDetails: newNode, - assessmentItems: context.rootGetters['assessmentItem/getAssessmentItems'](id), - files: context.rootGetters['file/getContentNodeFiles'](id), - }); + let complete = true; + if (checkComplete) { + complete = isNodeComplete({ + nodeDetails: newNode, + assessmentItems: context.rootGetters['assessmentItem/getAssessmentItems'](id), + files: context.rootGetters['file/getContentNodeFiles'](id), + }); + } + contentNodeData = { ...contentNodeData, complete, From 4f00c52322b57642eb851a7adeee39900f108c3b Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 26 Sep 2024 16:18:45 -0700 Subject: [PATCH 2/2] Don't set complete if completeCheck is false. Update tests. --- .../contentNode/__tests__/actions.spec.js | 28 ++++++++++++++++++- .../channelEdit/vuex/contentNode/actions.js | 20 ++++++------- 2 files changed, 36 insertions(+), 12 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 eedfa8a763..e25fe4ed5b 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/__tests__/actions.spec.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/__tests__/actions.spec.js @@ -111,7 +111,7 @@ describe('contentNode actions', () => { }); }); describe('updateContentNode action for an existing contentNode', () => { - it('should call ContentNode.update', () => { + it('should call ContentNode.update without complete if completeCheck is false', () => { store.commit('contentNode/ADD_CONTENTNODE', { id, title: 'test', @@ -125,6 +125,32 @@ describe('contentNode actions', () => { language: 'no', learning_activities: { test: true }, }) + .then(() => { + expect(updateSpy).toHaveBeenCalledWith(id, { + title: 'notatest', + description: 'very', + language: 'no', + changed: true, + learning_activities: { test: true }, + }); + updateSpy.mockRestore(); + }); + }); + it('should call ContentNode.update with complete false if completeCheck is true', () => { + store.commit('contentNode/ADD_CONTENTNODE', { + id, + title: 'test', + }); + const updateSpy = jest.spyOn(ContentNode, 'update'); + return store + .dispatch('contentNode/updateContentNode', { + id, + title: 'notatest', + description: 'very', + language: 'no', + learning_activities: { test: true }, + checkComplete: true, + }) .then(() => { expect(updateSpy).toHaveBeenCalledWith(id, { title: 'notatest', diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js index 802313c76c..f883725b71 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js @@ -428,24 +428,22 @@ export function updateContentNode( } } - const newNode = { - ...node, - ...contentNodeData, - }; - let complete = true; if (checkComplete) { - complete = isNodeComplete({ + const newNode = { + ...node, + ...contentNodeData, + }; + const complete = isNodeComplete({ nodeDetails: newNode, assessmentItems: context.rootGetters['assessmentItem/getAssessmentItems'](id), files: context.rootGetters['file/getContentNodeFiles'](id), }); + contentNodeData = { + ...contentNodeData, + complete, + }; } - contentNodeData = { - ...contentNodeData, - complete, - }; - context.commit('ADD_CONTENTNODE', { id, ...contentNodeData }); return ContentNode.update(id, contentNodeData); }