Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/backend/src/apps/formsg/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,8 @@ export interface ParsedMrfWorkflow {
trigger: Omit<ParsedMrfWorkflowStep, 'approvalField'>
actions: ParsedMrfWorkflowStep[]
}

export const stepApprovalConfigSchema = z.object({
branch: z.literal('reject'),
stepId: z.string().uuid(),
})
18 changes: 15 additions & 3 deletions packages/backend/src/graphql/mutations/create-step.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { IStepConfig } from '@plumber/types'

import { raw } from 'objection'

import { BadUserInputError } from '@/errors/graphql-errors'
import logger from '@/helpers/logger'
import { validateApprovalConfig } from '@/helpers/validate-approval-config'
import App from '@/models/app'
import FlowConnections from '@/models/flow-connections'
import Step from '@/models/step'
Expand Down Expand Up @@ -38,6 +41,7 @@ const createStep: MutationResolvers['createStep'] = async (
throw new BadUserInputError('Action can only be created by system')
}
}

return await Step.transaction(async (trx) => {
await trx.raw('SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;')

Expand Down Expand Up @@ -77,21 +81,29 @@ const createStep: MutationResolvers['createStep'] = async (
})
.throwIfNotFound()

const validationResult = await validateApprovalConfig(
input.config as IStepConfig,
previousStep,
)
if (!validationResult.isApprovalConfigValid) {
throw new BadUserInputError('Invalid approval config')
}

await flow
.$relatedQuery('steps', trx)
.patch({
position: raw(`position + 1`),
})
.where('position', '>=', previousStep.position + 1)
.where('position', '>=', validationResult.newStepPosition)

const step = await flow.$relatedQuery('steps', trx).insertAndFetch({
key: input.key,
appKey: input.appKey,
type: 'action',
position: previousStep.position + 1,
position: validationResult.newStepPosition,
parameters: input.parameters,
connectionId: input.connection?.id,
config: input.config,
config: input.config as IStepConfig,
})

// NOTE: add flow connection to the flow_connections table
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { IStepConfig } from '@plumber/types'

import { raw } from 'objection'

import { BadUserInputError } from '@/errors/graphql-errors'
Expand Down Expand Up @@ -78,7 +80,7 @@ const duplicateBranch: MutationResolvers['duplicateBranch'] = async (
position: previousStep.position + 1,
parameters,
connectionId: connection?.id,
config,
config: config as IStepConfig,
})

newSteps.push(step)
Expand Down
14 changes: 14 additions & 0 deletions packages/backend/src/graphql/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -590,8 +590,16 @@ input CreateStepInput {
config: StepConfigInput
}



input StepApprovalConfigInput {
branch: String!
stepId: String!
}

input StepConfigInput {
stepName: String
approval: StepApprovalConfigInput
}

