From ae4fb7d113d37ed16254451c781942aec03a9485 Mon Sep 17 00:00:00 2001 From: pau-not-paul <33595852+pau-not-paul@users.noreply.github.com> Date: Thu, 21 Nov 2024 14:02:09 +0100 Subject: [PATCH] fix: soft deleted records are read only (#8198) TODO: - [ ] It should not be possible to add tasks, notes, files, etc. Fix for https://github.com/twentyhq/twenty/issues/7172 Note for reviewer: - With these changes, `deletedAt` is now always included in `recordGqlFields`. --- Edit from Charles -- In this PR: 1) As mentionned by @pau-not-paul, we are adding deletedAt to our recordGqlFields for board and table 2) I'm removing cellReadOnly logic, it is now fully computed using useIsFieldValueReadonly like it's done in other places, there is no need to maintain two read only systems 3) refactoring useIsFieldValueReadonly to take all the business logics into one place together. It's now a function of the 5 factors (isRemote, isDeleted, objectName, fieldName, etc...). Later it's likely to get back to a function of Pick, Pick, record.isDeleted but we are not there yet Note: as all cells are listening to the record (for isDeleted), updating a field will trigger a re-rendering of the row which is not an issue right now. It will be if we introduce bulk edition. In this case we would need to use a selector on fields on top of record store --------- Co-authored-by: Charles Bochet --- .../__mocks__/fieldDefinitions.ts | 1 + .../__tests__/useIsFieldReadOnly.test.tsx | 50 -------- .../useIsFieldValueReadOnly.test.tsx | 74 ++++++++++++ .../record-field/hooks/useIsFieldReadOnly.ts | 18 --- .../hooks/useIsFieldValueReadOnly.ts | 30 +++++ .../__tests__/isFieldValueReadOnly.test.ts | 113 ++++++++++++++++++ .../utils/isFieldMetadataReadOnly.ts | 19 --- .../utils/isFieldValueReadOnly.ts | 54 +++++++++ .../hooks/useRecordBoardRecordGqlFields.ts | 1 + .../hooks/useRecordTableRecordGqlFields.ts | 1 + .../components/RecordInlineCell.tsx | 15 +-- .../record-show/components/FieldsCard.tsx | 6 +- .../record-show/components/SummaryCard.tsx | 8 +- .../RecordDetailRelationRecordsListItem.tsx | 6 +- .../RecordDetailRelationSection.tsx | 7 +- .../perf/RecordTableCell.perf.stories.tsx | 1 - .../contexts/RecordTableRowContext.ts | 1 - .../components/RecordTableCellFieldInput.tsx | 4 +- .../RecordTableCellSoftFocusMode.tsx | 18 ++- .../record-table-cell/hooks/__mocks__/cell.ts | 1 - .../hooks/useOpenRecordTableCellFromCell.ts | 9 +- .../components/RecordTableRowWrapper.tsx | 5 +- .../testing/jest/JestRecordStoreSetter.tsx | 20 ++++ 23 files changed, 327 insertions(+), 135 deletions(-) delete mode 100644 packages/twenty-front/src/modules/object-record/record-field/hooks/__tests__/useIsFieldReadOnly.test.tsx create mode 100644 packages/twenty-front/src/modules/object-record/record-field/hooks/__tests__/useIsFieldValueReadOnly.test.tsx delete mode 100644 packages/twenty-front/src/modules/object-record/record-field/hooks/useIsFieldReadOnly.ts create mode 100644 packages/twenty-front/src/modules/object-record/record-field/hooks/useIsFieldValueReadOnly.ts create mode 100644 packages/twenty-front/src/modules/object-record/record-field/utils/__tests__/isFieldValueReadOnly.test.ts delete mode 100644 packages/twenty-front/src/modules/object-record/record-field/utils/isFieldMetadataReadOnly.ts create mode 100644 packages/twenty-front/src/modules/object-record/record-field/utils/isFieldValueReadOnly.ts create mode 100644 packages/twenty-front/src/testing/jest/JestRecordStoreSetter.tsx diff --git a/packages/twenty-front/src/modules/object-record/record-field/__mocks__/fieldDefinitions.ts b/packages/twenty-front/src/modules/object-record/record-field/__mocks__/fieldDefinitions.ts index 0fe2fc3ca..03e81bf66 100644 --- a/packages/twenty-front/src/modules/object-record/record-field/__mocks__/fieldDefinitions.ts +++ b/packages/twenty-front/src/modules/object-record/record-field/__mocks__/fieldDefinitions.ts @@ -108,5 +108,6 @@ export const actorFieldDefinition: FieldDefinition = { defaultValue: { source: 'MANUAL', name: '' }, metadata: { fieldName: 'actor', + objectMetadataNameSingular: 'person', }, }; diff --git a/packages/twenty-front/src/modules/object-record/record-field/hooks/__tests__/useIsFieldReadOnly.test.tsx b/packages/twenty-front/src/modules/object-record/record-field/hooks/__tests__/useIsFieldReadOnly.test.tsx deleted file mode 100644 index f51cf795f..000000000 --- a/packages/twenty-front/src/modules/object-record/record-field/hooks/__tests__/useIsFieldReadOnly.test.tsx +++ /dev/null @@ -1,50 +0,0 @@ -import { renderHook } from '@testing-library/react'; -import { ReactNode } from 'react'; -import { RecoilRoot } from 'recoil'; - -import { - actorFieldDefinition, - phonesFieldDefinition, -} from '@/object-record/record-field/__mocks__/fieldDefinitions'; -import { FieldContext } from '@/object-record/record-field/contexts/FieldContext'; -import { useIsFieldReadOnly } from '@/object-record/record-field/hooks/useIsFieldReadOnly'; -import { FieldDefinition } from '@/object-record/record-field/types/FieldDefinition'; -import { FieldMetadata } from '@/object-record/record-field/types/FieldMetadata'; - -const recordId = 'recordId'; - -const getWrapper = - (fieldDefinition: FieldDefinition) => - ({ children }: { children: ReactNode }) => ( - - {children} - - ); - -const ActorWrapper = getWrapper(actorFieldDefinition); -const PhoneWrapper = getWrapper(phonesFieldDefinition); - -describe('useIsFieldReadOnly', () => { - it('should return true', () => { - const { result } = renderHook(() => useIsFieldReadOnly(), { - wrapper: ActorWrapper, - }); - - expect(result.current).toBe(true); - }); - - it('should return false', () => { - const { result } = renderHook(() => useIsFieldReadOnly(), { - wrapper: PhoneWrapper, - }); - - expect(result.current).toBe(false); - }); -}); diff --git a/packages/twenty-front/src/modules/object-record/record-field/hooks/__tests__/useIsFieldValueReadOnly.test.tsx b/packages/twenty-front/src/modules/object-record/record-field/hooks/__tests__/useIsFieldValueReadOnly.test.tsx new file mode 100644 index 000000000..a436b9753 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-field/hooks/__tests__/useIsFieldValueReadOnly.test.tsx @@ -0,0 +1,74 @@ +import { renderHook } from '@testing-library/react'; +import { ReactNode } from 'react'; +import { RecoilRoot } from 'recoil'; + +import { + actorFieldDefinition, + phonesFieldDefinition, +} from '@/object-record/record-field/__mocks__/fieldDefinitions'; +import { FieldContext } from '@/object-record/record-field/contexts/FieldContext'; +import { FieldDefinition } from '@/object-record/record-field/types/FieldDefinition'; +import { FieldMetadata } from '@/object-record/record-field/types/FieldMetadata'; + +import { ObjectRecord } from '@/object-record/types/ObjectRecord'; +import { JestObjectMetadataItemSetter } from '~/testing/jest/JestObjectMetadataItemSetter'; +import { JestRecordStoreSetter } from '~/testing/jest/JestRecordStoreSetter'; + +import { useIsFieldValueReadOnly } from '../useIsFieldValueReadOnly'; + +const recordId = 'recordId'; + +const getWrapper = + (fieldDefinition: FieldDefinition, isRecordDeleted: boolean) => + ({ children }: { children: ReactNode }) => { + return ( + + + + + {children} + + + + + ); + }; + +describe('useIsFieldValueReadOnly', () => { + it('should take fieldDefinition into account', () => { + const { result } = renderHook(() => useIsFieldValueReadOnly(), { + wrapper: getWrapper(phonesFieldDefinition, false), + }); + + expect(result.current).toBe(false); + + const { result: result2 } = renderHook(() => useIsFieldValueReadOnly(), { + wrapper: getWrapper(actorFieldDefinition, false), + }); + + expect(result2.current).toBe(true); + }); + + it('should take isRecordDeleted into account', () => { + const { result } = renderHook(() => useIsFieldValueReadOnly(), { + wrapper: getWrapper(phonesFieldDefinition, true), + }); + + expect(result.current).toBe(true); + }); +}); diff --git a/packages/twenty-front/src/modules/object-record/record-field/hooks/useIsFieldReadOnly.ts b/packages/twenty-front/src/modules/object-record/record-field/hooks/useIsFieldReadOnly.ts deleted file mode 100644 index 48bb03424..000000000 --- a/packages/twenty-front/src/modules/object-record/record-field/hooks/useIsFieldReadOnly.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { useContext } from 'react'; - -import { isFieldActor } from '@/object-record/record-field/types/guards/isFieldActor'; -import { isFieldRichText } from '@/object-record/record-field/types/guards/isFieldRichText'; -import { FieldContext } from '../contexts/FieldContext'; -import { isFieldMetadataReadOnly } from '../utils/isFieldMetadataReadOnly'; - -export const useIsFieldReadOnly = () => { - const { fieldDefinition } = useContext(FieldContext); - - const { metadata } = fieldDefinition; - - return ( - isFieldActor(fieldDefinition) || - isFieldRichText(fieldDefinition) || - isFieldMetadataReadOnly(metadata) - ); -}; diff --git a/packages/twenty-front/src/modules/object-record/record-field/hooks/useIsFieldValueReadOnly.ts b/packages/twenty-front/src/modules/object-record/record-field/hooks/useIsFieldValueReadOnly.ts new file mode 100644 index 000000000..aa8ec3515 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-field/hooks/useIsFieldValueReadOnly.ts @@ -0,0 +1,30 @@ +import { useContext } from 'react'; + +import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem'; +import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState'; +import { ObjectRecord } from '@/object-record/types/ObjectRecord'; +import { useRecoilValue } from 'recoil'; +import { FieldContext } from '../contexts/FieldContext'; +import { isFieldValueReadOnly } from '../utils/isFieldValueReadOnly'; + +export const useIsFieldValueReadOnly = () => { + const { fieldDefinition, recordId } = useContext(FieldContext); + + const { metadata, type } = fieldDefinition; + + const recordFromStore = useRecoilValue( + recordStoreFamilyState(recordId), + ); + + const { objectMetadataItem } = useObjectMetadataItem({ + objectNameSingular: metadata.objectMetadataNameSingular ?? '', + }); + + return isFieldValueReadOnly({ + objectNameSingular: metadata.objectMetadataNameSingular, + fieldName: metadata.fieldName, + fieldType: type, + isObjectRemote: objectMetadataItem.isRemote, + isRecordDeleted: recordFromStore?.deletedAt, + }); +}; diff --git a/packages/twenty-front/src/modules/object-record/record-field/utils/__tests__/isFieldValueReadOnly.test.ts b/packages/twenty-front/src/modules/object-record/record-field/utils/__tests__/isFieldValueReadOnly.test.ts new file mode 100644 index 000000000..a5dff94f7 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-field/utils/__tests__/isFieldValueReadOnly.test.ts @@ -0,0 +1,113 @@ +import { isFieldValueReadOnly } from '@/object-record/record-field/utils/isFieldValueReadOnly'; +import { FieldMetadataType } from '~/generated/graphql'; + +describe('isFieldValueReadOnly', () => { + it('should return true if fieldName is noteTargets or taskTargets', () => { + const result = isFieldValueReadOnly({ + fieldName: 'noteTargets', + }); + expect(result).toBe(true); + + const result2 = isFieldValueReadOnly({ + fieldName: 'taskTargets', + }); + + expect(result2).toBe(true); + }); + + it('should return false if fieldName is not noteTargets or taskTargets', () => { + const result = isFieldValueReadOnly({ + fieldName: 'test', + }); + + expect(result).toBe(false); + }); + + it('should return true if isObjectRemote is true', () => { + const result = isFieldValueReadOnly({ + isObjectRemote: true, + }); + + expect(result).toBe(true); + }); + + it('should return false if isObjectRemote is false', () => { + const result = isFieldValueReadOnly({ + isObjectRemote: false, + }); + + expect(result).toBe(false); + }); + + it('should return true if isRecordDeleted is true', () => { + const result = isFieldValueReadOnly({ + isRecordDeleted: true, + }); + + expect(result).toBe(true); + }); + + it('should return false if isRecordDeleted is false', () => { + const result = isFieldValueReadOnly({ + isRecordDeleted: false, + }); + + expect(result).toBe(false); + }); + + it('should return true if objectNameSingular is Workflow and fieldName is not name', () => { + const result = isFieldValueReadOnly({ + objectNameSingular: 'workflow', + fieldName: 'test', + }); + + expect(result).toBe(true); + }); + + it('should return false if objectNameSingular is Workflow and fieldName is name', () => { + const result = isFieldValueReadOnly({ + objectNameSingular: 'Workflow', + fieldName: 'name', + }); + + expect(result).toBe(false); + }); + + it('should return true if isWorkflowSubObjectMetadata is true', () => { + const result = isFieldValueReadOnly({ + objectNameSingular: 'workflowVersion', + }); + + expect(result).toBe(true); + }); + + it('should return true if fieldType is FieldMetadataType.Actor', () => { + const result = isFieldValueReadOnly({ + fieldType: FieldMetadataType.Actor, + }); + + expect(result).toBe(true); + }); + + it('should return true if fieldType is FieldMetadataType.RichText', () => { + const result = isFieldValueReadOnly({ + fieldType: FieldMetadataType.RichText, + }); + + expect(result).toBe(true); + }); + + it('should return false if fieldType is not FieldMetadataType.Actor or FieldMetadataType.RichText', () => { + const result = isFieldValueReadOnly({ + fieldType: FieldMetadataType.Text, + }); + + expect(result).toBe(false); + }); + + it('should return false if none of the conditions are met', () => { + const result = isFieldValueReadOnly({}); + + expect(result).toBe(false); + }); +}); diff --git a/packages/twenty-front/src/modules/object-record/record-field/utils/isFieldMetadataReadOnly.ts b/packages/twenty-front/src/modules/object-record/record-field/utils/isFieldMetadataReadOnly.ts deleted file mode 100644 index a24ed1003..000000000 --- a/packages/twenty-front/src/modules/object-record/record-field/utils/isFieldMetadataReadOnly.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; -import { isWorkflowSubObjectMetadata } from '@/object-metadata/utils/isWorkflowSubObjectMetadata'; -import { FieldMetadata } from '@/object-record/record-field/types/FieldMetadata'; - -export const isFieldMetadataReadOnly = (fieldMetadata: FieldMetadata) => { - if ( - fieldMetadata.fieldName === 'noteTargets' || - fieldMetadata.fieldName === 'taskTargets' - ) { - return true; - } - - return ( - isWorkflowSubObjectMetadata(fieldMetadata.objectMetadataNameSingular) || - (fieldMetadata.objectMetadataNameSingular === - CoreObjectNameSingular.Workflow && - fieldMetadata.fieldName !== 'name') - ); -}; diff --git a/packages/twenty-front/src/modules/object-record/record-field/utils/isFieldValueReadOnly.ts b/packages/twenty-front/src/modules/object-record/record-field/utils/isFieldValueReadOnly.ts new file mode 100644 index 000000000..e3e59574d --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-field/utils/isFieldValueReadOnly.ts @@ -0,0 +1,54 @@ +import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; +import { isWorkflowSubObjectMetadata } from '@/object-metadata/utils/isWorkflowSubObjectMetadata'; +import { isFieldActor } from '@/object-record/record-field/types/guards/isFieldActor'; +import { isFieldRichText } from '@/object-record/record-field/types/guards/isFieldRichText'; +import { isDefined } from 'twenty-ui'; +import { FieldMetadataType } from '~/generated-metadata/graphql'; + +type isFieldValueReadOnlyParams = { + objectNameSingular?: string; + fieldName?: string; + fieldType?: FieldMetadataType; + isObjectRemote?: boolean; + isRecordDeleted?: boolean; +}; + +export const isFieldValueReadOnly = ({ + objectNameSingular, + fieldName, + fieldType, + isObjectRemote = false, + isRecordDeleted = false, +}: isFieldValueReadOnlyParams) => { + if (fieldName === 'noteTargets' || fieldName === 'taskTargets') { + return true; + } + + if (isObjectRemote) { + return true; + } + + if (isRecordDeleted) { + return true; + } + + if (isWorkflowSubObjectMetadata(objectNameSingular)) { + return true; + } + + if ( + objectNameSingular === CoreObjectNameSingular.Workflow && + fieldName !== 'name' + ) { + return true; + } + + if ( + isDefined(fieldType) && + (isFieldActor({ type: fieldType }) || isFieldRichText({ type: fieldType })) + ) { + return true; + } + + return false; +}; diff --git a/packages/twenty-front/src/modules/object-record/record-index/hooks/useRecordBoardRecordGqlFields.ts b/packages/twenty-front/src/modules/object-record/record-index/hooks/useRecordBoardRecordGqlFields.ts index 64433aae2..aef238c2b 100644 --- a/packages/twenty-front/src/modules/object-record/record-index/hooks/useRecordBoardRecordGqlFields.ts +++ b/packages/twenty-front/src/modules/object-record/record-index/hooks/useRecordBoardRecordGqlFields.ts @@ -36,6 +36,7 @@ export const useRecordBoardRecordGqlFields = ({ const recordGqlFields: Record = { id: true, + deletedAt: true, ...Object.fromEntries( visibleFieldDefinitions.map((visibleFieldDefinition) => [ visibleFieldDefinition.metadata.fieldName, diff --git a/packages/twenty-front/src/modules/object-record/record-index/hooks/useRecordTableRecordGqlFields.ts b/packages/twenty-front/src/modules/object-record/record-index/hooks/useRecordTableRecordGqlFields.ts index ae0787744..a4588c6d5 100644 --- a/packages/twenty-front/src/modules/object-record/record-index/hooks/useRecordTableRecordGqlFields.ts +++ b/packages/twenty-front/src/modules/object-record/record-index/hooks/useRecordTableRecordGqlFields.ts @@ -41,6 +41,7 @@ export const useRecordTableRecordGqlFields = ({ const recordGqlFields: Record = { id: true, + deletedAt: true, ...Object.fromEntries( visibleTableColumns.map((column) => [column.metadata.fieldName, true]), ), diff --git a/packages/twenty-front/src/modules/object-record/record-inline-cell/components/RecordInlineCell.tsx b/packages/twenty-front/src/modules/object-record/record-inline-cell/components/RecordInlineCell.tsx index 55f3ccbf7..8b58072b0 100644 --- a/packages/twenty-front/src/modules/object-record/record-inline-cell/components/RecordInlineCell.tsx +++ b/packages/twenty-front/src/modules/object-record/record-inline-cell/components/RecordInlineCell.tsx @@ -13,7 +13,7 @@ import { RelationPickerHotkeyScope } from '@/object-record/relation-picker/types import { useInlineCell } from '../hooks/useInlineCell'; -import { useIsFieldReadOnly } from '@/object-record/record-field/hooks/useIsFieldReadOnly'; +import { useIsFieldValueReadOnly } from '@/object-record/record-field/hooks/useIsFieldValueReadOnly'; import { getRecordFieldInputId } from '@/object-record/utils/getRecordFieldInputId'; import { RecordInlineCellContainer } from './RecordInlineCellContainer'; import { @@ -26,21 +26,16 @@ type RecordInlineCellProps = { loading?: boolean; }; -export const RecordInlineCell = ({ - readonly, - loading, -}: RecordInlineCellProps) => { +export const RecordInlineCell = ({ loading }: RecordInlineCellProps) => { const { fieldDefinition, recordId, isCentered } = useContext(FieldContext); const buttonIcon = useGetButtonIcon(); const isFieldInputOnly = useIsFieldInputOnly(); - const isFieldReadOnly = useIsFieldReadOnly(); + const isFieldReadOnly = useIsFieldValueReadOnly(); const { closeInlineCell } = useInlineCell(); - const cellIsReadOnly = readonly || isFieldReadOnly; - const handleEnter: FieldInputEvent = (persistField) => { persistField(); closeInlineCell(); @@ -77,7 +72,7 @@ export const RecordInlineCell = ({ const { getIcon } = useIcons(); const RecordInlineCellContextValue: RecordInlineCellContextProps = { - readonly: cellIsReadOnly, + readonly: isFieldReadOnly, buttonIcon: buttonIcon, customEditHotkeyScope: isFieldRelation(fieldDefinition) ? { scope: RelationPickerHotkeyScope.RelationPicker } @@ -102,7 +97,7 @@ export const RecordInlineCell = ({ onTab={handleTab} onShiftTab={handleShiftTab} onClickOutside={handleClickOutside} - isReadOnly={cellIsReadOnly} + isReadOnly={isFieldReadOnly} /> ), displayModeContent: , diff --git a/packages/twenty-front/src/modules/object-record/record-show/components/FieldsCard.tsx b/packages/twenty-front/src/modules/object-record/record-show/components/FieldsCard.tsx index 1e2d3504b..3dfc2550f 100644 --- a/packages/twenty-front/src/modules/object-record/record-show/components/FieldsCard.tsx +++ b/packages/twenty-front/src/modules/object-record/record-show/components/FieldsCard.tsx @@ -84,7 +84,6 @@ export const FieldsCard = ({ fieldMetadataItem.name === 'taskTargets') ), ); - const isReadOnly = objectMetadataItem.isRemote; return ( <> @@ -149,10 +148,7 @@ export const FieldsCard = ({ hotkeyScope: InlineCellHotkeyScope.InlineCell, }} > - + ))} diff --git a/packages/twenty-front/src/modules/object-record/record-show/components/SummaryCard.tsx b/packages/twenty-front/src/modules/object-record/record-show/components/SummaryCard.tsx index 493854b12..d681cf199 100644 --- a/packages/twenty-front/src/modules/object-record/record-show/components/SummaryCard.tsx +++ b/packages/twenty-front/src/modules/object-record/record-show/components/SummaryCard.tsx @@ -1,6 +1,7 @@ import { useGetStandardObjectIcon } from '@/object-metadata/hooks/useGetStandardObjectIcon'; import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; import { FieldContext } from '@/object-record/record-field/contexts/FieldContext'; +import { isFieldValueReadOnly } from '@/object-record/record-field/utils/isFieldValueReadOnly'; import { RecordInlineCell } from '@/object-record/record-inline-cell/components/RecordInlineCell'; import { InlineCellHotkeyScope } from '@/object-record/record-inline-cell/types/InlineCellHotkeyScope'; import { RightDrawerTitleRecordInlineCell } from '@/object-record/record-right-drawer/components/RightDrawerTitleRecordInlineCell'; @@ -29,7 +30,6 @@ export const SummaryCard = ({ const { recordFromStore, recordLoading, - objectMetadataItem, labelIdentifierFieldMetadataItem, isPrefetchLoading, recordIdentifier, @@ -47,7 +47,11 @@ export const SummaryCard = ({ const { Icon, IconColor } = useGetStandardObjectIcon(objectNameSingular); const isMobile = useIsMobile() || isInRightDrawer; - const isReadOnly = objectMetadataItem.isRemote; + + const isReadOnly = isFieldValueReadOnly({ + objectNameSingular, + isRecordDeleted: recordFromStore?.isDeleted, + }); if (isNewRightDrawerItemLoading || !isDefined(recordFromStore)) { return ; diff --git a/packages/twenty-front/src/modules/object-record/record-show/record-detail-section/components/RecordDetailRelationRecordsListItem.tsx b/packages/twenty-front/src/modules/object-record/record-show/record-detail-section/components/RecordDetailRelationRecordsListItem.tsx index 3d51e9d38..6c9c2ed81 100644 --- a/packages/twenty-front/src/modules/object-record/record-show/record-detail-section/components/RecordDetailRelationRecordsListItem.tsx +++ b/packages/twenty-front/src/modules/object-record/record-show/record-detail-section/components/RecordDetailRelationRecordsListItem.tsx @@ -25,9 +25,9 @@ import { RecordUpdateHook, RecordUpdateHookParams, } from '@/object-record/record-field/contexts/FieldContext'; +import { useIsFieldValueReadOnly } from '@/object-record/record-field/hooks/useIsFieldValueReadOnly'; import { usePersistField } from '@/object-record/record-field/hooks/usePersistField'; import { FieldRelationMetadata } from '@/object-record/record-field/types/FieldMetadata'; -import { isFieldMetadataReadOnly } from '@/object-record/record-field/utils/isFieldMetadataReadOnly'; import { RecordInlineCell } from '@/object-record/record-inline-cell/components/RecordInlineCell'; import { PropertyBox } from '@/object-record/record-inline-cell/property-box/components/PropertyBox'; import { InlineCellHotkeyScope } from '@/object-record/record-inline-cell/types/InlineCellHotkeyScope'; @@ -189,7 +189,7 @@ export const RecordDetailRelationRecordsListItem = ({ [isExpanded], ); - const canEdit = !isFieldMetadataReadOnly(fieldDefinition.metadata); + const isReadOnly = useIsFieldValueReadOnly(); return ( <> @@ -206,7 +206,7 @@ export const RecordDetailRelationRecordsListItem = ({ accent="tertiary" /> - {canEdit && ( + {!isReadOnly && ( 0} rightAdornment={ - canEdit && ( + !isReadOnly && ( { useContext(RecordTableContext); const { recordId, fieldDefinition } = useContext(FieldContext); - const isFieldReadOnly = useIsFieldReadOnly(); + const isFieldReadOnly = useIsFieldValueReadOnly(); const handleEnter: FieldInputEvent = (persistField) => { onUpsertRecord({ diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/components/RecordTableCellSoftFocusMode.tsx b/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/components/RecordTableCellSoftFocusMode.tsx index b40114986..d9c47b826 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/components/RecordTableCellSoftFocusMode.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/components/RecordTableCellSoftFocusMode.tsx @@ -10,7 +10,6 @@ import { useIsFieldEmpty } from '@/object-record/record-field/hooks/useIsFieldEm import { useIsFieldInputOnly } from '@/object-record/record-field/hooks/useIsFieldInputOnly'; import { useToggleEditOnlyInput } from '@/object-record/record-field/hooks/useToggleEditOnlyInput'; import { RecordTableCellContext } from '@/object-record/record-table/contexts/RecordTableCellContext'; -import { RecordTableRowContext } from '@/object-record/record-table/contexts/RecordTableRowContext'; import { useCloseCurrentTableCellInEditMode } from '@/object-record/record-table/hooks/internal/useCloseCurrentTableCellInEditMode'; import { RecordTableCellButton } from '@/object-record/record-table/record-table-cell/components/RecordTableCellButton'; import { useOpenRecordTableCellFromCell } from '@/object-record/record-table/record-table-cell/hooks/useOpenRecordTableCellFromCell'; @@ -22,7 +21,7 @@ import { isDefined } from '~/utils/isDefined'; import { TableHotkeyScope } from '../../types/TableHotkeyScope'; -import { useIsFieldReadOnly } from '@/object-record/record-field/hooks/useIsFieldReadOnly'; +import { useIsFieldValueReadOnly } from '@/object-record/record-field/hooks/useIsFieldValueReadOnly'; import { RecordTableCellDisplayContainer } from './RecordTableCellDisplayContainer'; type RecordTableCellSoftFocusModeProps = { @@ -36,11 +35,8 @@ export const RecordTableCellSoftFocusMode = ({ }: RecordTableCellSoftFocusModeProps) => { const { columnIndex } = useContext(RecordTableCellContext); const closeCurrentTableCell = useCloseCurrentTableCellInEditMode(); - const { isReadOnly: isRowReadOnly } = useContext(RecordTableRowContext); - const isFieldReadOnly = useIsFieldReadOnly(); - - const isCellReadOnly = isFieldReadOnly || isRowReadOnly; + const isFieldReadOnly = useIsFieldValueReadOnly(); const { openTableCell } = useOpenRecordTableCellFromCell(); @@ -78,7 +74,7 @@ export const RecordTableCellSoftFocusMode = ({ useScopedHotkeys( Key.Enter, () => { - if (isCellReadOnly) { + if (isFieldReadOnly) { return; } @@ -95,7 +91,7 @@ export const RecordTableCellSoftFocusMode = ({ useScopedHotkeys( '*', (keyboardEvent) => { - if (isCellReadOnly) { + if (isFieldReadOnly) { return; } @@ -124,7 +120,7 @@ export const RecordTableCellSoftFocusMode = ({ ); const handleClick = () => { - if (!isFieldInputOnly && !isCellReadOnly) { + if (!isFieldInputOnly && !isFieldReadOnly) { openTableCell(); } }; @@ -156,9 +152,9 @@ export const RecordTableCellSoftFocusMode = ({ isDefined(buttonIcon) && !editModeContentOnly && (!isFirstColumn || !isEmpty) && - !isCellReadOnly; + !isFieldReadOnly; - const dontShowContent = isEmpty && isCellReadOnly; + const dontShowContent = isEmpty && isFieldReadOnly; return ( <> diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/__mocks__/cell.ts b/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/__mocks__/cell.ts index 9bbaeaec8..068a0ba42 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/__mocks__/cell.ts +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/__mocks__/cell.ts @@ -8,7 +8,6 @@ export const recordTableRow: RecordTableRowContextProps = { recordId: 'recordId', pathToShowPage: '/', objectNameSingular: 'objectNameSingular', - isReadOnly: false, dragHandleProps: {} as any, isDragging: false, inView: true, diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useOpenRecordTableCellFromCell.ts b/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useOpenRecordTableCellFromCell.ts index 002d638cf..67583b2f9 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useOpenRecordTableCellFromCell.ts +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useOpenRecordTableCellFromCell.ts @@ -1,7 +1,7 @@ import { useContext } from 'react'; import { FieldContext } from '@/object-record/record-field/contexts/FieldContext'; -import { useIsFieldReadOnly } from '@/object-record/record-field/hooks/useIsFieldReadOnly'; +import { useIsFieldValueReadOnly } from '@/object-record/record-field/hooks/useIsFieldValueReadOnly'; import { FieldDefinition } from '@/object-record/record-field/types/FieldDefinition'; import { FieldMetadata } from '@/object-record/record-field/types/FieldMetadata'; import { CellHotkeyScopeContext } from '@/object-record/record-table/contexts/CellHotkeyScopeContext'; @@ -32,11 +32,10 @@ export const useOpenRecordTableCellFromCell = () => { const cellPosition = useCurrentTableCellPosition(); const customCellHotkeyScope = useContext(CellHotkeyScopeContext); const { recordId, fieldDefinition } = useContext(FieldContext); - const { isReadOnly, pathToShowPage, objectNameSingular } = useContext( + const { pathToShowPage, objectNameSingular } = useContext( RecordTableRowContext, ); - const isFieldReadOnly = useIsFieldReadOnly(); - const cellIsReadOnly = isReadOnly || isFieldReadOnly; + const isFieldReadOnly = useIsFieldValueReadOnly(); const openTableCell = ( initialValue?: string, @@ -47,7 +46,7 @@ export const useOpenRecordTableCellFromCell = () => { customCellHotkeyScope, recordId, fieldDefinition, - isReadOnly: cellIsReadOnly, + isReadOnly: isFieldReadOnly, pathToShowPage, objectNameSingular, initialValue, diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-row/components/RecordTableRowWrapper.tsx b/packages/twenty-front/src/modules/object-record/record-table/record-table-row/components/RecordTableRowWrapper.tsx index 11d1ae1bb..6c7def1b0 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/record-table-row/components/RecordTableRowWrapper.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-row/components/RecordTableRowWrapper.tsx @@ -12,7 +12,7 @@ import { isRowSelectedComponentFamilyState } from '@/object-record/record-table/ import { tableCellWidthsComponentState } from '@/object-record/record-table/states/tableCellWidthsComponentState'; import { RecordTableWithWrappersScrollWrapperContext } from '@/ui/utilities/scroll/contexts/ScrollWrapperContexts'; import { useRecoilComponentFamilyValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentFamilyValueV2'; -import { useRecoilComponentStateV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentStateV2'; +import { useSetRecoilComponentStateV2 } from '@/ui/utilities/state/component-state/hooks/useSetRecoilComponentStateV2'; export const RecordTableRowWrapper = ({ recordId, @@ -48,7 +48,7 @@ export const RecordTableRowWrapper = ({ rootMargin: '1000px', }); - const [, setTableCellWidths] = useRecoilComponentStateV2( + const setTableCellWidths = useSetRecoilComponentStateV2( tableCellWidthsComponentState, ); @@ -108,7 +108,6 @@ export const RecordTableRowWrapper = ({ }) + recordId, objectNameSingular: objectMetadataItem.nameSingular, isSelected: currentRowSelected, - isReadOnly: objectMetadataItem.isRemote ?? false, isPendingRow, isDragging: draggableSnapshot.isDragging, dragHandleProps: draggableProvided.dragHandleProps, diff --git a/packages/twenty-front/src/testing/jest/JestRecordStoreSetter.tsx b/packages/twenty-front/src/testing/jest/JestRecordStoreSetter.tsx new file mode 100644 index 000000000..844b11d1c --- /dev/null +++ b/packages/twenty-front/src/testing/jest/JestRecordStoreSetter.tsx @@ -0,0 +1,20 @@ +import { ReactNode, useEffect } from 'react'; + +import { useUpsertRecordsInStore } from '@/object-record/record-store/hooks/useUpsertRecordsInStore'; +import { ObjectRecord } from '@/object-record/types/ObjectRecord'; + +export const JestRecordStoreSetter = ({ + children, + records, +}: { + children: ReactNode; + records: ObjectRecord[]; +}) => { + const { upsertRecords } = useUpsertRecordsInStore(); + + useEffect(() => { + upsertRecords(records); + }); + + return <>{children}; +};