From 5f7d1f92199e0c788536a3bf24af5706f157f3cc Mon Sep 17 00:00:00 2001 From: Ali BARIN Date: Wed, 18 Sep 2024 14:06:30 +0000 Subject: [PATCH 1/4] feat: write REST API endpoint to update step --- .../controllers/api/v1/steps/update-step.js | 33 +++ .../api/v1/steps/update-step.test.js | 211 ++++++++++++++++++ packages/backend/src/helpers/authorization.js | 4 + packages/backend/src/models/step.js | 24 ++ packages/backend/src/routes/api/v1/steps.js | 2 + .../mocks/rest/api/v1/steps/update-step.js | 26 +++ 6 files changed, 300 insertions(+) create mode 100644 packages/backend/src/controllers/api/v1/steps/update-step.js create mode 100644 packages/backend/src/controllers/api/v1/steps/update-step.test.js create mode 100644 packages/backend/test/mocks/rest/api/v1/steps/update-step.js diff --git a/packages/backend/src/controllers/api/v1/steps/update-step.js b/packages/backend/src/controllers/api/v1/steps/update-step.js new file mode 100644 index 00000000..5d8aa899 --- /dev/null +++ b/packages/backend/src/controllers/api/v1/steps/update-step.js @@ -0,0 +1,33 @@ +import { renderObject } from '../../../../helpers/renderer.js'; + +export default async (request, response) => { + let step = await request.currentUser.authorizedSteps + .findById(request.params.stepId) + .throwIfNotFound(); + + const stepData = stepParams(request); + + if (stepData.connectionId && (stepData.appKey || step.appKey)) { + await request.currentUser.authorizedConnections + .findOne({ + id: stepData.connectionId, + key: stepData.appKey || step.appKey, + }) + .throwIfNotFound(); + } + + step = await step.update(stepData); + + renderObject(response, step); +}; + +const stepParams = (request) => { + const { connectionId, appKey, key, parameters } = request.body; + + return { + connectionId, + appKey, + key, + parameters, + }; +}; diff --git a/packages/backend/src/controllers/api/v1/steps/update-step.test.js b/packages/backend/src/controllers/api/v1/steps/update-step.test.js new file mode 100644 index 00000000..1a5ae1b9 --- /dev/null +++ b/packages/backend/src/controllers/api/v1/steps/update-step.test.js @@ -0,0 +1,211 @@ +import { describe, it, beforeEach, expect } from 'vitest'; +import request from 'supertest'; +import Crypto from 'crypto'; +import app from '../../../../app.js'; +import createAuthTokenByUserId from '../../../../helpers/create-auth-token-by-user-id.js'; +import { createUser } from '../../../../../test/factories/user.js'; +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 updateStepMock from '../../../../../test/mocks/rest/api/v1/steps/update-step.js'; + +describe('PATCH /api/v1/steps/:stepId', () => { + let currentUser, token; + + beforeEach(async () => { + currentUser = await createUser(); + + token = await createAuthTokenByUserId(currentUser.id); + }); + + it('should update the step of the current user', async () => { + const currentUserFlow = await createFlow({ userId: currentUser.id }); + const currentUserConnection = await createConnection({ + key: 'deepl', + }); + + await createStep({ + flowId: currentUserFlow.id, + connectionId: currentUserConnection.id, + }); + + const actionStep = await createStep({ + flowId: currentUserFlow.id, + connectionId: currentUserConnection.id, + appKey: 'deepl', + key: 'translateText', + }); + + await createPermission({ + action: 'read', + subject: 'Flow', + roleId: currentUser.roleId, + conditions: ['isCreator'], + }); + + await createPermission({ + action: 'update', + subject: 'Flow', + roleId: currentUser.roleId, + conditions: ['isCreator'], + }); + + const response = await request(app) + .patch(`/api/v1/steps/${actionStep.id}`) + .set('Authorization', token) + .send({ + parameters: { + text: 'Hello world!', + targetLanguage: 'de', + }, + }) + .expect(200); + + const refetchedStep = await actionStep.$query(); + + const expectedResponse = updateStepMock(refetchedStep); + + expect(response.body).toStrictEqual(expectedResponse); + }); + + it('should update the step of the another user', async () => { + const anotherUser = await createUser(); + const anotherUserFlow = await createFlow({ userId: anotherUser.id }); + const anotherUserConnection = await createConnection({ + key: 'deepl', + }); + + await createStep({ + flowId: anotherUserFlow.id, + connectionId: anotherUserConnection.id, + }); + + const actionStep = await createStep({ + flowId: anotherUserFlow.id, + connectionId: anotherUserConnection.id, + }); + + await createPermission({ + action: 'read', + subject: 'Flow', + roleId: currentUser.roleId, + conditions: [], + }); + + await createPermission({ + action: 'update', + subject: 'Flow', + roleId: currentUser.roleId, + conditions: [], + }); + + const response = await request(app) + .patch(`/api/v1/steps/${actionStep.id}`) + .set('Authorization', token) + .send({ + parameters: { + text: 'Hello world!', + targetLanguage: 'de', + }, + }) + .expect(200); + + const refetchedStep = await actionStep.$query(); + + const expectedResponse = updateStepMock(refetchedStep); + + expect(response.body).toStrictEqual(expectedResponse); + }); + + it('should return not found response for inaccessible connection', async () => { + const currentUserFlow = await createFlow({ userId: currentUser.id }); + + const anotherUser = await createUser(); + const anotherUserConnection = await createConnection({ + key: 'deepl', + userId: anotherUser.id, + }); + + await createStep({ + flowId: currentUserFlow.id, + }); + + const actionStep = await createStep({ + flowId: currentUserFlow.id, + }); + + await createPermission({ + action: 'read', + subject: 'Flow', + roleId: currentUser.roleId, + conditions: ['isCreator'], + }); + + await createPermission({ + action: 'update', + subject: 'Flow', + roleId: currentUser.roleId, + conditions: ['isCreator'], + }); + + await createPermission({ + action: 'read', + subject: 'Connection', + roleId: currentUser.roleId, + conditions: ['isCreator'], + }); + + await request(app) + .patch(`/api/v1/steps/${actionStep.id}`) + .set('Authorization', token) + .send({ + connectionId: anotherUserConnection.id, + }) + .expect(404); + }); + + it('should return not found response for not existing step UUID', async () => { + await createPermission({ + action: 'update', + subject: 'Flow', + roleId: currentUser.roleId, + conditions: [], + }); + + await createPermission({ + action: 'read', + subject: 'Flow', + roleId: currentUser.roleId, + conditions: [], + }); + + const notExistingStepUUID = Crypto.randomUUID(); + + await request(app) + .patch(`/api/v1/steps/${notExistingStepUUID}`) + .set('Authorization', token) + .expect(404); + }); + + it('should return bad request response for invalid step UUID', async () => { + await createPermission({ + action: 'update', + subject: 'Flow', + roleId: currentUser.roleId, + conditions: [], + }); + + await createPermission({ + action: 'read', + subject: 'Flow', + roleId: currentUser.roleId, + conditions: [], + }); + + await request(app) + .patch('/api/v1/steps/invalidStepUUID') + .set('Authorization', token) + .expect(400); + }); +}); diff --git a/packages/backend/src/helpers/authorization.js b/packages/backend/src/helpers/authorization.js index b59cbcc0..c9f6329f 100644 --- a/packages/backend/src/helpers/authorization.js +++ b/packages/backend/src/helpers/authorization.js @@ -37,6 +37,10 @@ const authorizationList = { action: 'read', subject: 'Flow', }, + 'PATCH /api/v1/steps/:stepId': { + action: 'update', + subject: 'Flow', + }, 'POST /api/v1/steps/:stepId/test': { action: 'update', subject: 'Flow', diff --git a/packages/backend/src/models/step.js b/packages/backend/src/models/step.js index 4781d8b6..7cbf9a6e 100644 --- a/packages/backend/src/models/step.js +++ b/packages/backend/src/models/step.js @@ -334,6 +334,30 @@ class Step extends Base { await Promise.all(nextStepQueries); } + + async update(newStepData) { + const { connectionId, appKey, key, parameters } = newStepData; + + if (this.isTrigger && appKey && key) { + await App.checkAppAndTrigger(appKey, key); + } + + if (this.isAction && appKey && key) { + await App.checkAppAndAction(appKey, key); + } + + const updatedStep = await this.$query().patchAndFetch({ + key: key, + appKey: appKey, + connectionId: connectionId, + parameters: parameters, + status: 'incomplete', + }); + + await updatedStep.updateWebhookUrl(); + + return updatedStep; + } } export default Step; diff --git a/packages/backend/src/routes/api/v1/steps.js b/packages/backend/src/routes/api/v1/steps.js index 36305d9f..bcfc8074 100644 --- a/packages/backend/src/routes/api/v1/steps.js +++ b/packages/backend/src/routes/api/v1/steps.js @@ -7,6 +7,7 @@ import getPreviousStepsAction from '../../../controllers/api/v1/steps/get-previo import createDynamicFieldsAction from '../../../controllers/api/v1/steps/create-dynamic-fields.js'; import createDynamicDataAction from '../../../controllers/api/v1/steps/create-dynamic-data.js'; import deleteStepAction from '../../../controllers/api/v1/steps/delete-step.js'; +import updateStepAction from '../../../controllers/api/v1/steps/update-step.js'; const router = Router(); @@ -40,6 +41,7 @@ router.post( createDynamicDataAction ); +router.patch('/:stepId', authenticateUser, authorizeUser, updateStepAction); router.delete('/:stepId', authenticateUser, authorizeUser, deleteStepAction); export default router; diff --git a/packages/backend/test/mocks/rest/api/v1/steps/update-step.js b/packages/backend/test/mocks/rest/api/v1/steps/update-step.js new file mode 100644 index 00000000..87514ef9 --- /dev/null +++ b/packages/backend/test/mocks/rest/api/v1/steps/update-step.js @@ -0,0 +1,26 @@ +const updateStepMock = (step) => { + const data = { + id: step.id, + type: step.type || 'action', + key: step.key || null, + appKey: step.appKey || null, + iconUrl: step.iconUrl || null, + webhookUrl: step.webhookUrl || null, + status: step.status || 'incomplete', + position: step.position, + parameters: step.parameters || {}, + }; + + return { + data, + meta: { + type: 'Step', + count: 1, + isArray: false, + currentPage: null, + totalPages: null, + }, + }; +}; + +export default updateStepMock; From 074e7828f393a5afc03998c4a6265749e89f4d9d Mon Sep 17 00:00:00 2001 From: Ali BARIN Date: Wed, 18 Sep 2024 14:06:54 +0000 Subject: [PATCH 2/4] feat(web): use REST API endpoint to update step --- packages/web/src/components/Editor/index.jsx | 22 ++++++--------- .../src/components/EditorNew/EditorNew.jsx | 23 ++++++--------- packages/web/src/hooks/useUpdateStep.js | 28 +++++++++++++++++++ 3 files changed, 44 insertions(+), 29 deletions(-) create mode 100644 packages/web/src/hooks/useUpdateStep.js diff --git a/packages/web/src/components/Editor/index.jsx b/packages/web/src/components/Editor/index.jsx index 51de3de9..96ca3258 100644 --- a/packages/web/src/components/Editor/index.jsx +++ b/packages/web/src/components/Editor/index.jsx @@ -1,18 +1,17 @@ import * as React from 'react'; -import { useMutation } from '@apollo/client'; import Box from '@mui/material/Box'; import IconButton from '@mui/material/IconButton'; import AddIcon from '@mui/icons-material/Add'; import useCreateStep from 'hooks/useCreateStep'; -import { UPDATE_STEP } from 'graphql/mutations/update-step'; +import useUpdateStep from 'hooks/useUpdateStep'; import FlowStep from 'components/FlowStep'; import { FlowPropType } from 'propTypes/propTypes'; import { useQueryClient } from '@tanstack/react-query'; function Editor(props) { - const [updateStep] = useMutation(UPDATE_STEP); const { flow } = props; + const { mutateAsync: updateStep } = useUpdateStep(); const { mutateAsync: createStep, isPending: isCreateStepPending } = useCreateStep(flow?.id); const [triggerStep] = flow.steps; @@ -21,29 +20,24 @@ function Editor(props) { const onStepChange = React.useCallback( async (step) => { - const mutationInput = { + const payload = { id: step.id, key: step.key, parameters: step.parameters, - connection: { - id: step.connection?.id, - }, - flow: { - id: flow.id, - }, + connectionId: step.connection?.id, }; if (step.appKey) { - mutationInput.appKey = step.appKey; + payload.appKey = step.appKey; } - await updateStep({ variables: { input: mutationInput } }); + await updateStep(payload); + await queryClient.invalidateQueries({ queryKey: ['steps', step.id, 'connection'], }); - await queryClient.invalidateQueries({ queryKey: ['flows', flow.id] }); }, - [updateStep, flow.id, queryClient], + [updateStep, queryClient], ); const addStep = React.useCallback( diff --git a/packages/web/src/components/EditorNew/EditorNew.jsx b/packages/web/src/components/EditorNew/EditorNew.jsx index 8858af6e..89e655c4 100644 --- a/packages/web/src/components/EditorNew/EditorNew.jsx +++ b/packages/web/src/components/EditorNew/EditorNew.jsx @@ -5,8 +5,8 @@ import { FlowPropType } from 'propTypes/propTypes'; import ReactFlow, { useNodesState, useEdgesState } from 'reactflow'; import 'reactflow/dist/style.css'; -import { UPDATE_STEP } from 'graphql/mutations/update-step'; import useCreateStep from 'hooks/useCreateStep'; +import useUpdateStep from 'hooks/useUpdateStep'; import { useAutoLayout } from './useAutoLayout'; import { useScrollBoundaries } from './useScrollBoundaries'; @@ -35,7 +35,7 @@ const edgeTypes = { }; const EditorNew = ({ flow }) => { - const [updateStep] = useMutation(UPDATE_STEP); + const { mutateAsync: updateStep } = useUpdateStep(); const queryClient = useQueryClient(); const { mutateAsync: createStep, isPending: isCreateStepPending } = @@ -84,31 +84,24 @@ const EditorNew = ({ flow }) => { const onStepChange = useCallback( async (step) => { - const mutationInput = { + const payload = { id: step.id, key: step.key, parameters: step.parameters, - connection: { - id: step.connection?.id, - }, - flow: { - id: flow.id, - }, + connectionId: step.connection?.id, }; if (step.appKey) { - mutationInput.appKey = step.appKey; + payload.appKey = step.appKey; } - await updateStep({ - variables: { input: mutationInput }, - }); + await updateStep(payload); + await queryClient.invalidateQueries({ queryKey: ['steps', step.id, 'connection'], }); - await queryClient.invalidateQueries({ queryKey: ['flows', flow.id] }); }, - [flow.id, updateStep, queryClient], + [updateStep, queryClient], ); const onAddStep = useCallback( diff --git a/packages/web/src/hooks/useUpdateStep.js b/packages/web/src/hooks/useUpdateStep.js new file mode 100644 index 00000000..65e92170 --- /dev/null +++ b/packages/web/src/hooks/useUpdateStep.js @@ -0,0 +1,28 @@ +import { useMutation, useQueryClient } from '@tanstack/react-query'; + +import api from 'helpers/api'; + +export default function useUpdateStep() { + const queryClient = useQueryClient(); + + const query = useMutation({ + mutationFn: async ({ id, appKey, key, connectionId, parameters }) => { + const { data } = await api.patch(`/v1/steps/${id}`, { + appKey, + key, + connectionId, + parameters, + }); + + return data; + }, + + onSuccess: () => { + queryClient.invalidateQueries({ + queryKey: ['flows'], + }); + }, + }); + + return query; +} From cd16a3cc1581751011fd42d0bbf23e727fba1ded Mon Sep 17 00:00:00 2001 From: Ali BARIN Date: Wed, 18 Sep 2024 14:07:02 +0000 Subject: [PATCH 3/4] chore: remove redundant update step mutation --- .../backend/src/graphql/mutation-resolvers.js | 3 - .../src/graphql/mutations/update-step.js | 68 ------------------- packages/backend/src/graphql/schema.graphql | 12 ---- .../web/src/graphql/mutations/update-step.js | 17 ----- 4 files changed, 100 deletions(-) delete mode 100644 packages/backend/src/graphql/mutations/update-step.js delete mode 100644 packages/web/src/graphql/mutations/update-step.js diff --git a/packages/backend/src/graphql/mutation-resolvers.js b/packages/backend/src/graphql/mutation-resolvers.js index 5cf55a94..45c1d913 100644 --- a/packages/backend/src/graphql/mutation-resolvers.js +++ b/packages/backend/src/graphql/mutation-resolvers.js @@ -1,5 +1,3 @@ -import updateStep from './mutations/update-step.js'; - // Converted mutations import executeFlow from './mutations/execute-flow.js'; import updateUser from './mutations/update-user.ee.js'; @@ -21,7 +19,6 @@ const mutationResolvers = { updateConnection, updateCurrentUser, updateFlowStatus, - updateStep, updateUser, verifyConnection, }; diff --git a/packages/backend/src/graphql/mutations/update-step.js b/packages/backend/src/graphql/mutations/update-step.js deleted file mode 100644 index 398804d5..00000000 --- a/packages/backend/src/graphql/mutations/update-step.js +++ /dev/null @@ -1,68 +0,0 @@ -import App from '../../models/app.js'; -import Step from '../../models/step.js'; -import Connection from '../../models/connection.js'; - -const updateStep = async (_parent, params, context) => { - const { isCreator } = context.currentUser.can('update', 'Flow'); - const userSteps = context.currentUser.$relatedQuery('steps'); - const allSteps = Step.query(); - const baseQuery = isCreator ? userSteps : allSteps; - - const { input } = params; - - let step = await baseQuery - .findOne({ - 'steps.id': input.id, - flow_id: input.flow.id, - }) - .throwIfNotFound(); - - if (input.connection.id) { - let canSeeAllConnections = false; - try { - const conditions = context.currentUser.can('read', 'Connection'); - - canSeeAllConnections = !conditions.isCreator; - } catch { - // void - } - - const userConnections = context.currentUser.$relatedQuery('connections'); - const allConnections = Connection.query(); - const baseConnectionsQuery = canSeeAllConnections - ? allConnections - : userConnections; - - const connection = await baseConnectionsQuery - .clone() - .findById(input.connection?.id); - - if (!connection) { - throw new Error('The connection does not exist!'); - } - } - - if (step.isTrigger) { - await App.checkAppAndTrigger(input.appKey, input.key); - } - - if (step.isAction) { - await App.checkAppAndAction(input.appKey, input.key); - } - - step = await Step.query() - .patchAndFetchById(input.id, { - key: input.key, - appKey: input.appKey, - connectionId: input.connection.id, - parameters: input.parameters, - status: 'incomplete' - }) - .withGraphFetched('connection'); - - await step.updateWebhookUrl(); - - return step; -}; - -export default updateStep; diff --git a/packages/backend/src/graphql/schema.graphql b/packages/backend/src/graphql/schema.graphql index ae18e481..a96811b3 100644 --- a/packages/backend/src/graphql/schema.graphql +++ b/packages/backend/src/graphql/schema.graphql @@ -10,7 +10,6 @@ type Mutation { updateConnection(input: UpdateConnectionInput): Connection updateCurrentUser(input: UpdateCurrentUserInput): User updateFlowStatus(input: UpdateFlowStatusInput): Flow - updateStep(input: UpdateStepInput): Step updateUser(input: UpdateUserInput): User verifyConnection(input: VerifyConnectionInput): Connection } @@ -245,17 +244,6 @@ input ExecuteFlowInput { stepId: String! } -input UpdateStepInput { - id: String - previousStepId: String - key: String - appKey: String - connection: StepConnectionInput - flow: StepFlowInput - parameters: JSONObject - previousStep: PreviousStepInput -} - input CreateUserInput { fullName: String! email: String! diff --git a/packages/web/src/graphql/mutations/update-step.js b/packages/web/src/graphql/mutations/update-step.js deleted file mode 100644 index 44d91a2b..00000000 --- a/packages/web/src/graphql/mutations/update-step.js +++ /dev/null @@ -1,17 +0,0 @@ -import { gql } from '@apollo/client'; -export const UPDATE_STEP = gql` - mutation UpdateStep($input: UpdateStepInput) { - updateStep(input: $input) { - id - type - key - appKey - parameters - status - webhookUrl - connection { - id - } - } - } -`; From 8134b6db6a9b11a4f10d455f6e26aae56e97d3fd Mon Sep 17 00:00:00 2001 From: Ali BARIN Date: Thu, 19 Sep 2024 09:27:51 +0000 Subject: [PATCH 4/4] refactor(update-step): move connection authorization to model --- .../src/controllers/api/v1/steps/update-step.js | 13 +------------ packages/backend/src/models/step.js | 11 ++++++++++- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/backend/src/controllers/api/v1/steps/update-step.js b/packages/backend/src/controllers/api/v1/steps/update-step.js index 5d8aa899..c707726d 100644 --- a/packages/backend/src/controllers/api/v1/steps/update-step.js +++ b/packages/backend/src/controllers/api/v1/steps/update-step.js @@ -5,18 +5,7 @@ export default async (request, response) => { .findById(request.params.stepId) .throwIfNotFound(); - const stepData = stepParams(request); - - if (stepData.connectionId && (stepData.appKey || step.appKey)) { - await request.currentUser.authorizedConnections - .findOne({ - id: stepData.connectionId, - key: stepData.appKey || step.appKey, - }) - .throwIfNotFound(); - } - - step = await step.update(stepData); + step = await step.updateFor(request.currentUser, stepParams(request)); renderObject(response, step); }; diff --git a/packages/backend/src/models/step.js b/packages/backend/src/models/step.js index 7cbf9a6e..85f98a1d 100644 --- a/packages/backend/src/models/step.js +++ b/packages/backend/src/models/step.js @@ -335,9 +335,18 @@ class Step extends Base { await Promise.all(nextStepQueries); } - async update(newStepData) { + async updateFor(user, newStepData) { const { connectionId, appKey, key, parameters } = newStepData; + if (connectionId && (appKey || this.appKey)) { + await user.authorizedConnections + .findOne({ + id: connectionId, + key: appKey || this.appKey, + }) + .throwIfNotFound(); + } + if (this.isTrigger && appKey && key) { await App.checkAppAndTrigger(appKey, key); }