From 95326b2828bea25f2f7f0115c5e8d15939114c51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tha=C3=AFs?= Date: Sat, 13 Jan 2024 08:35:30 -0300 Subject: [PATCH] fix: fix Relation field optimistic effect on Record update (#3352) * fix: fix Relation field optimistic effect on Record update Related to #3099 * Fix lint * Fix * fix --------- Co-authored-by: Charles Bochet --- packages/twenty-front/.eslintrc-ci.cjs | 7 +- packages/twenty-front/nyc.config.cjs | 42 +- .../hooks/useOptimisticEffect.ts | 24 +- .../getRecordOptimisticEffectDefinition.ts | 7 +- .../object-record/hooks/useUpdateOneRecord.ts | 52 ++- .../__tests__/useFilterDropdown.test.tsx | 1 - .../utils/isRecordMatchingFilter.ts | 364 ++++++++---------- .../src/utils/parseApolloStoreFieldName.ts | 21 + 8 files changed, 269 insertions(+), 249 deletions(-) create mode 100644 packages/twenty-front/src/utils/parseApolloStoreFieldName.ts diff --git a/packages/twenty-front/.eslintrc-ci.cjs b/packages/twenty-front/.eslintrc-ci.cjs index 91aafc6f9..9b36dd30a 100644 --- a/packages/twenty-front/.eslintrc-ci.cjs +++ b/packages/twenty-front/.eslintrc-ci.cjs @@ -5,7 +5,12 @@ module.exports = { }, overrides: [ { - files: ['.storybook/**/*', '**/*.stories.tsx', '**/*.test.ts'], + files: [ + '.storybook/**/*', + '**/*.stories.tsx', + '**/*.test.ts', + '**/*.test.tsx', + ], rules: { 'no-console': 'off', }, diff --git a/packages/twenty-front/nyc.config.cjs b/packages/twenty-front/nyc.config.cjs index 9b21eec6b..a6175cb9c 100644 --- a/packages/twenty-front/nyc.config.cjs +++ b/packages/twenty-front/nyc.config.cjs @@ -1,35 +1,29 @@ const globalCoverage = { - "statements": 60, - "lines": 60, - "functions": 60, - "exclude": [ - "src/generated/**/*", - ] + statements: 60, + lines: 60, + functions: 60, + exclude: ['src/generated/**/*'], }; const modulesCoverage = { - "statements": 50, - "lines": 50, - "functions": 45, - "include": [ - "src/modules/**/*", - ] + statements: 50, + lines: 50, + functions: 45, + include: ['src/modules/**/*'], }; const pagesCoverage = { - "statements": 50, - "lines": 50, - "functions": 45, - "exclude": [ - "src/generated/**/*", - "src/modules/**/*", - ] + statements: 50, + lines: 50, + functions: 45, + exclude: ['src/generated/**/*', 'src/modules/**/*'], }; const storybookStoriesFolders = process.env.STORYBOOK_SCOPE; -module.exports = storybookStoriesFolders === 'pages' - ? pagesCoverage - : storybookStoriesFolders === 'modules' - ? modulesCoverage - : globalCoverage; +module.exports = + storybookStoriesFolders === 'pages' + ? pagesCoverage + : storybookStoriesFolders === 'modules' + ? modulesCoverage + : globalCoverage; diff --git a/packages/twenty-front/src/modules/apollo/optimistic-effect/hooks/useOptimisticEffect.ts b/packages/twenty-front/src/modules/apollo/optimistic-effect/hooks/useOptimisticEffect.ts index 15062b335..effcfe1ea 100644 --- a/packages/twenty-front/src/modules/apollo/optimistic-effect/hooks/useOptimisticEffect.ts +++ b/packages/twenty-front/src/modules/apollo/optimistic-effect/hooks/useOptimisticEffect.ts @@ -146,8 +146,8 @@ export const useOptimisticEffect = ({ ({ snapshot }) => ({ typename, - createdRecords, - updatedRecords, + createdRecords = [], + updatedRecords = [], deletedRecordIds, }: { typename: string; @@ -162,17 +162,17 @@ export const useOptimisticEffect = ({ for (const optimisticEffect of Object.values(optimisticEffects)) { // We need to update the typename when createObject type differs from listObject types // It is the case for apiKey, where the creation route returns an ApiKeyToken type - const formattedCreatedRecords = isNonEmptyArray(createdRecords) - ? createdRecords.map((data: any) => { - return { ...data, __typename: typename }; - }) - : []; + const formattedCreatedRecords = createdRecords.map((createdRecord) => + typename.endsWith('Edge') + ? createdRecord + : { ...createdRecord, __typename: typename }, + ); - const formattedUpdatedRecords = isNonEmptyArray(updatedRecords) - ? updatedRecords.map((data: any) => { - return { ...data, __typename: typename }; - }) - : []; + const formattedUpdatedRecords = updatedRecords.map((updatedRecord) => + typename.endsWith('Edge') + ? updatedRecord + : { ...updatedRecord, __typename: typename }, + ); if (optimisticEffect.typename === typename) { optimisticEffect.writer({ diff --git a/packages/twenty-front/src/modules/object-record/graphql/optimistic-effect-definition/getRecordOptimisticEffectDefinition.ts b/packages/twenty-front/src/modules/object-record/graphql/optimistic-effect-definition/getRecordOptimisticEffectDefinition.ts index 68e3c8395..3111bcef8 100644 --- a/packages/twenty-front/src/modules/object-record/graphql/optimistic-effect-definition/getRecordOptimisticEffectDefinition.ts +++ b/packages/twenty-front/src/modules/object-record/graphql/optimistic-effect-definition/getRecordOptimisticEffectDefinition.ts @@ -29,8 +29,13 @@ export const getRecordOptimisticEffectDefinition = ({ if (isNonEmptyArray(createdRecords)) { if (existingDataIsEmpty) { return { - __typename: `${capitalize(objectMetadataItem.nameSingular)}Edge`, + __typename: `${capitalize( + objectMetadataItem.nameSingular, + )}Connection`, edges: createdRecords.map((createdRecord) => ({ + __typename: `${capitalize( + objectMetadataItem.nameSingular, + )}Edge`, node: createdRecord, cursor: '', })), diff --git a/packages/twenty-front/src/modules/object-record/hooks/useUpdateOneRecord.ts b/packages/twenty-front/src/modules/object-record/hooks/useUpdateOneRecord.ts index b192ecfd7..b8f9b6724 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/useUpdateOneRecord.ts +++ b/packages/twenty-front/src/modules/object-record/hooks/useUpdateOneRecord.ts @@ -1,8 +1,10 @@ -import { useApolloClient } from '@apollo/client'; +import { Reference, useApolloClient } from '@apollo/client'; import { useOptimisticEffect } from '@/apollo/optimistic-effect/hooks/useOptimisticEffect'; import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem'; +import { isRecordMatchingFilter } from '@/object-record/record-filter/utils/isRecordMatchingFilter'; import { sanitizeRecordInput } from '@/object-record/utils/sanitizeRecordInput'; +import { parseApolloStoreFieldName } from '~/utils/parseApolloStoreFieldName'; import { capitalize } from '~/utils/string/capitalize'; type useUpdateOneRecordProps = { @@ -59,6 +61,54 @@ export const useUpdateOneRecord = ({ [`update${capitalize(objectMetadataItem.nameSingular)}`]: optimisticallyUpdatedRecord, }, + update: (cache, { data }) => { + const response = + data?.[`update${capitalize(objectMetadataItem.nameSingular)}`]; + + if (!response) return; + + cache.modify>({ + fields: { + [objectMetadataItem.namePlural]: ( + existingConnectionRef, + { readField, storeFieldName }, + ) => { + if ( + readField('__typename', existingConnectionRef) !== + `${capitalize(objectMetadataItem.nameSingular)}Connection` + ) + return existingConnectionRef; + + const { variables } = parseApolloStoreFieldName(storeFieldName); + + const edges = readField<{ node: Reference }[]>( + 'edges', + existingConnectionRef, + ); + + if ( + variables?.filter && + !isRecordMatchingFilter({ + record: response, + filter: variables.filter, + objectMetadataItem, + }) && + edges?.length + ) { + return { + ...existingConnectionRef, + edges: edges.filter( + (edge) => + readField('id', readField('node', edge)) !== response.id, + ), + }; + } + + return existingConnectionRef; + }, + }, + }); + }, }); if (!updatedRecord?.data) { diff --git a/packages/twenty-front/src/modules/object-record/object-filter-dropdown/hooks/__tests__/useFilterDropdown.test.tsx b/packages/twenty-front/src/modules/object-record/object-filter-dropdown/hooks/__tests__/useFilterDropdown.test.tsx index f47592352..f8ed8e1de 100644 --- a/packages/twenty-front/src/modules/object-record/object-filter-dropdown/hooks/__tests__/useFilterDropdown.test.tsx +++ b/packages/twenty-front/src/modules/object-record/object-filter-dropdown/hooks/__tests__/useFilterDropdown.test.tsx @@ -261,7 +261,6 @@ describe('useFilterDropdown', () => { }); it('should handle scopeId undefined on initial values', () => { - // eslint-disable-next-line no-console console.error = jest.fn(); const renderFunction = () => { diff --git a/packages/twenty-front/src/modules/object-record/record-filter/utils/isRecordMatchingFilter.ts b/packages/twenty-front/src/modules/object-record/record-filter/utils/isRecordMatchingFilter.ts index 336297442..c794e1b46 100644 --- a/packages/twenty-front/src/modules/object-record/record-filter/utils/isRecordMatchingFilter.ts +++ b/packages/twenty-front/src/modules/object-record/record-filter/utils/isRecordMatchingFilter.ts @@ -7,7 +7,6 @@ import { DateFilter, FloatFilter, FullNameFilter, - LeafObjectRecordFilter, NotObjectRecordFilter, ObjectRecordQueryFilter, OrObjectRecordFilter, @@ -24,6 +23,18 @@ import { FieldMetadataType } from '~/generated-metadata/graphql'; import { isDefined } from '~/utils/isDefined'; import { isEmptyObject } from '~/utils/isEmptyObject'; +const isAndFilter = ( + filter: ObjectRecordQueryFilter, +): filter is AndObjectRecordFilter => 'and' in filter && !!filter.and; + +const isOrFilter = ( + filter: ObjectRecordQueryFilter, +): filter is OrObjectRecordFilter => 'or' in filter && !!filter.or; + +const isNotFilter = ( + filter: ObjectRecordQueryFilter, +): filter is NotObjectRecordFilter => 'not' in filter && !!filter.not; + export const isRecordMatchingFilter = ({ record, filter, @@ -32,238 +43,173 @@ export const isRecordMatchingFilter = ({ record: any; filter: ObjectRecordQueryFilter; objectMetadataItem: ObjectMetadataItem; -}) => { +}): boolean => { if (Object.keys(filter).length === 0) { return true; } - const currentLevelFilterMatches: boolean[] = []; + if (isAndFilter(filter)) { + const filterValue = filter.and; - // We consider all the keys at the same level as an "and" - for (const filterKey in filter) { - if (filterKey === 'and') { - const filterValue = (filter as AndObjectRecordFilter).and; + if (!Array.isArray(filterValue)) { + throw new Error( + 'Unexpected value for "and" filter : ' + JSON.stringify(filterValue), + ); + } - if (!Array.isArray(filterValue)) { - throw new Error( - 'Unexpected value for "and" filter : ' + JSON.stringify(filterValue), - ); - } - - if (filterValue.length === 0) { - currentLevelFilterMatches.push(true); - continue; - } - - const recordIsMatchingAndFilters = filterValue.every((andFilter) => + return ( + filterValue.length === 0 || + filterValue.every((andFilter) => isRecordMatchingFilter({ record, filter: andFilter, objectMetadataItem, }), - ); + ) + ); + } - currentLevelFilterMatches.push(recordIsMatchingAndFilters); - } else if (filterKey === 'or') { - const filterValue = (filter as OrObjectRecordFilter).or; + if (isOrFilter(filter)) { + const filterValue = filter.or; - if (Array.isArray(filterValue)) { - if (filterValue.length === 0) { - currentLevelFilterMatches.push(true); - continue; - } - - const recordIsMatchingOrFilters = filterValue.some((orFilter) => + if (Array.isArray(filterValue)) { + return ( + filterValue.length === 0 || + filterValue.some((orFilter) => isRecordMatchingFilter({ record, filter: orFilter, objectMetadataItem, }), - ); + ) + ); + } - currentLevelFilterMatches.push(recordIsMatchingOrFilters); - } else if (isObject(filterValue)) { - // The API considers "or" with an object as an "and" - const recordIsMatchingOrFilters = isRecordMatchingFilter({ - record, - filter: filterValue, - objectMetadataItem, - }); - - currentLevelFilterMatches.push(recordIsMatchingOrFilters); - } else { - throw new Error('Unexpected value for "or" filter : ' + filterValue); - } - } else if (filterKey === 'not') { - const filterValue = (filter as NotObjectRecordFilter).not; - - if (!isDefined(filterValue)) { - throw new Error('Unexpected value for "not" filter : ' + filterValue); - } - - if (isEmptyObject(filterValue)) { - currentLevelFilterMatches.push(true); - continue; - } - - const recordIsMatchingNotFilters = !isRecordMatchingFilter({ + if (isObject(filterValue)) { + // The API considers "or" with an object as an "and" + return isRecordMatchingFilter({ record, filter: filterValue, objectMetadataItem, }); - - currentLevelFilterMatches.push(recordIsMatchingNotFilters); - } else { - const filterValue = (filter as LeafObjectRecordFilter)[filterKey]; - - if (!isDefined(filterValue)) { - throw new Error( - 'Unexpected value for filter key "' + - filterKey + - '" : ' + - filterValue, - ); - } - - if (isEmptyObject(filterValue)) { - currentLevelFilterMatches.push(true); - - continue; - } - - const objectMetadataField = objectMetadataItem.fields.find( - (field) => field.name === filterKey, - ); - - if (!isDefined(objectMetadataField)) { - throw new Error( - 'Field metadata item "' + - filterKey + - '" not found for object metadata item ' + - objectMetadataItem.nameSingular, - ); - } - - switch (objectMetadataField.type) { - case FieldMetadataType.Email: - case FieldMetadataType.Phone: - case FieldMetadataType.Text: { - const stringFilter = filterValue as StringFilter; - - currentLevelFilterMatches.push( - isMatchingStringFilter({ - stringFilter, - value: record[filterKey], - }), - ); - break; - } - case FieldMetadataType.Link: { - const urlFilter = filterValue as URLFilter; - - if (urlFilter.url !== undefined) { - currentLevelFilterMatches.push( - isMatchingStringFilter({ - stringFilter: urlFilter.url, - value: record[filterKey].url, - }), - ); - } - - if (urlFilter.label !== undefined) { - currentLevelFilterMatches.push( - isMatchingStringFilter({ - stringFilter: urlFilter.label, - value: record[filterKey].label, - }), - ); - } - break; - } - case FieldMetadataType.FullName: { - const fullNameFilter = filterValue as FullNameFilter; - - if (fullNameFilter.firstName !== undefined) { - currentLevelFilterMatches.push( - isMatchingStringFilter({ - stringFilter: fullNameFilter.firstName, - value: record[filterKey].firstName, - }), - ); - } - - if (fullNameFilter.lastName !== undefined) { - currentLevelFilterMatches.push( - isMatchingStringFilter({ - stringFilter: fullNameFilter.lastName, - value: record[filterKey].lastName, - }), - ); - } - break; - } - case FieldMetadataType.DateTime: { - const dateFilter = filterValue as DateFilter; - - currentLevelFilterMatches.push( - isMatchingDateFilter({ - dateFilter, - value: record[filterKey], - }), - ); - break; - } - case FieldMetadataType.Number: - case FieldMetadataType.Numeric: { - const numberFilter = filterValue as FloatFilter; - - currentLevelFilterMatches.push( - isMatchingFloatFilter({ - floatFilter: numberFilter, - value: record[filterKey], - }), - ); - break; - } - case FieldMetadataType.Uuid: { - const uuidFilter = filterValue as UUIDFilter; - - currentLevelFilterMatches.push( - isMatchingUUIDFilter({ - uuidFilter, - value: record[filterKey], - }), - ); - break; - } - case FieldMetadataType.Boolean: { - const booleanFilter = filterValue as BooleanFilter; - - currentLevelFilterMatches.push( - isMatchingBooleanFilter({ - booleanFilter, - value: record[filterKey], - }), - ); - break; - } - case FieldMetadataType.Relation: { - throw new Error( - `Not implemented yet, use UUID filter instead on the corredponding "${filterKey}Id" field`, - ); - } - case FieldMetadataType.Currency: - case FieldMetadataType.MultiSelect: - case FieldMetadataType.Select: - case FieldMetadataType.Probability: - case FieldMetadataType.Rating: { - throw new Error('Not implemented yet'); - } - } } + + throw new Error('Unexpected value for "or" filter : ' + filterValue); } - return currentLevelFilterMatches.length > 0 - ? currentLevelFilterMatches.every((match) => !!match) - : false; + if (isNotFilter(filter)) { + const filterValue = filter.not; + + if (!isDefined(filterValue)) { + throw new Error('Unexpected value for "not" filter : ' + filterValue); + } + + return ( + isEmptyObject(filterValue) || + !isRecordMatchingFilter({ + record, + filter: filterValue, + objectMetadataItem, + }) + ); + } + + return Object.entries(filter).every(([filterKey, filterValue]) => { + if (!isDefined(filterValue)) { + throw new Error( + 'Unexpected value for filter key "' + filterKey + '" : ' + filterValue, + ); + } + + if (isEmptyObject(filterValue)) return true; + + const objectMetadataField = objectMetadataItem.fields.find( + (field) => field.name === filterKey, + ); + + if (!isDefined(objectMetadataField)) { + throw new Error( + 'Field metadata item "' + + filterKey + + '" not found for object metadata item ' + + objectMetadataItem.nameSingular, + ); + } + + switch (objectMetadataField.type) { + case FieldMetadataType.Email: + case FieldMetadataType.Phone: + case FieldMetadataType.Text: { + return isMatchingStringFilter({ + stringFilter: filterValue as StringFilter, + value: record[filterKey], + }); + } + case FieldMetadataType.Link: { + const urlFilter = filterValue as URLFilter; + + return ( + (urlFilter.url === undefined || + isMatchingStringFilter({ + stringFilter: urlFilter.url, + value: record[filterKey].url, + })) && + (urlFilter.label === undefined || + isMatchingStringFilter({ + stringFilter: urlFilter.label, + value: record[filterKey].label, + })) + ); + } + case FieldMetadataType.FullName: { + const fullNameFilter = filterValue as FullNameFilter; + + return ( + (fullNameFilter.firstName === undefined || + isMatchingStringFilter({ + stringFilter: fullNameFilter.firstName, + value: record[filterKey].firstName, + })) && + (fullNameFilter.lastName === undefined || + isMatchingStringFilter({ + stringFilter: fullNameFilter.lastName, + value: record[filterKey].lastName, + })) + ); + } + case FieldMetadataType.DateTime: { + return isMatchingDateFilter({ + dateFilter: filterValue as DateFilter, + value: record[filterKey], + }); + } + case FieldMetadataType.Number: + case FieldMetadataType.Numeric: { + return isMatchingFloatFilter({ + floatFilter: filterValue as FloatFilter, + value: record[filterKey], + }); + } + case FieldMetadataType.Uuid: { + return isMatchingUUIDFilter({ + uuidFilter: filterValue as UUIDFilter, + value: record[filterKey], + }); + } + case FieldMetadataType.Boolean: { + return isMatchingBooleanFilter({ + booleanFilter: filterValue as BooleanFilter, + value: record[filterKey], + }); + } + case FieldMetadataType.Relation: { + throw new Error( + `Not implemented yet, use UUID filter instead on the corredponding "${filterKey}Id" field`, + ); + } + default: { + throw new Error('Not implemented yet'); + } + } + }); }; diff --git a/packages/twenty-front/src/utils/parseApolloStoreFieldName.ts b/packages/twenty-front/src/utils/parseApolloStoreFieldName.ts new file mode 100644 index 000000000..004c9f4f1 --- /dev/null +++ b/packages/twenty-front/src/utils/parseApolloStoreFieldName.ts @@ -0,0 +1,21 @@ +// There is a feature request for receiving variables in `cache.modify`: +// @see https://github.com/apollographql/apollo-feature-requests/issues/259 +// @see https://github.com/apollographql/apollo-client/issues/7129 +// For now we need to parse `storeFieldName` to retrieve the variables. +export const parseApolloStoreFieldName = (storeFieldName: string) => { + const matches = storeFieldName.match(/([a-zA-Z][a-zA-Z0-9 ]*)\((.*)\)/); + + if (!matches?.[1]) return {}; + + const [, fieldName, stringifiedVariables] = matches; + + try { + const variables = stringifiedVariables + ? (JSON.parse(stringifiedVariables) as Record) + : undefined; + + return { fieldName, variables }; + } catch { + return { fieldName }; + } +};