Skip to content

Commit

Permalink
Merge pull request #3590 from bjester/ensure-channel-id
Browse files Browse the repository at this point in the history
Ensure channel ID is set on move change to trash tree
  • Loading branch information
rtibbles authored Aug 29, 2022
2 parents 269e7e0 + 601b338 commit 28d993e
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ export function createContentNode(context, { parent, kind, ...payload }) {
learning_activities: {},
categories: {},
suggested_duration: 0,
channel_id: channel.id,
...payload,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,11 +277,12 @@ describe('ContentNode methods', () => {
where,
getNewSortOrder;
beforeEach(() => {
node = { id: uuid4(), title: 'Test node' };
node = { id: uuid4(), title: 'Test node', channel_id: uuid4() };
parent = {
id: uuid4(),
title: 'Test node parent',
root_id: uuid4(),
channel_id: uuid4(),
};
siblings = [];
resolveParent = mockMethod('resolveParent', () => Promise.resolve(parent));
Expand Down Expand Up @@ -331,6 +332,43 @@ describe('ContentNode methods', () => {
source: CLIENTID,
table: 'contentnode',
type: CHANGE_TYPES.MOVED,
channel_id: parent.channel_id,
},
});
});

it("should default `channel_id` to the node's", async () => {
let cb = jest.fn(() => Promise.resolve('results'));
parent.channel_id = null;
await expect(
ContentNode.resolveTreeInsert('abc123', 'target', 'position', false, cb)
).resolves.toEqual('results');
expect(resolveParent).toHaveBeenCalledWith('target', 'position');
expect(treeLock).toHaveBeenCalledWith(parent.root_id, expect.any(Function));
expect(get).toHaveBeenCalledWith('abc123', false);
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,
parent,
payload: {
id: 'abc123',
parent: parent.id,
lft: 1,
changed: true,
},
change: {
key: 'abc123',
from_key: null,
target: parent.id,
position: RELATIVE_TREE_POSITIONS.LAST_CHILD,
oldObj: node,
source: CLIENTID,
table: 'contentnode',
type: CHANGE_TYPES.MOVED,
channel_id: node.channel_id,
},
});
});
Expand Down Expand Up @@ -390,6 +428,22 @@ describe('ContentNode methods', () => {
expect(getNewSortOrder).toHaveBeenCalledWith('abc123', 'target', 'position', siblings);
expect(cb).not.toBeCalled();
});

it('should reject if null channel_id', async () => {
let cb = jest.fn(() => Promise.resolve('results'));
parent.channel_id = null;
node.channel_id = null;

await expect(
ContentNode.resolveTreeInsert('abc123', 'target', 'position', false, cb)
).rejects.toThrow('Missing channel_id for tree insertion change event');
expect(resolveParent).toHaveBeenCalledWith('target', 'position');
expect(treeLock).toHaveBeenCalledWith(parent.root_id, expect.any(Function));
expect(get).toHaveBeenCalledWith('abc123', false);
expect(where).toHaveBeenCalledWith({ parent: parent.id }, false);
expect(getNewSortOrder).not.toBeCalled();
expect(cb).not.toBeCalled();
});
});

describe('copying', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,24 @@ export const ContentNode = new TreeResource({
changed: true,
};

let channel_id = parent.channel_id;

// This should really only happen when this is a move operation to the trash tree.
// Other cases like copying shouldn't encounter this because they should always have the
// channel_id on the destination. The trash tree root node does not have the channel_id
// annotated on it, and using the source node's channel_id should be reliable
// in that case
if (!channel_id && node) {
channel_id = node.channel_id;

// The change would be disallowed anyway, so prevent the frontend from applying it
if (!channel_id) {
return Promise.reject(
new Error('Missing channel_id for tree insertion change event')
);
}
}

// Prep the change data tracked in the changes table
const change = {
key: payload.id,
Expand All @@ -1284,7 +1302,7 @@ export const ContentNode = new TreeResource({
source: CLIENTID,
table: this.tableName,
type: isCreate ? CHANGE_TYPES.COPIED : CHANGE_TYPES.MOVED,
channel_id: parent.channel_id,
channel_id,
};

return callback({
Expand Down

0 comments on commit 28d993e

Please sign in to comment.