From 2ecd7c643dff96f2f40ac12c1850aa9ef3e0461f Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Fri, 16 Jun 2023 20:44:16 +0300 Subject: [PATCH] Streamlines use of oldObj.parent --- .../__tests__/ContentNodeResource.spec.js | 6 +++--- .../shared/data/__tests__/changes.spec.js | 20 +++++++++---------- .../shared/data/applyRemoteChanges.js | 4 ++-- .../frontend/shared/data/changes.js | 4 ++-- .../frontend/shared/data/resources.js | 1 - 5 files changed, 16 insertions(+), 19 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/__tests__/ContentNodeResource.spec.js b/contentcuration/contentcuration/frontend/shared/data/__tests__/ContentNodeResource.spec.js index a7f58f81c2..514986558f 100644 --- a/contentcuration/contentcuration/frontend/shared/data/__tests__/ContentNodeResource.spec.js +++ b/contentcuration/contentcuration/frontend/shared/data/__tests__/ContentNodeResource.spec.js @@ -540,7 +540,7 @@ describe('ContentNode methods', () => { describe('tableMove method', () => { let node, - oldParent, + oldObj, parent, payload, change, @@ -553,9 +553,9 @@ describe('ContentNode methods', () => { put: jest.fn(() => Promise.resolve()), }; updated = true; - oldParent = { id: uuid4(), title: 'Parent' }; + oldObj = { id: uuid4(), title: 'Parent' }; parent = { id: uuid4(), root_id: uuid4(), title: 'Parent' }; - node = { id: uuid4(), parent: oldParent.id, title: 'Source node' }; + node = { id: uuid4(), parent: oldObj.id, title: 'Source node' }; payload = { id: uuid4(), parent: parent.id, changed: true, lft: 1, title: 'Payload' }; change = { key: payload.id, diff --git a/contentcuration/contentcuration/frontend/shared/data/__tests__/changes.spec.js b/contentcuration/contentcuration/frontend/shared/data/__tests__/changes.spec.js index 82cde0fc7d..13c4759263 100644 --- a/contentcuration/contentcuration/frontend/shared/data/__tests__/changes.spec.js +++ b/contentcuration/contentcuration/frontend/shared/data/__tests__/changes.spec.js @@ -182,30 +182,22 @@ describe('Change Types', () => { }); it('should persist only the specified fields in the MovedChange', async () => { + const oldObj = { parent: '4' }; const change = new MovedChange({ key: '1', table: TABLE_NAMES.CONTENTNODE, target: '2', position: RELATIVE_TREE_POSITIONS.LAST_CHILD, parent: '3', - oldParent: '4', source: CLIENTID, + oldObj, }); const rev = await change.saveChange(); const persistedChange = await db[CHANGES_TABLE].get(rev); expect(persistedChange).toEqual({ rev, channel_id, - ...pick(change, [ - 'type', - 'key', - 'table', - 'target', - 'position', - 'parent', - 'source', - 'oldParent', - ]), + ...pick(change, ['type', 'key', 'table', 'target', 'position', 'parent', 'source', 'oldObj']), }); }); @@ -427,6 +419,7 @@ describe('Change Types Unhappy Paths', () => { // MovedChange it('should throw error when MovedChange is instantiated without target', () => { + const oldObj = { a: 1, b: 2 }; expect( () => new MovedChange({ @@ -434,11 +427,13 @@ describe('Change Types Unhappy Paths', () => { table: TABLE_NAMES.CONTENTNODE, position: RELATIVE_TREE_POSITIONS.LAST_CHILD, source: CLIENTID, + oldObj, }) ).toThrow(new TypeError('target is required for a MovedChange but it was undefined')); }); it('should throw error when MovedChange is instantiated with incorrect position type', () => { + const oldObj = { a: 1, b: { c: 1 } }; expect( () => new MovedChange({ @@ -447,11 +442,13 @@ describe('Change Types Unhappy Paths', () => { target: '2', position: 'invalid', source: CLIENTID, + oldObj, }) ).toThrow(new ReferenceError('invalid is not a valid position value')); }); it('should throw error when MovedChange is instantiated without parent', () => { + const oldObj = { a: 1, b: 2, parent: 3 }; expect( () => new MovedChange({ @@ -460,6 +457,7 @@ describe('Change Types Unhappy Paths', () => { target: '2', position: RELATIVE_TREE_POSITIONS.LAST_CHILD, source: CLIENTID, + oldObj, }) ).toThrow(new ReferenceError('parent is required for a MovedChange but it was undefined')); }); diff --git a/contentcuration/contentcuration/frontend/shared/data/applyRemoteChanges.js b/contentcuration/contentcuration/frontend/shared/data/applyRemoteChanges.js index 109ab86f13..bed39ce5b1 100644 --- a/contentcuration/contentcuration/frontend/shared/data/applyRemoteChanges.js +++ b/contentcuration/contentcuration/frontend/shared/data/applyRemoteChanges.js @@ -295,13 +295,13 @@ class ResourceCounts extends ChangeDispatcher { */ async applyMove(change) { // Only if the node is being moved to a new parent do we need to update the ancestor counts - if (change.oldParent === change.parent) { + if (change.oldObj.parent === change.parent) { return; } const node = await this.table.get(change.key); await this.resource.updateAncestors( - { id: change.oldParent, includeSelf: true, ignoreChanges: true }, + { id: change.oldObj.parent, includeSelf: true, ignoreChanges: true }, this._applyDiff.bind(this, node, -1) ); await this.resource.updateAncestors( diff --git a/contentcuration/contentcuration/frontend/shared/data/changes.js b/contentcuration/contentcuration/frontend/shared/data/changes.js index 4129ea1515..f62a50a09e 100644 --- a/contentcuration/contentcuration/frontend/shared/data/changes.js +++ b/contentcuration/contentcuration/frontend/shared/data/changes.js @@ -383,7 +383,7 @@ export class DeletedChange extends Change { } export class MovedChange extends Change { - constructor({ target, position, parent, oldParent, ...fields }) { + constructor({ oldObj, target, position, parent, ...fields }) { fields.type = CHANGE_TYPES.MOVED; super(fields); if (this.table !== TABLE_NAMES.CONTENTNODE) { @@ -391,10 +391,10 @@ export class MovedChange extends Change { `${this.changeType} is only supported by ${TABLE_NAMES.CONTENTNODE} table but ${this.table} was passed instead` ); } + this.setAndValidateObj(oldObj, 'oldObj', omitIgnoredSubFields); this.setAndValidateIsDefined(target, 'target'); this.setAndValidateLookup(position, 'position', RELATIVE_TREE_POSITIONS_LOOKUP); this.setAndValidateIsDefined(parent, 'parent'); - this.setAndValidateIsDefined(oldParent, 'oldParent'); this.setChannelAndUserId(); } } diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index b18f62ec6a..4a7101f307 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -1395,7 +1395,6 @@ export const ContentNode = new TreeResource({ // so that we can avoid doing fetches while such changes // are pending. parent: parent.id, - oldParent: isCreate ? null : node.parent, oldObj: isCreate ? null : node, table: this.tableName, source: CLIENTID,