From 8b817569659346d104a026d7bcf3e3c53ce8f250 Mon Sep 17 00:00:00 2001 From: Chris Hills <31041837+chrishills@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:29:27 -0500 Subject: [PATCH] Added step child path field for explicit grouping (#401) --- ...240323145423_add_journey_step_child_key.js | 23 +++++++++++ .../platform/src/journey/JourneyController.ts | 4 ++ .../platform/src/journey/JourneyRepository.ts | 4 +- apps/platform/src/journey/JourneyStep.ts | 11 +++-- .../journey/__tests__/JourneyService.spec.ts | 23 +++++++---- apps/ui/src/types.ts | 7 ++-- apps/ui/src/views/journey/JourneyEditor.tsx | 40 ++++++++----------- apps/ui/src/views/journey/steps/Balancer.tsx | 2 +- .../ui/src/views/journey/steps/Experiment.tsx | 2 +- apps/ui/src/views/journey/steps/Gate.tsx | 2 +- 10 files changed, 75 insertions(+), 43 deletions(-) create mode 100644 apps/platform/db/migrations/20240323145423_add_journey_step_child_key.js diff --git a/apps/platform/db/migrations/20240323145423_add_journey_step_child_key.js b/apps/platform/db/migrations/20240323145423_add_journey_step_child_key.js new file mode 100644 index 00000000..f7a0bdec --- /dev/null +++ b/apps/platform/db/migrations/20240323145423_add_journey_step_child_key.js @@ -0,0 +1,23 @@ +exports.up = async function(knex) { + await knex.schema.alterTable('journey_step_child', function(table) { + table.string('path', 128) + }) + await knex.raw(` + update journey_step_child sc + inner join journey_steps s on sc.step_id = s.id + set sc.path = 'yes' + where s.type = 'gate' and priority = 0; + `) + await knex.raw(` + update journey_step_child sc + inner join journey_steps s on sc.step_id = s.id + set sc.path = 'no' + where s.type = 'gate' and priority > 0; + `) +} + +exports.down = async function(knex) { + await knex.schema.alterTable('journey_step_child', function(table) { + table.dropColumn('path') + }) +} diff --git a/apps/platform/src/journey/JourneyController.ts b/apps/platform/src/journey/JourneyController.ts index e97be788..fde239a8 100644 --- a/apps/platform/src/journey/JourneyController.ts +++ b/apps/platform/src/journey/JourneyController.ts @@ -146,6 +146,10 @@ const journeyStepsParamsSchema: JSONSchemaType = { external_id: { type: 'string', }, + path: { + type: 'string', + nullable: true, + }, data: { type: 'object', // TODO: this is also specific to the parent node's type nullable: true, diff --git a/apps/platform/src/journey/JourneyRepository.ts b/apps/platform/src/journey/JourneyRepository.ts index 72f3593e..d5f30de5 100644 --- a/apps/platform/src/journey/JourneyRepository.ts +++ b/apps/platform/src/journey/JourneyRepository.ts @@ -163,7 +163,7 @@ export const setJourneyStepMap = async (journeyId: number, stepMap: JourneyStepM const childIds: number[] = [] let ci = 0 - for (const { external_id, data = {} } of list) { + for (const { external_id, path, data = {} } of list) { const child = steps.find(s => s.external_id === external_id) if (!child) continue const idx = children.findIndex(sc => sc.step_id === step.id && sc.child_id === child.id) @@ -173,12 +173,14 @@ export const setJourneyStepMap = async (journeyId: number, stepMap: JourneyStepM step_id: step.id, child_id: child.id, data, + path, priority: ci, }, trx)) } else { stepChild = children[idx] children[idx] = await JourneyStepChild.updateAndFetch(stepChild.id, { data, + path, priority: ci, }, trx) } diff --git a/apps/platform/src/journey/JourneyStep.ts b/apps/platform/src/journey/JourneyStep.ts index 14638c44..a9a86cc8 100644 --- a/apps/platform/src/journey/JourneyStep.ts +++ b/apps/platform/src/journey/JourneyStep.ts @@ -49,6 +49,7 @@ export class JourneyStepChild extends Model { step_id!: number child_id!: number data?: Record + path?: string priority!: number static tableName = 'journey_step_child' @@ -302,9 +303,11 @@ export class JourneyGate extends JourneyStep { if (!this.rule) return const children = state.childrenOf(this.id) - if (!children.length) return + const passed = children.find(c => c.path === 'yes') + const failed = children.find(c => c.path === 'no') + + if (!passed && !failed) return - const [passed, failed] = children const events = await state.events() const params = { @@ -561,6 +564,7 @@ interface JourneyStepMapItem { y: number children?: Array<{ external_id: string + path?: string data?: Record }> } @@ -586,12 +590,13 @@ export async function toJourneyStepMap(steps: JourneyStep[], children: JourneySt data_key: step.data_key, x: step.x ?? 0, y: step.y ?? 0, - children: children.reduce((a, { step_id, child_id, data }) => { + children: children.reduce((a, { step_id, child_id, path, data }) => { if (step_id === step.id) { const child = steps.find(s => s.id === child_id) if (child) { a!.push({ external_id: child.external_id, + path, data, }) } diff --git a/apps/platform/src/journey/__tests__/JourneyService.spec.ts b/apps/platform/src/journey/__tests__/JourneyService.spec.ts index fe12efce..e9f70f70 100644 --- a/apps/platform/src/journey/__tests__/JourneyService.spec.ts +++ b/apps/platform/src/journey/__tests__/JourneyService.spec.ts @@ -5,7 +5,7 @@ import { UserEvent } from '../../users/UserEvent' import Journey from '../Journey' import { setupProject, setupTestJourney } from './helpers' import { JourneyState, enterJourneysFromEvent } from '../JourneyService' -import { JourneyStep, JourneyUserStep } from '../JourneyStep' +import { JourneyStep, JourneyStepMapParams, JourneyUserStep } from '../JourneyStep' import { make } from '../../rules/RuleEngine' import { uuid } from '../../utilities' @@ -33,14 +33,21 @@ describe('JourneyService', () => { } } - const gate = (rule: RuleTree, childIds: string[]) => { + const gate = (rule: RuleTree, yes?: string, no?: string) => { + const children: JourneyStepMapParams[string]['children'] = [] + if (yes) { + children.push({ external_id: yes, path: 'yes' }) + } + if (no) { + children.push({ external_id: no, path: 'no' }) + } return { ...baseStep, type: 'gate', data: { rule, }, - children: childIds.map(external_id => ({ external_id })), + children, } } @@ -186,7 +193,7 @@ describe('JourneyService', () => { path: 'state', operator: '=', value: 'AL', - }, ['g2', 'd1']), + }, 'g2', 'd1'), // match if users favorite guitar brand is Fender g2: gate({ @@ -207,7 +214,7 @@ describe('JourneyService', () => { value: 'Fender', }, ], - }, ['d2', 'd3']), + }, 'd2', 'd3'), d1: delay(0, ''), d2: delay(0, ''), @@ -307,9 +314,9 @@ describe('JourneyService', () => { const { steps } = await Journey.create(project.id, 'infinite loop journey', { e: entrance(0, 'g1'), - g1: gate(rule, ['g2', 'g3']), - g2: gate(rule, ['g1', 'g3']), - g3: gate(rule, []), + g1: gate(rule, 'g2', 'g3'), + g2: gate(rule, 'g1', 'g3'), + g3: gate(rule), }) const user = await User.insertAndFetch({ diff --git a/apps/ui/src/types.ts b/apps/ui/src/types.ts index d0f0f155..81f24727 100644 --- a/apps/ui/src/types.ts +++ b/apps/ui/src/types.ts @@ -265,6 +265,7 @@ export type JourneyStepParams = Omit interface JourneyStepMapChild { external_id: string + path?: string data?: E } @@ -306,10 +307,8 @@ export interface JourneyStepType { newEdgeData?: () => Promise Edit?: ComponentType> EditEdge?: ComponentType> - sources?: - | 'single' // single child (default) - | 'multi' // multiple children, one handle (unordered) - | string[] // enumerated handles (ordered) + sources?: string[] + multiChildSources?: boolean hasDataKey?: boolean } diff --git a/apps/ui/src/views/journey/JourneyEditor.tsx b/apps/ui/src/views/journey/JourneyEditor.tsx index 86fdaa0f..0ea5822d 100644 --- a/apps/ui/src/views/journey/JourneyEditor.tsx +++ b/apps/ui/src/views/journey/JourneyEditor.tsx @@ -151,7 +151,7 @@ function JourneyStepNode({ const validateConnection = useCallback((conn: Connection) => { if (!type) return false - if (type.sources === 'multi') return true + if (type.multiChildSources) return true const sourceNode = conn.source && getNode(conn.source) if (!sourceNode) return true const existing = getConnectedEdges([sourceNode], getEdges()) @@ -232,25 +232,25 @@ function JourneyStepNode({ Array.isArray(type.sources) ? type.sources : [''] - ).map((label, index, arr) => { + ).map((key, index, arr) => { const left = (((index + 1) / (arr.length + 1)) * 100) + '%' return ( - + { - label && ( + key && ( - {label} + {key} ) } | null + path?: string } function createEdge({ data, - index, sourceId, - stepType, targetId, + path, }: CreateEdgeParams): Edge { return { id: 'e-' + sourceId + '__' + targetId, source: sourceId, - sourceHandle: (Array.isArray(stepType?.sources) ? index : 0) + '-s-' + sourceId, + sourceHandle: (path ?? '') + '-s-' + sourceId, target: targetId, targetHandle: 't-' + targetId, data, @@ -395,20 +393,18 @@ function stepsToNodes(stepMap: JourneyStepMap) { stepId, }, }) - const stepType = getStepType(type) - children?.forEach(({ external_id, data }, index) => edges.push(createEdge({ + children?.forEach(({ external_id, path, data }) => edges.push(createEdge({ sourceId: id, targetId: external_id, - index, data, - stepType, + path, }))) } return { nodes, edges } } -const getSourceIndex = (handleId: string) => parseInt(handleId.substring(0, handleId.indexOf('-s-')), 10) +const getSourcePath = (handleId: string) => handleId.substring(0, handleId.indexOf('-s-')) function nodesToSteps(nodes: Node[], edges: Edge[]) { return nodes.reduce((a, { @@ -433,9 +429,9 @@ function nodesToSteps(nodes: Node[], edges: Edge[]) { y, children: edges .filter(e => e.source === id) - .sort((x, y) => getSourceIndex(x.sourceHandle!) - getSourceIndex(y.sourceHandle!)) - .map(({ data = {}, target }) => ({ + .map(({ data = {}, sourceHandle, target }) => ({ external_id: target, + path: getSourcePath(sourceHandle!), data, })), } @@ -464,15 +460,11 @@ function cloneNodes(edges: Edge[], targets: Node[]) { } const edgeCopies = getConnectedEdges(targets, edges) .filter(edge => edge.source in mapping && edge.target in mapping) - .map((edge, index) => createEdge({ + .map((edge) => createEdge({ sourceId: mapping[edge.source], targetId: mapping[edge.target], - index, data: edge.data ?? {}, - stepType: targets - .filter(n => n.id === edge.source) - .map(n => getStepType(n.data.type)) - .at(0)!, + path: getSourcePath(edge.sourceHandle!), })) return { nodeCopies, edgeCopies } } diff --git a/apps/ui/src/views/journey/steps/Balancer.tsx b/apps/ui/src/views/journey/steps/Balancer.tsx index 8267eafe..831010f9 100644 --- a/apps/ui/src/views/journey/steps/Balancer.tsx +++ b/apps/ui/src/views/journey/steps/Balancer.tsx @@ -47,5 +47,5 @@ export const balancerStep: JourneyStepType = { /> ), - sources: 'multi', + multiChildSources: true, } diff --git a/apps/ui/src/views/journey/steps/Experiment.tsx b/apps/ui/src/views/journey/steps/Experiment.tsx index 156b4e69..40e0c5b9 100644 --- a/apps/ui/src/views/journey/steps/Experiment.tsx +++ b/apps/ui/src/views/journey/steps/Experiment.tsx @@ -41,5 +41,5 @@ export const experimentStep: JourneyStepType<{}, ExperimentStepChildConfig> = { /> ) }, - sources: 'multi', + multiChildSources: true, } diff --git a/apps/ui/src/views/journey/steps/Gate.tsx b/apps/ui/src/views/journey/steps/Gate.tsx index 8bc11910..5fbe1f81 100644 --- a/apps/ui/src/views/journey/steps/Gate.tsx +++ b/apps/ui/src/views/journey/steps/Gate.tsx @@ -43,5 +43,5 @@ export const gateStep: JourneyStepType = { /> ) }, - sources: ['Yes', 'No'], + sources: ['yes', 'no'], }