Fixes Empty Label Identifer Preview in Settings/DataModel/Object/Edit

[Faisal-imtiyaz123]

fixes #6143

[greptile-apps]

PR Summary

This pull request addresses the issue of empty identifier previews in the settings data model by updating default values and adding new hooks and logic to ensure meaningful previews.

• Updated default values in packages/twenty-front/src/modules/settings/data-model/constants/SettingsFieldTypeConfigs.ts to provide clearer placeholder content.
• Passed fieldName prop in packages/twenty-front/src/modules/settings/data-model/fields/preview/components/SettingsDataModelFieldPreview.tsx to ensure correct field name usage.
• Added logic in packages/twenty-front/src/modules/settings/data-model/fields/preview/components/SettingsDataModelSetFieldValueEffect.tsx to check for upserted preview records.
• Introduced custom hooks in packages/twenty-front/src/modules/settings/data-model/fields/preview/hooks/previewRecordIdHooks.ts and packages/twenty-front/src/modules/settings/data-model/fields/preview/hooks/useUpsertedPreviewRecord.ts for managing preview record IDs.
• Wrapped form components with RecordFieldValueSelectorContextProvider in packages/twenty-front/src/pages/settings/data-model/SettingsObjectEdit.tsx for enhanced state management.

_{11 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings}

[FelixMalfait]

Hey @Faisal-imtiyaz123, I tried to review this but I couldn't figure out exactly what you did and why you did it.
We really value simple and elegant changes in the code base. Do you think you could do this fix without changing as many things, or explain why you went through this path in the PR description? Otherwise it's a bit hard to follow

[FelixMalfait]

Because you don't follow conventions very well (like naming the hook "[previewRecordIdHooks.ts]") then it makes the reviewer suspicious about the rest of the code, which might indeed be the best solution possible, but in that case it's best to provide context and explain why you had to make those choices

[FelixMalfait]

I will close the PR for now to keep the PR backlog clean but please feel free to raise a new one with a clean description and the most elegant solution possible without introducing un-necessary hooks or complex conditions. Thanks!

[Faisal-imtiyaz123]

@FelixMalfait There were couple of problems with this one. One was that FormProvider was not wrapped with RecordFieldValueSelectorContextProvider which caused nothing to show at all in Preview. Field. The other issue was with the form label selected. If u changed default form label let's say name to form label custom and did a refresh or navigated back , then when u come to this page again custom will appear as selected , but it is not because select component had no default value

[Faisal-imtiyaz123]

@FelixMalfait The hooks I made are to use the previewRecordId's. Not much code there . Just for refactoring I made them.

[Faisal-imtiyaz123]

@FelixMalfait If I raise I new PR I think I should only change previewRecordIdHooks file, by removing it and replacing it with two files usePreviewRecordId.ts and useSetPreviewRecordId.ts and create a folder state under preview folder to store the previewRecordIdState

[Faisal-imtiyaz123]

@FelixMalfait I would request you to please give it another look according to what I explained. I put quite some work in it.

[FelixMalfait]

@Faisal-imtiyaz123

[Faisal-imtiyaz123]

@FelixMalfait Thanks for reopening it. Do u want me to make a new PR with the refactoring of PreviewRecordIdHooks as I explained in this comment --->>>

@FelixMalfait If I raise I new PR I think I should only change previewRecordIdHooks file, by removing it and replacing it with two files usePreviewRecordId.ts and useSetPreviewRecordId.ts and create a folder state under preview folder to store the previewRecordIdState

Or Are you currently looking into it ? Please let me know.

[FelixMalfait]

Sorry I should not have closed it. I think you are a great problem solver, but since your code is sometimes not the cleanest it's easy to judge quickly and dismiss it. I should have spent more time on it. We'll merge it like this!

[FelixMalfait]

@lucasbordeau when you come back I'd appreciate if you can have a look at this merged PR

[github-actions]

Thanks @Faisal-imtiyaz123 for your contribution!
This marks your 0th PR on the repo. You're top 0% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

[Faisal-imtiyaz123]

@FelixMalfait I am deeply thankful to you for reopening it again and spending time to review it . Thanks for believing in me. I am working everyday to improve my code quality.

[greptile-apps]

PR Summary

(updates since last review)

This pull request addresses the issue of empty identifier previews in the Object settings by updating various components, hooks, and utilities to ensure meaningful previews are displayed.

• Updated package.json: Patched typeorm dependency to use a local patch file (./packages/twenty-server/patches/typeorm+0.3.20.patch).
• Modified GraphQL Enum: Renamed MessageChannelSyncStatusEnum.Completed to Active in packages/twenty-chrome-extension/src/generated/graphql.tsx.
• Simplified Image URI Handling: Replaced getImageAbsoluteURIOrBase64 with getImageAbsoluteURI in multiple files, including packages/twenty-emails/src/emails/send-invite-link.email.tsx.
• Refactored State Management: Removed activityIdInDrawerState and related states, simplifying state management across various components.
• Enhanced Field Handling: Added support for new field types (RichText, Actor) and updated hooks and components to handle these types correctly.

These changes ensure that the identifier preview functionality is improved, providing meaningful content and better state management across the application.

_{293 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings}