-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Remove ID field + fix relations + remove runs #12796
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
Conversation
There was a problem hiding this 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 crucial fixes for workflow actions and version views, enhancing relation handling and field visibility.
- Added proper relation formatting in
packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/components/WorkflowEditActionCreateRecord.tsx
to handle many-to-one relations with{id: string}
structure - Modified
shouldDisplayFormField.ts
to restrict ID field visibility to only FIND_RECORDS action type - Hidden 'runs' field in workflow versions view by setting
isVisible: false
inworkflow-versions-all.view.ts
- Improved type safety with new
RelationManyToOneField
type in action components - Added field definition validation before updates to prevent relation handling errors
4 files reviewed, 1 comment
Edit PR Review Bot Settings | Greptile
...dules/workflow/workflow-steps/workflow-actions/components/WorkflowEditActionCreateRecord.tsx
Show resolved
Hide resolved
6050321
to
ffe0f75
Compare
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:17422 This environment will automatically shut down when the PR is closed or after 5 hours. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! I left a small nitpick comment.
...dules/workflow/workflow-steps/workflow-actions/components/WorkflowEditActionCreateRecord.tsx
Show resolved
Hide resolved
@@ -41,24 +41,30 @@ export const shouldDisplayFormField = ({ | |||
isTypeAllowedForAction = | |||
fieldMetadataItem.type !== FieldMetadataType.RELATION || | |||
fieldMetadataItem.settings?.['relationType'] === 'MANY_TO_ONE'; | |||
break; | |||
return ( | |||
isTypeAllowedForAction && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we redefine the isTypeAllowedForAction
variable in each case of the switch statement, I wouldn't use a let
anymore and would instead declare a constant in each case. Wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it still makes the assertion easier to read. And I cannot declare the variable into the switch 😓
accountOwner: { id: string }
instead ofaccountOwner: string
runs
for version views as we did for workflows