From d916ec0af98e026246f5810b53a4366f15814653 Mon Sep 17 00:00:00 2001 From: Paul Rastoin <45004772+prastoin@users.noreply.github.com> Date: Tue, 15 Jul 2025 12:27:00 +0200 Subject: [PATCH] Fix link formatting (#13210) closes https://github.com/twentyhq/twenty/issues/13207 --------- Co-authored-by: Lucas Bordeau --- ...spreadsheetImportGetUnicityRowHook.test.ts | 7 ++- .../spreadsheetImportGetUnicityRowHook.ts | 4 +- .../utils/transform-links-value.util.ts | 22 ++++------ .../services/create-company.service.ts | 4 +- .../LabelIdentifierFieldMetadataTypes.ts | 2 +- .../__tests__/removeUndefinedFields.test.ts | 8 +--- .../twenty-shared/src/utils/getURLSafely.ts | 7 +++ packages/twenty-shared/src/utils/index.ts | 3 +- ...lowercaseUrlAndRemoveTrailingSlash.test.ts | 24 ---------- ...aseUrlOriginAndRemoveTrailingSlash.test.ts | 44 +++++++++++++++++++ .../url/lowercaseUrlAndRemoveTrailingSlash.ts | 7 --- ...owercaseUrlOriginAndRemoveTrailingSlash.ts | 15 +++++++ 12 files changed, 87 insertions(+), 60 deletions(-) create mode 100644 packages/twenty-shared/src/utils/getURLSafely.ts delete mode 100644 packages/twenty-shared/src/utils/url/__tests__/lowercaseUrlAndRemoveTrailingSlash.test.ts create mode 100644 packages/twenty-shared/src/utils/url/__tests__/lowercaseUrlOriginAndRemoveTrailingSlash.test.ts delete mode 100644 packages/twenty-shared/src/utils/url/lowercaseUrlAndRemoveTrailingSlash.ts create mode 100644 packages/twenty-shared/src/utils/url/lowercaseUrlOriginAndRemoveTrailingSlash.ts diff --git a/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/__tests__/spreadsheetImportGetUnicityRowHook.test.ts b/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/__tests__/spreadsheetImportGetUnicityRowHook.test.ts index 5cde13839..3575d398a 100644 --- a/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/__tests__/spreadsheetImportGetUnicityRowHook.test.ts +++ b/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/__tests__/spreadsheetImportGetUnicityRowHook.test.ts @@ -79,11 +79,10 @@ describe('spreadsheetImportGetUnicityRowHook', () => { it('should return row with error if row is not unique - index on composite field', () => { const hook = spreadsheetImportGetUnicityRowHook(mockObjectMetadataItem); - const testData: ImportedStructuredRow[] = [ - { 'Link URL (domainName)': 'duplicaTe.com' }, - { 'Link URL (domainName)': 'duplicate.com ' }, - { 'Link URL (domainName)': 'other.com' }, + { 'Link URL (domainName)': 'https://duplicaTe.com' }, + { 'Link URL (domainName)': 'https://duplicate.com' }, + { 'Link URL (domainName)': 'https://other.com' }, ]; const addErrorMock = jest.fn(); diff --git a/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/spreadsheetImportGetUnicityRowHook.ts b/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/spreadsheetImportGetUnicityRowHook.ts index eff0902be..52343e47d 100644 --- a/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/spreadsheetImportGetUnicityRowHook.ts +++ b/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/spreadsheetImportGetUnicityRowHook.ts @@ -12,7 +12,7 @@ import { isNonEmptyString } from '@sniptt/guards'; import { FieldMetadataType } from 'twenty-shared/types'; import { isDefined, - lowercaseUrlAndRemoveTrailingSlash, + lowercaseUrlOriginAndRemoveTrailingSlash, } from 'twenty-shared/utils'; type Column = { @@ -108,7 +108,7 @@ const getUniqueValues = ( .primaryLinkUrl, ) ) { - return lowercaseUrlAndRemoveTrailingSlash( + return lowercaseUrlOriginAndRemoveTrailingSlash( row?.[columnName]?.toString().trim() || '', ); } diff --git a/packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-links-value.util.ts b/packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-links-value.util.ts index 0b36b7cb8..776c607f9 100644 --- a/packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-links-value.util.ts +++ b/packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-links-value.util.ts @@ -1,7 +1,8 @@ import { isNonEmptyString } from '@sniptt/guards'; import { isDefined, - lowercaseUrlAndRemoveTrailingSlash, + lowercaseUrlOriginAndRemoveTrailingSlash, + parseJson, } from 'twenty-shared/utils'; import { removeEmptyLinks } from 'src/engine/core-modules/record-transformer/utils/remove-empty-links'; @@ -16,10 +17,11 @@ export type LinksFieldGraphQLInput = | null | undefined; +// TODO refactor this function handle partial composite field update export const transformLinksValue = ( value: LinksFieldGraphQLInput, ): LinksFieldGraphQLInput => { - if (!value) { + if (!isDefined(value)) { return value; } @@ -27,15 +29,9 @@ export const transformLinksValue = ( const primaryLinkLabelRaw = value.primaryLinkLabel as string | null; const secondaryLinksRaw = value.secondaryLinks as string | null; - let secondaryLinksArray: LinkMetadataNullable[] | null = null; - - if (isNonEmptyString(secondaryLinksRaw)) { - try { - secondaryLinksArray = JSON.parse(secondaryLinksRaw); - } catch { - /* empty */ - } - } + const secondaryLinksArray = isNonEmptyString(secondaryLinksRaw) + ? parseJson(secondaryLinksRaw) + : null; const { primaryLinkLabel, primaryLinkUrl, secondaryLinks } = removeEmptyLinks( { @@ -48,14 +44,14 @@ export const transformLinksValue = ( return { ...value, primaryLinkUrl: isDefined(primaryLinkUrl) - ? lowercaseUrlAndRemoveTrailingSlash(primaryLinkUrl) + ? lowercaseUrlOriginAndRemoveTrailingSlash(primaryLinkUrl) : primaryLinkUrl, primaryLinkLabel, secondaryLinks: JSON.stringify( secondaryLinks?.map((link) => ({ ...link, url: isDefined(link.url) - ? lowercaseUrlAndRemoveTrailingSlash(link.url) + ? lowercaseUrlOriginAndRemoveTrailingSlash(link.url) : link.url, })), ), diff --git a/packages/twenty-server/src/modules/contact-creation-manager/services/create-company.service.ts b/packages/twenty-server/src/modules/contact-creation-manager/services/create-company.service.ts index 1782b49e4..d45152c59 100644 --- a/packages/twenty-server/src/modules/contact-creation-manager/services/create-company.service.ts +++ b/packages/twenty-server/src/modules/contact-creation-manager/services/create-company.service.ts @@ -5,7 +5,7 @@ import axios, { AxiosInstance } from 'axios'; import uniqBy from 'lodash.uniqby'; import { TWENTY_COMPANIES_BASE_URL } from 'twenty-shared/constants'; import { ConnectedAccountProvider } from 'twenty-shared/types'; -import { lowercaseUrlAndRemoveTrailingSlash } from 'twenty-shared/utils'; +import { lowercaseUrlOriginAndRemoveTrailingSlash } from 'twenty-shared/utils'; import { DeepPartial, ILike, Repository } from 'typeorm'; import { DatabaseEventAction } from 'src/engine/api/graphql/graphql-query-runner/enums/database-event-action'; @@ -79,7 +79,7 @@ export class CreateCompanyService { const companiesWithoutTrailingSlash = companies.map((company) => ({ ...company, domainName: company.domainName - ? lowercaseUrlAndRemoveTrailingSlash(company.domainName) + ? lowercaseUrlOriginAndRemoveTrailingSlash(company.domainName) : undefined, })); diff --git a/packages/twenty-shared/src/constants/LabelIdentifierFieldMetadataTypes.ts b/packages/twenty-shared/src/constants/LabelIdentifierFieldMetadataTypes.ts index 66eefa316..8bca62a86 100644 --- a/packages/twenty-shared/src/constants/LabelIdentifierFieldMetadataTypes.ts +++ b/packages/twenty-shared/src/constants/LabelIdentifierFieldMetadataTypes.ts @@ -1,4 +1,4 @@ -import { FieldMetadataType } from "@/types"; +import { FieldMetadataType } from '@/types'; export const LABEL_IDENTIFIER_FIELD_METADATA_TYPES = [ FieldMetadataType.TEXT, diff --git a/packages/twenty-shared/src/utils/__tests__/removeUndefinedFields.test.ts b/packages/twenty-shared/src/utils/__tests__/removeUndefinedFields.test.ts index 25917b3e0..956393b90 100644 --- a/packages/twenty-shared/src/utils/__tests__/removeUndefinedFields.test.ts +++ b/packages/twenty-shared/src/utils/__tests__/removeUndefinedFields.test.ts @@ -105,11 +105,7 @@ describe('removeUndefinedFields', () => { }, expected: { names: ['John', 'Jane', null], - tags: [ - { id: 1, label: 'active' }, - { label: 'pending' }, - { id: 3 }, - ], + tags: [{ id: 1, label: 'active' }, { label: 'pending' }, { id: 3 }], }, }, { @@ -131,4 +127,4 @@ describe('removeUndefinedFields', () => { expect(removeUndefinedFields(input)).toEqual(expected); }); }); -}); \ No newline at end of file +}); diff --git a/packages/twenty-shared/src/utils/getURLSafely.ts b/packages/twenty-shared/src/utils/getURLSafely.ts new file mode 100644 index 000000000..a207ea62f --- /dev/null +++ b/packages/twenty-shared/src/utils/getURLSafely.ts @@ -0,0 +1,7 @@ +export const getURLSafely = (url: string) => { + try { + return new URL(url); + } catch (e: unknown) { + return null; + } +}; diff --git a/packages/twenty-shared/src/utils/index.ts b/packages/twenty-shared/src/utils/index.ts index e7d336990..acfb641fb 100644 --- a/packages/twenty-shared/src/utils/index.ts +++ b/packages/twenty-shared/src/utils/index.ts @@ -9,6 +9,7 @@ export { assertUnreachable } from './assertUnreachable'; export { isFieldMetadataDateKind } from './fieldMetadata/isFieldMetadataDateKind'; +export { getURLSafely } from './getURLSafely'; export { getImageAbsoluteURI } from './image/getImageAbsoluteURI'; export { sanitizeURL, @@ -26,7 +27,7 @@ export { getAbsoluteUrlOrThrow } from './url/getAbsoluteUrlOrThrow'; export { getUrlHostnameOrThrow } from './url/getUrlHostnameOrThrow'; export { isValidHostname } from './url/isValidHostname'; export { isValidUrl } from './url/isValidUrl'; -export { lowercaseUrlAndRemoveTrailingSlash } from './url/lowercaseUrlAndRemoveTrailingSlash'; +export { lowercaseUrlOriginAndRemoveTrailingSlash } from './url/lowercaseUrlOriginAndRemoveTrailingSlash'; export { isDefined } from './validation/isDefined'; export { isLabelIdentifierFieldMetadataTypes } from './validation/isLabelIdentifierFieldMetadataTypes'; export { isValidLocale } from './validation/isValidLocale'; diff --git a/packages/twenty-shared/src/utils/url/__tests__/lowercaseUrlAndRemoveTrailingSlash.test.ts b/packages/twenty-shared/src/utils/url/__tests__/lowercaseUrlAndRemoveTrailingSlash.test.ts deleted file mode 100644 index a70c3f26c..000000000 --- a/packages/twenty-shared/src/utils/url/__tests__/lowercaseUrlAndRemoveTrailingSlash.test.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { lowercaseUrlAndRemoveTrailingSlash } from '@/utils/url/lowercaseUrlAndRemoveTrailingSlash'; - -describe('lowercaseUrlAndRemoveTrailingSlash', () => { - it('should leave lowcased domain unchanged', () => { - const primaryLinkUrl = 'https://www.example.com/test'; - const result = lowercaseUrlAndRemoveTrailingSlash(primaryLinkUrl); - - expect(result).toBe('https://www.example.com/test'); - }); - - it('should lowercase the domain of the primary link url', () => { - const primaryLinkUrl = 'htTps://wwW.exAmple.coM/TEST'; - const result = lowercaseUrlAndRemoveTrailingSlash(primaryLinkUrl); - - expect(result).toBe('https://www.example.com/TEST'); - }); - - it('should not add a trailing slash', () => { - const primaryLinkUrl = 'https://www.example.com'; - const result = lowercaseUrlAndRemoveTrailingSlash(primaryLinkUrl); - - expect(result).toBe('https://www.example.com'); - }); -}); diff --git a/packages/twenty-shared/src/utils/url/__tests__/lowercaseUrlOriginAndRemoveTrailingSlash.test.ts b/packages/twenty-shared/src/utils/url/__tests__/lowercaseUrlOriginAndRemoveTrailingSlash.test.ts new file mode 100644 index 000000000..a9a54dc27 --- /dev/null +++ b/packages/twenty-shared/src/utils/url/__tests__/lowercaseUrlOriginAndRemoveTrailingSlash.test.ts @@ -0,0 +1,44 @@ +import { lowercaseUrlOriginAndRemoveTrailingSlash } from '@/utils/url/lowercaseUrlOriginAndRemoveTrailingSlash'; + +interface TestContext { + title: string; + input: string; + expected: string; +} + +describe('lowercaseUrlOriginAndRemoveTrailingSlash', () => { + test.each([ + { + title: 'should leave lowcased domain unchanged', + input: 'https://www.example.com/test', + expected: 'https://www.example.com/test', + }, + { + title: 'should lowercase the domain while preserving path case', + input: 'htTps://wwW.exAmple.coM/TEST', + expected: 'https://www.example.com/TEST', + }, + { + title: 'should not add a trailing slash', + input: 'https://www.example.com', + expected: 'https://www.example.com', + }, + { + title: 'should remove trailing slash', + input: 'https://www.example.com/', + expected: 'https://www.example.com', + }, + { + title: 'should handle query parameters', + input: 'htTps://wwW.exAmple.coM/TEST?Param=Value', + expected: 'https://www.example.com/TEST?Param=Value', + }, + { + title: 'should handle hash fragments', + input: 'htTps://wwW.exAmple.coM/TEST#Hash', + expected: 'https://www.example.com/TEST#Hash', + }, + ])('$title', ({ input, expected }) => { + expect(lowercaseUrlOriginAndRemoveTrailingSlash(input)).toBe(expected); + }); +}); diff --git a/packages/twenty-shared/src/utils/url/lowercaseUrlAndRemoveTrailingSlash.ts b/packages/twenty-shared/src/utils/url/lowercaseUrlAndRemoveTrailingSlash.ts deleted file mode 100644 index 359fe16e2..000000000 --- a/packages/twenty-shared/src/utils/url/lowercaseUrlAndRemoveTrailingSlash.ts +++ /dev/null @@ -1,7 +0,0 @@ -export const lowercaseUrlAndRemoveTrailingSlash = (url: string) => { - try { - return new URL(url).toString().toLowerCase().replace(/\/$/, ''); - } catch { - return url.toLowerCase(); - } -}; diff --git a/packages/twenty-shared/src/utils/url/lowercaseUrlOriginAndRemoveTrailingSlash.ts b/packages/twenty-shared/src/utils/url/lowercaseUrlOriginAndRemoveTrailingSlash.ts new file mode 100644 index 000000000..b21769c71 --- /dev/null +++ b/packages/twenty-shared/src/utils/url/lowercaseUrlOriginAndRemoveTrailingSlash.ts @@ -0,0 +1,15 @@ +import { getURLSafely } from '@/utils/getURLSafely'; +import { isDefined } from '@/utils/validation'; + +export const lowercaseUrlOriginAndRemoveTrailingSlash = (rawUrl: string) => { + const url = getURLSafely(rawUrl); + + if (!isDefined(url)) { + return rawUrl; + } + + const lowercaseOrigin = url.origin.toLowerCase(); + const path = url.pathname + url.search + url.hash; + + return (lowercaseOrigin + path).replace(/\/$/, ''); +};