From 70cc3e75fea9c8ec521d18e13afbfb51ba99703b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Bosi?= <71827178+bosiraphael@users.noreply.github.com> Date: Tue, 3 Jun 2025 12:20:57 +0200 Subject: [PATCH] Eliminate unnecessary API calls when persisting field (#12429) Fixes #10177 Modified `usePersistField` to check for deep equality between the value to persist and the current record store value before sending an update query. --- .../hooks/__tests__/usePersistField.test.tsx | 82 ++++++++++++++++--- .../record-field/hooks/usePersistField.ts | 19 ++++- 2 files changed, 87 insertions(+), 14 deletions(-) diff --git a/packages/twenty-front/src/modules/object-record/record-field/hooks/__tests__/usePersistField.test.tsx b/packages/twenty-front/src/modules/object-record/record-field/hooks/__tests__/usePersistField.test.tsx index 6fde4fedc..4d5b354d1 100644 --- a/packages/twenty-front/src/modules/object-record/record-field/hooks/__tests__/usePersistField.test.tsx +++ b/packages/twenty-front/src/modules/object-record/record-field/hooks/__tests__/usePersistField.test.tsx @@ -1,9 +1,3 @@ -import { gql } from '@apollo/client'; -import { MockedResponse } from '@apollo/client/testing'; -import { act, renderHook, waitFor } from '@testing-library/react'; -import { ReactNode } from 'react'; -import { useRecoilValue } from 'recoil'; - import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; import { PERSON_FRAGMENT_WITH_DEPTH_ONE_RELATIONS } from '@/object-record/hooks/__mocks__/personFragments'; import { useUpdateOneRecord } from '@/object-record/hooks/useUpdateOneRecord'; @@ -20,6 +14,11 @@ import { usePersistField } from '@/object-record/record-field/hooks/usePersistFi import { FieldDefinition } from '@/object-record/record-field/types/FieldDefinition'; import { FieldMetadata } from '@/object-record/record-field/types/FieldMetadata'; import { recordStoreFamilySelector } from '@/object-record/record-store/states/selectors/recordStoreFamilySelector'; +import { gql } from '@apollo/client'; +import { MockedResponse } from '@apollo/client/testing'; +import { renderHook, waitFor } from '@testing-library/react'; +import { ReactNode, act } from 'react'; +import { useRecoilValue } from 'recoil'; import { getJestMetadataAndApolloMocksWrapper } from '~/testing/jest/getJestMetadataAndApolloMocksWrapper'; const query = gql` @@ -30,6 +29,13 @@ const query = gql` } `; +const phoneMock = { + primaryPhoneNumber: '123 456', + primaryPhoneCountryCode: 'US', + primaryPhoneCallingCode: '+1', + additionalPhones: [], +}; + const mocks: MockedResponse[] = [ { request: { @@ -37,12 +43,7 @@ const mocks: MockedResponse[] = [ variables: { idToUpdate: 'recordId', input: { - phones: { - primaryPhoneNumber: '123 456', - primaryPhoneCountryCode: 'US', - primaryPhoneCallingCode: '+1', - additionalPhones: [], - }, + phones: phoneMock, }, }, }, @@ -117,11 +118,15 @@ const PhoneWrapper = getWrapper(phonesFieldDefinition); const RelationWrapper = getWrapper(relationFieldDefinition); describe('usePersistField', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('should work as expected', async () => { const { result } = renderHook( () => { const entityFields = useRecoilValue( - recordStoreFamilySelector({ recordId, fieldName: 'phone' }), + recordStoreFamilySelector({ recordId, fieldName: 'phones' }), ); return { @@ -172,4 +177,55 @@ describe('usePersistField', () => { expect(mocks[1].result).toHaveBeenCalled(); }); }); + + it('should skip persistence value has not changed', async () => { + const { result } = renderHook( + () => { + const entityFields = useRecoilValue( + recordStoreFamilySelector({ recordId, fieldName: 'phones' }), + ); + + return { + persistField: usePersistField(), + entityFields, + }; + }, + { wrapper: PhoneWrapper }, + ); + + act(() => { + result.current.persistField(phoneMock); + }); + + await waitFor(() => { + expect(mocks[0].result).not.toHaveBeenCalled(); + }); + }); + + it('should skip persistence when relation field value has not changed', async () => { + const { result } = renderHook( + () => { + const entityFields = useRecoilValue( + recordStoreFamilySelector({ + recordId, + fieldName: 'company', + }), + ); + + return { + persistField: usePersistField(), + entityFields, + }; + }, + { wrapper: RelationWrapper }, + ); + + act(() => { + result.current.persistField({ id: 'companyId' }); + }); + + await waitFor(() => { + expect(mocks[1].result).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/twenty-front/src/modules/object-record/record-field/hooks/usePersistField.ts b/packages/twenty-front/src/modules/object-record/record-field/hooks/usePersistField.ts index a0f6054b6..6c33a7f9c 100644 --- a/packages/twenty-front/src/modules/object-record/record-field/hooks/usePersistField.ts +++ b/packages/twenty-front/src/modules/object-record/record-field/hooks/usePersistField.ts @@ -33,6 +33,7 @@ import { isFieldRichTextV2 } from '@/object-record/record-field/types/guards/isF import { isFieldRichTextValue } from '@/object-record/record-field/types/guards/isFieldRichTextValue'; import { isFieldRichTextV2Value } from '@/object-record/record-field/types/guards/isFieldRichTextValueV2'; import { getForeignKeyNameFromRelationFieldName } from '@/object-record/utils/getForeignKeyNameFromRelationFieldName'; +import { isDeeplyEqual } from '~/utils/isDeeplyEqual'; import { FieldContext } from '../contexts/FieldContext'; import { isFieldBoolean } from '../types/guards/isFieldBoolean'; import { isFieldBooleanValue } from '../types/guards/isFieldBooleanValue'; @@ -57,7 +58,7 @@ export const usePersistField = () => { const [updateRecord] = useUpdateRecord(); const persistField = useRecoilCallback( - ({ set }) => + ({ set, snapshot }) => (valueToPersist: unknown) => { const fieldIsRelationToOneObject = isFieldRelationToOneObject( @@ -156,6 +157,22 @@ export const usePersistField = () => { if (isValuePersistable) { const fieldName = fieldDefinition.metadata.fieldName; + + const currentValue: any = snapshot + .getLoadable(recordStoreFamilySelector({ recordId, fieldName })) + .getValue(); + + if ( + fieldIsRelationToOneObject && + valueToPersist?.id === currentValue?.id + ) { + return; + } + + if (isDeeplyEqual(valueToPersist, currentValue)) { + return; + } + set( recordStoreFamilySelector({ recordId, fieldName }), valueToPersist,