Skip to content

Add missing unit tests for ListView miscellaneous properties #803

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.

Sign up for GitHub

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

Merged
hotlong merged 3 commits into from
Feb 23, 2026
Merged

Add missing unit tests for ListView miscellaneous properties #803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
252d5ac
Initial plan
Copilot Feb 23, 2026
35d09f0
Add missing filterableFields and rowHeight short/extra_tall tests
Copilot Feb 23, 2026
35b07df
Improve filterableFields tests with filter button assertions
Copilot Feb 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions packages/plugin-list/src/__tests__/ListView.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1973,4 +1973,61 @@ describe('ListView', () => {
expect(shareButton).toHaveAttribute('title', 'Sharing: collaborative');
});
});

// ============================
// filterableFields whitelist
// ============================
describe('filterableFields', () => {
it('should render with filterableFields whitelist restricting available fields', () => {
const schema: ListViewSchema = {
type: 'list-view',
objectName: 'contacts',
viewType: 'grid',
fields: [
{ name: 'name', label: 'Name', type: 'text' },
{ name: 'email', label: 'Email', type: 'text' },
{ name: 'phone', label: 'Phone', type: 'text' },
] as any,
filterableFields: ['name', 'email'],
};

renderWithProvider(<ListView schema={schema} />);
// Filter button should still be visible
const filterButton = screen.getByRole('button', { name: /filter/i });
expect(filterButton).toBeInTheDocument();
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test for filterableFields whitelist only verifies that the filter button renders, but doesn't actually verify that the whitelist is being applied. According to the implementation at ListView.tsx L883-887, filterableFields restricts which fields are available in the FilterBuilder.

To properly test this functionality, the test should:

  1. Click the filter button to open the filter panel
  2. Verify that only the whitelisted fields (name and email) are available in the field selector
  3. Verify that non-whitelisted fields (phone) are not available

Similar to how other tests in this file verify behavior after interactions (e.g., lines 148-168 for filter panel toggle, lines 536-543 for auto-derived userFilters), this test should verify the actual filtering behavior, not just button presence.

Suggested change
expect(filterButton).toBeInTheDocument();
expect(filterButton).toBeInTheDocument();
// Open the filter panel
fireEvent.click(filterButton);
// Collect available field options from the filter builder
const fieldOptions = screen.getAllByRole('option').map(
(option) => option.textContent?.trim()
);
// Only whitelisted fields should be available
expect(fieldOptions).toEqual(
expect.arrayContaining(['Name', 'Email'])
);
// Non-whitelisted fields should not be available
expect(fieldOptions).not.toContain('Phone');

Copilot uses AI. Check for mistakes.
});

it('should render filter button when filterableFields is not set', () => {
const schema: ListViewSchema = {
type: 'list-view',
objectName: 'contacts',
viewType: 'grid',
fields: [
{ name: 'name', label: 'Name', type: 'text' },
{ name: 'email', label: 'Email', type: 'text' },
] as any,
};

renderWithProvider(<ListView schema={schema} />);
const filterButton = screen.getByRole('button', { name: /filter/i });
expect(filterButton).toBeInTheDocument();
});

it('should render filter button when filterableFields is empty array', () => {
const schema: ListViewSchema = {
type: 'list-view',
objectName: 'contacts',
viewType: 'grid',
fields: [
{ name: 'name', label: 'Name', type: 'text' },
{ name: 'email', label: 'Email', type: 'text' },
] as any,
filterableFields: [],
};

renderWithProvider(<ListView schema={schema} />);
const filterButton = screen.getByRole('button', { name: /filter/i });
expect(filterButton).toBeInTheDocument();
});
Comment on lines +2000 to +2031
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test cases for "filterableFields not set" (lines 2000-2014) and "filterableFields empty array" (lines 2016-2031) are redundant. Both test the exact same behavior: verifying that the filter button renders.

According to the implementation at ListView.tsx L883-887, an empty array should behave differently from undefined:

  • undefined/not set: no whitelist, all fields available
  • empty array: whitelist with no fields, effectively no fields available
  • non-empty array: only whitelisted fields available

To properly differentiate these cases and provide meaningful test coverage, the tests should verify the actual field availability in the filter panel, not just button presence.

Copilot uses AI. Check for mistakes.
});
});
6 changes: 6 additions & 0 deletions packages/react/src/spec-bridge/__tests__/SpecBridge.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,12 @@ describe('SpecBridge', () => {

const small = bridgeListView({ rowHeight: 'small' }, {});
expect(small.density).toBe('compact');

const short = bridgeListView({ rowHeight: 'short' }, {});
expect(short.density).toBe('compact');

const extraTall = bridgeListView({ rowHeight: 'extra_tall' }, {});
expect(extraTall.density).toBe('spacious');
});

it('includes optional list properties', () => {
Expand Down