ability to have null and not null operators in filter & case when op#1703
Conversation
|
Caution Review failedPull request was closed or merged during review WalkthroughAdds IS NULL and IS NOT NULL operators to two form components. When a null-check operator is selected, operand UI controls are hidden and the form emits empty/null operands; non-null operators retain existing operand handling and validation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/components/TransformWorkflow/FlowEditor/Components/OperationPanel/Forms/CaseWhenOpForm.tsx`:
- Around line 393-399: The current logic sets operands to [] for null operators
which causes useFieldArray to have zero rows and prevents adding operands when
switching to non-null operators; change the initialization in CaseWhenOpForm so
that when clause.operator is a null-check ('IS NULL'/'IS NOT NULL') you still
provide a minimal operand array (e.g., reuse clause.operands if present or
supply a single empty GenericOperand placeholder) instead of an empty array;
update the operands mapping where operands: ... is created (referencing
clause.operator and clause.operands) so the field array always has at least one
operand row to render and edit when the operator changes.
- Around line 117-118: The compile error arises because data.logicalOp?.id is
string | undefined and is passed to Array.prototype.includes; fix by narrowing
the type before calling includes—add a small helper (e.g., function
isNullCheckOp(id?: string): boolean) in CaseWhenOpForm that returns id !==
undefined && ['IS NULL', 'IS NOT NULL'].includes(id), then replace occurrences
of ['IS NULL', 'IS NOT NULL'].includes(data.logicalOp?.id) with
isNullCheckOp(data.logicalOp?.id); ensure both spots (the usage in the
render/logic and the other occurrence) call the helper so includes always
receives a string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9db4b710-2c9a-4cb2-bc4d-5f4dafccb03b
📒 Files selected for processing (2)
src/components/TransformWorkflow/FlowEditor/Components/OperationPanel/Forms/CaseWhenOpForm.tsxsrc/components/TransformWorkflow/FlowEditor/Components/OperationPanel/Forms/WhereFilterOpForm.tsx
| operands: ['IS NULL', 'IS NOT NULL'].includes(clause.operator) | ||
| ? [] // Empty operands for null operators | ||
| : clause.operands.map((op: GenericOperand) => ({ | ||
| type: op.is_col ? 'col' : 'val', | ||
| col_val: op.is_col ? op.value : '', | ||
| const_val: !op.is_col ? op.value : '', | ||
| })), |
There was a problem hiding this comment.
Prefilling operands: [] can block editing from null-op to non-null-op.
In edit mode, initializing null operators with an empty operand array can leave useFieldArray with zero operand rows; switching to =/between then has no operand controls to populate.
Suggested fix
- operands: ['IS NULL', 'IS NOT NULL'].includes(clause.operator)
- ? [] // Empty operands for null operators
+ operands: ['IS NULL', 'IS NOT NULL'].includes(clause.operator)
+ ? [
+ { type: 'val', col_val: '', const_val: '' },
+ { type: 'val', col_val: '', const_val: '' },
+ ]
: clause.operands.map((op: GenericOperand) => ({
type: op.is_col ? 'col' : 'val',
col_val: op.is_col ? op.value : '',
const_val: !op.is_col ? op.value : '',
})),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/components/TransformWorkflow/FlowEditor/Components/OperationPanel/Forms/CaseWhenOpForm.tsx`
around lines 393 - 399, The current logic sets operands to [] for null operators
which causes useFieldArray to have zero rows and prevents adding operands when
switching to non-null operators; change the initialization in CaseWhenOpForm so
that when clause.operator is a null-check ('IS NULL'/'IS NOT NULL') you still
provide a minimal operand array (e.g., reuse clause.operands if present or
supply a single empty GenericOperand placeholder) instead of an empty array;
update the operands mapping where operands: ... is created (referencing
clause.operator and clause.operands) so the field array always has at least one
operand row to render and edit when the operator changes.
| is_col: op.type === 'col', | ||
| })) | ||
| .slice(0, clause.logicalOp?.id === 'between' ? 2 : 1), | ||
| operands: ['IS NULL', 'IS NOT NULL'].includes(clause.logicalOp?.id) |
There was a problem hiding this comment.
This shows some type script error can you check.
Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
Type 'undefined' is not assignable to type 'string'.ts(2345)
(property) logicalOp: {
id: string;
label: string;
} | null
| .slice(0, data.logicalOp?.id === 'between' ? 2 : 1) | ||
| .slice( | ||
| 0, | ||
| ['IS NULL', 'IS NOT NULL'].includes(data.logicalOp?.id) |
There was a problem hiding this comment.
This has a typescript error,
Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
Type 'undefined' is not assignable to type 'string'.ts(2345)
(property) logicalOp: {
id: string;
label: string;
} | null
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1703 +/- ##
==========================================
- Coverage 75.36% 75.32% -0.05%
==========================================
Files 107 107
Lines 7973 7991 +18
Branches 1929 1946 +17
==========================================
+ Hits 6009 6019 +10
- Misses 1840 1934 +94
+ Partials 124 38 -86 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
Summary by CodeRabbit