Merge pull request #2441 from automatisch/aut-1530

feat: Implement isOwner flag for get flow API endpoint

packages/backend/src/controllers/internal/api/v1/apps/get-flows.js

@@ -1,6 +1,5 @@
 import { renderObject } from '../../../../../helpers/renderer.js';
 import App from '../../../../../models/app.js';
-import Flow from '../../../../../models/flow.js';
 import paginateRest from '../../../../../helpers/pagination.js';
 
 export default async (request, response) => {
@@ -15,10 +14,6 @@ export default async (request, response) => {
     .withGraphFetched({
       steps: true,
     })
-    .select('flows.*')
-    .select(
-      Flow.raw('flows.user_id = ? as "isOwner"', [request.currentUser.id])
-    )
     .where('steps.app_key', app.key)
     .orderBy('active', 'desc')
     .orderBy('updated_at', 'desc');

packages/backend/src/controllers/internal/api/v1/apps/get-flows.test.js

@@ -6,7 +6,7 @@ import { createUser } from '../../../../../../test/factories/user.js';
 import { createFlow } from '../../../../../../test/factories/flow.js';
 import { createStep } from '../../../../../../test/factories/step.js';
 import { createPermission } from '../../../../../../test/factories/permission.js';
-import getFlowsMock from '../../../../../../test/mocks/rest/internal/api/v1/apps/get-flows.js';
+import getFlowsMock from '../../../../../../test/mocks/rest/internal/api/v1/flows/get-flows.js';
 
 describe('GET /internal/api/v1/apps/:appKey/flows', () => {
   let currentUser, currentUserRole, token;
@@ -59,8 +59,7 @@ describe('GET /internal/api/v1/apps/:appKey/flows', () => {
 
     const expectedPayload = await getFlowsMock(
       [currentUserFlowOne],
-      [triggerStepFlowOne, actionStepFlowOne],
+      [triggerStepFlowOne, actionStepFlowOne]
-      currentUser.id
     );
 
     expect(response.body).toStrictEqual(expectedPayload);
@@ -108,8 +107,7 @@ describe('GET /internal/api/v1/apps/:appKey/flows', () => {
 
     const expectedPayload = await getFlowsMock(
       [anotherUserFlowOne],
-      [triggerStepFlowOne, actionStepFlowOne],
+      [triggerStepFlowOne, actionStepFlowOne]
-      currentUser.id
     );
 
     expect(response.body).toStrictEqual(expectedPayload);

packages/backend/src/controllers/internal/api/v1/connections/get-flows.js

@@ -1,6 +1,5 @@
 import { renderObject } from '../../../../../helpers/renderer.js';
 import paginateRest from '../../../../../helpers/pagination.js';
-import Flow from '../../../../../models/flow.js';
 
 export default async (request, response) => {
   const flowsQuery = request.currentUser.authorizedFlows
@@ -12,10 +11,6 @@ export default async (request, response) => {
     .withGraphFetched({
       steps: true,
     })
-    .select('flows.*')
-    .select(
-      Flow.raw('flows.user_id = ? as "isOwner"', [request.currentUser.id])
-    )
     .where('steps.connection_id', request.params.connectionId)
     .orderBy('active', 'desc')
     .orderBy('updated_at', 'desc');

packages/backend/src/controllers/internal/api/v1/connections/get-flows.test.js

@@ -7,7 +7,7 @@ import { createConnection } from '../../../../../../test/factories/connection.js
 import { createFlow } from '../../../../../../test/factories/flow.js';
 import { createStep } from '../../../../../../test/factories/step.js';
 import { createPermission } from '../../../../../../test/factories/permission.js';
-import getFlowsMock from '../../../../../../test/mocks/rest/internal/api/v1/connections/get-flows.js';
+import getFlowsMock from '../../../../../../test/mocks/rest/internal/api/v1/flows/get-flows.js';
 
 describe('GET /internal/api/v1/connections/:connectionId/flows', () => {
   let currentUser, currentUserRole, token;
@@ -66,8 +66,7 @@ describe('GET /internal/api/v1/connections/:connectionId/flows', () => {
 
     const expectedPayload = await getFlowsMock(
       [currentUserFlowOne],
-      [triggerStepFlowOne, actionStepFlowOne],
+      [triggerStepFlowOne, actionStepFlowOne]
-      currentUser.id
     );
 
     expect(response.body).toStrictEqual(expectedPayload);
@@ -121,8 +120,7 @@ describe('GET /internal/api/v1/connections/:connectionId/flows', () => {
 
     const expectedPayload = await getFlowsMock(
       [anotherUserFlowOne],
-      [triggerStepFlowOne, actionStepFlowOne],
+      [triggerStepFlowOne, actionStepFlowOne]
-      currentUser.id
     );
 
     expect(response.body).toStrictEqual(expectedPayload);

packages/backend/src/controllers/internal/api/v1/flows/get-flow.js

@@ -1,9 +1,13 @@
+import Flow from '../../../../../models/flow.js';
 import { renderObject } from '../../../../../helpers/renderer.js';
 
 export default async (request, response) => {
   const flow = await request.currentUser.authorizedFlows
     .clone()
     .withGraphJoined({ steps: true })
+    .select(
+      Flow.raw('flows.user_id = ? as "isOwner"', [request.currentUser.id])
+    )
     .orderBy('steps.position', 'asc')
     .findOne({ 'flows.id': request.params.flowId })
     .throwIfNotFound();

packages/backend/src/controllers/internal/api/v1/flows/get-flow.test.js

@@ -36,10 +36,11 @@ describe('GET /internal/api/v1/flows/:flowId', () => {
       .set('Authorization', token)
       .expect(200);
 
-    const expectedPayload = await getFlowMock(currentUserflow, [
+    const expectedPayload = await getFlowMock(
-      triggerStep,
+      currentUserflow,
-      actionStep,
+      [triggerStep, actionStep],
-    ]);
+      currentUser.id
+    );
 
     expect(response.body).toStrictEqual(expectedPayload);
   });
@@ -62,10 +63,11 @@ describe('GET /internal/api/v1/flows/:flowId', () => {
       .set('Authorization', token)
       .expect(200);
 
-    const expectedPayload = await getFlowMock(anotherUserFlow, [
+    const expectedPayload = await getFlowMock(
-      triggerStep,
+      anotherUserFlow,
-      actionStep,
+      [triggerStep, actionStep],
-    ]);
+      currentUser.id
+    );
 
     expect(response.body).toStrictEqual(expectedPayload);
   });

packages/backend/src/controllers/internal/api/v1/flows/get-flows.test.js

@@ -61,8 +61,7 @@ describe('GET /internal/api/v1/flows', () => {
         actionStepFlowOne,
         triggerStepFlowTwo,
         actionStepFlowTwo,
-      ],
+      ]
-      currentUser.id
     );
 
     expect(response.body).toStrictEqual(expectedPayload);
@@ -112,8 +111,7 @@ describe('GET /internal/api/v1/flows', () => {
         actionStepFlowOne,
         triggerStepFlowTwo,
         actionStepFlowTwo,
-      ],
+      ]
-      currentUser.id
     );
 
     expect(response.body).toStrictEqual(expectedPayload);
@@ -186,8 +184,7 @@ describe('GET /internal/api/v1/flows', () => {
         actionStepFlowTwo,
         triggerStepFlowThree,
         actionStepFlowThree,
-      ],
+      ]
-      currentUser.id
     );
 
     expect(response.body).toStrictEqual(expectedPayload);
@@ -273,8 +270,7 @@ describe('GET /internal/api/v1/flows', () => {
         actionStepFlowThree,
         triggerStepFlowFour,
         actionStepFlowFour,
-      ],
+      ]
-      currentUser.id
     );
 
     expect(response.body).toStrictEqual(expectedPayload);
@@ -360,8 +356,7 @@ describe('GET /internal/api/v1/flows', () => {
         actionStepFlowOne,
         triggerStepFlowTwo,
         actionStepFlowTwo,
-      ],
+      ]
-      currentUser.id
     );
 
     expect(response.body).toStrictEqual(expectedPayload);

packages/backend/src/controllers/internal/api/v1/flows/update-flow.test.js

@@ -6,7 +6,7 @@ import createAuthTokenByUserId from '../../../../../helpers/create-auth-token-by
 import { createUser } from '../../../../../../test/factories/user.js';
 import { createFlow } from '../../../../../../test/factories/flow.js';
 import { createPermission } from '../../../../../../test/factories/permission.js';
-import getFlowMock from '../../../../../../test/mocks/rest/internal/api/v1/flows/get-flow.js';
+import updateFlowMock from '../../../../../../test/mocks/rest/internal/api/v1/flows/update-flow.js';
 
 describe('PATCH /internal/api/v1/flows/:flowId', () => {
   let currentUser, currentUserRole, token;
@@ -45,7 +45,7 @@ describe('PATCH /internal/api/v1/flows/:flowId', () => {
 
     const refetchedCurrentUserFlow = await currentUserFlow.$query();
 
-    const expectedPayload = await getFlowMock({
+    const expectedPayload = await updateFlowMock({
       ...refetchedCurrentUserFlow,
       name: 'Updated flow',
     });
@@ -81,7 +81,7 @@ describe('PATCH /internal/api/v1/flows/:flowId', () => {
 
     const refetchedAnotherUserFlow = await anotherUserFlow.$query();
 
-    const expectedPayload = await getFlowMock({
+    const expectedPayload = await updateFlowMock({
       ...refetchedAnotherUserFlow,
       name: 'Updated flow',
     });

packages/backend/src/models/user.js

@@ -537,8 +537,6 @@ class User extends Base {
       .withGraphFetched({
         steps: true,
       })
-      .select('flows.*')
-      .select(Flow.raw('flows.user_id = ? as "isOwner"', [this.id]))
       .where((builder) => {
         if (name) {
           builder.where('flows.name', 'ilike', `%${name}%`);

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

@@ -1333,23 +1333,6 @@ describe('User model', () => {
       );
     });
 
-    it('should return isOwner false if the flow is owned by another user', async () => {
-      const anotherUser = await createUser();
-
-      await createFlow({
-        userId: anotherUser.id,
-        folderId: folder.id,
-        active: true,
-        name: 'Another User Flow One',
-      });
-
-      const flows = await currentUser.getFlows({ onlyOwnedFlows: false }, [
-        folder.id,
-      ]);
-
-      expect(flows[0].isOwner).toBe(false);
-    });
-
     it('should return specified flows with all filters together', async () => {
       const flows = await currentUser.getFlows(
         {

packages/backend/test/mocks/rest/internal/api/v1/apps/get-flows.js

@@ -1,40 +0,0 @@
-const getFlowsMock = async (flows, steps, currentUserId) => {
-  const data = flows.map((flow) => {
-    const flowSteps = steps.filter((step) => step.flowId === flow.id);
-
-    return {
-      active: flow.active,
-      id: flow.id,
-      name: flow.name,
-      status: flow.active ? 'published' : 'draft',
-      isOwner: flow.userId === currentUserId,
-      createdAt: flow.createdAt.getTime(),
-      updatedAt: flow.updatedAt.getTime(),
-      steps: flowSteps.map((step) => ({
-        appKey: step.appKey,
-        iconUrl: step.iconUrl,
-        id: step.id,
-        key: step.key,
-        name: step.name,
-        parameters: step.parameters,
-        position: step.position,
-        status: step.status,
-        type: step.type,
-        webhookUrl: step.webhookUrl,
-      })),
-    };
-  });
-
-  return {
-    data: data,
-    meta: {
-      count: data.length,
-      currentPage: 1,
-      isArray: true,
-      totalPages: 1,
-      type: 'Flow',
-    },
-  };
-};
-
-export default getFlowsMock;

packages/backend/test/mocks/rest/internal/api/v1/connections/get-flows.js

@@ -1,40 +0,0 @@
-const getFlowsMock = async (flows, steps, currentUserId) => {
-  const data = flows.map((flow) => {
-    const flowSteps = steps.filter((step) => step.flowId === flow.id);
-
-    return {
-      active: flow.active,
-      id: flow.id,
-      name: flow.name,
-      status: flow.active ? 'published' : 'draft',
-      isOwner: flow.userId === currentUserId,
-      createdAt: flow.createdAt.getTime(),
-      updatedAt: flow.updatedAt.getTime(),
-      steps: flowSteps.map((step) => ({
-        appKey: step.appKey,
-        iconUrl: step.iconUrl,
-        id: step.id,
-        key: step.key,
-        name: step.name,
-        parameters: step.parameters,
-        position: step.position,
-        status: step.status,
-        type: step.type,
-        webhookUrl: step.webhookUrl,
-      })),
-    };
-  });
-
-  return {
-    data: data,
-    meta: {
-      count: data.length,
-      currentPage: 1,
-      isArray: true,
-      totalPages: 1,
-      type: 'Flow',
-    },
-  };
-};
-
-export default getFlowsMock;

packages/backend/test/mocks/rest/internal/api/v1/flows/get-flow.js

@@ -1,9 +1,10 @@
-const getFlowMock = async (flow, steps = []) => {
+const getFlowMock = async (flow, steps = [], currentUserId) => {
   const data = {
     active: flow.active,
     id: flow.id,
     name: flow.name,
     status: flow.active ? 'published' : 'draft',
+    isOwner: flow.userId === currentUserId,
     createdAt: flow.createdAt.getTime(),
     updatedAt: flow.updatedAt.getTime(),
   };

packages/backend/test/mocks/rest/internal/api/v1/flows/get-flows.js

@@ -1,4 +1,4 @@
-const getFlowsMock = async (flows, steps, currentUserId) => {
+const getFlowsMock = async (flows, steps) => {
   const data = flows.map((flow) => {
     const flowSteps = steps.filter((step) => step.flowId === flow.id);
 
@@ -9,7 +9,6 @@ const getFlowsMock = async (flows, steps, currentUserId) => {
       status: flow.active ? 'published' : 'draft',
       createdAt: flow.createdAt.getTime(),
       updatedAt: flow.updatedAt.getTime(),
-      isOwner: flow.userId === currentUserId,
       steps: flowSteps.map((step) => ({
         appKey: step.appKey,
         iconUrl: step.iconUrl,

packages/backend/test/mocks/rest/internal/api/v1/flows/update-flow.js

@@ -0,0 +1,23 @@
+const updateFlowMock = async (flow) => {
+  const data = {
+    id: flow.id,
+    active: flow.active,
+    name: flow.name,
+    status: flow.status,
+    createdAt: flow.createdAt.getTime(),
+    updatedAt: flow.updatedAt.getTime(),
+  };
+
+  return {
+    data: data,
+    meta: {
+      count: 1,
+      currentPage: null,
+      isArray: false,
+      totalPages: null,
+      type: 'Flow',
+    },
+  };
+};
+
+export default updateFlowMock;