-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
feat: Filters support choosing any flagset category #3797
Changes from all commits
05a7c98
3b6c4d9
96ef764
5eab50f
0412cda
5e2fb17
181d2f6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,141 @@ | ||
import { | ||
ComponentType as TYPES, | ||
DEFAULT_FLAG_CATEGORY, | ||
flatFlags, | ||
} from "@opensystemslab/planx-core/types"; | ||
import { fireEvent, screen, waitFor } from "@testing-library/react"; | ||
import React from "react"; | ||
import { setup } from "testUtils"; | ||
import { vi } from "vitest"; | ||
|
||
import Filter from "./Editor"; | ||
|
||
test("Adding a filter without explicit props uses the default flagset", async () => { | ||
const handleSubmit = vi.fn(); | ||
|
||
setup(<Filter handleSubmit={handleSubmit} />); | ||
|
||
expect(screen.getByTestId("flagset-category-select")).toHaveValue( | ||
DEFAULT_FLAG_CATEGORY, | ||
); | ||
|
||
fireEvent.submit(screen.getByTestId("filter-component-form")); | ||
|
||
await waitFor(() => | ||
expect(handleSubmit).toHaveBeenCalledWith( | ||
{ | ||
type: TYPES.Filter, | ||
data: { | ||
fn: "flag", | ||
category: DEFAULT_FLAG_CATEGORY, | ||
}, | ||
}, | ||
mockDefaultFlagOptions, | ||
), | ||
); | ||
}); | ||
|
||
test("Adding a filter and selecting a flagset category", async () => { | ||
const handleSubmit = vi.fn(); | ||
|
||
setup(<Filter handleSubmit={handleSubmit} />); | ||
|
||
expect(screen.getByTestId("flagset-category-select")).toHaveValue( | ||
DEFAULT_FLAG_CATEGORY, | ||
); | ||
|
||
fireEvent.change(screen.getByTestId("flagset-category-select"), { | ||
target: { value: "Community infrastructure levy" }, | ||
}); | ||
|
||
fireEvent.submit(screen.getByTestId("filter-component-form")); | ||
|
||
await waitFor(() => | ||
expect(handleSubmit).toHaveBeenCalledWith( | ||
{ | ||
type: TYPES.Filter, | ||
data: { | ||
fn: "flag", | ||
category: "Community infrastructure levy", | ||
}, | ||
}, | ||
mockCILFlagOptions, | ||
), | ||
); | ||
}); | ||
|
||
test("Updating an existing filter to another category", async () => { | ||
const handleSubmit = vi.fn(); | ||
|
||
setup( | ||
<Filter | ||
id="filterNodeId" | ||
node={mockExistingFilterNode} | ||
handleSubmit={handleSubmit} | ||
/>, | ||
); | ||
|
||
expect(screen.getByTestId("flagset-category-select")).toHaveValue( | ||
"Listed building consent", | ||
); | ||
|
||
fireEvent.change(screen.getByTestId("flagset-category-select"), { | ||
target: { value: "Community infrastructure levy" }, | ||
}); | ||
|
||
fireEvent.submit(screen.getByTestId("filter-component-form")); | ||
|
||
await waitFor(() => | ||
expect(handleSubmit).toHaveBeenCalledWith( | ||
{ | ||
type: TYPES.Filter, | ||
data: { | ||
fn: "flag", | ||
category: "Community infrastructure levy", | ||
}, | ||
}, | ||
mockCILFlagOptions, | ||
), | ||
); | ||
}); | ||
|
||
const mockExistingFilterNode = { | ||
data: { | ||
fn: "flag", | ||
category: "Listed building consent", | ||
}, | ||
type: 500, | ||
edges: ["flag1", "flag2", "flag3", "flag4", "blankFlag"], | ||
}; | ||
|
||
const mockDefaultFlagOptions = [ | ||
...flatFlags.filter((flag) => flag.category === DEFAULT_FLAG_CATEGORY), | ||
{ | ||
category: DEFAULT_FLAG_CATEGORY, | ||
text: "No flag result", | ||
value: "", | ||
}, | ||
].map((flag) => ({ | ||
type: TYPES.Answer, | ||
data: { | ||
text: flag.text, | ||
val: flag.value, | ||
}, | ||
})); | ||
|
||
const mockCILFlagOptions = [ | ||
...flatFlags.filter( | ||
(flag) => flag.category === "Community infrastructure levy", | ||
), | ||
{ | ||
category: "Community infrastructure levy", | ||
text: "No flag result", | ||
value: "", | ||
}, | ||
].map((flag) => ({ | ||
type: TYPES.Answer, | ||
data: { | ||
text: flag.text, | ||
val: flag.value, | ||
}, | ||
})); |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I'd expect the Editor model to be similar to the Result component, where I can see the filter options before updating. I am an uneducated user, so it is feasible that this is redundant for people who know what to expect from the filters. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Filter options are not editable, they are hard-coded only still - therefore in my opinion there's no reason to display them in the modal here (they'd have to be disabled fields?). In the future, when we may have customisable filter options (aka flags and flagsets), then I imagine it will make sense to expose them directly here rather select from a pre-existing/pre-configured category ! But that's out of scope of these changes. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,52 +1,84 @@ | ||
import Typography from "@mui/material/Typography"; | ||
import { | ||
ComponentType as TYPES, | ||
DEFAULT_FLAG_CATEGORY, | ||
flatFlags, | ||
} from "@opensystemslab/planx-core/types"; | ||
import { ComponentType as TYPES } from "@opensystemslab/planx-core/types"; | ||
import { useFormik } from "formik"; | ||
import React from "react"; | ||
import ModalSection from "ui/editor/ModalSection"; | ||
import ModalSectionContent from "ui/editor/ModalSectionContent"; | ||
|
||
import { ICONS } from "../ui"; | ||
|
||
export interface Props { | ||
id?: string; | ||
handleSubmit?: (d: any, c?: any) => void; | ||
handleSubmit?: (data: any, children?: any) => void; | ||
node?: any; | ||
} | ||
|
||
const Filter: React.FC<Props> = (props) => { | ||
const formik = useFormik({ | ||
initialValues: {}, | ||
initialValues: { | ||
fn: "flag", | ||
category: props?.node?.data?.category || DEFAULT_FLAG_CATEGORY, | ||
}, | ||
onSubmit: (newValues) => { | ||
if (props.handleSubmit) { | ||
const children = props.id | ||
? undefined | ||
: [ | ||
...flatFlags, | ||
{ | ||
category: DEFAULT_FLAG_CATEGORY, | ||
text: "(No Result)", | ||
value: "", | ||
}, | ||
] | ||
.filter((f) => f.category === DEFAULT_FLAG_CATEGORY) | ||
.map((f) => ({ | ||
type: TYPES.Answer, | ||
data: { | ||
text: f.text, | ||
val: f.value, | ||
}, | ||
})); | ||
if (props?.handleSubmit) { | ||
const children = [ | ||
...flatFlags, | ||
{ | ||
category: formik.values.category, | ||
text: "No flag result", | ||
value: "", | ||
}, | ||
] | ||
.filter((f) => f.category === formik.values.category) | ||
.map((f) => ({ | ||
type: TYPES.Answer, | ||
data: { | ||
text: f.text, | ||
val: f.value, | ||
}, | ||
})); | ||
|
||
props.handleSubmit( | ||
{ type: TYPES.Filter, data: { newValues, fn: "flag" } }, | ||
children, | ||
); | ||
props.handleSubmit({ type: TYPES.Filter, data: newValues }, children); | ||
} | ||
}, | ||
validate: () => {}, | ||
}); | ||
|
||
const categories = new Set(flatFlags.map((flag) => flag.category)); | ||
|
||
return ( | ||
<form onSubmit={formik.handleSubmit} id="modal"> | ||
<h1>Filter Component</h1> | ||
<form | ||
onSubmit={formik.handleSubmit} | ||
id="modal" | ||
data-testid="filter-component-form" | ||
> | ||
<ModalSection> | ||
<ModalSectionContent title="Filter" Icon={ICONS[TYPES.Filter]}> | ||
<Typography variant="body2"> | ||
Filters automatically sort based on collected flags. Flags within a | ||
category are ordered heirarchically and the filter will route | ||
through the left-most matching flag option only. | ||
</Typography> | ||
</ModalSectionContent> | ||
<ModalSectionContent title="Pick a flagset category (coming soon)"> | ||
<select | ||
data-testid="flagset-category-select" | ||
name="category" | ||
value={formik.values.category} | ||
onChange={formik.handleChange} | ||
disabled | ||
> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My current thinking is that we can merge this to Then I can rebase into #3764, correctly adjust filter automations, and un-disable? Does this sound solid, anything I might be forgetting? Please test flows on this pizza with existing filter components to confirm they work correctly before any type of node "Update" ! |
||
{Array.from(categories).map((category) => ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. First time seeing this, its so slick with the new Set() above! |
||
<option key={category} value={category}> | ||
{category} | ||
</option> | ||
))} | ||
</select> | ||
</ModalSectionContent> | ||
</ModalSection> | ||
</form> | ||
); | ||
}; | ||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: why would we use
fireEvent
here rather thanuserEvent
?Does it make selecting from a
<select>
easier?(rather than going through user.click, find the options, user.click again...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using
fireEvent
throughout this file because the modal submit buttons like "Create filter" / "Update filter" aren't easily accessible touser.click()
- thereforefireEvent.submit()
is handy to have (this is commonly used throughout other "Editor" test files as well).I think the select interaction specifically could be migrated to
userEvent
going forward - which would mean we'd be testing the interaction more realistically to how the user would experience it in the browser.