-
Notifications
You must be signed in to change notification settings - Fork 4.6k
fix: use value from filter if recordUUID is unavailable #15762
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
base: main
Are you sure you want to change the base?
Conversation
Greptile OverviewGreptile SummaryThis PR fixes a bug where filtering records by UUID using the "Is" operator in workflow Search Records actions was ignored, returning all records instead of the matching one. Key Changes:
Impact:
Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant WF as Workflow Engine
participant CF as computeRecordGqlOperationFilter
participant TF as turnRecordFilterIntoRecordGqlOperationFilter
participant DB as Database
WF->>CF: Filter by UUID with "Is" operator
Note over WF,CF: recordFilter.value = "uuid-string"<br/>recordIdsForUuid = undefined
CF->>TF: Process filter
Note over CF,TF: No recordIdsForUuid provided
TF->>TF: Check UUID case
TF->>TF: recordIds = undefined
TF->>TF: Extract uuidValue from recordFilter.value
alt recordIds exists and has values
TF->>DB: { id: { in: recordIds } }
else uuidValue exists and is non-empty (NEW)
TF->>DB: { id: { eq: uuidValue } }
Note over TF,DB: This path is the fix
else neither exists
TF-->>CF: undefined (no filter)
end
DB-->>WF: Filtered records
|
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.
1 file reviewed, 2 comments
| }; | ||
| } | ||
|
|
||
| if (isDefined(uuidValue) && uuidValue.trim() !== '') { |
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: Use isNonEmptyString utility instead of manual validation.
| if (isDefined(uuidValue) && uuidValue.trim() !== '') { | |
| if (isNonEmptyString(uuidValue)) { |
Context Used: Context from dashboard - Use the 'isNonEmptyString' utility function to check for non-empty strings instead of checking for e... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-shared/src/utils/filter/turnRecordFilterIntoGqlOperationFilter.ts
Line: 1320:1320
Comment:
**style:** Use `isNonEmptyString` utility instead of manual validation.
```suggestion
if (isNonEmptyString(uuidValue)) {
```
**Context Used:** Context from `dashboard` - Use the 'isNonEmptyString' utility function to check for non-empty strings instead of checking for e... ([source](https://app.greptile.com/review/custom-context?memory=1553f120-8908-4962-bc22-796896959d06))
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| import { type DateTimeFilter } from '@/types/RecordGqlOperationFilter'; | ||
| import { | ||
| CustomError, |
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: Import order change for CustomError is inconsistent with alphabetical ordering.
| CustomError, | |
| checkIfShouldComputeEmptinessFilter, | |
| checkIfShouldSkipFiltering, | |
| CustomError, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-shared/src/utils/filter/turnRecordFilterIntoGqlOperationFilter.ts
Line: 48:48
Comment:
**style:** Import order change for `CustomError` is inconsistent with alphabetical ordering.
```suggestion
checkIfShouldComputeEmptinessFilter,
checkIfShouldSkipFiltering,
CustomError,
```
How can I resolve this? If you propose a fix, please make it concise.
Closes #15067
When filtering by a record's
idusing the "Is" operator in a workflow'sSearch Recordsaction, the filter was ignored. The action returned all records for that object type instead of the single record matching theid.Root cause
Inside
computeRecordGqlOperationFilter.ts, all filters are iterated over and passed on toturnRecordFilterIntoRecordGqlOperationFilter. InturnRecordFilterIntoRecordGqlOperationFilter, in the case ofuuid,recordIdsForUuidis required. But since it is undefined, it is early returned, causing the filter to be dropped.Solution
In the case where
recordIdsForUuid, there is an additional check:If
uuidValueis valid, theneqis used withuuidValueand returned.Another potential solution could be to pass
recordIdsForUuidas a singular array for each iteration incomputeRecordGqlOperationFilter.Screenshot
Note
gqlOperationFilter