Skip to content

[APP-526] feat: Use NBS components for Advanced Filter UI #3315

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

Open
emyl3 wants to merge 39 commits into
base: main
Choose a base branch
from
Open

[APP-526] feat: Use NBS components for Advanced Filter UI #3315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
736da60
[APP-526] feat: Use NBS components for Advanced Filter UI
emyl3 Jun 4, 2026
959c71c
fix: styling cleanup
mcmcgrath13 Jun 15, 2026
3c905c0
fix: commented out code
mcmcgrath13 Jun 15, 2026
1836fa6
Merge branch 'main' into el/app-526/adv-filter-nbs-components
emyl3 Jun 15, 2026
26775c2
fix: fix styling
emyl3 Jun 15, 2026
0c101ac
Merge branch 'main' into el/app-526/adv-filter-nbs-components
emyl3 Jun 15, 2026
e25367f
address comments
emyl3 Jun 15, 2026
a1449ab
wip: fix validation and add tests
emyl3 Jun 16, 2026
644a817
Merge remote-tracking branch 'origin/main' into el/app-526/adv-filter…
emyl3 Jun 18, 2026
7b5df69
fix: remove refs
emyl3 Jun 18, 2026
de78d75
fix: address comments
emyl3 Jun 18, 2026
751d940
fix: error msg
emyl3 Jun 18, 2026
c8b240a
fix: nested rule group labels and changing operators
emyl3 Jun 18, 2026
36a078d
fix: update buttons
emyl3 Jun 18, 2026
854dad2
Merge remote-tracking branch 'origin/main' into el/app-526/adv-filter…
emyl3 Jun 18, 2026
1f09b21
Apply suggestions from code review
emyl3 Jun 18, 2026
ff86178
address comments
emyl3 Jun 22, 2026
629a9b5
Apply suggestions from code review
emyl3 Jun 22, 2026
1b86b2c
fix styles
emyl3 Jun 22, 2026
9e14147
cleanup
emyl3 Jun 22, 2026
04c2a34
cleanup
emyl3 Jun 22, 2026
c89f8f6
address comments
emyl3 Jun 22, 2026
b16ff03
add error msg
emyl3 Jun 22, 2026
bd54eca
add more tests
emyl3 Jun 22, 2026
d5949fe
Merge branch 'main' into el/app-526/adv-filter-nbs-components
emyl3 Jun 22, 2026
7b41118
fix test
emyl3 Jun 22, 2026
62bcba7
fix test
emyl3 Jun 22, 2026
31016e7
Merge branch 'main' into el/app-526/adv-filter-nbs-components
emyl3 Jun 22, 2026
b9888d2
Fix typo
emyl3 Jun 24, 2026
cbd5649
Merge branch 'main' into el/app-526/adv-filter-nbs-components
emyl3 Jun 24, 2026
5498b62
address comments
emyl3 Jun 24, 2026
8751a85
run format
emyl3 Jun 24, 2026
5f06177
Merge remote-tracking branch 'origin/main' into el/app-526/adv-filter…
emyl3 Jun 24, 2026
dd484a4
run format
emyl3 Jun 24, 2026
52cef3a
fix test
emyl3 Jun 24, 2026
4d210e7
fix: cypress test
emyl3 Jun 25, 2026
c31d8e7
Apply suggestions from code review
emyl3 Jun 25, 2026
3ecb947
Merge branch 'main' into el/app-526/adv-filter-nbs-components
emyl3 Jun 25, 2026
0c45444
move constant
emyl3 Jun 25, 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
11 changes: 10 additions & 1 deletion apps/modernization-ui/src/apps/report/run/ReportConfigurationPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,16 @@ const SECTIONS = [
id: 'advanced-filter',
hasData: (config: ReportConfiguration) => !!config.advancedFilter,
Component: ({ config, id, title }: { config: ReportConfiguration; id: string; title: string }) => (
<Card id={id} title={title} collapsible={true}>
<Card
id={id}
title={title}
collapsible={true}
subtext="Add rules and rule groups to narrow or broaden your results.
Use AND to require all connected rules or groups to match, or OR to require
only one to match. Your advanced filter combines with your basic filters
using AND logic. The WHERE clause preview shows your advanced filter as you build it."
contentWidth="widescreen"
>
<AdvancedFilter filter={config.advancedFilter!} columns={config.columns} />
</Card>
),
Expand Down
114 changes: 60 additions & 54 deletions apps/modernization-ui/src/apps/report/run/ReportRunPage.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import { fireEvent, render, waitFor, within } from '@testing-library/react';
import { ReportRunPage } from './ReportRunPage';
import * as generated from 'generated';
import userEvent from '@testing-library/user-event';
Expand Down Expand Up @@ -2448,26 +2448,26 @@ describe('report run page', () => {
const mockResultApi = vi
.mocked(generated.ReportControllerService.exportReport)
.mockResolvedValue(MOCK_RESULT);
const { findAllByText, queryByText, findByRole } = renderWithRouter();
const { findAllByText, queryByText, findByRole, findByLabelText, getByLabelText, container } =
renderWithRouter();

expect(mockApi).toHaveBeenCalled();

expect(await findAllByText('Advanced filter')).toHaveLength(2);
expect(queryByText('Basic filters')).toBeNull();

const fieldSelect = await findByRole('combobox', { name: 'Field' });
const fieldSelect = getByLabelText('Field');
expect(fieldSelect).toHaveValue('~');
const user = userEvent.setup();
await user.selectOptions(fieldSelect, 'Full Name');
const opSelect = await findByRole('combobox', { name: 'Operator' });
const opSelect = await findByLabelText('Logic');
expect(opSelect).toHaveValue('~');
await user.selectOptions(opSelect, 'contains');
const valueBox = await findByRole('textbox', { name: 'Value' });
const valueBox = await findByLabelText('Value');
expect(valueBox).toHaveValue('');
await user.type(valueBox, 'hi');

// currently not working, but should once we put in our own components
// expect(await axe(container)).toHaveNoViolations();
expect(await axe(container)).toHaveNoViolations();

const exportButton = await findByRole('button', { name: 'Export' });
await user.click(exportButton);
Expand Down Expand Up @@ -2533,17 +2533,17 @@ describe('report run page', () => {
getByText,
findByText,
findAllByText,
getByLabelText,
getByTestId,
findByLabelText,
findByRole,
findAllByRole,
findByTestId,
} = renderWithRouter();

expect(mockApi).toHaveBeenCalled();

expect(await findAllByText('Advanced filter')).toHaveLength(2);

const fieldSelect = await findByRole('combobox', { name: 'Field' });
const fieldSelect = getByLabelText('Field');
expect(fieldSelect).toHaveValue('~');
const user = userEvent.setup();
await user.selectOptions(fieldSelect, 'Full Name');
Expand All @@ -2552,40 +2552,40 @@ describe('report run page', () => {
const exportButton = await findByRole('button', { name: 'Export' });
await user.click(exportButton);

expect(await findByText('Must select an operator and value')).toBeVisible();
expect(await findByText('Enter a logic value for Full Name.')).toBeVisible();

// generally filled in text value
const opSelect = await findByRole('combobox', { name: 'Operator' });
const opSelect = getByLabelText('Logic');
expect(opSelect).toHaveValue('~');
await user.selectOptions(opSelect, 'contains');

expect(await findByText('Value cannot be empty')).toBeVisible();
expect(await findByText('Enter a value for Full Name.')).toBeVisible();

const valueBox = await findByRole('textbox', { name: 'Value' });
const valueBox = getByLabelText('Value');
expect(valueBox).toHaveValue('');
await user.type(valueBox, 'hi');

expect(queryByText('Value cannot be empty')).toBeNull();
expect(queryByText('Enter a value for Full Name.')).toBeNull();

// generally filled in number value
await user.selectOptions(fieldSelect, 'DAYS_OLD');
expect(opSelect).toHaveValue('~');
await user.selectOptions(opSelect, '=');

expect(await findByText('Value cannot be empty')).toBeVisible();
expect(await findByText('Enter a value for Days Old.')).toBeVisible();

const numberBox = await findByRole('spinbutton', { name: 'Value' });
const numberBox = await findByLabelText('Value');
expect(numberBox).toHaveValue(null);
await user.type(numberBox, '0');
await user.type(numberBox, '0{tab}');

expect(queryByText('Value cannot be empty')).toBeNull();
expect(queryByText('Enter a value for Days Old.')).toBeNull();

// generally filled in coded list
await user.selectOptions(fieldSelect, 'Condition Code');
expect(opSelect).toHaveValue('~');
await user.selectOptions(opSelect, 'in');

expect(await findByText('Value cannot be empty')).toBeVisible();
expect(await findByText('Enter a value for Condition Code.')).toBeVisible();

await waitFor(() => expect(codedValueGetter).toHaveBeenCalledWith(`/nbs/api/options/races`));

Expand All @@ -2594,46 +2594,51 @@ describe('report run page', () => {
await user.click(dropDown);
await user.click(getByText('Terrible disease'));

expect(queryByText('Value cannot be empty')).toBeNull();
expect(queryByText('Enter a value for Condition Code.')).toBeNull();

// dates between
await user.selectOptions(fieldSelect, 'DATE_OF_BIRTH');
expect(opSelect).toHaveValue('~');
await user.selectOptions(opSelect, 'between');

expect(await findByText('Both low and high values required')).toBeVisible();
expect(await findByText('Enter from and to values for Date of Birth.')).toBeVisible();

// The date entry will likely need to change once we switch to NBS components
const dtInputs = (await findByTestId('value-editor')).children;
await user.type(dtInputs[0], '2022-10-18');

expect(await findByText('Both low and high values required')).toBeVisible();
const dtContainer = getByTestId('date-range-editor');
const dtInputFrom = await within(dtContainer).findByLabelText('From');
const dtInputTo = await within(dtContainer).findByLabelText('To');
await user.type(dtInputFrom, '10/18/2022{tab}');

await user.type(dtInputs[1], '2022-10-17');
expect(await findByText('Enter from and to values for Date of Birth.')).toBeVisible();

expect(await findByText('High value must be greater than or equal to low value')).toBeVisible();
await user.type(dtInputTo, '10/17/2022{tab}');

await user.type(dtInputs[1], '{backspace}9');
expect(await findByText('From date must be before to date for Date of Birth.')).toBeVisible();
await user.clear(dtInputTo);
await user.type(dtInputTo, '10/20/2022{tab}');

expect(queryByText('High value must be greater than or equal to low value')).toBeNull();
expect(queryByText('From date must be before to date for Date of Birth.')).toBeNull();

// numbers between
await user.selectOptions(fieldSelect, 'DAYS_OLD');
expect(opSelect).toHaveValue('~');
await user.selectOptions(opSelect, 'between');

expect(await findByText('Both low and high values required')).toBeVisible();
expect(await findByText('Enter from and to values for Days Old.')).toBeVisible();

const numInputs = await findAllByRole('spinbutton');
await user.type(numInputs[0], '10');
const numContainer = getByTestId('number-range-editor');
const numInputFrom = await within(numContainer).findByLabelText('From');
const numInputTo = await within(numContainer).findByLabelText('To');
await user.type(numInputFrom, '10');

expect(await findByText('Both low and high values required')).toBeVisible();
expect(await findByText('Enter from and to values for Days Old.')).toBeVisible();

await user.type(numInputs[1], '0');
await user.type(numInputTo, '0');

expect(await findByText('High value must be greater than or equal to low value')).toBeVisible();
expect(await findByText('From value must be before to value for Days Old.')).toBeVisible();

await user.type(numInputs[1], '{backspace}20');
await user.clear(numInputTo);
await user.type(numInputTo, '20');

await user.click(exportButton);

Expand Down Expand Up @@ -2683,7 +2688,7 @@ describe('report run page', () => {
id: '126-126-126',
columnId: 2002,
operator: 'GT',
value: '2020-01-01', // format should be mm/dd/yyyy when we switch components
value: '01/01/2020',
},
{
id: '127-127-127',
Expand All @@ -2710,44 +2715,46 @@ describe('report run page', () => {
{ value: '123', name: 'Disease, terrible' },
{ value: '456', name: 'Disease, not so bad' },
]);
const { findAllByText, findByRole, findAllByRole, findAllByTitle } = renderWithRouter();
const { getByTestId, findAllByText, findByRole, findAllByRole, findAllByLabelText } = renderWithRouter();

expect(mockApi).toHaveBeenCalled();

expect(await findAllByText('Advanced filter')).toHaveLength(2);

const combinators = await findAllByRole('combobox', { name: 'Combinator' });
const combinators = await findAllByLabelText('Combinator');
expect(combinators).toHaveLength(2);
expect(combinators[0]).toHaveValue('or');
expect(combinators[1]).toHaveValue('and');

const fields = await findAllByRole('combobox', { name: 'Field' });
const fields = await findAllByLabelText('Field');
expect(fields).toHaveLength(4);
expect(fields[0]).toHaveValue('FULL_NAME');
expect(fields[1]).toHaveValue('DATE_OF_BIRTH');
expect(fields[2]).toHaveValue('DAYS_OLD');
expect(fields[3]).toHaveValue('CONDITION');

const operators = await findAllByRole('combobox', { name: 'Operator' });
const operators = await findAllByLabelText('Logic');
expect(operators).toHaveLength(4);
expect(operators[0]).toHaveValue('beginswith');
expect(operators[1]).toHaveValue('>');
expect(operators[2]).toHaveValue('between');
expect(operators[3]).toHaveValue('notIn');

const values = await findAllByTitle('Value');
const values = await findAllByLabelText('Value');
expect(values).toHaveLength(3);
expect(values[0]).toHaveValue('prefix');
expect(values[1]).toHaveValue('2020-01-01');
const [low, high] = values[2].children;
expect(low).toHaveValue(10);
expect(high).toHaveValue(20);
expect(values[1]).toHaveValue('01/01/2020');
const numContainer = getByTestId('number-range-editor');
const numLow = await within(numContainer).findByLabelText('From');
const numHigh = await within(numContainer).findByLabelText('To');
expect(numLow).toHaveValue(10);
expect(numHigh).toHaveValue(20);
expect(await findByRole('button', { name: 'Remove Disease, terrible' })).toBeVisible();
expect(await findByRole('button', { name: 'Remove Disease, not so bad' })).toBeVisible();

const user = userEvent.setup();
await user.type(high, '1');
expect(high).toHaveValue(201);
await user.type(numHigh, '1');
expect(numHigh).toHaveValue(201);

const exportButton = await findByRole('button', { name: 'Export' });
await user.click(exportButton);
Expand Down Expand Up @@ -2776,7 +2783,7 @@ describe('report run page', () => {
columnId: 2002,
operator: 'GT',
// format should be mm/dd/yyyy when we switch components
value: '2020-01-01',
value: '01/01/2020',
},
{
id: '127-127-127',
Expand Down Expand Up @@ -2836,8 +2843,7 @@ describe('report run page', () => {
id: '129-129-129',
columnId: 2002,
operator: 'GT',
// format should be mm/dd/yyyy when we switch components
value: '2020-01-01',
value: '01/01/2020',
},
{
id: '130-130-130',
Expand Down Expand Up @@ -2959,7 +2965,7 @@ describe('report run page', () => {
columnId: 2002,
operator: 'GT',
// format should be mm/dd/yyyy when we switch components
value: '2020-01-01',
value: '01/01/2020',
},
{
id: '130-130-130',
Expand Down Expand Up @@ -3059,7 +3065,7 @@ describe('report run page', () => {
expect(announcementEl).toHaveTextContent('You have moved the group up to path 2-2')
);

const addRuleBtn = (await findAllByRole('button', { name: '+ Rule' }))[0];
const addRuleBtn = (await findAllByRole('button', { name: 'Add rule' }))[0];
await user.click(addRuleBtn);
await waitFor(() =>
expect(announcementEl).toHaveTextContent('The group has returned to its starting position')
Expand Down
17 changes: 17 additions & 0 deletions apps/modernization-ui/src/apps/report/run/filters/advanced/AddButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import React from 'react';
import { ActionProps } from 'react-querybuilder';
import { Button } from '../../../../../design-system/button';

@JordanGuinn JordanGuinn Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(thought, super-nb): ENTIRELY unrelated to this PR, but it'd be great if we could get path aliasing working at some point, and remove the need to traverse up the codebase like this for some of our dependencies!

@emyl3 emyl3 Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes! Agree! Do you prefer I create a separate ticket to keep track of this or just a good thing to keep in mind when we are out of the crunch period? 👀

@JordanGuinn JordanGuinn Jun 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A tech debt ticket couldn't hurt! Thanks Elisa 🙏


const AddButton = (props: ActionProps) => {
return (
<Button
type={'button'}
onClick={(e) => props.handleOnClick(e)}
secondary={props.className === 'ruleGroup-addGroup'}
>
{props.title}
</Button>
);
};

export { AddButton };
Loading
Loading