mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 09:25:06 +03:00
fix(FilterBar): prevent column named 'group' from creating nested filter group (#46849)
The 'New Group' action used value `'group'` which collided with any
column named `'group'`. Changed to unique sentinel `'__new_group__'`.
## How to test
Create a table with the column Group
```
create table public.test_group_filter (
id bigint generated always as identity primary key,
name text,
"group" text
);
insert into public.test_group_filter (name, "group")
values
('Alice', 'admin'),
('Bob', 'member'),
('Charlie', 'guest');
```
Try to filter on Group, it should work
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Resolved a naming collision in FilterBar where properties named
"group" conflicted with the group creation action, preventing users from
accurately filtering by properties with this name.
* **Tests**
* Added comprehensive test coverage for FilterBar's property selection
and group creation features, verifying correct handling of edge cases
and reserved property names.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
1 parent
f402b4d464
commit
715f003cdd
5 files changed
+129
-4
No files matched your search
@@ -521,4 +521,69 @@ describe('FilterBar', () => {
|
||||
expect(screen.getByDisplayValue('active')).toBeInTheDocument()
|
||||
expect(screen.queryByText('AND')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('selects a column named "group" without creating a nested group', async () => {
|
||||
const user = userEvent.setup()
|
||||
const propertiesWithGroup: FilterProperty[] = [
|
||||
{
|
||||
label: 'group',
|
||||
name: 'group',
|
||||
type: 'string',
|
||||
operators: ['=', '!=', 'CONTAINS'],
|
||||
},
|
||||
{
|
||||
label: 'Name',
|
||||
name: 'name',
|
||||
type: 'string',
|
||||
operators: ['=', '!='],
|
||||
},
|
||||
]
|
||||
|
||||
let currentFilters: FilterGroup = initialFilters
|
||||
const handleFilterChange = vi.fn((filters) => {
|
||||
currentFilters = filters
|
||||
})
|
||||
|
||||
const { rerender } = render(
|
||||
<FilterBar
|
||||
filterProperties={propertiesWithGroup}
|
||||
filters={currentFilters}
|
||||
onFilterChange={handleFilterChange}
|
||||
freeformText=""
|
||||
onFreeformTextChange={mockOnFreeformTextChange}
|
||||
/>
|
||||
)
|
||||
|
||||
const freeform = screen.getByPlaceholderText('Filter by group, Name')
|
||||
await user.click(freeform)
|
||||
|
||||
expect(await screen.findByText('group')).toBeInTheDocument()
|
||||
|
||||
await user.click(screen.getByText('group'))
|
||||
|
||||
await waitFor(() => {
|
||||
expect(handleFilterChange).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
rerender(
|
||||
<FilterBar
|
||||
filterProperties={propertiesWithGroup}
|
||||
filters={currentFilters}
|
||||
onFilterChange={handleFilterChange}
|
||||
freeformText=""
|
||||
onFreeformTextChange={mockOnFreeformTextChange}
|
||||
/>
|
||||
)
|
||||
|
||||
// Should create a filter condition for the "group" column, NOT a nested filter group
|
||||
expect(currentFilters.conditions).toHaveLength(1)
|
||||
const condition = currentFilters.conditions[0]
|
||||
expect('logicalOperator' in condition).toBe(false)
|
||||
expect((condition as { propertyName: string }).propertyName).toBe('group')
|
||||
|
||||
// Should show the operator input for the "group" column
|
||||
await waitFor(() => {
|
||||
expect(screen.getByLabelText('Operator for group')).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { buildOperatorItems, buildValueItems } from './menuItems'
|
||||
import { buildOperatorItems, buildPropertyItems, buildValueItems } from './menuItems'
|
||||
import { FilterGroup, FilterProperty } from './types'
|
||||
|
||||
const stringProperty: FilterProperty = {
|
||||
@@ -146,3 +146,63 @@ describe('buildValueItems', () => {
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
describe('buildPropertyItems', () => {
|
||||
const properties: FilterProperty[] = [
|
||||
{ label: 'Name', name: 'name', type: 'string', operators: ['='] },
|
||||
{ label: 'Status', name: 'status', type: 'string', operators: ['='] },
|
||||
]
|
||||
|
||||
it('returns property items matching the input', () => {
|
||||
const items = buildPropertyItems({
|
||||
filterProperties: properties,
|
||||
inputValue: 'nam',
|
||||
})
|
||||
|
||||
expect(items).toEqual([{ value: 'name', label: 'Name' }])
|
||||
})
|
||||
|
||||
it('uses a sentinel value for the "New Group" action when supportsOperators is true', () => {
|
||||
const items = buildPropertyItems({
|
||||
filterProperties: properties,
|
||||
inputValue: '',
|
||||
supportsOperators: true,
|
||||
})
|
||||
|
||||
const groupItem = items.find((item) => item.label === 'New Group')
|
||||
expect(groupItem).toBeDefined()
|
||||
expect(groupItem?.value).toBe('__new_group__')
|
||||
})
|
||||
|
||||
it('does not include "New Group" when supportsOperators is false', () => {
|
||||
const items = buildPropertyItems({
|
||||
filterProperties: properties,
|
||||
inputValue: '',
|
||||
supportsOperators: false,
|
||||
})
|
||||
|
||||
expect(items.find((item) => item.label === 'New Group')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('does not collide with a property named "group"', () => {
|
||||
const propertiesWithGroup: FilterProperty[] = [
|
||||
{ label: 'group', name: 'group', type: 'string', operators: ['='] },
|
||||
{ label: 'Name', name: 'name', type: 'string', operators: ['='] },
|
||||
]
|
||||
|
||||
const items = buildPropertyItems({
|
||||
filterProperties: propertiesWithGroup,
|
||||
inputValue: '',
|
||||
supportsOperators: true,
|
||||
})
|
||||
|
||||
const groupPropertyItem = items.find((item) => item.label === 'group')
|
||||
const newGroupItem = items.find((item) => item.label === 'New Group')
|
||||
|
||||
expect(groupPropertyItem).toBeDefined()
|
||||
expect(groupPropertyItem?.value).toBe('group')
|
||||
expect(newGroupItem).toBeDefined()
|
||||
expect(newGroupItem?.value).toBe('__new_group__')
|
||||
expect(groupPropertyItem?.value).not.toBe(newGroupItem?.value)
|
||||
})
|
||||
})
|
||||
@@ -94,7 +94,7 @@ export function buildPropertyItems(params: {
|
||||
)
|
||||
|
||||
if (supportsOperators) {
|
||||
items.push({ value: 'group', label: 'New Group' })
|
||||
items.push({ value: '__new_group__', label: 'New Group' })
|
||||
}
|
||||
|
||||
if (actions && trimmedInput.length > 0) {
|
||||
|
||||
@@ -175,7 +175,7 @@ export function useCommandHandling({
|
||||
return
|
||||
}
|
||||
|
||||
if (item.value === 'group') {
|
||||
if (item.value === '__new_group__') {
|
||||
handleGroupCommand()
|
||||
return
|
||||
}
|
||||
|
||||
@@ -52,7 +52,7 @@ export function useCommandMenu({
|
||||
|
||||
if (supportsOperators) {
|
||||
items.push({
|
||||
value: 'group',
|
||||
value: '__new_group__',
|
||||
label: 'New Group',
|
||||
})
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user