From e13dfe220d04b652a22edc288351f809f4f8e5ab Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Wed, 12 Mar 2025 13:48:53 +0100 Subject: [PATCH 1/4] fix: Specify uncategorized flows by using not-owned folders --- .../src/controllers/api/v1/flows/get-flows.js | 7 ++++++- packages/backend/src/models/user.js | 12 ++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/controllers/api/v1/flows/get-flows.js b/packages/backend/src/controllers/api/v1/flows/get-flows.js index e4951cce..98373ce5 100644 --- a/packages/backend/src/controllers/api/v1/flows/get-flows.js +++ b/packages/backend/src/controllers/api/v1/flows/get-flows.js @@ -3,8 +3,13 @@ import paginateRest from '../../../../helpers/pagination-rest.js'; export default async (request, response) => { await request.currentUser.hasFolderAccess(request.body.folderId); + const currentUserFolderIds = await request.currentUser.getFolderIds(); + + const flowsQuery = request.currentUser.getFlows( + flowParams(request), + currentUserFolderIds + ); - const flowsQuery = request.currentUser.getFlows(flowParams(request)); const flows = await paginateRest(flowsQuery, request.query.page); renderObject(response, flows); diff --git a/packages/backend/src/models/user.js b/packages/backend/src/models/user.js index 27bbe8b2..98af0fc3 100644 --- a/packages/backend/src/models/user.js +++ b/packages/backend/src/models/user.js @@ -525,7 +525,13 @@ class User extends Base { return true; } - getFlows({ folderId, name }) { + async getFolderIds() { + const folders = await this.$relatedQuery('folders').select('id'); + + return folders.map((folder) => folder.id); + } + + getFlows({ folderId, name }, ownedFolderIds) { return this.authorizedFlows .clone() .withGraphFetched({ @@ -537,7 +543,9 @@ class User extends Base { } if (folderId === 'null') { - builder.whereNull('flows.folder_id'); + builder + .whereNull('flows.folder_id') + .orWhereNotIn('flows.folder_id', ownedFolderIds); } else if (folderId) { builder.where('flows.folder_id', folderId); } From f616ec5c8db05374d7daf05dc2fcca0d5f7e91bc Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Wed, 12 Mar 2025 16:12:59 +0100 Subject: [PATCH 2/4] test: Update getFlows test for user model --- packages/backend/src/models/user.test.js | 79 +++++++++++++++++------- 1 file changed, 58 insertions(+), 21 deletions(-) diff --git a/packages/backend/src/models/user.test.js b/packages/backend/src/models/user.test.js index 7a12abb7..6b85064d 100644 --- a/packages/backend/src/models/user.test.js +++ b/packages/backend/src/models/user.test.js @@ -1181,14 +1181,24 @@ describe('User model', () => { }); }); - describe('getFlows', () => { - let currentUser, currentUserRole, folder, flowOne, flowTwo; + describe.only('getFlows', () => { + let currentUser, + currentUserRole, + anotherUser, + folder, + anotherUserFolder, + flowOne, + flowTwo, + flowThree; beforeEach(async () => { currentUser = await createUser(); currentUserRole = await currentUser.$relatedQuery('role'); + anotherUser = await createUser(); + folder = await createFolder({ userId: currentUser.id }); + anotherUserFolder = await createFolder({ userId: anotherUser.id }); flowOne = await createFlow({ userId: currentUser.id, @@ -1201,11 +1211,16 @@ describe('User model', () => { name: 'Flow Two', }); + flowThree = await createFlow({ + userId: anotherUser.id, + name: 'Flow Three', + }); + await createPermission({ action: 'read', subject: 'Flow', roleId: currentUserRole.id, - conditions: ['isCreator'], + conditions: [], }); currentUser = await currentUser.$query().withGraphFetched({ @@ -1214,44 +1229,66 @@ describe('User model', () => { }); }); - it('should return flows filtered by folderId', async () => { - const flows = await currentUser.getFlows({ folderId: folder.id }); - - expect(flows).toHaveLength(1); - expect(flows[0].id).toBe(flowOne.id); - }); - it('should return flows filtered by name', async () => { - const flows = await currentUser.getFlows({ name: 'Flow Two' }); + const flows = await currentUser.getFlows({ name: 'Flow Two' }, [ + folder.id, + ]); expect(flows).toHaveLength(1); expect(flows[0].id).toBe(flowTwo.id); }); + it('should return flows with specific folder ID', async () => { + const flows = await currentUser.getFlows({ folderId: folder.id }, [ + folder.id, + ]); + + expect(flows.length).toBe(1); + expect(flows[0].id).toBe(flowOne.id); + }); + it('should return flows filtered by folderId and name', async () => { - const flows = await currentUser.getFlows({ - folderId: folder.id, - name: 'Flow One', - }); + const flows = await currentUser.getFlows( + { + folderId: folder.id, + name: 'Flow One', + }, + [folder.id] + ); expect(flows).toHaveLength(1); expect(flows[0].id).toBe(flowOne.id); }); it('should return all flows if no filters are provided', async () => { - const flows = await currentUser.getFlows({}); + const flows = await currentUser.getFlows({}, [folder.id]); - expect(flows).toHaveLength(2); + expect(flows).toHaveLength(3); expect(flows.map((flow) => flow.id)).toEqual( - expect.arrayContaining([flowOne.id, flowTwo.id]) + expect.arrayContaining([flowOne.id, flowTwo.id, flowThree.id]) ); }); it('should return uncategorized flows if the folderId is null', async () => { - const flows = await currentUser.getFlows({ folderId: 'null' }); + const flows = await currentUser.getFlows({ folderId: 'null' }, [ + folder.id, + ]); - expect(flows).toHaveLength(1); - expect(flows[0].id).toBe(flowTwo.id); + expect(flows).toHaveLength(2); + expect(flows.map((flow) => flow.id)).toEqual( + expect.arrayContaining([flowTwo.id, flowThree.id]) + ); + }); + + it('should return other users flow as uncategorized flows if the folderId is null', async () => { + const flows = await currentUser.getFlows({ folderId: 'null' }, [ + folder.id, + ]); + + expect(flows).toHaveLength(2); + expect(flows.map((flow) => flow.id)).toEqual( + expect.arrayContaining([flowTwo.id, flowThree.id]) + ); }); }); From a359cf0b29c26b06a06b14859ac7659c85ad5654 Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Wed, 12 Mar 2025 16:15:03 +0100 Subject: [PATCH 3/4] fix: Lint errors of user model test --- packages/backend/src/models/user.test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/backend/src/models/user.test.js b/packages/backend/src/models/user.test.js index 6b85064d..9f71d312 100644 --- a/packages/backend/src/models/user.test.js +++ b/packages/backend/src/models/user.test.js @@ -1186,7 +1186,6 @@ describe('User model', () => { currentUserRole, anotherUser, folder, - anotherUserFolder, flowOne, flowTwo, flowThree; @@ -1198,7 +1197,7 @@ describe('User model', () => { anotherUser = await createUser(); folder = await createFolder({ userId: currentUser.id }); - anotherUserFolder = await createFolder({ userId: anotherUser.id }); + await createFolder({ userId: anotherUser.id }); flowOne = await createFlow({ userId: currentUser.id, From b9163a8b7399b1f687d214c3fadb73672166d0da Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Wed, 12 Mar 2025 16:18:26 +0100 Subject: [PATCH 4/4] chore: Remove redundant only blocks from tests --- packages/backend/src/models/user.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/models/user.test.js b/packages/backend/src/models/user.test.js index 9f71d312..0d4ffb47 100644 --- a/packages/backend/src/models/user.test.js +++ b/packages/backend/src/models/user.test.js @@ -1181,7 +1181,7 @@ describe('User model', () => { }); }); - describe.only('getFlows', () => { + describe('getFlows', () => { let currentUser, currentUserRole, anotherUser,