From d00c75b110c7a46a359d6e8eddd25523fff08500 Mon Sep 17 00:00:00 2001 From: Jason Gill Date: Fri, 3 May 2024 17:58:23 -0600 Subject: [PATCH 1/4] Use CheckboxTree for data map filtering --- .../__tests__/features/common-utils.test.ts | 17 +- .../admin-ui/cypress/e2e/datamap-report.cy.ts | 28 ++ .../src/features/common/CheckboxTree.tsx | 24 +- .../features/common/modals/FilterModal.tsx | 15 +- .../table/v2/filters/GlobalFilterV2.tsx | 1 + clients/admin-ui/src/features/common/utils.ts | 10 + .../reporting/DatamapReportFilterModal.tsx | 279 ++++++++---------- .../datamap/reporting/DatamapReportTable.tsx | 65 ++-- 8 files changed, 228 insertions(+), 211 deletions(-) diff --git a/clients/admin-ui/__tests__/features/common-utils.test.ts b/clients/admin-ui/__tests__/features/common-utils.test.ts index b17163a8c0..bc249c2f24 100644 --- a/clients/admin-ui/__tests__/features/common-utils.test.ts +++ b/clients/admin-ui/__tests__/features/common-utils.test.ts @@ -1,4 +1,7 @@ -import { getFileNameFromContentDisposition } from "~/features/common/utils"; +import { + getFileNameFromContentDisposition, + getQueryParamsFromArray, +} from "~/features/common/utils"; describe("common utils", () => { describe(getFileNameFromContentDisposition.name, () => { @@ -13,6 +16,18 @@ describe("common utils", () => { ); }); }); + describe(getQueryParamsFromArray.name, () => { + it("should return undefined when strings is empty", () => { + expect(getQueryParamsFromArray([], "test")).toBeUndefined(); + }); + it("should return query params from strings", () => { + const strings = ["a", "b", "c"]; + const queryParam = "test"; + expect(getQueryParamsFromArray(strings, queryParam)).toEqual( + "test=a&test=b&test=c" + ); + }); + }); }); export default undefined; diff --git a/clients/admin-ui/cypress/e2e/datamap-report.cy.ts b/clients/admin-ui/cypress/e2e/datamap-report.cy.ts index d70e485dc4..7de01e6ecf 100644 --- a/clients/admin-ui/cypress/e2e/datamap-report.cy.ts +++ b/clients/admin-ui/cypress/e2e/datamap-report.cy.ts @@ -165,6 +165,34 @@ describe("Minimal datamap report table", () => { }); }); + describe("Filtering", () => { + it("should filter the table by making a selection", () => { + cy.getByTestId("filter-multiple-systems-btn").click(); + cy.getByTestId("datamap-report-filter-modal").should("be.visible"); + cy.getByTestId("filter-modal-accordion-button").eq(1).click(); + cy.getByTestId("filter-modal-checkbox-tree-categories").should( + "be.visible" + ); + cy.getByTestId("filter-modal-checkbox-tree-categories") + .find("input") + .first() + .click({ force: true }); + cy.getByTestId("datamap-report-filter-modal-continue-btn").click(); + cy.get("@getDatamapMinimal") + .its("request.url") + .should("include", "data_categories=custom"); + cy.getByTestId("datamap-report-filter-modal").should("not.exist"); + + // should clear the filters + cy.getByTestId("filter-multiple-systems-btn").click(); + cy.getByTestId("datamap-report-filter-modal-cancel-btn").click(); + cy.getByTestId("datamap-report-filter-modal").should("not.exist"); + cy.wait("@getDatamapMinimal") + .its("request.url") + .should("not.include", "data_categories=custom"); + }); + }); + describe("Exporting", () => { it("should open the export modal", () => { cy.getByTestId("export-btn").click(); diff --git a/clients/admin-ui/src/features/common/CheckboxTree.tsx b/clients/admin-ui/src/features/common/CheckboxTree.tsx index 883665e2dc..409086929d 100644 --- a/clients/admin-ui/src/features/common/CheckboxTree.tsx +++ b/clients/admin-ui/src/features/common/CheckboxTree.tsx @@ -8,10 +8,10 @@ */ import { - ArrowDownLineIcon, - ArrowUpLineIcon, Box, + BoxProps, Checkbox, + ChevronDownIcon, IconButton, } from "@fidesui/react"; import { Fragment, ReactNode, useEffect, useState } from "react"; @@ -115,16 +115,11 @@ const CheckboxItem = ({ - ) : ( - - ) - } + icon={} variant="ghost" onClick={() => onExpanded(node)} size="sm" + style={{ transform: isExpanded ? "rotate(180deg)" : "" }} /> ) : null} @@ -133,13 +128,18 @@ const CheckboxItem = ({ ); }; -interface CheckboxTreeProps { +interface CheckboxTreeProps extends BoxProps { nodes: TreeNode[]; selected: string[]; onSelected: (newSelected: string[]) => void; } -const CheckboxTree = ({ nodes, selected, onSelected }: CheckboxTreeProps) => { +const CheckboxTree = ({ + nodes, + selected, + onSelected, + ...props +}: CheckboxTreeProps) => { const [checked, setChecked] = useState([]); const [expanded, setExpanded] = useState([]); @@ -236,7 +236,7 @@ const CheckboxTree = ({ nodes, selected, onSelected }: CheckboxTreeProps) => { }; return ( - + {nodes.map((child) => ( {createTree(child)} ))} diff --git a/clients/admin-ui/src/features/common/modals/FilterModal.tsx b/clients/admin-ui/src/features/common/modals/FilterModal.tsx index 56f9556966..0a5213de73 100644 --- a/clients/admin-ui/src/features/common/modals/FilterModal.tsx +++ b/clients/admin-ui/src/features/common/modals/FilterModal.tsx @@ -15,6 +15,7 @@ import { ModalFooter, ModalHeader, ModalOverlay, + ModalProps, SimpleGrid, Text, } from "@fidesui/react"; @@ -167,19 +168,17 @@ export const FilterSection = ({ heading, children }: FilterSectionProps) => ( ); -interface FilterModalProps { - isOpen: boolean; - onClose: () => void; +export interface FilterModalProps extends ModalProps { resetFilters: () => void; } - -export const FilterModal: React.FC = ({ +export const FilterModal = ({ + resetFilters, isOpen, onClose, children, - resetFilters, -}) => ( - + ...props +}: FilterModalProps): JSX.Element => ( + Filters diff --git a/clients/admin-ui/src/features/common/table/v2/filters/GlobalFilterV2.tsx b/clients/admin-ui/src/features/common/table/v2/filters/GlobalFilterV2.tsx index 1e252fe20c..646b95c577 100644 --- a/clients/admin-ui/src/features/common/table/v2/filters/GlobalFilterV2.tsx +++ b/clients/admin-ui/src/features/common/table/v2/filters/GlobalFilterV2.tsx @@ -36,6 +36,7 @@ export const GlobalFilterV2 = ({ onClear={onClear} search={value || ""} placeholder={placeholder} + data-testid="global-text-filter" /> ); diff --git a/clients/admin-ui/src/features/common/utils.ts b/clients/admin-ui/src/features/common/utils.ts index fac360a030..499f045cd5 100644 --- a/clients/admin-ui/src/features/common/utils.ts +++ b/clients/admin-ui/src/features/common/utils.ts @@ -47,3 +47,13 @@ export const getFileNameFromContentDisposition = ( const match = contentDisposition.match(/filename=(.+)/); return match ? match[1] : defaultName; }; + +export const getQueryParamsFromArray = ( + strings: string[], + queryParam: string +) => { + if (strings.length > 0) { + return `${queryParam}=${strings.join(`&${queryParam}=`)}`; + } + return undefined; +}; diff --git a/clients/admin-ui/src/features/datamap/reporting/DatamapReportFilterModal.tsx b/clients/admin-ui/src/features/datamap/reporting/DatamapReportFilterModal.tsx index fb1537a24a..edfb6964ec 100644 --- a/clients/admin-ui/src/features/datamap/reporting/DatamapReportFilterModal.tsx +++ b/clients/admin-ui/src/features/datamap/reporting/DatamapReportFilterModal.tsx @@ -1,12 +1,21 @@ -import { Accordion, useDisclosure } from "@fidesui/react"; -import { useEffect, useState } from "react"; +import { + Accordion, + AccordionButton, + AccordionIcon, + AccordionItem, + AccordionItemProps, + AccordionPanel, + Box, + Heading, +} from "@fidesui/react"; +import { useMemo, useState } from "react"; import { useAppSelector } from "~/app/hooks"; -import { - AccordionMultifieldFilter, - FilterModal, - Option, -} from "~/features/common/modals/FilterModal"; +import CheckboxTree from "~/features/common/CheckboxTree"; +import StandardDialog, { + StandardDialogProps, +} from "~/features/common/modals/StandardDialog"; +import { TreeNode } from "~/features/common/types"; import { selectDataSubjects, useGetAllDataSubjectsQuery, @@ -15,167 +24,131 @@ import { selectDataUses, useGetAllDataUsesQuery, } from "~/features/data-use/data-use.slice"; +import { transformTaxonomyEntityToNodes } from "~/features/taxonomy/helpers"; import { selectDataCategories, useGetAllDataCategoriesQuery, } from "~/features/taxonomy/taxonomy.slice"; -export const useDatamapReportFilters = () => { - const { isOpen, onClose, onOpen } = useDisclosure(); +export type DatamapReportFilterSelections = { + dataUses: string[]; + dataSubjects: string[]; + dataCategories: string[]; +}; + +interface DatamapReportFilterModalProps + extends Omit { + onFilterChange: (selectedFilters: DatamapReportFilterSelections) => void; +} + +const FilterModalAccordionItem = ({ + label, + children, + ...props +}: { label: string } & AccordionItemProps): JSX.Element => ( + + + + {label} + + + + {children} + +); + +export const DatamapReportFilterModal = ({ + onClose, + onFilterChange, + ...props +}: DatamapReportFilterModalProps): JSX.Element => { useGetAllDataUsesQuery(); - const dataUses = useAppSelector(selectDataUses); useGetAllDataSubjectsQuery(); - const dataSubjects = useAppSelector(selectDataSubjects); useGetAllDataCategoriesQuery(); - const dataCategories = useAppSelector(selectDataCategories); - const [dataUseOptions, setDataUseOptions] = useState([]); - const [dataCategoriesOptions, setDataCategoriesOptions] = useState( - [] - ); - const [dataSubjectOptions, setDataSubjectOptions] = useState([]); - - useEffect(() => { - if (dataUseOptions.length === 0) { - setDataUseOptions( - dataUses.map((dataUse) => ({ - value: dataUse.fides_key, - displayText: dataUse.name || dataUse.fides_key, - isChecked: false, - })) - ); - } - }, [dataUses, dataUseOptions, setDataUseOptions]); + const dataUses = useAppSelector(selectDataUses); + const dataSubjects = useAppSelector(selectDataSubjects); + const dataCategories = useAppSelector(selectDataCategories); - useEffect(() => { - if (dataCategoriesOptions.length === 0) { - setDataCategoriesOptions( - dataCategories.map((dataCategory) => ({ - value: dataCategory.fides_key, - displayText: dataCategory.name || dataCategory.fides_key, - isChecked: false, - })) - ); - } - }, [dataCategories, dataCategoriesOptions, setDataCategoriesOptions]); + const [checkedUses, setCheckedUses] = useState([]); + const [checkedSubjects, setCheckedSubjects] = useState([]); + const [checkedCategories, setCheckedCategories] = useState([]); - useEffect(() => { - if (dataSubjectOptions.length === 0) { - setDataSubjectOptions( - dataSubjects.map((dataSubject) => ({ - value: dataSubject.fides_key, - displayText: dataSubject.name || dataSubject.fides_key, - isChecked: false, - })) - ); - } - }, [dataSubjects, dataSubjectOptions, setDataSubjectOptions]); + const dataUseNodes: TreeNode[] = useMemo( + () => transformTaxonomyEntityToNodes(dataUses), + [dataUses] + ); + const dataSubjectNodes: TreeNode[] = useMemo( + () => transformTaxonomyEntityToNodes(dataSubjects), + [dataSubjects] + ); + const dataCategoryNodes: TreeNode[] = useMemo( + () => transformTaxonomyEntityToNodes(dataCategories), + [dataCategories] + ); const resetFilters = () => { - setDataUseOptions((prev) => prev.map((o) => ({ ...o, isChecked: false }))); - setDataCategoriesOptions((prev) => - prev.map((o) => ({ ...o, isChecked: false })) - ); - setDataSubjectOptions((prev) => - prev.map((o) => ({ ...o, isChecked: false })) - ); - }; - - const onCheckBoxChange = ( - newValue: string, - checked: boolean, - options: Option[], - setOptions: (options: Option[]) => void - ) => { - const newOptions = options.map((option) => { - if (option.value === newValue) { - return { - ...option, - isChecked: checked, - }; - } - return option; + setCheckedUses([]); + setCheckedSubjects([]); + setCheckedCategories([]); + onFilterChange({ + dataUses: [], + dataSubjects: [], + dataCategories: [], }); - - setOptions(newOptions); - }; - - const onDataUseChange = (fidesKey: string, checked: boolean) => { - onCheckBoxChange(fidesKey, checked, dataUseOptions, setDataUseOptions); - }; - - const onDataCategoriesChange = (fidesKey: string, checked: boolean) => { - onCheckBoxChange( - fidesKey, - checked, - dataCategoriesOptions, - setDataCategoriesOptions - ); - }; - - const onDataSubjectChange = (fidesKey: string, checked: boolean) => { - onCheckBoxChange( - fidesKey, - checked, - dataSubjectOptions, - setDataSubjectOptions - ); + onClose(); }; - return { - isOpen, - onClose, - onOpen, - resetFilters, - dataUseOptions, - onDataUseChange, - dataCategoriesOptions, - onDataCategoriesChange, - dataSubjectOptions, - onDataSubjectChange, + const handleFilterChange = () => { + onFilterChange({ + dataUses: checkedUses, + dataSubjects: checkedSubjects, + dataCategories: checkedCategories, + }); + onClose(); }; + return ( + + + + + + + + + + + + + + ); }; - -type Props = { - isOpen: boolean; - onClose: () => void; - resetFilters: () => void; - dataUseOptions: Option[]; - onDataUseChange: (fidesKey: string, checked: boolean) => void; - dataCategoriesOptions: Option[]; - onDataCategoriesChange: (fidesKey: string, checked: boolean) => void; - dataSubjectOptions: Option[]; - onDataSubjectChange: (fidesKey: string, checked: boolean) => void; -}; - -export const DatamapReportFilterModal = ({ - isOpen, - onClose, - resetFilters, - dataUseOptions, - onDataUseChange, - dataCategoriesOptions, - onDataCategoriesChange, - dataSubjectOptions, - onDataSubjectChange, -}: Props) => ( - - - - - - - -); diff --git a/clients/admin-ui/src/features/datamap/reporting/DatamapReportTable.tsx b/clients/admin-ui/src/features/datamap/reporting/DatamapReportTable.tsx index a010ebd3bc..603c35f6bd 100644 --- a/clients/admin-ui/src/features/datamap/reporting/DatamapReportTable.tsx +++ b/clients/admin-ui/src/features/datamap/reporting/DatamapReportTable.tsx @@ -38,7 +38,7 @@ import { useAppSelector } from "~/app/hooks"; import { useLocalStorage } from "~/features/common/hooks/useLocalStorage"; import useTaxonomies from "~/features/common/hooks/useTaxonomies"; import { DownloadLightIcon } from "~/features/common/Icon"; -import { getQueryParamsFromList } from "~/features/common/modals/FilterModal"; +import { getQueryParamsFromArray } from "~/features/common/utils"; import { DATAMAP_LOCAL_STORAGE_KEYS, ExportFormat, @@ -50,7 +50,7 @@ import { import ReportExportModal from "~/features/datamap/modals/ReportExportModal"; import { DatamapReportFilterModal, - useDatamapReportFilters, + DatamapReportFilterSelections, } from "~/features/datamap/reporting/DatamapReportFilterModal"; import { selectAllCustomFieldDefinitions, @@ -223,17 +223,10 @@ export const DatamapReportTable = () => { } = useServerSidePagination(); const { - isOpen, - onClose, - onOpen, - resetFilters, - dataUseOptions, - onDataUseChange, - dataCategoriesOptions, - onDataCategoriesChange, - dataSubjectOptions, - onDataSubjectChange, - } = useDatamapReportFilters(); + isOpen: isFilterModalOpen, + onClose: onFilterModalClose, + onOpen: onFilterModalOpen, + } = useDisclosure(); const { getDataUseDisplayName, @@ -242,20 +235,12 @@ export const DatamapReportTable = () => { isLoading: isLoadingFidesLang, } = useTaxonomies(); - const selectedDataUseFilters = useMemo( - () => getQueryParamsFromList(dataUseOptions, "data_uses"), - [dataUseOptions] - ); - - const selectedDataCategoriesFilters = useMemo( - () => getQueryParamsFromList(dataCategoriesOptions, "data_categories"), - [dataCategoriesOptions] - ); - - const selectedDataSubjectFilters = useMemo( - () => getQueryParamsFromList(dataSubjectOptions, "data_subjects"), - [dataSubjectOptions] - ); + const [selectedDataUseFilters, setSelectedDataUseFilters] = + useState(); + const [selectedDataCategoriesFilters, setSelectedDataCategoriesFilters] = + useState(); + const [selectedDataSubjectFilters, setSelectedDataSubjectFilters] = + useState(); const [groupChangeStarted, setGroupChangeStarted] = useState(false); const [globalFilter, setGlobalFilter] = useState(""); @@ -1076,6 +1061,18 @@ export const DatamapReportTable = () => { return ; } + const handleFilterChange = (newFilters: DatamapReportFilterSelections) => { + setSelectedDataUseFilters( + getQueryParamsFromArray(newFilters.dataUses, "data_uses") + ); + setSelectedDataCategoriesFilters( + getQueryParamsFromArray(newFilters.dataCategories, "data_categories") + ); + setSelectedDataSubjectFilters( + getQueryParamsFromArray(newFilters.dataSubjects, "data_subjects") + ); + }; + return ( { Data map report isOpen={isColumnSettingsOpen} @@ -1168,7 +1159,7 @@ export const DatamapReportTable = () => { data-testid="filter-multiple-systems-btn" size="xs" variant="outline" - onClick={onOpen} + onClick={onFilterModalOpen} > Filter From 2a67078ae3e330e4718328894fb4162975ae21bc Mon Sep 17 00:00:00 2001 From: Jason Gill Date: Fri, 3 May 2024 18:09:51 -0600 Subject: [PATCH 2/4] Update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e4ae58f44..7dfff6636a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,9 @@ The types of changes are: - Added ability to export the contents of datamap report [#1545](https://ethyca.atlassian.net/browse/PROD-1545) - Added state persistence across sessions to the datamap report table [#4853](https://github.com/ethyca/fides/pull/4853) +### Changed +- Changed filters on the data map report table to use checkbox collapsible tree view [#4864](https://github.com/ethyca/fides/pull/4864) + ### Fixed - Remove the extra 'white-space: normal' CSS for FidesJS HTML descriptions [#4850](https://github.com/ethyca/fides/pull/4850) - Fixed data map report to display second level names from the taxonomy as primary (bold) label [#4856](https://github.com/ethyca/fides/pull/4856) From b8de28d4cc9c5c670b21020fc4eb63e6cb88fc7e Mon Sep 17 00:00:00 2001 From: Jason Gill Date: Tue, 7 May 2024 16:30:06 -0600 Subject: [PATCH 3/4] fix varying heights on rows w/ or w/o dropdown --- clients/admin-ui/src/features/common/CheckboxTree.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/clients/admin-ui/src/features/common/CheckboxTree.tsx b/clients/admin-ui/src/features/common/CheckboxTree.tsx index 409086929d..305f62b9b0 100644 --- a/clients/admin-ui/src/features/common/CheckboxTree.tsx +++ b/clients/admin-ui/src/features/common/CheckboxTree.tsx @@ -98,6 +98,7 @@ const CheckboxItem = ({ justifyContent="space-between" _hover={{ backgroundColor: "gray.100", cursor: "pointer" }} onClick={() => onExpanded(node)} + minHeight={8} > Date: Tue, 7 May 2024 16:51:00 -0600 Subject: [PATCH 4/4] better param name for util and jsDocs to supplement --- .../__tests__/features/common-utils.test.ts | 4 ++-- clients/admin-ui/src/features/common/utils.ts | 18 +++++++++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/clients/admin-ui/__tests__/features/common-utils.test.ts b/clients/admin-ui/__tests__/features/common-utils.test.ts index bc249c2f24..e4c698ff3a 100644 --- a/clients/admin-ui/__tests__/features/common-utils.test.ts +++ b/clients/admin-ui/__tests__/features/common-utils.test.ts @@ -21,9 +21,9 @@ describe("common utils", () => { expect(getQueryParamsFromArray([], "test")).toBeUndefined(); }); it("should return query params from strings", () => { - const strings = ["a", "b", "c"]; + const valueList = ["a", "b", "c"]; const queryParam = "test"; - expect(getQueryParamsFromArray(strings, queryParam)).toEqual( + expect(getQueryParamsFromArray(valueList, queryParam)).toEqual( "test=a&test=b&test=c" ); }); diff --git a/clients/admin-ui/src/features/common/utils.ts b/clients/admin-ui/src/features/common/utils.ts index 499f045cd5..a589175833 100644 --- a/clients/admin-ui/src/features/common/utils.ts +++ b/clients/admin-ui/src/features/common/utils.ts @@ -48,12 +48,24 @@ export const getFileNameFromContentDisposition = ( return match ? match[1] : defaultName; }; +/** + * Constructs a query string from an array of values. + * + * @param valueList - An array of string values to be included in the query string. + * @param queryParam - The name of the query parameter. + * @returns A query string where each value from the array is assigned to the query parameter. + * If the array is empty, the function returns undefined. + * + * @example + * getQueryParamsFromArray(['1', '2', '3'], 'id'); + * // returns 'id=1&id=2&id=3' + */ export const getQueryParamsFromArray = ( - strings: string[], + valueList: string[], queryParam: string ) => { - if (strings.length > 0) { - return `${queryParam}=${strings.join(`&${queryParam}=`)}`; + if (valueList.length > 0) { + return `${queryParam}=${valueList.join(`&${queryParam}=`)}`; } return undefined; };