Skip to content

Commit e079fa4

Browse files
waleedlatif1waleedlatif1
committed
fix(tables): drop nested same-column conditions when filtering by cell value
1 parent 2201895 commit e079fa4

2 files changed

Lines changed: 60 additions & 16 deletions

File tree

‎apps/sim/lib/table/query-builder/__tests__/cell-filter.test.ts‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,42 @@ describe('withCellValueFilter', () => {
130130
expect(withCellValueFilter(current, [eqA])).toEqual({ all: [eqA] })
131131
})
132132

133-
it('narrows an any-group without reaching inside it', () => {
133+
it('keeps an any-group that does not touch the column', () => {
134134
const current: TablePredicate = {
135-
any: [{ all: [{ field: 'col_a', op: 'eq', value: 'old' }] }],
135+
any: [{ all: [{ field: 'col_b', op: 'eq', value: 'x' }] }],
136136
}
137137
expect(withCellValueFilter(current, [eqA])).toEqual({ all: [current, eqA] })
138138
})
139+
140+
// Reachable from the panel: an `or` rule produces an `any` group, and a cell
141+
// filter on a column inside it would otherwise AND against the disjunction
142+
// and empty the table.
143+
it('drops a nested group that constrains the same column', () => {
144+
const current: TablePredicate = {
145+
any: [
146+
{ all: [{ field: 'col_a', op: 'eq', value: 'old' }] },
147+
{ all: [{ field: 'col_b', op: 'eq', value: 'keep' }] },
148+
],
149+
}
150+
expect(withCellValueFilter(current, [eqA])).toEqual({ all: [eqA] })
151+
})
152+
153+
it('drops a same-column leaf nested inside an all-group', () => {
154+
const current: TablePredicate = {
155+
all: [
156+
{ all: [{ field: 'col_a', op: 'eq', value: 'old' }] },
157+
{ field: 'col_b', op: 'eq', value: 'keep' },
158+
],
159+
}
160+
expect(withCellValueFilter(current, [eqA])).toEqual({
161+
all: [{ field: 'col_b', op: 'eq', value: 'keep' }, eqA],
162+
})
163+
})
164+
165+
// An `{ all: [] }` group is not a valid predicate — the server rejects it.
166+
it('leaves the filter untouched when there are no conditions', () => {
167+
const current: TablePredicate = { all: [{ field: 'col_a', op: 'eq', value: 'x' }] }
168+
expect(withCellValueFilter(current, [])).toBe(current)
169+
expect(withCellValueFilter(null, [])).toBeNull()
170+
})
139171
})

‎apps/sim/lib/table/query-builder/cell-filter.ts‎

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import type {
1717
FilterOp,
1818
JsonValue,
1919
Predicate,
20+
PredicateNode,
2021
TablePredicate,
2122
} from '@/lib/table/types'
2223

@@ -70,26 +71,37 @@ export function cellValueFilterConditions(
7071
return [{ field, op: 'eq', value: value as JsonValue }]
7172
}
7273

74+
/** True when any leaf anywhere under `node` filters on `field`. */
75+
function mentionsField(node: PredicateNode, field: string): boolean {
76+
if ('field' in node) return node.field === field
77+
const members = 'all' in node ? node.all : node.any
78+
return members.some((member) => mentionsField(member, field))
79+
}
80+
7381
/**
7482
* Narrows `current` with one cell's conditions.
7583
*
76-
* Existing top-level conditions on the same column are dropped first, so
77-
* filtering twice on one column swaps the value instead of ANDing two
78-
* equalities into a guaranteed-empty result. Conditions on other columns are
79-
* kept — the action narrows what the user is looking at rather than replacing
80-
* it.
84+
* Anything already constraining this column is dropped first, so filtering
85+
* twice on one column swaps the value instead of ANDing two conditions the
86+
* same row cannot satisfy — which would empty the table the user is looking at
87+
* and give them no clue why. Conditions on other columns are kept: the action
88+
* narrows the current view rather than replacing it.
89+
*
90+
* A whole nested group is dropped when it mentions the column ANYWHERE, not
91+
* just its top-level leaves. Reaching inside an `any` group to pull one leaf
92+
* out would silently WIDEN the user's disjunction — dropping the group loses
93+
* the other columns it mentioned, but it is visible in the panel afterwards
94+
* and never contradicts what was just asked for.
8195
*/
8296
export function withCellValueFilter(
8397
current: TablePredicate | null,
8498
conditions: readonly Predicate[]
85-
): TablePredicate {
99+
): TablePredicate | null {
100+
// Nothing to add leaves the filter exactly as it was — an `{ all: [] }` group
101+
// is not a valid predicate and the server rejects it.
102+
const field = conditions[0]?.field
103+
if (field === undefined) return current
86104
if (!current) return { all: [...conditions] }
87-
if ('all' in current) {
88-
const field = conditions[0]?.field
89-
const kept = current.all.filter((node) => !('field' in node && node.field === field))
90-
return { all: [...kept, ...conditions] }
91-
}
92-
// An `any` group is a whole disjunction — narrowing it means AND-ing the new
93-
// conditions onto the group, not reaching inside it.
94-
return { all: [current, ...conditions] }
105+
const members = 'all' in current ? current.all : [current]
106+
return { all: [...members.filter((node) => !mentionsField(node, field)), ...conditions] }
95107
}

0 commit comments

Comments
 (0)