10000 feat(messaging): start using Hogflow inputs in workflow editor by havenbarnes · Pull Request #33678 · PostHog/posthog · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(messaging): start using Hogflow inputs in workflow editor #33678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 195 commits into
base: master
Choose a base branch
from
Open

feat(messaging): start using Hogflow inputs in workflow editor #33678

wants to merge 195 commits into from
+359 −388

Conversation

havenbarnes
10000 Copy link
Contributor

Important

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Problem

Changes

Did you write or update any docs for this change?

How did you test this code?

havenbarnes and others added 30 commits May 6, 2025 18:31
@havenbarnes
chore(messaging): add typedefs for messaging workflows
b3a0d79
@havenbarnes
tweak
8ca9b4a
@github-actions
Update UI snapshots for chromium (7)
ccc913b
@github-actions
Update UI snapshots for chromium (7)
0da72ff
@havenbarnes
feat(messaging): add drag n drop workflow builder UI
0b57bc1
@havenbarnes
tweak
36ba665
@havenbarnes
Merge branch 'messaging-workflow-frontend' of https://github.com/Post…
722076b 
…Hog/posthog into messaging-workflow-frontend
@havenbarnes
Merge branch 'messaging-workflow-frontend' of https://github.com/Post…
a3450b6 
…Hog/posthog into messaging-workflow-frontend-reactflow
@havenbarnes
Merge branch 'master' of https://github.com/PostHog/posthog into mess…
b5e811a 
…aging-workflow-frontend-reactflow
@havenbarnes
tweak
bbbbbee
@havenbarnes
Merge branch 'master' of https://github.com/PostHog/posthog into mess…
37c76ad 
…aging-workflow-frontend
@havenbarnes
tweak
e94ec49
@github-actions
Update UI snapshots for chromium (5)
9373e57
@havenbarnes
tweak
7909419
@havenbarnes
Merge branch 'messaging-workflow-frontend' of https://github.com/Post…
9f5a0ad 
…Hog/posthog into messaging-workflow-frontend
@github-actions
Update UI snapshots for chromium (5)
a3ce57a
@havenbarnes
Merge branch 'master' of https://github.com/PostHog/posthog into mess…
54110fd 
…aging-workflow-frontend
@havenbarnes
Merge branch 'master' of https://github.com/PostHog/posthog into mess…
dedecf5 
…aging-workflow-frontend
@havenbarnes
Merge branch 'messaging-workflow-frontend' of https://github.com/Post…
fa2d7b5 
…Hog/posthog into messaging-workflow-frontend
@havenbarnes
Merge branch 'master' of https://github.com/PostHog/posthog into mess…
7a4b495 
…aging-workflow-frontend
@havenbarnes
Merge branch 'messaging-workflow-frontend' of https://github.com/Post…
f89cfc8 
…Hog/posthog into messaging-workflow-frontend-reactflow
@havenbarnes
stash
6b7a896
@havenbarnes
tweak
7e6e9af
@havenbarnes
stash
ba0bba8
@havenbarnes
tweak
9080335
@havenbarnes
Merge branch 'master' of https://github.com/PostHog/posthog into mess…
72d8e6b 
…aging-workflow-frontend
@havenbarnes
Merge branch 'messaging-workflow-frontend' of https://github.com/Post…
f998e76 
…Hog/posthog into messaging-workflow-frontend-reactflow
@benjackwhite
Rename to shared types and added hogflows
c562f80
@havenbarnes
stash
a87d2d3
@benjackwhite
Fixes
113c96d
havenbarnes and others added 16 commits June 16, 2025 14:27
@havenbarnes
Merge branch 'master' of https://github.com/PostHog/posthog into hogf…
343f8ac 
…unctioninputs-hogflows-refactor
@github-actions
Update UI snapshots for chromium (1)
bae5feb
@havenbarnes
Merge branch 'hogfunctioninputs-hogflows-refactor' of https://github.…
fc2714a 
…com/PostHog/posthog into hogflow-inputs-ux
@havenbarnes
tweak
336f605
@havenbarnes
Merge branch 'hogflow-inputs-ux' of https://github.com/PostHog/posthog …
0f64698 
…into hogflow-inputs-ux
@havenbarnes
Merge branch 'master' of https://github.com/PostHog/posthog into hogf…
3b0e419 
…low-inputs-ux
@havenbarnes
Merge branch 'master' into hogfunctioninputs-hogflows-refactor
0a1afc9
@havenbarnes
Merge branch 'hogfunctioninputs-hogflows-refactor' into hogflow-input…
175ce81 
…s-ux
@havenbarnes
Merge branch 'master' into hogfunctioninputs-hogflows-refactor
08ebbaf
@havenbarnes
Merge branch 'hogfunctioninputs-hogflows-refactor' of https://github.…
cc8cf3e 
…com/PostHog/posthog into hogfunctioninputs-hogflows-refactor
@havenbarnes
rm screenshot diffs
89fd1e6
@havenbarnes
Merge branch 'master' into hogfunctioninputs-hogflows-refactor
7e52e54
@havenbarnes
Merge branch 'hogfunctioninputs-hogflows-refactor' into hogflow-input…
fa77c44 
…s-ux
@havenbarnes
fixes and cleanup
9a8e919
@havenbarnes
Merge branch 'hogflow-inputs-ux' of https://github.com/PostHog/posthog …
1feec93 
…into hogflow-inputs-ux
@havenbarnes
tweak
e1be1da
Base automatically changed from hogfunctioninputs-hogflows-refactor to master June 17, 2025 10:52
havenbarnes
havenbarnes commented Jun 17, 2025
View reviewed changes
products/messaging/frontend/Campaigns/Workflows/Nodes/utils.ts
@@ -1,6 +1,7 @@
import { Edge, getSmoothStepPath, Handle, Node, Position, XYPosition } from '@xyflow/react'
import { NEW_TEMPLATE } from 'products/messaging/frontend/TemplateLibrary/constants'
Copy link
Contributor Author
@havenbarnes havenbarnes Jun 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This whole file needs a fresh coat of paint, will be refactoring in a follow-up PR to make this a bit more organized and less of a pile of export functions

