-
Notifications
You must be signed in to change notification settings - Fork 3.6k
getLabelIdentifierFieldValue
should always return string
#12772
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
Conversation
getLabelIdentifierFieldValue
should always return string
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
Fixed a bug in packages/twenty-front/src/modules/object-metadata/utils/getLabelIdentifierFieldValue.ts
where non-string values were causing a 't.trim is not a function' error when opening custom object settings.
- Modified
getLabelIdentifierFieldValue
to safely convert values to strings using template literals, preventing runtime errors with non-string types - Using template literals (
${value}
) for type coercion could mask underlying type issues that should be handled explicitly - Consider adding type guards or explicit type handling for null/undefined values instead of relying on implicit conversion
1 file reviewed, 1 comment
Edit PR Review Bot Settings | Greptile
packages/twenty-front/src/modules/object-metadata/utils/getLabelIdentifierFieldValue.ts
Outdated
Show resolved
Hide resolved
@greptileai trigger |
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
Improves the handling of label identifier field values in the metadata settings by ensuring proper string conversion for numeric fields.
- Consider replacing template literal string conversion (
${value}
) with more explicit methods likeString(value)
or.toString()
for better type clarity - Add JSDoc documentation specifying the expected return type and parameter constraints for
getLabelIdentifierFieldValue
- Consider adding unit tests specifically for numeric field identifier cases to prevent regression
- The
ObjectRecord
type usingany
for unknown fields could be improved with a generic type parameter to better handle field value types
1 file reviewed, no comments
Edit PR Review Bot Settings | Greptile
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:23525 This environment will automatically shut down when the PR is closed or after 5 hours. |
Introduction
For a custom object if the selected identifier field metadata is an number type than it wouldn't get be converted to a string
#closes #12717
Concerns
Kinda the same than for #12728
Here ObjectRecord unknown fields are typed as any, we might wanna do a poc in order to migrate to
unknown
usage