mukeshs/twenty_crm
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Removing trailing slashes (#12658)

Browse Source 
Fix inconsistent domain URL formats : removing the last / that was
caused by URL method

Standardize URL formatting to ensure consistent links storage and
retrieval of domain URLs across the application. Will improve the
dedpulicates in the links

Note: there is another temporary issue from google that was solved on
the 13th of june https://groups.google.com/g/adwords-api/c/tRSQMRZrJYM
but we consider this out of this scope

Fixes #12621
This commit is contained in:
Guillim
2025-06-17 16:29:14 +02:00
committed by GitHub
parent cc7a37b0cc
commit 1cee587709
14 changed files with 414 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/__tests__/query-runner.util.spec.ts
View File

@ -1,17 +1,31 @@
import { lowercaseDomain } from 'src/engine/api/graphql/workspace-query-runner/utils/query-runner-links.util';
import { lowercaseDomainAndRemoveTrailingSlash } from 'src/engine/api/graphql/workspace-query-runner/utils/query-runner-links.util';
describe('queryRunner LINKS util', () => {
it('should leave lowcased domain unchanged', () => {
const primaryLinkUrl = 'https://www.example.com/test';
const result = lowercaseDomain(primaryLinkUrl);
const result = lowercaseDomainAndRemoveTrailingSlash(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 = lowercaseDomain(primaryLinkUrl);
const result = lowercaseDomainAndRemoveTrailingSlash(primaryLinkUrl);
expect(result).toBe('https://www.example.com/TEST');
});
it('should not add a trailing slash', () => {
const primaryLinkUrl = 'https://www.example.com';
const result = lowercaseDomainAndRemoveTrailingSlash(primaryLinkUrl);
expect(result).toBe('https://www.example.com');
});
it('should not add a trailing slash', () => {
const primaryLinkUrl = 'https://www.example.com/toto/';
const result = lowercaseDomainAndRemoveTrailingSlash(primaryLinkUrl);
expect(result).toBe('https://www.example.com/toto');
});
});

4
packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/query-runner-links.util.ts
View File

@ -1,6 +1,6 @@
export const lowercaseDomain = (url: string) => {
export const lowercaseDomainAndRemoveTrailingSlash = (url: string) => {
try {
return new URL(url).toString();
return new URL(url).toString().replace(/\/$/, '');
} catch {
return url;
}

52
packages/twenty-server/src/engine/core-modules/record-transformer/services/record-input-transformer.service.ts
View File

@ -1,15 +1,15 @@
import { Injectable } from '@nestjs/common';
import { isNonEmptyString } from '@sniptt/guards';
import { FieldMetadataType } from 'twenty-shared/types';
import { isDefined } from 'twenty-shared/utils';
import { FieldMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata.interface';
import { lowercaseDomain } from 'src/engine/api/graphql/workspace-query-runner/utils/query-runner-links.util';
import { removeEmptyLinks } from 'src/engine/core-modules/record-transformer/utils/remove-empty-links';
import {
LinksFieldGraphQLInput,
transformLinksValue,
} from 'src/engine/core-modules/record-transformer/utils/transform-links-value.util';
import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types';
import { LinkMetadataNullable } from 'src/engine/metadata-modules/field-metadata/composite-types/links.composite-type';
import {
RichTextV2Metadata,
richTextV2ValueSchema,
@ -86,7 +86,7 @@ export class RecordInputTransformerService {
case FieldMetadataType.RICH_TEXT_V2:
return this.transformRichTextV2Value(value);
case FieldMetadataType.LINKS:
return this.transformLinksValue(value);
return transformLinksValue(value as LinksFieldGraphQLInput);
case FieldMetadataType.EMAILS:
return this.transformEmailsValue(value);
default:
@ -132,48 +132,6 @@ export class RecordInputTransformerService {
};
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
private transformLinksValue(value: any): any {
if (!value) {
return value;
}
const primaryLinkUrlRaw = value.primaryLinkUrl as string | null;
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 { primaryLinkLabel, primaryLinkUrl, secondaryLinks } =
removeEmptyLinks({
primaryLinkUrl: primaryLinkUrlRaw,
primaryLinkLabel: primaryLinkLabelRaw,
secondaryLinks: secondaryLinksArray,
});
return {
...value,
primaryLinkUrl: isDefined(primaryLinkUrl)
? lowercaseDomain(primaryLinkUrl)
: primaryLinkUrl,
primaryLinkLabel,
secondaryLinks: JSON.stringify(
secondaryLinks?.map((link) => ({
...link,
url: isDefined(link.url) ? lowercaseDomain(link.url) : link.url,
})),
),
};
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
// eslint-disable-next-line @typescript-eslint/no-explicit-any
private transformEmailsValue(value: any): any {

79
packages/twenty-server/src/engine/core-modules/record-transformer/utils/__tests__/transform-links-value.util.spec.ts Normal file
View File

@ -0,0 +1,79 @@
import { transformLinksValue } from 'src/engine/core-modules/record-transformer/utils/transform-links-value.util';
describe('transformLinksValue', () => {
it('should handle null/undefined/empty object values', () => {
expect(transformLinksValue(null)).toBeNull();
expect(transformLinksValue(undefined)).toBeUndefined();
expect(transformLinksValue({})).toEqual({
primaryLinkLabel: null,
primaryLinkUrl: null,
secondaryLinks: '[]',
});
});
describe('primary link', () => {
it('should transform uppercase', () => {
const input = {
primaryLinkUrl: 'HTTPS://EXAMPLE.COM',
primaryLinkLabel: 'Example',
secondaryLinks: '[]',
};
const expected = {
primaryLinkUrl: 'https://example.com',
primaryLinkLabel: 'Example',
secondaryLinks: '[]',
};
expect(transformLinksValue(input)).toEqual(expected);
});
it('should remove trailing slash', () => {
const input = {
primaryLinkUrl: 'https://example.com/',
primaryLinkLabel: 'Example',
secondaryLinks: '[]',
};
const expected = {
primaryLinkUrl: 'https://example.com',
primaryLinkLabel: 'Example',
secondaryLinks: '[]',
};
expect(transformLinksValue(input)).toEqual(expected);
});
it('should work fine without protocol', () => {
const input = {
primaryLinkUrl: 'example.com',
primaryLinkLabel: 'Example',
secondaryLinks: '[]',
};
const expected = {
primaryLinkUrl: 'example.com',
primaryLinkLabel: 'Example',
secondaryLinks: '[]',
};
expect(transformLinksValue(input)).toEqual(expected);
});
it('should work fine with www', () => {
const input = {
primaryLinkUrl: 'www.example.com',
primaryLinkLabel: 'Example',
secondaryLinks: '[]',
};
const expected = {
primaryLinkUrl: 'www.example.com',
primaryLinkLabel: 'Example',
secondaryLinks: '[]',
};
expect(transformLinksValue(input)).toEqual(expected);
});
});
});

61
packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-links-value.util.ts Normal file
View File

@ -0,0 +1,61 @@
import { isNonEmptyString } from '@sniptt/guards';
import { isDefined } from 'twenty-shared/utils';
import { lowercaseDomainAndRemoveTrailingSlash } from 'src/engine/api/graphql/workspace-query-runner/utils/query-runner-links.util';
import { removeEmptyLinks } from 'src/engine/core-modules/record-transformer/utils/remove-empty-links';
import { LinkMetadataNullable } from 'src/engine/metadata-modules/field-metadata/composite-types/links.composite-type';
export type LinksFieldGraphQLInput =
| {
primaryLinkUrl?: string | null;
primaryLinkLabel?: string | null;
secondaryLinks?: string | null;
}
| null
| undefined;
export const transformLinksValue = (
value: LinksFieldGraphQLInput,
): LinksFieldGraphQLInput => {
if (!value) {
return value;
}
const primaryLinkUrlRaw = value.primaryLinkUrl as string | null;
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 { primaryLinkLabel, primaryLinkUrl, secondaryLinks } = removeEmptyLinks(
{
primaryLinkUrl: primaryLinkUrlRaw,
primaryLinkLabel: primaryLinkLabelRaw,
secondaryLinks: secondaryLinksArray,
},
);
return {
...value,
primaryLinkUrl: isDefined(primaryLinkUrl)
? lowercaseDomainAndRemoveTrailingSlash(primaryLinkUrl)
: primaryLinkUrl,
primaryLinkLabel,
secondaryLinks: JSON.stringify(
secondaryLinks?.map((link) => ({
...link,
url: isDefined(link.url)
? lowercaseDomainAndRemoveTrailingSlash(link.url)
: link.url,
})),
),
};
};

206
packages/twenty-server/src/modules/contact-creation-manager/services/__tests__/create-company.service.spec.ts Normal file
View File

@ -0,0 +1,206 @@
import { Test, TestingModule } from '@nestjs/testing';
import { getRepositoryToken } from '@nestjs/typeorm';
import { ConnectedAccountProvider } from 'twenty-shared/types';
import { FieldActorSource } from 'src/engine/metadata-modules/field-metadata/composite-types/actor.composite-type';
import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
import { WorkspaceEventEmitter } from 'src/engine/workspace-event-emitter/workspace-event-emitter';
import { STANDARD_OBJECT_IDS } from 'src/engine/workspace-manager/workspace-sync-metadata/constants/standard-object-ids';
import {
CompanyToCreate,
CreateCompanyService,
} from 'src/modules/contact-creation-manager/services/create-company.service';
describe('CreateCompanyService', () => {
let service: CreateCompanyService;
let mockCompanyRepository: any;
const workspaceId = 'workspace-1';
const companyToCreate1: CompanyToCreate = {
domainName: 'example1.com',
createdBySource: FieldActorSource.MANUAL,
createdByContext: {
provider: ConnectedAccountProvider.GOOGLE,
},
};
const companyToCreate1withSlash: CompanyToCreate = {
domainName: 'example1.com/',
createdBySource: FieldActorSource.MANUAL,
createdByContext: {
provider: ConnectedAccountProvider.GOOGLE,
},
};
const companyToCreate2: CompanyToCreate = {
domainName: 'example2.com',
createdBySource: FieldActorSource.MANUAL,
createdByContext: {
provider: ConnectedAccountProvider.GOOGLE,
},
};
const companyToCreateExisting: CompanyToCreate = {
domainName: 'existing-company.com',
createdBySource: FieldActorSource.MANUAL,
createdByContext: {
provider: ConnectedAccountProvider.GOOGLE,
},
};
const inputForCompanyToCreate1 = {
address: {
addressCity: undefined,
},
createdBy: {
context: {
provider: 'google',
},
name: '',
source: 'MANUAL',
workspaceMemberId: undefined,
},
domainName: {
primaryLinkUrl: 'https://example1.com',
},
name: 'Example1',
position: 1,
};
const inputForCompanyToCreate2 = {
address: {
addressCity: '',
},
createdBy: {
context: {
provider: 'google',
},
name: '',
source: 'MANUAL',
workspaceMemberId: undefined,
},
domainName: {
primaryLinkUrl: 'https://example2.com',
},
name: 'BNQ',
position: 2,
};
beforeEach(async () => {
mockCompanyRepository = {
find: jest.fn(),
save: jest.fn(),
maximum: jest.fn().mockResolvedValue(0),
};
const module: TestingModule = await Test.createTestingModule({
providers: [
CreateCompanyService,
{
provide: TwentyORMGlobalManager,
useValue: {
getRepositoryForWorkspace: jest
.fn()
.mockResolvedValue(mockCompanyRepository),
},
},
{
provide: WorkspaceEventEmitter,
useValue: {
emitDatabaseBatchEvent: jest.fn(),
},
},
{
provide: getRepositoryToken(ObjectMetadataEntity, 'core'),
useValue: {
findOne: jest.fn().mockResolvedValue({
id: 'mock-object-metadata-id',
standardId: STANDARD_OBJECT_IDS.company,
workspaceId,
nameSingular: 'company',
namePlural: 'companies',
labelSingular: 'Company',
labelPlural: 'Companies',
targetTableName: 'company',
isCustom: false,
isRemote: false,
isActive: true,
isSystem: false,
isAuditLogged: true,
isSearchable: true,
isLabelSyncedWithName: false,
}),
},
},
],
}).compile();
service = module.get<CreateCompanyService>(CreateCompanyService);
});
describe('With no existing companies', () => {
beforeEach(() => {
mockCompanyRepository.find.mockResolvedValue([]);
// it is useless to check results here, we can only check the input it was called with
mockCompanyRepository.save.mockResolvedValue([]);
});
it('should successfully create a company', async () => {
await service.createCompanies([companyToCreate1], workspaceId);
expect(mockCompanyRepository.find).toHaveBeenCalled();
expect(mockCompanyRepository.save).toHaveBeenCalledWith([
inputForCompanyToCreate1,
]);
});
it('should successfully two companies', async () => {
await service.createCompanies(
[companyToCreate1, companyToCreate2],
workspaceId,
);
expect(mockCompanyRepository.find).toHaveBeenCalled();
expect(mockCompanyRepository.save).toHaveBeenCalledWith([
inputForCompanyToCreate1,
inputForCompanyToCreate2,
]);
});
it('should create only one of example.com & example.com/ ', async () => {
await service.createCompanies(
[companyToCreate1, companyToCreate1withSlash],
workspaceId,
);
expect(mockCompanyRepository.find).toHaveBeenCalled();
expect(mockCompanyRepository.save).toHaveBeenCalledWith(
expect.arrayContaining([
expect.objectContaining({
domainName: {
primaryLinkUrl: 'https://example1.com',
},
}),
]),
);
});
});
describe('With existing companies', () => {
beforeEach(() => {
mockCompanyRepository.find.mockResolvedValue([
{
id: 'existing-company-1',
domainName: { primaryLinkUrl: 'https://existing-company.com' },
},
]);
mockCompanyRepository.save.mockResolvedValue([]);
});
it('should not create a company if it already exists', async () => {
await service.createCompanies([companyToCreateExisting], workspaceId);
expect(mockCompanyRepository.find).toHaveBeenCalled();
expect(mockCompanyRepository.save).not.toHaveBeenCalled();
});
});
});

15
packages/twenty-server/src/modules/contact-creation-manager/services/create-company.service.ts
View File

@ -8,6 +8,7 @@ import { ConnectedAccountProvider } from 'twenty-shared/types';
import { DeepPartial, ILike, Repository } from 'typeorm';
import { DatabaseEventAction } from 'src/engine/api/graphql/graphql-query-runner/enums/database-event-action';
import { lowercaseDomainAndRemoveTrailingSlash } from 'src/engine/api/graphql/workspace-query-runner/utils/query-runner-links.util';
import { FieldActorSource } from 'src/engine/metadata-modules/field-metadata/composite-types/actor.composite-type';
import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
import { WorkspaceRepository } from 'src/engine/twenty-orm/repository/workspace.repository';
@ -20,7 +21,7 @@ import { getCompanyNameFromDomainName } from 'src/modules/contact-creation-manag
import { WorkspaceMemberWorkspaceEntity } from 'src/modules/workspace-member/standard-objects/workspace-member.workspace-entity';
import { computeDisplayName } from 'src/utils/compute-display-name';
type CompanyToCreate = {
export type CompanyToCreate = {
domainName: string | undefined;
createdBySource: FieldActorSource;
createdByWorkspaceMember?: WorkspaceMemberWorkspaceEntity | null;
@ -74,8 +75,16 @@ export class CreateCompanyService {
},
);
// Avoid creating duplicate companies
const uniqueCompanies = uniqBy(companies, 'domainName');
// Remove trailing slash from domain names
const companiesWithoutTrailingSlash = companies.map((company) => ({
...company,
domainName: company.domainName
? lowercaseDomainAndRemoveTrailingSlash(company.domainName)
: undefined,
}));
// Avoid creating duplicate companies, e.g. example.com and example.com/
const uniqueCompanies = uniqBy(companiesWithoutTrailingSlash, 'domainName');
const conditions = uniqueCompanies.map((companyToCreate) => ({
domainName: {
primaryLinkUrl: ILike(`%${companyToCreate.domainName}%`),

1
packages/twenty-server/test/integration/constants/test-primary-link-url.constant.ts
View File

@ -1 +1,2 @@
export const TEST_PRIMARY_LINK_URL = 'https://test.com/';
export const TEST_PRIMARY_LINK_URL_WIITHOUT_TRAILING_SLASH = 'https://test.com';

7
packages/twenty-server/test/integration/rest/suites/rest-api-core-create-one.integration-spec.ts
View File

@ -1,6 +1,9 @@
import { TEST_COMPANY_1_ID } from 'test/integration/constants/test-company-ids.constants';
import { TEST_PERSON_1_ID } from 'test/integration/constants/test-person-ids.constants';
import { TEST_PRIMARY_LINK_URL } from 'test/integration/constants/test-primary-link-url.constant';
import {
TEST_PRIMARY_LINK_URL,
TEST_PRIMARY_LINK_URL_WIITHOUT_TRAILING_SLASH,
} from 'test/integration/constants/test-primary-link-url.constant';
import { TIM_ACCOUNT_ID } from 'test/integration/graphql/integration.constants';
import { makeRestAPIRequest } from 'test/integration/rest/utils/make-rest-api-request.util';
import { deleteAllRecords } from 'test/integration/utils/delete-all-records';
@ -136,7 +139,7 @@ describe('Core REST API Create One endpoint', () => {
expect(createdPerson.company).toBeDefined();
expect(createdPerson.company.domainName.primaryLinkUrl).toBe(
TEST_PRIMARY_LINK_URL,
TEST_PRIMARY_LINK_URL_WIITHOUT_TRAILING_SLASH,
);
expect(createdPerson.company.people).not.toBeDefined();
});

7
packages/twenty-server/test/integration/rest/suites/rest-api-core-find-many.integration-spec.ts
View File

@ -5,7 +5,10 @@ import {
TEST_PERSON_3_ID,
TEST_PERSON_4_ID,
} from 'test/integration/constants/test-person-ids.constants';
import { TEST_PRIMARY_LINK_URL } from 'test/integration/constants/test-primary-link-url.constant';
import {
TEST_PRIMARY_LINK_URL,
TEST_PRIMARY_LINK_URL_WIITHOUT_TRAILING_SLASH,
} from 'test/integration/constants/test-primary-link-url.constant';
import { makeRestAPIRequest } from 'test/integration/rest/utils/make-rest-api-request.util';
import { deleteAllRecords } from 'test/integration/utils/delete-all-records';
import { generateRecordName } from 'test/integration/utils/generate-record-name';
@ -453,7 +456,7 @@ describe('Core REST API Find Many endpoint', () => {
expect(person.company).toBeDefined();
expect(person.company.domainName.primaryLinkUrl).toBe(
TEST_PRIMARY_LINK_URL,
TEST_PRIMARY_LINK_URL_WIITHOUT_TRAILING_SLASH,
);
expect(person.company.people).not.toBeDefined();

7
packages/twenty-server/test/integration/rest/suites/rest-api-core-find-one.integration-spec.ts
View File

@ -3,7 +3,10 @@ import {
NOT_EXISTING_TEST_PERSON_ID,
TEST_PERSON_1_ID,
} from 'test/integration/constants/test-person-ids.constants';
import { TEST_PRIMARY_LINK_URL } from 'test/integration/constants/test-primary-link-url.constant';
import {
TEST_PRIMARY_LINK_URL,
TEST_PRIMARY_LINK_URL_WIITHOUT_TRAILING_SLASH,
} from 'test/integration/constants/test-primary-link-url.constant';
import { makeRestAPIRequest } from 'test/integration/rest/utils/make-rest-api-request.util';
import { deleteAllRecords } from 'test/integration/utils/delete-all-records';
import { generateRecordName } from 'test/integration/utils/generate-record-name';
@ -106,7 +109,7 @@ describe('Core REST API Find One endpoint', () => {
expect(person.company).toBeDefined();
expect(person.company.domainName.primaryLinkUrl).toBe(
TEST_PRIMARY_LINK_URL,
TEST_PRIMARY_LINK_URL_WIITHOUT_TRAILING_SLASH,
);
expect(person.company.people).not.toBeDefined();
});

11
packages/twenty-server/test/integration/rest/suites/rest-api-core-update.integration-spec.ts
View File

@ -1,12 +1,15 @@
import { TEST_COMPANY_1_ID } from 'test/integration/constants/test-company-ids.constants';
import {
NOT_EXISTING_TEST_PERSON_ID,
TEST_PERSON_1_ID,
} from 'test/integration/constants/test-person-ids.constants';
import {
TEST_PRIMARY_LINK_URL,
TEST_PRIMARY_LINK_URL_WIITHOUT_TRAILING_SLASH,
} from 'test/integration/constants/test-primary-link-url.constant';
import { makeRestAPIRequest } from 'test/integration/rest/utils/make-rest-api-request.util';
import { generateRecordName } from 'test/integration/utils/generate-record-name';
import { TEST_COMPANY_1_ID } from 'test/integration/constants/test-company-ids.constants';
import { TEST_PRIMARY_LINK_URL } from 'test/integration/constants/test-primary-link-url.constant';
import { deleteAllRecords } from 'test/integration/utils/delete-all-records';
import { generateRecordName } from 'test/integration/utils/generate-record-name';
describe('Core REST API Update One endpoint', () => {
const updatedData = {
@ -96,7 +99,7 @@ describe('Core REST API Update One endpoint', () => {
expect(updatedPerson.company).toBeDefined();
expect(updatedPerson.company.domainName.primaryLinkUrl).toBe(
TEST_PRIMARY_LINK_URL,
TEST_PRIMARY_LINK_URL_WIITHOUT_TRAILING_SLASH,
);
expect(updatedPerson.company.people).not.toBeDefined();
});

7
packages/twenty-shared/src/utils/url/absoluteUrlSchema.ts
View File

@ -8,7 +8,9 @@ export const absoluteUrlSchema = z.string().transform((value, ctx) => {
const valueWithoutProtocol = absoluteUrl
.replace('https://', '')
.replace('http://', '');
.replace('http://', '')
.replace('HTTPS://', '')
.replace('HTTP://', '');
if (/^\d+(?:\/[a-zA-Z]*)?$/.test(valueWithoutProtocol)) {
// if the hostname is a number, it's not a valid url
@ -20,14 +22,11 @@ export const absoluteUrlSchema = z.string().transform((value, ctx) => {
return z.NEVER;
}
try {
const url = new URL(absoluteUrl);
if (isValidHostname(url.hostname)) {
return absoluteUrl;
}
ctx.addIssue({
code: z.ZodIssueCode.custom,
message: 'domain is not a valid url',

7
packages/twenty-shared/src/utils/url/getAbsoluteUrl.ts
View File

@ -1,5 +1,10 @@
export const getAbsoluteUrl = (value: string): string => {
if (value.startsWith('http://') || value.startsWith('https://')) {
if (
value.startsWith('http://') ||
value.startsWith('https://') ||
value.startsWith('HTTPS://') ||
value.startsWith('HTTP://')
) {
return value;
}