@havenbarnes havenbarnes marked this pull request as ready for review June 17, 2025 18:01
greptile-apps[bot]
greptile-apps bot reviewed Jun 17, 2025
View reviewed changes
Copy link
Contributor
@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Major refactor of the messaging workflow editor to use Hogflow inputs system, replacing the previous basic input implementation with a more robust configuration framework.

  • Introduced products/messaging/frontend/Campaigns/Workflows/Nodes/NodeDetailsPanel.tsx replacing StepDetails.tsx, providing enhanced node configuration with CyclotronJobInputs integration
  • Restructured node data model in products/messaging/frontend/Campaigns/Workflows/types.ts by moving inputs into a config object and simplifying the CyclotronJob input schema
  • Added products/messaging/frontend/Campaigns/Workflows/Nodes/nodeLogic.ts implementing Kea form logic for handling node inputs
  • Improved campaign integration in products/messaging/frontend/Campaigns/campaignLogic.ts with proper API handling using api.hogFlows
  • Added type-specific input schemas in products/messaging/frontend/Campaigns/Workflows/Nodes/utils.ts for different node types (message, delay, conditional_branch)

15 files reviewed, 4 comments
Edit PR Review Bot Settings | Greptile

products/messaging/frontend/Campaigns/campaignLogic.ts
Comment on lines +17 to 29
const NEW_CAMPAIGN: HogFlow = {
id: 'new',
name: 'Untitled campaign',
name: '',
edges: [],
actions: [],
trigger: { type: 'event' },
trigger_masking: { ttl: 0, hash: '', threshold: 0 },
conversion: { window_minutes: 0, filters: [] },
exit_condition: 'exit_on_conversion',
exit_condition: 'exit_only_at_end',
version: 1,
status: 'draft',
team_id: 0,
team_id: -1,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider defining these default values in a constants file for better maintainability and reuse across components

products/messaging/frontend/Campaigns/campaignLogic.ts
router.actions.replace(
urls.messagingCampaign(campaign.id, campaignSceneLogic.findMounted()?.values.currentTab)
)
actions.resetCampaign(campaign)
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: resetCampaign action is used but not defined in the actions block

products/messaging/frontend/Campaigns/Workflows/Nodes/utils.ts
Comment on lines +231 to +235
type: 'string',
key: 'duration',
label: 'Duration (minutes)',
required: true,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Duration is defined as type 'string' but the default value in getNodeInputs is a number (15). This type mismatch could cause runtime issues.

Suggested change
type: 'string',
key: 'duration',
label: 'Duration (minutes)',
required: true,
},
{
type: 'number',
key: 'duration',
label: 'Duration (minutes)',
required: true,
},

products/messaging/frontend/Campaigns/Workflows/Nodes/NodeDetailsPanel.tsx
Comment on lines +26 to +33
const canBeDeleted = (): boolean => {
const outgoingNodes = getOutgoers(node, nodes, edges)
if (outgoingNodes.length === 1) {
return true
}

return new Set(outgoingNodes.map((node) => node.id)).size === 1
}
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: the canBeDeleted logic appears to have an edge case - it returns true if there's 1 outgoing node OR if all outgoing nodes have the same ID, which seems contradictory to the warning about cleaning up branching steps

graphite-app[bot]
graphite-app bot reviewed Jun 17, 2025
View reviewed changes
products/messaging/frontend/Campaigns/Workflows/Nodes/NodeDetailsPanel.tsx Outdated
inputs: node.data.config.inputs,
inputs_schema: getNodeInputsSchema(node.data),
}}
setConfigurationValue={handleInputChange}
Copy link
Contributor

Choose a reason for hiding this comment

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

The CyclotronJobInputs component expects an onInputChange prop, not setConfigurationValue. This prop name mismatch will prevent the inputs from updating correctly. Please change:

setConfigurationValue={handleInputChange}

to:

onInputChange={handleInputChange}
Suggested change
setConfigurationValue={handleInputChange}
onInputChange={handleInputChange}

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@graphite-app graphite-app[bot] graphite-app[bot] left review comments

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

@benjackwhite benjackwhite Awaiting requested review from benjackwhite

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@havenbarnes @posthog-bot @benjackwhite @MoustafaEid
0