Merge pull request #2391 from automatisch/uncategorized-flow-fix

fix: Specify uncategorized flows by using not-owned folders

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);

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);
         }

packages/backend/src/models/user.test.js

@@ -1182,13 +1182,22 @@ describe('User model', () => {
   });
 
   describe('getFlows', () => {
-    let currentUser, currentUserRole, folder, flowOne, flowTwo;
+    let currentUser,
+      currentUserRole,
+      anotherUser,
+      folder,
+      flowOne,
+      flowTwo,
+      flowThree;
 
     beforeEach(async () => {
       currentUser = await createUser();
       currentUserRole = await currentUser.$relatedQuery('role');
 
+      anotherUser = await createUser();
+
       folder = await createFolder({ userId: currentUser.id });
+      await createFolder({ userId: anotherUser.id });
 
       flowOne = await createFlow({
         userId: currentUser.id,
@@ -1201,11 +1210,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 +1228,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({
+      const flows = await currentUser.getFlows(
-        folderId: folder.id,
+        {
-        name: 'Flow One',
+          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).toHaveLength(2);
-      expect(flows[0].id).toBe(flowTwo.id);
+      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])
+      );
     });
   });