Skip to content

Commit

Permalink
Fix the options target label selection code
Browse files Browse the repository at this point in the history
  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 <[email protected]>
  • Loading branch information
sjd78 committed Aug 1, 2024
1 parent bebdc17 commit ed417ab
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -226,11 +226,13 @@ export const AnalysisWizard: React.FC<IAnalysisWizard> = ({
);

const ruleSetRefsFromSelectedTargets: Ref[] = fieldValues.selectedTargets
.filter((target) => !!target.ruleset)
.map<Ref>(({ ruleset }) => ({
id: ruleset.id ?? -1,
name: ruleset.name ?? "",
}));
.map(({ ruleset }) => ruleset)
.filter(Boolean)
.map<Ref>(({ 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<Ruleset> or similar to avoid these issues.

return {
...currentTaskgroup,
Expand Down
32 changes: 9 additions & 23 deletions client/src/app/pages/applications/analysis-wizard/set-options.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,16 +107,15 @@ export const SetTargets: React.FC<SetTargetsProps> = ({ applications }) => {
target,
selectedTargets
);
setValue("selectedTargets", updatedSelectedTargets);

const updatedFormLabels = getUpdatedFormLabels(
isSelecting,
selectedLabelName,
target,
formLabels
);

setValue("formLabels", updatedFormLabels);
setValue("selectedTargets", updatedSelectedTargets);
};

const allProviders = targets.flatMap((target) => target.provider);
Expand Down
57 changes: 30 additions & 27 deletions client/src/app/pages/applications/analysis-wizard/utils.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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];
};

0 comments on commit ed417ab

Please sign in to comment.