From b4e92dd8da07d874ebe018a9f994e4c81959fb66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Gr=C3=A4=C3=9Fl?= Date: Mon, 27 Jan 2025 11:24:43 +0100 Subject: [PATCH] feat(Tailorings): Implement group selection (#2306) --- .../useAsyncTableTools/useAsyncTableTools.js | 2 +- .../__snapshots__/useBulkSelect.test.js.snap | 5 ++ .../hooks/useBulkSelect/useBulkSelect.js | 2 +- .../hooks/useTableView/helpers.js | 53 ++++++++++++++++ .../hooks/useTableView/views/index.js | 26 +++----- .../hooks/useTableView/views/treeChopper.js | 62 ++++++++++--------- .../RulesTable/RulesTableRest.js | 3 +- .../Tailorings/components/TailoringTab.js | 9 +-- .../Tailorings/helpers.js | 3 +- .../Tailorings/helpers.test.js | 44 +++++++------ .../EditPolicyProfilesRulesRest.js | 1 - 11 files changed, 128 insertions(+), 82 deletions(-) create mode 100644 src/Frameworks/AsyncTableTools/hooks/useTableView/helpers.js diff --git a/src/Frameworks/AsyncTableTools/hooks/useAsyncTableTools/useAsyncTableTools.js b/src/Frameworks/AsyncTableTools/hooks/useAsyncTableTools/useAsyncTableTools.js index 527c7865f..d6c897c6b 100644 --- a/src/Frameworks/AsyncTableTools/hooks/useAsyncTableTools/useAsyncTableTools.js +++ b/src/Frameworks/AsyncTableTools/hooks/useAsyncTableTools/useAsyncTableTools.js @@ -131,10 +131,10 @@ const useAsyncTableTools = (items, columns, options = {}) => { ...sortableTableProps, ...bulkSelectTableProps, ...expandableTableProps, - ...tableViewTableProps, ...tablePropsOption, onSelect: bulkSelectTableProps?.onSelect || tablePropsOption?.onSelect, actionResolver: actionResolverEnabled && actionResolver, + ...tableViewTableProps, }), [ managedColumns, diff --git a/src/Frameworks/AsyncTableTools/hooks/useBulkSelect/__snapshots__/useBulkSelect.test.js.snap b/src/Frameworks/AsyncTableTools/hooks/useBulkSelect/__snapshots__/useBulkSelect.test.js.snap index 44cb31337..b5282eee2 100644 --- a/src/Frameworks/AsyncTableTools/hooks/useBulkSelect/__snapshots__/useBulkSelect.test.js.snap +++ b/src/Frameworks/AsyncTableTools/hooks/useBulkSelect/__snapshots__/useBulkSelect.test.js.snap @@ -8,7 +8,12 @@ exports[`useBulkSelect returns a bulk select configuration 1`] = ` "onSelect": undefined, }, "tableView": { + "clear": [Function], + "deselect": [Function], "markRowSelected": [Function], + "select": [Function], + "selectOne": [Function], + "set": [Function], }, "toolbarProps": { "bulkSelect": { diff --git a/src/Frameworks/AsyncTableTools/hooks/useBulkSelect/useBulkSelect.js b/src/Frameworks/AsyncTableTools/hooks/useBulkSelect/useBulkSelect.js index 8b1316100..48e25b507 100644 --- a/src/Frameworks/AsyncTableTools/hooks/useBulkSelect/useBulkSelect.js +++ b/src/Frameworks/AsyncTableTools/hooks/useBulkSelect/useBulkSelect.js @@ -94,7 +94,7 @@ const useBulkSelect = ({ return enableBulkSelect ? { - tableView: { markRowSelected }, + tableView: { markRowSelected, selectOne, set, select, deselect, clear }, tableProps: { onSelect: total > 0 ? selectOne : undefined, canSelectAll: false, diff --git a/src/Frameworks/AsyncTableTools/hooks/useTableView/helpers.js b/src/Frameworks/AsyncTableTools/hooks/useTableView/helpers.js new file mode 100644 index 000000000..044149024 --- /dev/null +++ b/src/Frameworks/AsyncTableTools/hooks/useTableView/helpers.js @@ -0,0 +1,53 @@ +import { treeRow } from '@patternfly/react-table'; + +export const treeColumns = (columns, onCollapse, onSelect) => [ + { + ...columns[0], + cellTransforms: [ + ...(columns[0].cellTransforms || []), + treeRow( + (...args) => onCollapse?.(...args), + onSelect && ((...args) => onSelect?.(...args)) + ), + ], + }, + ...columns.slice(1), +]; + +export const collectLeaves = (tableTree, itemId) => { + const pickBranch = (basket, branch, _idx, _arr, inBranchArg) => { + const inBranch = inBranchArg || (itemId ? branch.itemId === itemId : true); + const twigLeaves = branch?.twigs?.flatMap((twig) => + pickBranch([], twig, _idx, _arr, inBranch) + ); + + return [ + ...basket, + ...(twigLeaves || []), + ...(inBranch ? branch.leaves || [] : []), + ...(inBranch ? (branch.leave ? [branch.leave] : []) : []), + ]; + }; + + return tableTree.reduce(pickBranch, []); +}; + +export const getOnTreeSelect = (options) => { + const { select, deselect } = options.bulkSelect || {}; + + return ( + options.bulkSelect && + ((...args) => { + const row = args[4]; + const idsForSelect = row.isTreeBranch + ? collectLeaves(options.tableTree, row.itemId).map( + ({ itemId }) => itemId + ) + : row.itemId; + + !(row.props?.isChecked || row.isChecked) + ? select(idsForSelect) + : deselect(idsForSelect); + }) + ); +}; diff --git a/src/Frameworks/AsyncTableTools/hooks/useTableView/views/index.js b/src/Frameworks/AsyncTableTools/hooks/useTableView/views/index.js index 983ae1090..caacaf155 100644 --- a/src/Frameworks/AsyncTableTools/hooks/useTableView/views/index.js +++ b/src/Frameworks/AsyncTableTools/hooks/useTableView/views/index.js @@ -1,26 +1,11 @@ import React from 'react'; import { ListIcon, TreeviewIcon } from '@patternfly/react-icons'; import { Spinner } from '@patternfly/react-core'; -import { treeRow } from '@patternfly/react-table'; import NoResultsTable from 'Utilities/hooks/useTableTools/Components/NoResultsTable'; - +import { treeColumns, getOnTreeSelect } from '../helpers'; import rowsBuilder from './rowsBuilder'; import treeChopper from './treeChopper'; -const treeColumns = (columns, onCollapse) => [ - { - ...columns[0], - cellTransforms: [ - ...(columns[0].cellTransforms || []), - treeRow( - (...args) => onCollapse?.(...args) - // TODO add Selection feature - ), - ], - }, - ...columns.slice(1), -]; - const views = { loading: { tableProps: (_items, columns) => ({ @@ -70,19 +55,26 @@ const views = { tree: { tableProps: (items, columns, options) => { const rows = treeChopper(items, columns, options); - const cells = treeColumns(columns, options.expandable.onCollapse); + const onSelect = getOnTreeSelect(options); + const cells = treeColumns( + columns, + options.expandable?.onCollapse, + options.bulkSelect && onSelect + ); return rows ? { cells, rows, isTreeTable: true, + onSelect: undefined, } : {}; }, icon: TreeviewIcon, toolbarProps: () => ({ variant: 'compact', + bulkSelect: undefined, }), checkOptions: ({ tableTree }) => !!tableTree, }, diff --git a/src/Frameworks/AsyncTableTools/hooks/useTableView/views/treeChopper.js b/src/Frameworks/AsyncTableTools/hooks/useTableView/views/treeChopper.js index 770846309..18b95c73d 100644 --- a/src/Frameworks/AsyncTableTools/hooks/useTableView/views/treeChopper.js +++ b/src/Frameworks/AsyncTableTools/hooks/useTableView/views/treeChopper.js @@ -27,6 +27,8 @@ const childRowForItem = (item, _idx, DetailsComponent, colSpan) => ({ ), props: { ...(colSpan ? { colSpan } : {}), + // TODO This removes the checkbox, however this should maybe be fixed differently + className: 'compliance-rule-details', }, }, ], @@ -78,7 +80,7 @@ const buildTreeBranch = ( ...item, ...(items?.find(({ id }) => id === item.itemId) || {}), props: { - // ...(selectable ? { isChecked: item.rowProps?.selected } : {}), + ...(selectable ? { isChecked: item.isChecked } : {}), isExpanded: openItems.includes(item.itemId) || false, 'aria-level': nextLevel, 'aria-setsize': 1, @@ -114,27 +116,27 @@ const buildTreeBranch = ( }) : []; - // const isChecked = () => { - // const anySprouts = leaves.length > 0 || twigs.length > 0; - // const allSprouts = [...(twigs || []), ...(leaves || [])]; - // if ( - // anySprouts && - // allSprouts - // .filter(({ props: { isDetailsRow } }) => !isDetailsRow) - // .every((leaf) => leaf.props.isChecked === true) - // ) { - // return true; - // } - // - // if ( - // anySprouts && - // allSprouts.some((leave) => leave.props.isChecked === true) - // ) { - // return null; - // } - // - // return false; - // }; + const isChecked = () => { + const anySprouts = leaves.length > 0 || twigs.length > 0; + const allSprouts = [...(twigs || []), ...(leaves || [])]; + if ( + anySprouts && + allSprouts + .filter(({ props: { isDetailsRow } }) => !isDetailsRow) + .every((leaf) => leaf.props.isChecked === true) + ) { + return true; + } + + if ( + anySprouts && + allSprouts.some((leave) => leave.props.isChecked === true) + ) { + return null; + } + + return false; + }; const branchRow = level === 1 || twigs.length > 0 || leaves.length > 0 @@ -150,11 +152,11 @@ const buildTreeBranch = ( isTreeBranch: true, isExpanded, props: { - // ...(selectable - // ? { - // isChecked: isChecked(), - // } - // : {}), + ...(selectable + ? { + isChecked: isChecked(), + } + : {}), isExpanded, 'aria-level': level, 'aria-setsize': @@ -210,7 +212,7 @@ const treeChopper = (items, columns, options = {}) => { const { tableTree, expandable: { openItems } = {}, - // bulkSelect: { markRowSelected } = {}, // TODO Enable selection for groups and rows in groups + bulkSelect: { markRowSelected } = {}, detailsComponent, } = options; @@ -219,9 +221,9 @@ const treeChopper = (items, columns, options = {}) => { items, columns, openItems, - [], + [markRowSelected], detailsComponent, - false + true ); return choppedTree; diff --git a/src/PresentationalComponents/RulesTable/RulesTableRest.js b/src/PresentationalComponents/RulesTable/RulesTableRest.js index 03d763cb8..fb93f3db6 100644 --- a/src/PresentationalComponents/RulesTable/RulesTableRest.js +++ b/src/PresentationalComponents/RulesTable/RulesTableRest.js @@ -60,7 +60,6 @@ const RulesTable = ({ function Row(props) { // eslint-disable-next-line react/prop-types const { itemId, valueDefinitions } = props?.item || {}; - const rule = rules?.find(({ id }) => itemId === id); const ruleValueDefinitions = rule?.value_checks?.map((checkId) => valueDefinitions?.data?.find(({ id }) => id === checkId) @@ -108,7 +107,7 @@ const RulesTable = ({ ]); const itemsWithValueDefinitions = useMemo( - () => rules?.map((rule) => ({ ...rule, rowProps: { valueDefinitions } })), + () => rules?.map((rule) => ({ ...rule, valueDefinitions })), [rules, valueDefinitions] ); diff --git a/src/PresentationalComponents/Tailorings/components/TailoringTab.js b/src/PresentationalComponents/Tailorings/components/TailoringTab.js index 91ec141cd..4d168a0c9 100644 --- a/src/PresentationalComponents/Tailorings/components/TailoringTab.js +++ b/src/PresentationalComponents/Tailorings/components/TailoringTab.js @@ -82,7 +82,8 @@ const TailoringTab = ({ ruleGroups && (tailoringRuleTree || securityGuideRuleTree) ? buildTreeTable( tailoringRuleTree || securityGuideRuleTree, - ruleGroups?.data + ruleGroups?.data, + preselected ) : undefined; @@ -151,11 +152,7 @@ const TailoringTab = ({ }} onRuleValueReset={onRuleValueReset} onValueOverrideSave={onValueSave} - onSelect={ - onSelect && tableState?.tableState?.tableView !== 'tree' - ? onSelectRule - : undefined - } + onSelect={onSelect ? onSelectRule : undefined} selectedRules={preselected} options={{ exporter, diff --git a/src/PresentationalComponents/Tailorings/helpers.js b/src/PresentationalComponents/Tailorings/helpers.js index d9de73dc4..ca77fd459 100644 --- a/src/PresentationalComponents/Tailorings/helpers.js +++ b/src/PresentationalComponents/Tailorings/helpers.js @@ -1,7 +1,7 @@ export const eventKey = ({ id, os_minor_version }) => `tailoring-${id}-${os_minor_version}`; -export const buildTreeTable = (ruleTree, ruleGroups) => { +export const buildTreeTable = (ruleTree, ruleGroups, selected) => { const growTree = (ruleTree) => ruleTree .map((branch) => { @@ -22,6 +22,7 @@ export const buildTreeTable = (ruleTree, ruleGroups) => { } else { return { itemId: branch.id, + ...(selected ? { isChecked: selected.includes(branch.id) } : {}), ...branch, }; } diff --git a/src/PresentationalComponents/Tailorings/helpers.test.js b/src/PresentationalComponents/Tailorings/helpers.test.js index 24992ad30..22e460780 100644 --- a/src/PresentationalComponents/Tailorings/helpers.test.js +++ b/src/PresentationalComponents/Tailorings/helpers.test.js @@ -10,7 +10,7 @@ describe('buildTreeTable', () => { }); }); -describe.skip('skips', () => { +describe('skips', () => { const policy = {}; const tailoring = {}; const securityGuide = {}; @@ -50,7 +50,7 @@ describe.skip('skips', () => { }); }); - it('should return an object with all skip options true if a tableState is present, but no policy, tailoring or SSG', () => { + it('should return an object with all skip options true, except SSG, if a tableState is present, but no policy, tailoring or SSG', () => { expect( skips({ policy: undefined, @@ -61,9 +61,9 @@ describe.skip('skips', () => { }) ).toEqual({ securityGuide: { - ruleGroups: true, - ruleTree: true, - rules: true, + ruleGroups: false, + ruleTree: false, + rules: false, valueDefinitions: true, profile: { rules: true, @@ -90,12 +90,12 @@ describe.skip('skips', () => { ).toEqual({ securityGuide: { ruleGroups: false, - ruleTree: true, - rules: true, + ruleTree: false, + rules: false, valueDefinitions: true, profile: { - rules: true, - ruleTree: true, + rules: false, + ruleTree: false, }, }, tailoring: { @@ -119,8 +119,8 @@ describe.skip('skips', () => { ).toEqual({ securityGuide: { ruleGroups: false, - ruleTree: true, - rules: true, + ruleTree: false, + rules: false, valueDefinitions: true, profile: { rules: false, @@ -128,8 +128,8 @@ describe.skip('skips', () => { }, }, tailoring: { - rules: true, - ruleTree: true, + rules: false, + ruleTree: false, }, }); }); @@ -156,13 +156,13 @@ describe.skip('skips', () => { rules: false, valueDefinitions: true, profile: { - rules: true, - ruleTree: true, + rules: false, + ruleTree: false, }, }, tailoring: { - rules: true, - ruleTree: true, + rules: false, + ruleTree: false, }, }); }); @@ -185,8 +185,8 @@ describe.skip('skips', () => { ).toEqual({ securityGuide: { ruleGroups: false, - ruleTree: true, - rules: true, + ruleTree: false, + rules: false, valueDefinitions: false, profile: { rules: true, @@ -201,9 +201,7 @@ describe.skip('skips', () => { }); // Same as above, but with NO open items - // TODO When rules are skipped it'll result in the items being undefined in case the treeview is active. - // When the tree view is active no rules should be fetched until a group is expanded - it.skip('should skip tailoring rules if there are no open items', () => { + it('should skip tailoring rules if there are no open items', () => { expect( skips({ policy, @@ -220,7 +218,7 @@ describe.skip('skips', () => { ).toEqual({ securityGuide: { ruleGroups: false, - ruleTree: true, + ruleTree: false, rules: true, valueDefinitions: true, profile: { diff --git a/src/SmartComponents/CreatePolicy/EditPolicyProfilesRules/EditPolicyProfilesRulesRest.js b/src/SmartComponents/CreatePolicy/EditPolicyProfilesRules/EditPolicyProfilesRulesRest.js index 97ed869ce..eb4449cca 100644 --- a/src/SmartComponents/CreatePolicy/EditPolicyProfilesRules/EditPolicyProfilesRulesRest.js +++ b/src/SmartComponents/CreatePolicy/EditPolicyProfilesRules/EditPolicyProfilesRulesRest.js @@ -187,7 +187,6 @@ export const EditPolicyProfilesRulesRest = ({ preselected={preselected} enableSecurityGuideRulesToggle rulesPageLink={true} - defaultTableView="rows" onValueOverrideSave={onValueOverrideSave} selectedVersionCounts={osMinorVersionCounts.reduce( (prev, cur) => ({