fix: when field metadata SELECT type is edited update view groups #8344
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR adds functionality to update view groups when SELECT type field metadata is edited, ensuring synchronization between field changes and their associated views.
- Added
FieldMetadataRelatedRecordsServicein/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata-related-records.service.tsto manage view group updates - Added
useObjectMetadataViewshook in/packages/twenty-front/src/modules/views/hooks/useObjectMetadataViews.tsto fetch view metadata with network-only policy - Added
refetchfunction touseFindManyRecordshook return value for manual query refreshes - Added view group update logic in
field-metadata.service.tsto handle SELECT field modifications - Added type guard utility
isSelectFieldMetadataTypefor type-safe SELECT field checks
8 file(s) reviewed, 10 comment(s)
Edit PR Review Bot Settings | Greptile
| @@ -171,6 +171,8 @@ export const RecordIndexContainer = () => { | |||
| return; | |||
| } | |||
|
|
|||
| console.log('CURRENT VIEW CHANGE !'); | |||
There was a problem hiding this comment.
style: Remove debug console.log before merging to production
| if (!objectMetadataId) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
logic: Early return should return null or empty array instead of undefined for consistent return types
| return await apolloClient.query({ | ||
| query: findManyRecordsQuery, | ||
| variables: { | ||
| filter: { | ||
| objectMetadataId: { | ||
| eq: objectMetadataId, | ||
| }, | ||
| }, | ||
| }, | ||
| fetchPolicy: 'network-only', | ||
| }); |
There was a problem hiding this comment.
style: Consider adding error handling for failed queries
| @@ -157,6 +162,8 @@ export const SettingsObjectFieldEdit = () => { | |||
| }); | |||
| } | |||
|
|
|||
| fetchObjectMetadataViews(); | |||
|
|
|||
| navigate(`/settings/objects/${objectSlug}`); | |||
|
|
|||
| refetchRecords(); | |||
There was a problem hiding this comment.
logic: refetchRecords is called after navigation, which could lead to a race condition or unnecessary fetch if component unmounts
| @@ -157,6 +162,8 @@ export const SettingsObjectFieldEdit = () => { | |||
| }); | |||
| } | |||
|
|
|||
| fetchObjectMetadataViews(); | |||
There was a problem hiding this comment.
logic: fetchObjectMetadataViews result is not awaited, which could lead to stale data if navigation happens too quickly
| console.log('updatedFieldMetadata', updatedFieldMetadata); | ||
| if ( |
There was a problem hiding this comment.
style: Remove console.log statements before merging to production
| console.log('updateRelatedViewGroups'); | ||
| await this.fieldMetadataRelatedRecordsService.updateRelatedViewGroups( | ||
| existingFieldMetadata, | ||
| updatedFieldMetadata, | ||
| ); | ||
| } |
There was a problem hiding this comment.
style: Consider wrapping updateRelatedViewGroups in try/catch to handle potential errors gracefully
| return; | ||
| } | ||
|
|
||
| const view = await this.getFieldMetadataView(newFieldMetadata); |
There was a problem hiding this comment.
style: Consider wrapping all database operations in a transaction to ensure atomicity
| if (!viewGroup) { | ||
| throw new Error('View group not found'); | ||
| } |
There was a problem hiding this comment.
style: Error should include more context like fieldMetadataId and fieldValue to help debugging
| return await viewRepository.findOneOrFail({ | ||
| where: { | ||
| kanbanFieldMetadataId: fieldMetadata.id, | ||
| }, | ||
| relations: ['viewGroups'], | ||
| }); |
There was a problem hiding this comment.
logic: Add error handling if view is not found for the given kanbanFieldMetadataId
| @@ -311,7 +314,7 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit | |||
| FieldMetadataEntity, | |||
| ); | |||
|
|
|||
| const [existingFieldMetadata] = await fieldMetadataRepository.find({ | |||
| const existingFieldMetadata = await fieldMetadataRepository.findOne({ | |||
There was a problem hiding this comment.
nope this is not equivalent :p typeORM makes two queries for a findOne and one for find!
No description provided.