From 934cc39d7d3a138d5a8d1387b4ff7fe6d24b718a Mon Sep 17 00:00:00 2001 From: Scott J Dickerson Date: Wed, 31 Jul 2024 09:38:14 -0400 Subject: [PATCH 1/3] :bug: Analysis - Update the TaskGroup to id selected rulesets Signed-off-by: Scott J Dickerson --- client/src/app/api/models.ts | 1 + .../analysis-wizard/analysis-wizard.tsx | 12 ++++++ .../applications/analysis-wizard/schema.ts | 4 +- .../analysis-wizard/set-targets.tsx | 4 +- .../applications/analysis-wizard/utils.ts | 43 +++++++++++-------- 5 files changed, 41 insertions(+), 23 deletions(-) diff --git a/client/src/app/api/models.ts b/client/src/app/api/models.ts index e3047d8abd..e1a2f83ac0 100644 --- a/client/src/app/api/models.ts +++ b/client/src/app/api/models.ts @@ -416,6 +416,7 @@ export interface TaskData { included: string[]; excluded: string[]; }; + ruleSets?: Ref[]; // Target.ruleset.{ id, name } }; } diff --git a/client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx b/client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx index 64f784393d..ab4c891acf 100644 --- a/client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx +++ b/client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx @@ -15,6 +15,7 @@ import { useTranslation } from "react-i18next"; import { Application, New, + Ref, TaskData, Taskgroup, TaskgroupTask, @@ -223,6 +224,14 @@ export const AnalysisWizard: React.FC = ({ const matchingSourceCredential = identities.find( (identity) => identity.name === fieldValues.associatedCredentials ); + + const selectedTargetsWithRuleset: Ref[] = fieldValues.selectedTargets + .filter((target) => !!target.ruleset) + .map(({ ruleset }) => ({ + id: ruleset.id ?? -1, + name: ruleset.name ?? "", + })); + return { ...currentTaskgroup, tasks: analyzableApplications.map((app: Application) => initTask(app)), @@ -286,6 +295,9 @@ export const AnalysisWizard: React.FC = ({ name: matchingSourceCredential.name, }, }), + ...(selectedTargetsWithRuleset.length > 0 && { + ruleSets: selectedTargetsWithRuleset, + }), }, }, }; diff --git a/client/src/app/pages/applications/analysis-wizard/schema.ts b/client/src/app/pages/applications/analysis-wizard/schema.ts index 9b6682f8f6..770dbff5a2 100644 --- a/client/src/app/pages/applications/analysis-wizard/schema.ts +++ b/client/src/app/pages/applications/analysis-wizard/schema.ts @@ -4,6 +4,7 @@ import { IReadFile, FileLoadError, TargetLabel, + Target, } from "@app/api/models"; import { useTranslation } from "react-i18next"; import { useAnalyzableApplicationsByMode } from "./utils"; @@ -55,7 +56,7 @@ const useModeStepSchema = ({ export interface TargetsStepValues { formLabels: TargetLabel[]; - selectedTargets: number[]; + selectedTargets: Target[]; } const useTargetsStepSchema = (): yup.SchemaOf => { @@ -114,7 +115,6 @@ export const customRulesFilesSchema: yup.SchemaOf = yup.object({ }); const useCustomRulesStepSchema = (): yup.SchemaOf => { - const { t } = useTranslation(); return yup.object({ rulesKind: yup.string().defined(), customRulesFiles: yup diff --git a/client/src/app/pages/applications/analysis-wizard/set-targets.tsx b/client/src/app/pages/applications/analysis-wizard/set-targets.tsx index f4c419a16d..2fbc20fae6 100644 --- a/client/src/app/pages/applications/analysis-wizard/set-targets.tsx +++ b/client/src/app/pages/applications/analysis-wizard/set-targets.tsx @@ -104,7 +104,7 @@ export const SetTargets: React.FC = ({ applications }) => { target: Target ) => { const updatedSelectedTargets = updateSelectedTargets( - target.id, + target, selectedTargets ); @@ -176,7 +176,7 @@ export const SetTargets: React.FC = ({ applications }) => { id === target.id)} onSelectedCardTargetChange={(selectedTarget) => { handleOnSelectedCardTargetChange(selectedTarget); }} diff --git a/client/src/app/pages/applications/analysis-wizard/utils.ts b/client/src/app/pages/applications/analysis-wizard/utils.ts index 07494fc4c4..2c3124f7c6 100644 --- a/client/src/app/pages/applications/analysis-wizard/utils.ts +++ b/client/src/app/pages/applications/analysis-wizard/utils.ts @@ -1,6 +1,7 @@ import * as React from "react"; import { Application, Target, TargetLabel } from "@app/api/models"; import { AnalysisMode, ANALYSIS_MODES } from "./schema"; +import { toggle } from "radash"; export const isApplicationBinaryEnabled = ( application: Application @@ -61,14 +62,14 @@ export const useAnalyzableApplicationsByMode = ( [applications] ); +/** + * Toggle the existence of a target within the array and return the array + */ export const updateSelectedTargets = ( - targetId: number, - selectedTargetIDs: number[] -) => { - const isSelected = selectedTargetIDs.includes(targetId); - return isSelected - ? selectedTargetIDs.filter((id) => id !== targetId) - : [...selectedTargetIDs, targetId]; + target: Target, + selectedTargets: Target[] +): Target[] => { + return toggle(selectedTargets, target, (t) => t.id); }; export const getUpdatedFormLabels = ( @@ -106,33 +107,37 @@ export const findLabelBySelector = (labels: TargetLabel[], selector: string) => export const isLabelInFormLabels = (formLabels: TargetLabel[], label: string) => formLabels.some((formLabel) => formLabel.label === label); -export const labelToTargetId = (labelName: string, targets: Target[]) => { +export const labelToTarget = (labelName: string, targets: Target[]) => { const target = targets.find( (t) => t.labels?.some((l) => l.name === labelName) ); - return target ? target.id : null; + return target ? target : null; }; export const updateSelectedTargetsBasedOnLabels = ( currentFormLabels: TargetLabel[], - selectedTargets: number[], + selectedTargets: Target[], targets: Target[] -) => { +): Target[] => { + const targetsFromLabels = currentFormLabels + .map((label) => labelToTarget(label.name, targets)) + .filter(Boolean); + const newSelectedTargets = currentFormLabels.reduce( - (acc: number[], formLabel) => { - const targetId = labelToTargetId(formLabel.name, targets); - if (targetId && !acc.includes(targetId)) { - acc.push(targetId); + (acc: Target[], formLabel) => { + const target = labelToTarget(formLabel.name, targets); + if (target && !acc.some((v) => v.id === target.id)) { + acc.push(target); } return acc; }, [] ); - const filteredSelectedTargets = selectedTargets.filter((targetId) => - currentFormLabels.some( - (formLabel) => labelToTargetId(formLabel.name, targets) === targetId - ) + // TODO: If a Target doesn't have a label (i.e. is a custom repo), it should + // stick around and not get dropped here + const filteredSelectedTargets = selectedTargets.filter((target) => + targetsFromLabels.some((current) => current?.id === target.id) ); return [...new Set([...newSelectedTargets, ...filteredSelectedTargets])]; From bebdc17633de434c1a790de6d5a171e7c222bfc9 Mon Sep 17 00:00:00 2001 From: Scott J Dickerson Date: Wed, 31 Jul 2024 12:33:32 -0400 Subject: [PATCH 2/3] s/updateSelectedTargets/toggleSelectedTargets/ Signed-off-by: Scott J Dickerson --- .../pages/applications/analysis-wizard/analysis-wizard.tsx | 6 +++--- .../app/pages/applications/analysis-wizard/set-targets.tsx | 4 ++-- client/src/app/pages/applications/analysis-wizard/utils.ts | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx b/client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx index ab4c891acf..41465aa04b 100644 --- a/client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx +++ b/client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx @@ -225,7 +225,7 @@ export const AnalysisWizard: React.FC = ({ (identity) => identity.name === fieldValues.associatedCredentials ); - const selectedTargetsWithRuleset: Ref[] = fieldValues.selectedTargets + const ruleSetRefsFromSelectedTargets: Ref[] = fieldValues.selectedTargets .filter((target) => !!target.ruleset) .map(({ ruleset }) => ({ id: ruleset.id ?? -1, @@ -295,8 +295,8 @@ export const AnalysisWizard: React.FC = ({ name: matchingSourceCredential.name, }, }), - ...(selectedTargetsWithRuleset.length > 0 && { - ruleSets: selectedTargetsWithRuleset, + ...(ruleSetRefsFromSelectedTargets.length > 0 && { + ruleSets: ruleSetRefsFromSelectedTargets, }), }, }, diff --git a/client/src/app/pages/applications/analysis-wizard/set-targets.tsx b/client/src/app/pages/applications/analysis-wizard/set-targets.tsx index 2fbc20fae6..e525fe3438 100644 --- a/client/src/app/pages/applications/analysis-wizard/set-targets.tsx +++ b/client/src/app/pages/applications/analysis-wizard/set-targets.tsx @@ -19,7 +19,7 @@ import { useFetchTargets } from "@app/queries/targets"; import { Application, TagCategory, Target } from "@app/api/models"; import { useFetchTagCategories } from "@app/queries/tags"; import { SimpleSelectCheckbox } from "@app/components/SimpleSelectCheckbox"; -import { getUpdatedFormLabels, updateSelectedTargets } from "./utils"; +import { getUpdatedFormLabels, toggleSelectedTargets } from "./utils"; interface SetTargetsProps { applications: Application[]; @@ -103,7 +103,7 @@ export const SetTargets: React.FC = ({ applications }) => { selectedLabelName: string, target: Target ) => { - const updatedSelectedTargets = updateSelectedTargets( + const updatedSelectedTargets = toggleSelectedTargets( target, selectedTargets ); diff --git a/client/src/app/pages/applications/analysis-wizard/utils.ts b/client/src/app/pages/applications/analysis-wizard/utils.ts index 2c3124f7c6..a6fe9ba995 100644 --- a/client/src/app/pages/applications/analysis-wizard/utils.ts +++ b/client/src/app/pages/applications/analysis-wizard/utils.ts @@ -65,7 +65,7 @@ export const useAnalyzableApplicationsByMode = ( /** * Toggle the existence of a target within the array and return the array */ -export const updateSelectedTargets = ( +export const toggleSelectedTargets = ( target: Target, selectedTargets: Target[] ): Target[] => { From ed417ab01fe5111b8a73695ecb81ebf0d797e923 Mon Sep 17 00:00:00 2001 From: Scott J Dickerson Date: Wed, 31 Jul 2024 21:01:49 -0400 Subject: [PATCH 3/3] Fix the options target label selection code Targets getting selected will add labels. The set of labels isn't visible until the options step. On that step the labels can be changed. When the labels change, the new set of labels need to be mapped back to the targets. The mapping code needs to: - Keep the correct set of target cards selected as per the current set of selected target labels - Keep targets selected that lack any labels (currently only custom source repository targets have no labels) Signed-off-by: Scott J Dickerson --- .../analysis-wizard/analysis-wizard.tsx | 12 ++-- .../analysis-wizard/set-options.tsx | 32 +++-------- .../analysis-wizard/set-targets.tsx | 3 +- .../applications/analysis-wizard/utils.ts | 57 ++++++++++--------- 4 files changed, 47 insertions(+), 57 deletions(-) diff --git a/client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx b/client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx index 41465aa04b..a432b85be9 100644 --- a/client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx +++ b/client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx @@ -226,11 +226,13 @@ export const AnalysisWizard: React.FC = ({ ); const ruleSetRefsFromSelectedTargets: Ref[] = fieldValues.selectedTargets - .filter((target) => !!target.ruleset) - .map(({ ruleset }) => ({ - id: ruleset.id ?? -1, - name: ruleset.name ?? "", - })); + .map(({ ruleset }) => ruleset) + .filter(Boolean) + .map(({ id, name }) => ({ id: id ?? 0, name: name ?? "" })); + // TODO: Type `Ruleset` has the id and name as optional/undefined to support + // object creation. At runtime, id and name will always be defined on + // existing objects. In future, update the Ruleset creation code to use + // New or similar to avoid these issues. return { ...currentTaskgroup, diff --git a/client/src/app/pages/applications/analysis-wizard/set-options.tsx b/client/src/app/pages/applications/analysis-wizard/set-options.tsx index f5597388d1..b67fb45af3 100644 --- a/client/src/app/pages/applications/analysis-wizard/set-options.tsx +++ b/client/src/app/pages/applications/analysis-wizard/set-options.tsx @@ -29,11 +29,8 @@ import { DEFAULT_SELECT_MAX_HEIGHT } from "@app/Constants"; import { useFetchTargets } from "@app/queries/targets"; import defaultSources from "./sources"; import { QuestionCircleIcon } from "@patternfly/react-icons"; -import { - findLabelBySelector, - isLabelInFormLabels, - updateSelectedTargetsBasedOnLabels, -} from "./utils"; +import { updateSelectedTargetsBasedOnLabels } from "./utils"; +import { toggle } from "radash"; export const SetOptions: React.FC = () => { const { t } = useTranslation(); @@ -124,25 +121,14 @@ export const SetOptions: React.FC = () => { selections={targetSelections} isOpen={isSelectTargetsOpen} onSelect={(_, selection) => { - const selectionWithLabelSelector = `konveyor.io/target=${selection}`; - const matchingLabel = findLabelBySelector( - defaultTargetsAndTargetsLabels, - selectionWithLabelSelector + const selectionLabel = `konveyor.io/target=${selection}`; + const matchingLabel = defaultTargetsAndTargetsLabels.find( + (label) => label.label === selectionLabel ); - let updatedFormLabels = []; - if ( - matchingLabel && - !isLabelInFormLabels(formLabels, matchingLabel.label) - ) { - updatedFormLabels = [...formLabels, matchingLabel]; - onChange(updatedFormLabels); - } else { - updatedFormLabels = formLabels.filter( - (formLabel) => - formLabel.label !== selectionWithLabelSelector - ); - onChange(updatedFormLabels); - } + const updatedFormLabels = !matchingLabel + ? formLabels + : toggle(formLabels, matchingLabel, (tl) => tl.label); + onChange(updatedFormLabels); const updatedSelectedTargets = updateSelectedTargetsBasedOnLabels( diff --git a/client/src/app/pages/applications/analysis-wizard/set-targets.tsx b/client/src/app/pages/applications/analysis-wizard/set-targets.tsx index e525fe3438..4faca34b7d 100644 --- a/client/src/app/pages/applications/analysis-wizard/set-targets.tsx +++ b/client/src/app/pages/applications/analysis-wizard/set-targets.tsx @@ -107,6 +107,7 @@ export const SetTargets: React.FC = ({ applications }) => { target, selectedTargets ); + setValue("selectedTargets", updatedSelectedTargets); const updatedFormLabels = getUpdatedFormLabels( isSelecting, @@ -114,9 +115,7 @@ export const SetTargets: React.FC = ({ applications }) => { target, formLabels ); - setValue("formLabels", updatedFormLabels); - setValue("selectedTargets", updatedSelectedTargets); }; const allProviders = targets.flatMap((target) => target.provider); diff --git a/client/src/app/pages/applications/analysis-wizard/utils.ts b/client/src/app/pages/applications/analysis-wizard/utils.ts index a6fe9ba995..d21d682e60 100644 --- a/client/src/app/pages/applications/analysis-wizard/utils.ts +++ b/client/src/app/pages/applications/analysis-wizard/utils.ts @@ -1,7 +1,8 @@ import * as React from "react"; import { Application, Target, TargetLabel } from "@app/api/models"; import { AnalysisMode, ANALYSIS_MODES } from "./schema"; -import { toggle } from "radash"; +import { toggle, unique } from "radash"; +import { getParsedLabel } from "@app/utils/rules-utils"; export const isApplicationBinaryEnabled = ( application: Application @@ -101,44 +102,46 @@ export const getUpdatedFormLabels = ( return otherSelectedLabels; } }; -export const findLabelBySelector = (labels: TargetLabel[], selector: string) => - labels.find((label) => label.label === selector) || ""; -export const isLabelInFormLabels = (formLabels: TargetLabel[], label: string) => - formLabels.some((formLabel) => formLabel.label === label); +/** + * Match a target to a set of target type labels based on if the target supports + * label choice. + */ +const matchTargetToLabels = (target: Target, labels: TargetLabel[]) => { + if (!target.labels?.length) { + return false; + } -export const labelToTarget = (labelName: string, targets: Target[]) => { - const target = targets.find( - (t) => t.labels?.some((l) => l.name === labelName) + const targetTargetLabelCount = target.labels?.reduce( + (count, tl) => + getParsedLabel(tl.label).labelType === "target" ? count + 1 : count, + 0 ); - return target ? target : null; + + const matches = labels + .map((l) => target.labels?.find((tl) => tl.label === l.label) ?? false) + .filter(Boolean).length; + + return target.choice ? matches >= 1 : matches === targetTargetLabelCount; }; +/** + * Given a set of selected labels, return a set of targets where (1) the target's labels + * properly match the select labels or (2) the target is selected but has no labels. + */ export const updateSelectedTargetsBasedOnLabels = ( currentFormLabels: TargetLabel[], selectedTargets: Target[], targets: Target[] ): Target[] => { - const targetsFromLabels = currentFormLabels - .map((label) => labelToTarget(label.name, targets)) - .filter(Boolean); - - const newSelectedTargets = currentFormLabels.reduce( - (acc: Target[], formLabel) => { - const target = labelToTarget(formLabel.name, targets); - if (target && !acc.some((v) => v.id === target.id)) { - acc.push(target); - } - return acc; - }, - [] + const targetsFromLabels = unique( + targets.filter((target) => matchTargetToLabels(target, currentFormLabels)), + (target) => target.id ); - // TODO: If a Target doesn't have a label (i.e. is a custom repo), it should - // stick around and not get dropped here - const filteredSelectedTargets = selectedTargets.filter((target) => - targetsFromLabels.some((current) => current?.id === target.id) + const selectedTargetsWithNoLabel = selectedTargets.filter( + (target) => (target.labels?.length ?? 0) === 0 ); - return [...new Set([...newSelectedTargets, ...filteredSelectedTargets])]; + return [...targetsFromLabels, ...selectedTargetsWithNoLabel]; };