Skip to content
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.

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

Trigger workflow on database event #6480

Merged
merged 2 commits into from
Aug 1, 2024
Merged

Conversation

thomtrp
Copy link
Contributor

@thomtrp thomtrp commented Jul 31, 2024

  • Add global listener on database event
  • Fetch event listener associated
  • Trigger associated workflow

Also updated the runner so it expects the input to be in the payload rather than the trigger

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

This pull request introduces a global listener for database events to trigger associated workflows, along with updates to the workflow runner to handle input in the payload.

  • Module Renaming: packages/twenty-server/src/engine/core-modules/workflow/core-workflow-trigger.module.ts renamed to WorkflowTriggerCoreModule for better organization.
  • New Input Type: Added packages/twenty-server/src/engine/core-modules/workflow/dtos/workflow-input.dto.ts to handle JSON payloads for workflow triggers.
  • Listener Addition: Introduced packages/twenty-server/src/modules/workflow/workflow-trigger/listeners/database-event-trigger.listener.ts to dynamically trigger workflows based on database events.
  • Job Handling: Added packages/twenty-server/src/modules/workflow/workflow-trigger/jobs/workflow-event-trigger.job.ts to manage workflow event triggers via message queues.
  • Module Integration: Updated packages/twenty-server/src/modules/workflow/workflow.module.ts to include WorkflowTriggerModule, ensuring seamless integration of the new event-triggering functionality.

15 file(s) reviewed, 32 comment(s)
Edit PR Review Bot Settings

Comment on lines +26 to +27
workflowId,
payload,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should be consistent between those handle function parameters and the runWorkflowparameters. Lets use the workflowId in runWorkflow then. Also, we should update the workflowCommonService.getWorkflowVersion so that we don't need twentyORMGlobalManager service here
Then the runWorkflow method can also use the updated version of workflowCommonService.getWorkflowVersion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@martmull agreed with the input update. But thinking again about the endpoint, I think this is the version we want to run. This endpoint will be used to test versions. I rename it to be consistent. Happy to discuss it if you disagree!

Comment on lines +53 to +63
const workflowEventListenerRepository =
await this.twentyORMGlobalManager.getRepositoryForWorkspace<WorkflowEventListenerWorkspaceEntity>(
workspaceId,
'workflowEventListener',
);

const eventListeners = await workflowEventListenerRepository.find({
where: {
eventName,
},
});
Copy link
Contributor

Choose a reason for hiding this comment

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

think you can move that in workflow-common.service.ts in a getEventListeners method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only place where we fetch eventListeners and not sure we will have others. I prefer put in common functions that will be used in several places, across sub-modules. Wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

Copy link
Contributor

@martmull martmull left a comment

Choose a reason for hiding this comment

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

Some comments

@thomtrp thomtrp force-pushed the tt-add-listener-on-workflow-event branch from 8cdc8fe to a850682 Compare August 1, 2024 09:35
@martmull martmull merged commit 8c8f192 into main Aug 1, 2024
6 checks passed
@martmull martmull deleted the tt-add-listener-on-workflow-event branch August 1, 2024 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants