Consider null values as empty values for link fields (#12113)

This pull request introduces changes to improve handling of nullable
values in link-related data structures and simplifies field value
generation logic. Key updates include adjustments to type definitions,
utility functions, and component logic to support `null` values for
links, along with the removal of the `generateDefaultFieldValue`
function in favor of `generateEmptyFieldValue`.

There will be a few more follow-up Pull Requests.

---

Closes https://github.com/twentyhq/twenty/issues/11844
This commit is contained in:
Baptiste Devessier
2025-05-21 11:30:15 +02:00
committed by GitHub
parent 7461b7ac58
commit c29ed1c0c9
17 changed files with 432 additions and 104 deletions

View File

@ -15,7 +15,7 @@ type FormTextFieldInputProps = {
label?: string;
error?: string;
hint?: string;
defaultValue: string | undefined;
defaultValue: string | undefined | null;
onChange: (value: string) => void;
onBlur?: () => void;
multiline?: boolean;

View File

@ -13,7 +13,7 @@ type UseTextVariableEditorProps = {
placeholder: string | undefined;
multiline: boolean | undefined;
readonly: boolean | undefined;
defaultValue: string | undefined;
defaultValue: string | undefined | null;
onUpdate: (editor: Editor) => void;
};

View File

@ -1,10 +1,11 @@
import { useLinksField } from '@/object-record/record-field/meta-types/hooks/useLinksField';
import { LinksFieldMenuItem } from '@/object-record/record-field/meta-types/input/components/LinksFieldMenuItem';
import { getFieldLinkDefinedLinks } from '@/object-record/record-field/meta-types/input/utils/getFieldLinkDefinedLinks';
import { recordFieldInputIsFieldInErrorComponentState } from '@/object-record/record-field/states/recordFieldInputIsFieldInErrorComponentState';
import { DEFAULT_CELL_SCOPE } from '@/object-record/record-table/record-table-cell/hooks/useOpenRecordTableCellV2';
import { useSetRecoilComponentStateV2 } from '@/ui/utilities/state/component-state/hooks/useSetRecoilComponentStateV2';
import { useMemo } from 'react';
import { absoluteUrlSchema, isDefined } from 'twenty-shared/utils';
import { absoluteUrlSchema } from 'twenty-shared/utils';
import { FieldMetadataType } from '~/generated-metadata/graphql';
import { MultiItemFieldInput } from './MultiItemFieldInput';
@ -19,26 +20,13 @@ export const LinksFieldInput = ({
}: LinksFieldInputProps) => {
const { persistLinksField, fieldValue, fieldDefinition } = useLinksField();
const links = useMemo<{ url: string; label: string }[]>(
() =>
[
fieldValue.primaryLinkUrl
? {
url: fieldValue.primaryLinkUrl,
label: fieldValue.primaryLinkLabel,
}
: null,
...(fieldValue.secondaryLinks ?? []),
].filter(isDefined),
[
fieldValue.primaryLinkLabel,
fieldValue.primaryLinkUrl,
fieldValue.secondaryLinks,
],
const links = useMemo<{ url: string; label: string | null }[]>(
() => getFieldLinkDefinedLinks(fieldValue),
[fieldValue],
);
const handlePersistLinks = (
updatedLinks: { url: string; label: string }[],
updatedLinks: { url: string | null; label: string | null }[],
) => {
const [nextPrimaryLink, ...nextSecondaryLinks] = updatedLinks;
persistLinksField({

View File

@ -4,11 +4,11 @@ import { MultiItemFieldMenuItem } from './MultiItemFieldMenuItem';
type LinksFieldMenuItemProps = {
dropdownId: string;
isPrimary?: boolean;
label: string;
label: string | null;
url: string;
onEdit?: () => void;
onSetAsPrimary?: () => void;
onDelete?: () => void;
url: string;
};
export const LinksFieldMenuItem = ({

View File

@ -0,0 +1,162 @@
import { getFieldLinkDefinedLinks } from '../getFieldLinkDefinedLinks';
describe('getFieldLinkDefinedLinks', () => {
describe('Primary link', () => {
it('should not return primary link when primaryLinkUrl is null', () => {
expect(
getFieldLinkDefinedLinks({
primaryLinkUrl: null,
primaryLinkLabel: 'Twenty',
secondaryLinks: [],
}),
).toEqual([]);
});
it('should not return primary link when primaryLinkUrl is empty string', () => {
expect(
getFieldLinkDefinedLinks({
primaryLinkUrl: '',
primaryLinkLabel: 'Twenty',
secondaryLinks: [],
}),
).toEqual([]);
});
it('should return primary link when primaryLinkUrl is defined but primaryLinkLabel is null', () => {
expect(
getFieldLinkDefinedLinks({
primaryLinkUrl: 'https://twenty.com',
primaryLinkLabel: null,
secondaryLinks: [],
}),
).toEqual([
{
url: 'https://twenty.com',
label: null,
},
]);
});
});
describe('Secondary links', () => {
it('should handle null secondaryLinks', () => {
expect(
getFieldLinkDefinedLinks({
primaryLinkUrl: '',
primaryLinkLabel: '',
secondaryLinks: null,
}),
).toEqual([]);
});
it('should filter out secondary links with null url', () => {
expect(
getFieldLinkDefinedLinks({
primaryLinkUrl: '',
primaryLinkLabel: '',
secondaryLinks: [
{
url: null,
label: 'Twenty',
},
{
url: 'https://docs.twenty.com',
label: 'Documentation',
},
],
}),
).toEqual([
{
url: 'https://docs.twenty.com',
label: 'Documentation',
},
]);
});
it('should filter out secondary links with empty url', () => {
expect(
getFieldLinkDefinedLinks({
primaryLinkUrl: '',
primaryLinkLabel: '',
secondaryLinks: [
{
url: '',
label: 'Twenty',
},
{
url: 'https://docs.twenty.com',
label: 'Documentation',
},
],
}),
).toEqual([
{
url: 'https://docs.twenty.com',
label: 'Documentation',
},
]);
});
it('should keep secondary links with null label if url is defined', () => {
expect(
getFieldLinkDefinedLinks({
primaryLinkUrl: '',
primaryLinkLabel: '',
secondaryLinks: [
{
url: 'https://twenty.com',
label: null,
},
],
}),
).toEqual([
{
url: 'https://twenty.com',
label: null,
},
]);
});
it('should correctly combine primary and secondary links with edge cases', () => {
expect(
getFieldLinkDefinedLinks({
primaryLinkUrl: 'https://twenty.com',
primaryLinkLabel: null,
secondaryLinks: [
{
url: '',
label: 'Invalid Link',
},
{
url: 'https://docs.twenty.com',
label: null,
},
{
url: null,
label: 'Another Invalid Link',
},
],
}),
).toEqual([
{
url: 'https://twenty.com',
label: null,
},
{
url: 'https://docs.twenty.com',
label: null,
},
]);
});
it('should handle empty secondaryLinks array', () => {
expect(
getFieldLinkDefinedLinks({
primaryLinkUrl: '',
primaryLinkLabel: '',
secondaryLinks: [],
}),
).toEqual([]);
});
});
});

View File

@ -0,0 +1,27 @@
import { FieldLinksValue } from '@/object-record/record-field/types/FieldMetadata';
import { isNonEmptyString } from '@sniptt/guards';
import { isDefined } from 'twenty-shared/utils';
export const getFieldLinkDefinedLinks = (fieldValue: FieldLinksValue) => {
return [
isNonEmptyString(fieldValue.primaryLinkUrl)
? {
url: fieldValue.primaryLinkUrl,
label: fieldValue.primaryLinkLabel,
}
: null,
...(fieldValue.secondaryLinks ?? []),
]
.filter(isDefined)
.map((link) => {
if (!isNonEmptyString(link.url)) {
return undefined;
}
return {
url: link.url,
label: link.label,
};
})
.filter(isDefined);
};

View File

@ -201,9 +201,9 @@ export type FieldEmailsValue = {
additionalEmails: string[] | null;
};
export type FieldLinksValue = {
primaryLinkLabel: string;
primaryLinkUrl: string;
secondaryLinks?: { label: string; url: string }[] | null;
primaryLinkLabel: string | null;
primaryLinkUrl: string | null;
secondaryLinks?: { label: string | null; url: string | null }[] | null;
};
export type FieldCurrencyValue = {
currencyCode: CurrencyCode;

View File

@ -3,10 +3,15 @@ import { z } from 'zod';
import { FieldLinksValue } from '../FieldMetadata';
export const linksSchema = z.object({
primaryLinkLabel: z.string(),
primaryLinkUrl: absoluteUrlSchema.or(z.string().length(0)),
primaryLinkLabel: z.string().nullable(),
primaryLinkUrl: absoluteUrlSchema.or(z.string().length(0)).nullable(),
secondaryLinks: z
.array(z.object({ label: z.string(), url: absoluteUrlSchema }))
.array(
z.object({
label: z.string().nullable(),
url: absoluteUrlSchema.nullable(),
}),
)
.nullable(),
}) satisfies z.ZodType<FieldLinksValue>;

View File

@ -37,11 +37,9 @@ import { isFieldText } from '@/object-record/record-field/types/guards/isFieldTe
import { isFieldTsVector } from '@/object-record/record-field/types/guards/isFieldTsVectorValue';
import { isFieldUuid } from '@/object-record/record-field/types/guards/isFieldUuid';
import { isDefined } from 'twenty-shared/utils';
import { stripSimpleQuotesFromString } from '~/utils/string/stripSimpleQuotesFromString';
const isValueEmpty = (value: unknown) =>
!isDefined(value) ||
(isString(value) && stripSimpleQuotesFromString(value) === '');
!isDefined(value) || (isString(value) && value === '');
export const isFieldValueEmpty = ({
fieldDefinition,

View File

@ -1,30 +0,0 @@
import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem';
import { isFieldValueEmpty } from '@/object-record/record-field/utils/isFieldValueEmpty';
import { generateEmptyFieldValue } from '@/object-record/utils/generateEmptyFieldValue';
import { v4 } from 'uuid';
import { stripSimpleQuotesFromString } from '~/utils/string/stripSimpleQuotesFromString';
type GenerateEmptyFieldValueArgs = {
fieldMetadataItem: Pick<FieldMetadataItem, 'defaultValue' | 'type'>;
};
export const generateDefaultFieldValue = ({
fieldMetadataItem,
}: GenerateEmptyFieldValueArgs) => {
const defaultValue = isFieldValueEmpty({
fieldValue: fieldMetadataItem.defaultValue,
fieldDefinition: fieldMetadataItem,
})
? generateEmptyFieldValue({
fieldMetadataItem,
})
: stripSimpleQuotesFromString(fieldMetadataItem.defaultValue);
switch (defaultValue) {
case 'uuid':
return v4();
case 'now':
return new Date().toISOString();
default:
return defaultValue;
}
};

View File

@ -113,7 +113,7 @@ export const generateEmptyFieldValue = ({
};
}
case FieldMetadataType.TS_VECTOR: {
throw new Error('TS_VECTOR not implemented yet');
return null;
}
default: {
return assertUnreachable(

View File

@ -3,9 +3,9 @@ import { isUndefined } from '@sniptt/guards';
import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem';
import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem';
import { ObjectRecord } from '@/object-record/types/ObjectRecord';
import { generateDefaultFieldValue } from '@/object-record/utils/generateDefaultFieldValue';
import { FieldMetadataType, RelationDefinitionType } from '~/generated/graphql';
import { generateEmptyFieldValue } from '@/object-record/utils/generateEmptyFieldValue';
import { isDefined } from 'twenty-shared/utils';
import { FieldMetadataType, RelationDefinitionType } from '~/generated/graphql';
type PrefillRecordArgs = {
objectMetadataItem: ObjectMetadataItem;
@ -28,7 +28,7 @@ export const prefillRecord = <T extends ObjectRecord>({
}
const fieldValue = isUndefined(inputValue)
? generateDefaultFieldValue({ fieldMetadataItem })
? generateEmptyFieldValue({ fieldMetadataItem })
: inputValue;
return [fieldMetadataItem.name, fieldValue];
})

View File

@ -29,7 +29,7 @@ describe('getFieldPreviewValue', () => {
});
// Then
expect(result).toBe(false);
expect(result).toBe(true);
});
it('returns a placeholder defaultValue if the field metadata does not have a defaultValue', () => {

View File

@ -1,9 +1,10 @@
import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem';
import { isFieldValueEmpty } from '@/object-record/record-field/utils/isFieldValueEmpty';
import { generateDefaultFieldValue } from '@/object-record/utils/generateDefaultFieldValue';
import { generateEmptyFieldValue } from '@/object-record/utils/generateEmptyFieldValue';
import { getSettingsFieldTypeConfig } from '@/settings/data-model/utils/getSettingsFieldTypeConfig';
import { isFieldTypeSupportedInSettings } from '@/settings/data-model/utils/isFieldTypeSupportedInSettings';
import { isDefined } from 'twenty-shared/utils';
import { stripSimpleQuotesFromStringRecursive } from '~/utils/string/stripSimpleQuotesFromString';
type getFieldPreviewValueArgs = {
fieldMetadataItem: Pick<FieldMetadataItem, 'type' | 'defaultValue'>;
@ -16,10 +17,12 @@ export const getFieldPreviewValue = ({
if (
!isFieldValueEmpty({
fieldDefinition: { type: fieldMetadataItem.type },
fieldValue: fieldMetadataItem.defaultValue,
fieldValue: stripSimpleQuotesFromStringRecursive(
fieldMetadataItem.defaultValue,
),
})
) {
return generateDefaultFieldValue({
return generateEmptyFieldValue({
fieldMetadataItem,
});
}

View File

@ -2,11 +2,11 @@ import { isNonEmptyString } from '@sniptt/guards';
import { LinkType, RoundedLink, SocialLink } from 'twenty-ui/navigation';
type LinkDisplayProps = {
value?: { url: string; label?: string };
value: { url: string; label?: string | null };
};
export const LinkDisplay = ({ value }: LinkDisplayProps) => {
const url = value?.url;
const url = value.url;
if (!isNonEmptyString(url)) {
return <></>;
@ -18,8 +18,8 @@ export const LinkDisplay = ({ value }: LinkDisplayProps) => {
: 'https://' + url
: '';
const displayedValue = isNonEmptyString(value?.label)
? value?.label
const displayedValue = isNonEmptyString(value.label)
? value.label
: url?.replace(/^http[s]?:\/\/(?:[w]+\.)?/gm, '').replace(/^[w]+\./gm, '');
const type = displayedValue.startsWith('linkedin.')

View File

@ -1,50 +1,43 @@
import { useMemo } from 'react';
import { getFieldLinkDefinedLinks } from '@/object-record/record-field/meta-types/input/utils/getFieldLinkDefinedLinks';
import { FieldLinksValue } from '@/object-record/record-field/types/FieldMetadata';
import { ExpandableList } from '@/ui/layout/expandable-list/components/ExpandableList';
import { checkUrlType } from '~/utils/checkUrlType';
import {
getAbsoluteUrlOrThrow,
getUrlHostnameOrThrow,
isDefined,
} from 'twenty-shared/utils';
import { LinkType, RoundedLink, SocialLink } from 'twenty-ui/navigation';
import { checkUrlType } from '~/utils/checkUrlType';
type LinksDisplayProps = {
value?: FieldLinksValue;
};
export const LinksDisplay = ({ value }: LinksDisplayProps) => {
const links = useMemo(
() =>
[
value?.primaryLinkUrl
? {
url: value.primaryLinkUrl,
label: value.primaryLinkLabel,
}
: null,
...(value?.secondaryLinks ?? []),
]
.filter(isDefined)
.map(({ url, label }) => {
let absoluteUrl = '';
let hostname = '';
try {
absoluteUrl = getAbsoluteUrlOrThrow(url);
hostname = getUrlHostnameOrThrow(absoluteUrl);
} catch {
absoluteUrl = '';
hostname = '';
}
return {
url: absoluteUrl,
label: label || hostname,
type: checkUrlType(absoluteUrl),
};
}),
[value?.primaryLinkLabel, value?.primaryLinkUrl, value?.secondaryLinks],
);
const links = useMemo(() => {
if (!isDefined(value)) {
return [];
}
return getFieldLinkDefinedLinks(value).map(({ url, label }) => {
let absoluteUrl = '';
let hostname = '';
try {
absoluteUrl = getAbsoluteUrlOrThrow(url);
hostname = getUrlHostnameOrThrow(absoluteUrl);
} catch {
absoluteUrl = '';
hostname = '';
}
return {
url: absoluteUrl,
label: label || hostname,
type: checkUrlType(absoluteUrl),
};
});
}, [value]);
return (
<ExpandableList>

View File

@ -0,0 +1,182 @@
import { Meta, StoryObj } from '@storybook/react';
import { expect, waitFor, within } from '@storybook/test';
import { LinksDisplay } from '@/ui/field/display/components/LinksDisplay';
import { ComponentDecorator } from 'twenty-ui/testing';
const meta: Meta<typeof LinksDisplay> = {
title: 'UI/Display/LinksDisplay',
component: LinksDisplay,
decorators: [ComponentDecorator],
argTypes: {
value: {
control: 'object',
description:
'The value object containing primaryLinkUrl, primaryLinkLabel, and secondaryLinks',
},
},
};
export default meta;
type Story = StoryObj<typeof LinksDisplay>;
export const NoLinks: Story = {
args: {
value: {
primaryLinkUrl: '',
primaryLinkLabel: '',
secondaryLinks: [],
},
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await waitFor(() => {
expect(canvas.queryByRole('link')).toBeNull();
});
},
};
export const NullLinks: Story = {
args: {
value: {
primaryLinkUrl: null,
primaryLinkLabel: 'Primary Link',
secondaryLinks: [
{ url: null, label: 'Secondary Link' },
{ url: 'https://www.twenty.com', label: 'Valid Link' },
],
},
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await waitFor(() => {
const links = canvas.queryAllByRole('link');
expect(links).toHaveLength(1);
});
const validLink = await canvas.findByText('Valid Link');
expect(validLink).toBeVisible();
expect(validLink).toHaveAttribute('href', 'https://www.twenty.com');
expect(canvas.queryByText('Primary Link')).not.toBeInTheDocument();
expect(canvas.queryByText('Secondary Link')).not.toBeInTheDocument();
},
};
export const SingleLink: Story = {
args: {
value: {
primaryLinkUrl: 'https://www.twenty.com',
primaryLinkLabel: 'Twenty Website',
secondaryLinks: null,
},
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
const link = await canvas.findByRole('link');
expect(link).toBeVisible();
expect(link).toHaveAttribute('href', 'https://www.twenty.com');
expect(link).toHaveTextContent('Twenty Website');
await waitFor(() => {
expect(canvas.getAllByRole('link')).toHaveLength(1);
});
},
};
export const MultipleLinks: Story = {
args: {
value: {
primaryLinkUrl: 'https://www.twenty.com',
primaryLinkLabel: 'Twenty Website',
secondaryLinks: [
{ url: 'https://docs.twenty.com', label: 'Documentation' },
{ url: 'https://blog.twenty.com', label: 'Blog' },
],
},
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await waitFor(() => {
const links = canvas.queryAllByRole('link');
expect(links).toHaveLength(3);
});
const primaryLink = await canvas.findByText('Twenty Website');
expect(primaryLink).toBeVisible();
expect(primaryLink).toHaveAttribute('href', 'https://www.twenty.com');
const docsLink = await canvas.findByText('Documentation');
expect(docsLink).toBeVisible();
expect(docsLink).toHaveAttribute('href', 'https://docs.twenty.com');
const blogLink = await canvas.findByText('Blog');
expect(blogLink).toBeVisible();
expect(blogLink).toHaveAttribute('href', 'https://blog.twenty.com');
},
};
export const SocialMediaLinks: Story = {
args: {
value: {
primaryLinkUrl: 'https://www.linkedin.com/company/twenty',
primaryLinkLabel: 'Twenty on LinkedIn',
secondaryLinks: [
{ url: 'https://twitter.com/twentycrm', label: 'Twenty on Twitter' },
],
},
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await waitFor(() => {
const links = canvas.queryAllByRole('link');
expect(links).toHaveLength(2);
});
const linkedinLink = await canvas.findByText('twenty');
expect(linkedinLink).toBeVisible();
expect(linkedinLink).toHaveAttribute(
'href',
'https://www.linkedin.com/company/twenty',
);
const twitterLink = await canvas.findByText('@twentycrm');
expect(twitterLink).toBeVisible();
expect(twitterLink).toHaveAttribute(
'href',
'https://twitter.com/twentycrm',
);
},
};
export const AutomaticLabelFromURL: Story = {
args: {
value: {
primaryLinkUrl: 'https://www.example.com',
primaryLinkLabel: '',
secondaryLinks: [{ url: 'https://test.example.com', label: null }],
},
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await waitFor(() => {
const links = canvas.queryAllByRole('link');
expect(links).toHaveLength(2);
});
const primaryLink = await canvas.findByText('www.example.com');
expect(primaryLink).toBeVisible();
expect(primaryLink).toHaveAttribute('href', 'https://www.example.com');
const secondaryLink = await canvas.findByText('test.example.com');
expect(secondaryLink).toBeVisible();
expect(secondaryLink).toHaveAttribute('href', 'https://test.example.com');
},
};