From 83bf2d1739f57541153ccff371eb5b3f2cb6fba2 Mon Sep 17 00:00:00 2001 From: martmull Date: Tue, 11 Feb 2025 17:30:11 +0100 Subject: [PATCH] Webhook follow up (#10124) - fix webhook creation - fix pointer --- .../hooks/useCreateManyRecords.ts | 6 +- .../object-record/hooks/useCreateOneRecord.ts | 8 +- .../components/FormNumberFieldInput.tsx | 2 +- .../components/FormTextFieldInput.tsx | 2 +- .../SettingsDevelopersWebhookTableRow.tsx | 5 +- .../developers/hooks/useWebhookUpdateForm.ts | 100 ++++++++++++------ .../ui/input/components/InputErrorHelper.tsx | 17 ++- .../ui/input/components/TextInputV2.tsx | 12 +-- .../WorkflowEditTriggerCronForm.tsx | 2 +- .../developers/SettingsDevelopers.tsx | 32 ++---- .../SettingsDevelopersWebhookDetail.tsx | 13 ++- .../src/utils/url/getUrlHostname.ts | 9 +- .../assert-version-can-be-activated.util.ts | 89 ++++++++++------ 13 files changed, 184 insertions(+), 113 deletions(-) diff --git a/packages/twenty-front/src/modules/object-record/hooks/useCreateManyRecords.ts b/packages/twenty-front/src/modules/object-record/hooks/useCreateManyRecords.ts index 46d3bfcfe..de86ff086 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/useCreateManyRecords.ts +++ b/packages/twenty-front/src/modules/object-record/hooks/useCreateManyRecords.ts @@ -26,7 +26,7 @@ type PartialObjectRecordWithId = Partial & { type useCreateManyRecordsProps = { objectNameSingular: string; recordGqlFields?: RecordGqlOperationGqlRecordFields; - skipPostOptmisticEffect?: boolean; + skipPostOptimisticEffect?: boolean; shouldMatchRootQueryFilter?: boolean; }; @@ -35,7 +35,7 @@ export const useCreateManyRecords = < >({ objectNameSingular, recordGqlFields, - skipPostOptmisticEffect = false, + skipPostOptimisticEffect = false, shouldMatchRootQueryFilter, }: useCreateManyRecordsProps) => { const apolloClient = useApolloClient(); @@ -135,7 +135,7 @@ export const useCreateManyRecords = < update: (cache, { data }) => { const records = data?.[mutationResponseField]; - if (!isDefined(records?.length) || skipPostOptmisticEffect) return; + if (!isDefined(records?.length) || skipPostOptimisticEffect) return; triggerCreateRecordsOptimisticEffect({ cache, diff --git a/packages/twenty-front/src/modules/object-record/hooks/useCreateOneRecord.ts b/packages/twenty-front/src/modules/object-record/hooks/useCreateOneRecord.ts index 16a2205d6..c8379ec6d 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/useCreateOneRecord.ts +++ b/packages/twenty-front/src/modules/object-record/hooks/useCreateOneRecord.ts @@ -23,7 +23,7 @@ import { isDefined } from 'twenty-shared'; type useCreateOneRecordProps = { objectNameSingular: string; recordGqlFields?: RecordGqlOperationGqlRecordFields; - skipPostOptmisticEffect?: boolean; + skipPostOptimisticEffect?: boolean; shouldMatchRootQueryFilter?: boolean; }; @@ -32,7 +32,7 @@ export const useCreateOneRecord = < >({ objectNameSingular, recordGqlFields, - skipPostOptmisticEffect = false, + skipPostOptimisticEffect = false, shouldMatchRootQueryFilter, }: useCreateOneRecordProps) => { const apolloClient = useApolloClient(); @@ -95,7 +95,7 @@ export const useCreateOneRecord = < computeReferences: false, }); - if (optimisticRecordNode !== null) { + if (skipPostOptimisticEffect === false && optimisticRecordNode !== null) { triggerCreateRecordsOptimisticEffect({ cache: apolloClient.cache, objectMetadataItem, @@ -117,7 +117,7 @@ export const useCreateOneRecord = < }, update: (cache, { data }) => { const record = data?.[mutationResponseField]; - if (skipPostOptmisticEffect === false && isDefined(record)) { + if (skipPostOptimisticEffect === false && isDefined(record)) { triggerCreateRecordsOptimisticEffect({ cache, objectMetadataItem, diff --git a/packages/twenty-front/src/modules/object-record/record-field/form-types/components/FormNumberFieldInput.tsx b/packages/twenty-front/src/modules/object-record/record-field/form-types/components/FormNumberFieldInput.tsx index f19b5ac22..8e72ea6a2 100644 --- a/packages/twenty-front/src/modules/object-record/record-field/form-types/components/FormNumberFieldInput.tsx +++ b/packages/twenty-front/src/modules/object-record/record-field/form-types/components/FormNumberFieldInput.tsx @@ -139,7 +139,7 @@ export const FormNumberFieldInput = ({ {hint ? {hint} : null} - {error && {error}} + {error} ); }; diff --git a/packages/twenty-front/src/modules/object-record/record-field/form-types/components/FormTextFieldInput.tsx b/packages/twenty-front/src/modules/object-record/record-field/form-types/components/FormTextFieldInput.tsx index 35d531086..7190a6ff8 100644 --- a/packages/twenty-front/src/modules/object-record/record-field/form-types/components/FormTextFieldInput.tsx +++ b/packages/twenty-front/src/modules/object-record/record-field/form-types/components/FormTextFieldInput.tsx @@ -91,7 +91,7 @@ export const FormTextFieldInput = ({ ) : null} {hint && {hint}} - {error && {error}} + {error} ); }; diff --git a/packages/twenty-front/src/modules/settings/developers/components/SettingsDevelopersWebhookTableRow.tsx b/packages/twenty-front/src/modules/settings/developers/components/SettingsDevelopersWebhookTableRow.tsx index d559f89a2..c848cd0d0 100644 --- a/packages/twenty-front/src/modules/settings/developers/components/SettingsDevelopersWebhookTableRow.tsx +++ b/packages/twenty-front/src/modules/settings/developers/components/SettingsDevelopersWebhookTableRow.tsx @@ -5,6 +5,7 @@ import { IconChevronRight } from 'twenty-ui'; import { Webhook } from '@/settings/developers/types/webhook/Webhook'; import { TableCell } from '@/ui/layout/table/components/TableCell'; import { TableRow } from '@/ui/layout/table/components/TableRow'; +import { getUrlHostname } from '~/utils/url/getUrlHostname'; export const StyledApisFieldTableRow = styled(TableRow)` grid-template-columns: 1fr 28px; @@ -37,7 +38,9 @@ export const SettingsDevelopersWebhookTableRow = ({ return ( - {fieldItem.targetUrl} + + {getUrlHostname(fieldItem.targetUrl, { keepPath: true })} + { +export const useWebhookUpdateForm = ({ + webhookId, + isCreationMode, +}: { + webhookId: string; + isCreationMode: boolean; +}) => { const navigate = useNavigateSettings(); - const [loading, setLoading] = useState(true); + const [isCreated, setIsCreated] = useState(!isCreationMode); + const [loading, setLoading] = useState(!isCreationMode); + const [title, setTitle] = useState(isCreationMode ? 'New Webhook' : ''); const [formData, setFormData] = useState({ targetUrl: '', @@ -37,6 +47,10 @@ export const useWebhookUpdateForm = ({ webhookId }: { webhookId: string }) => { objectNameSingular: CoreObjectNameSingular.Webhook, }); + const { createOneRecord } = useCreateOneRecord({ + objectNameSingular: CoreObjectNameSingular.Webhook, + }); + const addEmptyOperationIfNecessary = ( newOperations: WebhookOperationType[], ) => { @@ -49,34 +63,6 @@ export const useWebhookUpdateForm = ({ webhookId }: { webhookId: string }) => { return newOperations; }; - useFindOneRecord({ - objectNameSingular: CoreObjectNameSingular.Webhook, - objectRecordId: webhookId, - onCompleted: (data) => { - const baseOperations = data?.operations - ? data.operations.map((op: string) => { - const [object, action] = op.split('.'); - return { object, action }; - }) - : data?.operation - ? [ - { - object: data.operation.split('.')[0], - action: data.operation.split('.')[1], - }, - ] - : []; - const operations = addEmptyOperationIfNecessary(baseOperations); - setFormData({ - targetUrl: data.targetUrl, - description: data.description, - operations, - secret: data.secret, - }); - setLoading(false); - }, - }); - const cleanAndFormatOperations = (operations: WebhookOperationType[]) => { return Array.from( new Set( @@ -90,6 +76,19 @@ export const useWebhookUpdateForm = ({ webhookId }: { webhookId: string }) => { const handleSave = useDebouncedCallback(async () => { const cleanedOperations = cleanAndFormatOperations(formData.operations); + const webhookData = { + ...(isTargetUrlValid && { targetUrl: formData.targetUrl.trim() }), + operations: cleanedOperations, + description: formData.description, + secret: formData.secret, + }; + + if (!isCreated) { + await createOneRecord({ id: webhookId, ...webhookData }); + setIsCreated(true); + return; + } + await updateOneRecord({ idToUpdate: webhookId, updateOneRecordInput: { @@ -104,7 +103,13 @@ export const useWebhookUpdateForm = ({ webhookId }: { webhookId: string }) => { const validateData = (data: Partial) => { if (isDefined(data?.targetUrl)) { const trimmedUrl = data.targetUrl.trim(); - setIsTargetUrlValid(isValidUrl(trimmedUrl)); + const isTargetUrlValid = isValidUrl(trimmedUrl); + setIsTargetUrlValid(isTargetUrlValid); + if (isTargetUrlValid) { + setTitle( + getUrlHostname(trimmedUrl, { keepPath: true }) || 'New Webhook', + ); + } } }; @@ -147,8 +152,41 @@ export const useWebhookUpdateForm = ({ webhookId }: { webhookId: string }) => { navigate(SettingsPath.Developers); }; + useFindOneRecord({ + skip: isCreationMode, + objectNameSingular: CoreObjectNameSingular.Webhook, + objectRecordId: webhookId, + onCompleted: (data) => { + const baseOperations = data?.operations + ? data.operations.map((op: string) => { + const [object, action] = op.split('.'); + return { object, action }; + }) + : data?.operation + ? [ + { + object: data.operation.split('.')[0], + action: data.operation.split('.')[1], + }, + ] + : []; + const operations = addEmptyOperationIfNecessary(baseOperations); + setFormData({ + targetUrl: data.targetUrl, + description: data.description, + operations, + secret: data.secret, + }); + setTitle( + getUrlHostname(data.targetUrl, { keepPath: true }) || 'New Webhook', + ); + setLoading(false); + }, + }); + return { formData, + title, isTargetUrlValid, updateWebhook, updateOperation, diff --git a/packages/twenty-front/src/modules/ui/input/components/InputErrorHelper.tsx b/packages/twenty-front/src/modules/ui/input/components/InputErrorHelper.tsx index 5c0b1deb5..2f15b31a7 100644 --- a/packages/twenty-front/src/modules/ui/input/components/InputErrorHelper.tsx +++ b/packages/twenty-front/src/modules/ui/input/components/InputErrorHelper.tsx @@ -4,12 +4,25 @@ import styled from '@emotion/styled'; const StyledInputErrorHelper = styled.div` color: ${({ theme }) => theme.color.red}; font-size: ${({ theme }) => theme.font.size.xs}; + position: absolute; +`; + +const StyledErrorContainer = styled.div` + margin-top: ${({ theme }) => theme.spacing(1)}; `; export const InputErrorHelper = ({ children, + isVisible = true, }: { - children: React.ReactNode; + children?: React.ReactNode; + isVisible?: boolean; }) => ( - {children} + + {children && isVisible && ( + + {children} + + )} + ); diff --git a/packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx b/packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx index b4bb09178..e9a0e6e53 100644 --- a/packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx +++ b/packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx @@ -76,18 +76,14 @@ const StyledInput = styled.input< } &:focus { - ${({ theme }) => { + ${({ theme, error }) => { return ` - border-color: ${theme.color.blue}; + border-color: ${error ? theme.border.color.danger : theme.color.blue}; `; }}; } `; -const StyledErrorHelper = styled(InputErrorHelper)` - padding: ${({ theme }) => theme.spacing(1)}; -`; - const StyledLeftIconContainer = styled.div<{ sizeVariant: TextInputV2Size }>` align-items: center; display: flex; @@ -270,9 +266,7 @@ const TextInputV2Component = ( )} - {error && !noErrorHelper && ( - {error} - )} + {error} ); }; diff --git a/packages/twenty-front/src/modules/workflow/workflow-trigger/components/WorkflowEditTriggerCronForm.tsx b/packages/twenty-front/src/modules/workflow/workflow-trigger/components/WorkflowEditTriggerCronForm.tsx index e56a16c74..3ef5e6172 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-trigger/components/WorkflowEditTriggerCronForm.tsx +++ b/packages/twenty-front/src/modules/workflow/workflow-trigger/components/WorkflowEditTriggerCronForm.tsx @@ -111,7 +111,7 @@ export const WorkflowEditTriggerCronForm = ({ placeholder="0 */1 * * *" error={errorMessagesVisible ? errorMessages.CUSTOM : undefined} onBlur={onBlur} - hint="Format: [Second] [Minute] [Hour] [Day of Month] [Month] [Day of Week]" + hint="Format: [Minute] [Hour] [Day of Month] [Month] [Day of Week]" readonly={triggerOptions.readonly} defaultValue={trigger.settings.pattern} onPersist={(newPattern: string) => { diff --git a/packages/twenty-front/src/pages/settings/developers/SettingsDevelopers.tsx b/packages/twenty-front/src/pages/settings/developers/SettingsDevelopers.tsx index b3858e301..e76b5bf3a 100644 --- a/packages/twenty-front/src/pages/settings/developers/SettingsDevelopers.tsx +++ b/packages/twenty-front/src/pages/settings/developers/SettingsDevelopers.tsx @@ -1,3 +1,4 @@ +import { v4 } from 'uuid'; import { SettingsPageContainer } from '@/settings/components/SettingsPageContainer'; import { SettingsApiKeysTable } from '@/settings/developers/components/SettingsApiKeysTable'; import { SettingsReadDocumentationButton } from '@/settings/developers/components/SettingsReadDocumentationButton'; @@ -9,10 +10,6 @@ import styled from '@emotion/styled'; import { Trans, useLingui } from '@lingui/react/macro'; import { Button, H2Title, IconPlus, MOBILE_VIEWPORT, Section } from 'twenty-ui'; import { getSettingsPath } from '~/utils/navigation/getSettingsPath'; -import { useCreateOneRecord } from '@/object-record/hooks/useCreateOneRecord'; -import { Webhook } from '@/settings/developers/types/webhook/Webhook'; -import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; -import { useNavigateSettings } from '~/hooks/useNavigateSettings'; const StyledButtonContainer = styled.div` display: flex; @@ -32,24 +29,7 @@ const StyledContainer = styled.div<{ isMobile: boolean }>` export const SettingsDevelopers = () => { const isMobile = useIsMobile(); - const navigate = useNavigateSettings(); const { t } = useLingui(); - const { createOneRecord: createOneWebhook } = useCreateOneRecord({ - objectNameSingular: CoreObjectNameSingular.Webhook, - }); - - const createNewWebhook = async () => { - const newWebhook = await createOneWebhook({ - targetUrl: '', - operations: ['*.*'], - }); - if (!newWebhook) { - return; - } - navigate(SettingsPath.DevelopersNewWebhookDetail, { - webhookId: newWebhook.id, - }); - }; return ( { title={t`Create Webhook`} size="small" variant="secondary" - onClick={createNewWebhook} + to={getSettingsPath( + SettingsPath.DevelopersNewWebhookDetail, + { + webhookId: v4(), + }, + { + creationMode: true, + }, + )} /> diff --git a/packages/twenty-front/src/pages/settings/developers/webhooks/components/SettingsDevelopersWebhookDetail.tsx b/packages/twenty-front/src/pages/settings/developers/webhooks/components/SettingsDevelopersWebhookDetail.tsx index dccedc335..15d71d551 100644 --- a/packages/twenty-front/src/pages/settings/developers/webhooks/components/SettingsDevelopersWebhookDetail.tsx +++ b/packages/twenty-front/src/pages/settings/developers/webhooks/components/SettingsDevelopersWebhookDetail.tsx @@ -1,7 +1,7 @@ import { useIsMobile } from '@/ui/utilities/responsive/hooks/useIsMobile'; import styled from '@emotion/styled'; import { useMemo, useState } from 'react'; -import { useParams } from 'react-router-dom'; +import { useParams, useSearchParams } from 'react-router-dom'; import { Button, H2Title, @@ -33,6 +33,7 @@ import { useRecoilValue } from 'recoil'; import { FeatureFlagKey } from '~/generated/graphql'; import { getSettingsPath } from '~/utils/navigation/getSettingsPath'; import { useWebhookUpdateForm } from '@/settings/developers/hooks/useWebhookUpdateForm'; +import { isDefined } from 'twenty-shared'; const OBJECT_DROPDOWN_WIDTH = 340; const ACTION_DROPDOWN_WIDTH = 140; @@ -68,8 +69,13 @@ export const SettingsDevelopersWebhooksDetail = () => { const { webhookId = '' } = useParams(); + const [searchParams] = useSearchParams(); + + const isCreationMode = isDefined(searchParams.get('creationMode')); + const { formData, + title, loading, isTargetUrlValid, updateWebhook, @@ -78,6 +84,7 @@ export const SettingsDevelopersWebhooksDetail = () => { deleteWebhook, } = useWebhookUpdateForm({ webhookId, + isCreationMode, }); const [isDeleteWebhookModalOpen, setIsDeleteWebhookModalOpen] = @@ -114,7 +121,7 @@ export const SettingsDevelopersWebhooksDetail = () => { return ( { fullWidth /> - {isAnalyticsEnabled && isAnalyticsV2Enabled && ( + {!isCreationMode && isAnalyticsEnabled && isAnalyticsV2Enabled && ( diff --git a/packages/twenty-front/src/utils/url/getUrlHostname.ts b/packages/twenty-front/src/utils/url/getUrlHostname.ts index ce3164f85..601fc48f6 100644 --- a/packages/twenty-front/src/utils/url/getUrlHostname.ts +++ b/packages/twenty-front/src/utils/url/getUrlHostname.ts @@ -1,9 +1,12 @@ import { getAbsoluteUrl } from '~/utils/url/getAbsoluteUrl'; -export const getUrlHostname = (url: string) => { +export const getUrlHostname = ( + url: string, + options?: { keepPath?: boolean }, +) => { try { - const absoluteUrl = getAbsoluteUrl(url); - return new URL(absoluteUrl).hostname.replace(/^www\./i, ''); + const parsedUrl = new URL(getAbsoluteUrl(url)); + return `${parsedUrl.hostname.replace(/^www\./i, '')}${options?.keepPath && parsedUrl.pathname !== '/' ? parsedUrl.pathname : ''}`; } catch { return ''; } diff --git a/packages/twenty-server/src/modules/workflow/workflow-trigger/utils/assert-version-can-be-activated.util.ts b/packages/twenty-server/src/modules/workflow/workflow-trigger/utils/assert-version-can-be-activated.util.ts index 1e8686308..12b421e24 100644 --- a/packages/twenty-server/src/modules/workflow/workflow-trigger/utils/assert-version-can-be-activated.util.ts +++ b/packages/twenty-server/src/modules/workflow/workflow-trigger/utils/assert-version-can-be-activated.util.ts @@ -88,40 +88,65 @@ function assertCronTriggerSettingsAreValid(settings: any) { WorkflowTriggerExceptionCode.INVALID_WORKFLOW_TRIGGER, ); } - if (settings.type === 'CUSTOM' && !settings.pattern) { - throw new WorkflowTriggerException( - 'No pattern provided in CUSTOM cron trigger', - WorkflowTriggerExceptionCode.INVALID_WORKFLOW_TRIGGER, - ); - } - if (!settings.schedule) { - throw new WorkflowTriggerException( - 'No schedule provided in cron trigger', - WorkflowTriggerExceptionCode.INVALID_WORKFLOW_TRIGGER, - ); - } - if (settings.type === 'HOURS' && settings.schedule.hour <= 0) { - throw new WorkflowTriggerException( - 'Invalid hour value. Should be integer greater than 1', - WorkflowTriggerExceptionCode.INVALID_WORKFLOW_TRIGGER, - ); - } + switch (settings.type) { + case 'CUSTOM': { + if (!settings.pattern) { + throw new WorkflowTriggerException( + 'No pattern provided in CUSTOM cron trigger', + WorkflowTriggerExceptionCode.INVALID_WORKFLOW_TRIGGER, + ); + } - if ( - settings.type === 'HOURS' && - (settings.schedule.minute < 0 || settings.schedule.minute > 59) - ) { - throw new WorkflowTriggerException( - 'Invalid minute value. Should be integer between 0 and 59', - WorkflowTriggerExceptionCode.INVALID_WORKFLOW_TRIGGER, - ); - } + return; + } - if (settings.type === 'MINUTES' && settings.schedule.minute <= 0) { - throw new WorkflowTriggerException( - 'Invalid minute value. Should be integer greater than 1', - WorkflowTriggerExceptionCode.INVALID_WORKFLOW_TRIGGER, - ); + case 'HOURS': { + if (!settings.schedule) { + throw new WorkflowTriggerException( + 'No schedule provided in cron trigger', + WorkflowTriggerExceptionCode.INVALID_WORKFLOW_TRIGGER, + ); + } + if (settings.schedule.hour <= 0) { + throw new WorkflowTriggerException( + 'Invalid hour value. Should be integer greater than 1', + WorkflowTriggerExceptionCode.INVALID_WORKFLOW_TRIGGER, + ); + } + + if (settings.schedule.minute < 0 || settings.schedule.minute > 59) { + throw new WorkflowTriggerException( + 'Invalid minute value. Should be integer between 0 and 59', + WorkflowTriggerExceptionCode.INVALID_WORKFLOW_TRIGGER, + ); + } + + return; + } + + case 'MINUTES': { + if (!settings.schedule) { + throw new WorkflowTriggerException( + 'No schedule provided in cron trigger', + WorkflowTriggerExceptionCode.INVALID_WORKFLOW_TRIGGER, + ); + } + + if (settings.schedule.minute <= 0) { + throw new WorkflowTriggerException( + 'Invalid minute value. Should be integer greater than 1', + WorkflowTriggerExceptionCode.INVALID_WORKFLOW_TRIGGER, + ); + } + + return; + } + + default: + throw new WorkflowTriggerException( + 'Invalid setting type provided in cron trigger', + WorkflowTriggerExceptionCode.INVALID_WORKFLOW_TRIGGER, + ); } }