-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Record filter greaterThan
becomes inclusive as lowerThan
#12786
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
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
Modified rating filter behavior in packages/twenty-front/src/modules/object-record/object-filter-dropdown/components/ObjectFilterDropdownRatingInput.tsx
to fix issues with 'greater than' comparisons in the 5-star rating system.
- Changed greater than filter logic to be inclusive (>= instead of >) to prevent errors when filtering 5-star ratings
- Simplified function syntax by converting blocks to single-line arrow functions for better maintainability
- Updated
convertGreaterThanRatingToArrayOfRatingValues
to ensure proper filtering of maximum rating values
1 file reviewed, no comments
Edit PR Review Bot Settings | Greptile
7512b29
to
16fd872
Compare
@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
Fixed a critical bug in rating filter functionality by changing from array index-based comparison to actual numeric rating values, ensuring accurate filtering behavior.
- Modified
convertFieldRatingValueToNumber
to extract actual numeric values from rating strings (e.g., '3' from 'RATING_3') instead of using array indices - Changed comparison logic in
convertGreaterThanRatingToArrayOfRatingValues
to use actual rating values for more accurate filtering - Fixed an edge case where rating value 5 wasn't being handled correctly due to zero-based array indexing
1 file reviewed, no comments
Edit PR Review Bot Settings | Greptile
greaterThan
becomes inclusive as lowerThan
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:25649 This environment will automatically shut down when the PR is closed or after 5 hours. |
@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
Refactored rating value handling by removing dependency on RATING_VALUES array order and implementing more robust type validation and filtering.
- Switched RATING_VALUES from static array to dynamically generated values using Array.from() in
record-field/meta-types/constants/RatingValues.ts
- Added new type guard
isFieldRatingValue
replacing Zod schema with simpler Array.includes() check - Updated FieldMetadata types to use template literal type (
RATING_${number}
| null) instead of array-based validation - Improved spreadsheet validation by extracting rating field validation into dedicated guard function
4 files reviewed, 3 comments
Edit PR Review Bot Settings | Greptile
export const RATING_VALUES = Array.from<unknown, NonNullable<FieldRatingValue>>( | ||
{ length: 5 }, | ||
(_value, index) => `RATING_${index + 1}`, | ||
); |
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.
style: Consider extracting '5' to a named constant like MAX_RATING
to make the rating scale more maintainable
packages/twenty-front/src/modules/object-record/record-field/types/FieldMetadata.ts
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/object-record/record-field/types/guards/isFieldRatingValue.ts
Show resolved
Hide resolved
54b620e
to
cf68dbd
Compare
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.
LGTM
Introduction
Greater than filtering wasn't inclusive whereas lower than was, resulting in sending empty array to filtering resolver
Also refactored the transpilation methods to avoid asserting on the
RATING_VALUES
ordercloses #12779