input UpdateStepInput {
Expand Down Expand Up @@ -710,12 +718,18 @@ type Step {
type StepConfig {
templateConfig: StepTemplateConfig
stepName: String
approval: StepApprovalConfig
}

type StepTemplateConfig {
appEventKey: String
}

type StepApprovalConfig {
branch: String
stepId: String
}

input StepConnectionInput {
id: String
}
Expand Down
149 changes: 149 additions & 0 deletions packages/backend/src/helpers/validate-approval-config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
import { IStep, IStepConfig } from '@plumber/types'

import get from 'lodash.get'

import { stepApprovalConfigSchema } from '@/apps/formsg/common/types'
import Step from '@/models/step'

import logger from './logger'

/**
* TODO: write unit test for this function
*/
export async function validateApprovalConfig(
config: IStepConfig,
prevStep: IStep,
): Promise<
| {
isApprovalConfigValid: true
newStepPosition: number
}
| {
isApprovalConfigValid: false
}
> {
/**
* Case 1: new step is a trigger step, return early
*/
if (!prevStep) {
return {
isApprovalConfigValid: true,
newStepPosition: 1,
}
}

const isPreviousStepMrfApprovalStep =
prevStep.appKey === 'formsg' &&
prevStep.key === 'mrfSubmission' &&
// if no approval field, it's just a normal mrf step
!!get(prevStep.parameters, 'mrf.approvalField')

/**
* Case 2: Prev step has no approval config and is not an mrf approval step,
* current step should have no approval config either
*/
if (!prevStep.config.approval && !isPreviousStepMrfApprovalStep) {
if (!config?.approval) {
return {
isApprovalConfigValid: true,
newStepPosition: prevStep.position + 1,
}
}
logger.error(
'Invalid approval config: previous step has no approval config and is not an mrf approval step. This step should not have an approval config.',
)
return {
isApprovalConfigValid: false,
}
}

/**
* Case 3: Prev step is part of an approval branch, check that
* the new step has the same approval config
*/
if (prevStep.config.approval) {
const isSameApprovalConfig =
prevStep.config.approval?.branch === config?.approval?.branch &&
prevStep.config.approval?.stepId === config?.approval?.stepId
if (isSameApprovalConfig) {
return {
isApprovalConfigValid: true,
newStepPosition: prevStep.position + 1,
}
}
logger.error(
'Invalid approval config: new step approval config is not the same as previous step',
)
return {
isApprovalConfigValid: false,
}
}

/**
* Case 4: Previous step is an approval step
*/
if (isPreviousStepMrfApprovalStep) {
// If previous step is an approval step, but this step has no approval config, it is part of the approve flow
if (!config?.approval) {
return {
isApprovalConfigValid: true,
newStepPosition: prevStep.position + 1,
}
}
// validate approval config
const { success } = stepApprovalConfigSchema.safeParse(config?.approval)
// validate approval config step id is the same as prev step id
if (!success || config.approval.stepId !== prevStep.id) {
logger.error(
'Invalid approval config: invalid approval config or new step id is not the same as approval step id',
{
approvalConfig: config?.approval,
},
)
return {
isApprovalConfigValid: false,
}
}
// For reject branch, find the end of the approval branch and add one to that step
// first find the next mrf step (if any)
const nextMrfStep = await Step.query()
.where('flow_id', prevStep.flowId)
.andWhere('type', 'action')
.andWhere('key', 'mrfSubmission')
.andWhere('position', '>', prevStep.position)
.orderBy('position', 'asc')
.first()
// then find the step in the approve branch (if any)
const lastStepOfApproveFlowQuery = Step.query()
.where('flow_id', prevStep.flowId)
.andWhere('position', '>', prevStep.position)

.orderBy('position', 'desc')
.first()

if (nextMrfStep) {
lastStepOfApproveFlowQuery.andWhere('position', '<', nextMrfStep.position)
}
const lastStepOfapproveFlow = await lastStepOfApproveFlowQuery.first()
// If last approval step exists, return the position after that
if (lastStepOfapproveFlow) {
return {
isApprovalConfigValid: true,
newStepPosition: lastStepOfapproveFlow.position + 1,
}
} else {
// If no last approval step, return the position after the previous step
return {
isApprovalConfigValid: true,
newStepPosition: prevStep.position + 1,
}
}
}

/**
* Catch all branch, something seems fishy
*/
return {
isApprovalConfigValid: false,
}
}
7 changes: 7 additions & 0 deletions packages/frontend/src/graphql/mutations/create-step.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ export const CREATE_STEP = gql`
}
config {
stepName
templateConfig {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clarification: do we need to add the templateConfig?
don't think we add this in any of the createStep mutations

appEventKey
}
approval {
branch
stepId
}
}
flow {
updatedAt
Expand Down
4 changes: 4 additions & 0 deletions packages/frontend/src/graphql/mutations/update-step.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ export const UPDATE_STEP = graphql(`
templateConfig {
appEventKey
}
approval {
branch
stepId
}
}
flow {
updatedAt
Expand Down
4 changes: 4 additions & 0 deletions packages/frontend/src/graphql/queries/get-flow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ export const FLOW_FIELDS = gql`
templateConfig {
appEventKey
}
approval {
branch
stepId
}
}
}
config {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ export const GET_TEST_EXECUTION_STEPS = gql`
appEventKey
}
stepName
approval {
branch
stepId
}
}
key
appKey
Expand Down
6 changes: 6 additions & 0 deletions packages/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,14 @@ export interface IExecution {

export type IStepApprovalBranch = 'approve' | 'reject'

export interface IStepApprovalConfig {
branch: 'reject'
stepId: string
}

export interface IStepConfig {
stepName?: string
approval?: IStepApprovalConfig
templateConfig?: IStepTemplateConfig
adminOverride?: IJSONObject
}
Expand Down