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

8723 workflow add editor in serverless function code step #8805

Merged

Conversation

martmull
Copy link
Contributor

@martmull martmull commented Nov 29, 2024

  • create a serverless function when creating a new workflow code step
  • add code editor in workflow code step
  • move workflowVersion steps management from frontend to backend
    • add a custom resolver for workflow-version management
    • fix optimistic rendering on frontend
  • fix css
  • delete serverless function when deleting workflow code step

TODO

  • Don't update serverlessFunction if no code change
  • Factorize what can be between crud trigger and crud step
  • Publish serverless version when activating workflow
  • delete serverless functions when deleting workflow or workflowVersion
  • fix optimistic rendering for code updates
  • Unify CRUD types
image

@martmull martmull linked an issue Nov 29, 2024 that may be closed by this pull request
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 PR implements serverless function code editing in workflow steps and moves workflow version step management from frontend to backend, with significant changes to both the UI and backend architecture.

  • Added new WorkflowVersionResolver and DTOs in backend to handle workflow step CRUD operations through dedicated endpoints
  • Implemented code editor integration in WorkflowEditActionFormServerlessFunction with Monaco editor and auto-completion support
  • Added serverless function lifecycle management (create/delete) tied to workflow code steps
  • Created DeleteServerlessFunctionJob for batch deletion through message queue system
  • Refactored workflow action types to simplify CRUD operations by consolidating subtypes into single RECORD_CRUD type

56 file(s) reviewed, 52 comment(s)
Edit PR Review Bot Settings | Greptile

@@ -35,7 +35,7 @@ export const WorkflowRunActionEffect = () => {
type: ActionMenuEntryType.WorkflowRun,
key: `workflow-run-${activeWorkflowVersion.id}`,
scope: ActionMenuEntryScope.Global,
label: capitalize(activeWorkflowVersion.workflow.name),
label: capitalize(activeWorkflowVersion.workflow?.name || ''),
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Optional chaining added for workflow?.name but not used consistently - line 48 still accesses workflow.name directly which could cause runtime errors

Comment on lines +44 to +58
findOneRecord: async ({
objectRecordId,
onCompleted,
}: FindOneRecordParams<T>) => {
await findOneRecord({
variables: { objectRecordId },
fetchPolicy,
onCompleted: (data) => {
const record = getRecordFromRecordNode<T>({
recordNode: data[objectNameSingular],
});
onCompleted?.(record);
},
}),
});
},
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Making findOneRecord async but not handling potential rejection could cause unhandled promise rejections. Consider adding try/catch.

Comment on lines +34 to +37
const { serverlessFunctionId = '' } = useParams();
const { availablePackages } = useGetAvailablePackages({
id: serverlessFunctionId,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: serverlessFunctionId defaults to empty string which could cause issues if ID is required by useGetAvailablePackages

} from '~/generated-metadata/graphql';

export const useGetAvailablePackages = () => {
export const useGetAvailablePackages = (input: ServerlessFunctionIdInput) => {
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 making input parameter optional with a default value to maintain backward compatibility with existing usage

Copy link
Contributor

@thomtrp thomtrp left a comment

Choose a reason for hiding this comment

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

Huge work @martmull 💪

@martmull martmull force-pushed the 8723-workflow-add-editor-in-serverless-function-code-step branch from 99f8f19 to 5d36f1c Compare November 29, 2024 16:44
@martmull martmull force-pushed the 8723-workflow-add-editor-in-serverless-function-code-step branch from 5f598a0 to 42673fc Compare December 2, 2024 10:39
@martmull martmull force-pushed the 8723-workflow-add-editor-in-serverless-function-code-step branch from 42673fc to ecf5e2d Compare December 2, 2024 10:43
Copy link
Contributor

@Devessier Devessier left a comment

Choose a reason for hiding this comment

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

Thank you a lot for your pull request, @martmull. It's really nice to edit a serverless function directly from the workflow visualizer.

I spotted a few things that we could discuss.

@martmull martmull force-pushed the 8723-workflow-add-editor-in-serverless-function-code-step branch 2 times, most recently from 07703e8 to 11b0ea1 Compare December 2, 2024 21:23
@martmull martmull force-pushed the 8723-workflow-add-editor-in-serverless-function-code-step branch from 11b0ea1 to 3382033 Compare December 2, 2024 21:41
@martmull martmull enabled auto-merge (squash) December 2, 2024 21:47
@martmull martmull enabled auto-merge (squash) December 2, 2024 22:06
@martmull martmull merged commit d0ff1ff into main Dec 3, 2024
19 checks passed
@martmull martmull deleted the 8723-workflow-add-editor-in-serverless-function-code-step branch December 3, 2024 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Workflow] Add editor in serverless function code step
4 participants