-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[REFACTOR][FRONT]: Remove objectMetadata and fieldMetadata sluggification #9441
Conversation
object
field
sluggification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR implements a significant refactoring to remove slugification (kebab-case formatting) across the codebase, directly using object and field names instead. Here are the key changes:
- Removed
getObjectSlug.ts
andgetFieldSlug.ts
utility files and their associated tests - Replaced
findObjectMetadataItemBySlug
withfindObjectMetadataItemByNamePlural
in metadata hooks - Updated URL construction to use raw
objectMetadataItem.namePlural
andfieldMetadataItem.name
values - Modified test files to reflect new naming conventions and removed slug-related tests
- Renamed methods and variables across components to align with the new direct naming approach
The main risk is potential dead links for users who bookmarked URLs using the old kebab-case format, though this has been acknowledged as a controlled risk by the team.
💡 (2/5) Greptile learns from your feedback when you react with 👍/👎!
19 file(s) reviewed, 15 comment(s)
Edit PR Review Bot Settings | Greptile
packages/twenty-front/src/modules/object-metadata/utils/__tests__/getFieldSlug.test.ts
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/navigation/hooks/useLastVisitedObjectMetadataItem.ts
Show resolved
Hide resolved
packages/twenty-front/src/modules/object-metadata/utils/getObjectSlug.ts
Outdated
Show resolved
Hide resolved
...ct-record/record-table/record-table-header/components/RecordTableHeaderPlusButtonContent.tsx
Show resolved
Hide resolved
...ct-record/record-table/record-table-header/components/RecordTableHeaderPlusButtonContent.tsx
Show resolved
Hide resolved
...enty-front/src/modules/settings/data-model/object-details/components/tabs/ObjectSettings.tsx
Show resolved
Hide resolved
packages/twenty-front/src/modules/views/view-picker/hooks/useGetAvailableFieldsForKanban.ts
Show resolved
Hide resolved
packages/twenty-front/src/modules/object-metadata/utils/getFieldSlug.ts
Outdated
Show resolved
Hide resolved
...-front/src/pages/settings/data-model/SettingsObjectNewField/SettingsObjectNewFieldSelect.tsx
Outdated
Show resolved
Hide resolved
object
field
sluggificationThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
This PR continues the refactoring work to remove slugification, with significant changes to URL handling and state management. Here are the key updates:
- Replaced
updatedObjectSlugState
with newupdatedObjectNamePluralState
for managing raw object names - Updated dropdown components to use
objectNamePlural
instead ofobjectSlug
in URL construction - Modified settings page components to handle raw field names and object names in navigation
- Changed route parameters from
:objectSlug/:fieldSlug
to:objectNamePlural/:fieldName
across stories and tests - Removed kebab-case formatting from all URL paths in
SettingsPath
enum
The changes maintain consistency with the new direct naming approach while acknowledging the controlled risk of breaking existing bookmarked URLs.
18 file(s) reviewed, 12 comment(s)
Edit PR Review Bot Settings | Greptile
...bject-record/object-options-dropdown/components/ObjectOptionsDropdownHiddenFieldsContent.tsx
Show resolved
Hide resolved
...record/object-options-dropdown/components/ObjectOptionsDropdownHiddenRecordGroupsContent.tsx
Show resolved
Hide resolved
...-record/object-options-dropdown/components/ObjectOptionsDropdownRecordGroupFieldsContent.tsx
Show resolved
Hide resolved
packages/twenty-front/src/pages/settings/data-model/SettingsObjectDetailPage.tsx
Show resolved
Hide resolved
...ont/src/pages/settings/data-model/SettingsObjectNewField/SettingsObjectNewFieldConfigure.tsx
Show resolved
Hide resolved
...ages/twenty-front/src/pages/settings/data-model/__stories__/SettingsObjectDetail.stories.tsx
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/pages/settings/data-model/states/updatedObjectNamePluralState.ts
Show resolved
Hide resolved
packages/twenty-front/src/pages/settings/data-model/states/updatedObjectSlugState.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice refactor
...rc/modules/settings/data-model/graph-overview/components/SettingsDataModelOverviewObject.tsx
Show resolved
Hide resolved
...enty-front/src/modules/settings/data-model/object-details/components/tabs/ObjectSettings.tsx
Outdated
Show resolved
Hide resolved
Log
|
Introduction
For motivations and context please have a look to #9394 whom this PR results from.
In this pull-request we remove any
metadataField
andobjectMetadata
sluggification. We directly consumeobjectMetadata.namePlural
andmetadataField.name
, it seems like that historically the consumedmetadataField.name
are we sure that we wanna change this behavior ?Notes
Unless I'm mistaken by reverting the
kebabcase
url formatting we might be creating deadlinks that user could have save beforehand => Discussed with Charles said it's controlled risk.