🐛 Analysis - Update the TaskGroup to id selected rulesets #2034

sjd78 · 2024-08-01T01:25:29Z

Supports: https://issues.redhat.com/browse/MTA-3330

The way custom targets are handled by the analysis wizard has been updated. There are no visual changes.

At analysis start time, a Taskgroup is submitted containing all of the user's choices. To better support custom targets, the ruleSet of any selected custom targets will no be submitted as part of the Taskgroup.TaskData.rules. Identifying the ruleSet will allow the custom target's rules to be enabled in analysis.

In support of this change:

The wizard's form data now stores a Target[] instead of a number[] to make working with the selected targets more convenient across the wizard components.
Any code working with just the targetId has been updated to use the full Target object
The handlers for changes to the target labels on the wizard's options step better handle mapping the selected label changes to target card selection

codecov · 2024-08-01T01:31:08Z

Codecov Report

Attention: Patch coverage is 18.91892% with 30 lines in your changes missing coverage. Please review.

Project coverage is 42.37%. Comparing base (b654645) to head (a38bd49).
Report is 207 commits behind head on main.

Files	Patch %	Lines
...rc/app/pages/applications/analysis-wizard/utils.ts	20.00%	15 Missing and 1 partial ⚠️
...pages/applications/analysis-wizard/set-options.tsx	22.22%	7 Missing ⚠️
...s/applications/analysis-wizard/analysis-wizard.tsx	0.00%	4 Missing ⚠️
...pages/applications/analysis-wizard/set-targets.tsx	25.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2034      +/-   ##
==========================================
+ Coverage   39.20%   42.37%   +3.17%     
==========================================
  Files         146      172      +26     
  Lines        4857     5520     +663     
  Branches     1164     1360     +196     
==========================================
+ Hits         1904     2339     +435     
- Misses       2939     3065     +126     
- Partials       14      116     +102

Flag	Coverage Δ
client	`42.37% <18.91%> (+3.17%)`	⬆️
server	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

https://issues.redhat.com/browse/MTA-3330 A custom target that references a repository will be created with a related `RuleSet`. A repository may be defined on the ruleSet. An analysis task defines _included_ labels which determine which rulesets are pulled (downloaded) from the hub. Since ruleSets that reference repositories do not have labels, the addon did not know to download it and clone the repository. Solution: The new rules.ruleSets will be used to pass a list of RuleSet (refs) selected by the user (by selecting targets). Each will be be unconditionally fetched by the addon. For each ruleset with a repository the addon will: - clone the repository - update .yaml files - replacing the `labels` element with `konveyor.io/include=always`. related: konveyor/tackle2-ui#2034 --------- Signed-off-by: Jeff Ortel <[email protected]>

https://issues.redhat.com/browse/MTA-3330 A custom target that references a repository will be created with a related `RuleSet`. A repository may be defined on the ruleSet. An analysis task defines _included_ labels which determine which rulesets are pulled (downloaded) from the hub. Since ruleSets that reference repositories do not have labels, the addon did not know to download it and clone the repository. Solution: The new rules.ruleSets will be used to pass a list of RuleSet (refs) selected by the user (by selecting targets). Each will be be unconditionally fetched by the addon. For each ruleset with a repository the addon will: - clone the repository - update .yaml files - replacing the `labels` element with `konveyor.io/include=always`. related: konveyor/tackle2-ui#2034 --------- Signed-off-by: Jeff Ortel <[email protected]> Signed-off-by: Cherry Picker <[email protected]>

Signed-off-by: Scott J Dickerson <[email protected]>

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]>

https://issues.redhat.com/browse/MTA-3330 A custom target that references a repository will be created with a related `RuleSet`. A repository may be defined on the ruleSet. An analysis task defines _included_ labels which determine which rulesets are pulled (downloaded) from the hub. Since ruleSets that reference repositories do not have labels, the addon did not know to download it and clone the repository. Solution: The new rules.ruleSets will be used to pass a list of RuleSet (refs) selected by the user (by selecting targets). Each will be be unconditionally fetched by the addon. For each ruleset with a repository the addon will: - clone the repository - update .yaml files - replacing the `labels` element with `konveyor.io/include=always`. related: konveyor/tackle2-ui#2034 --------- Signed-off-by: Jeff Ortel <[email protected]> Signed-off-by: Cherry Picker <[email protected]> Signed-off-by: Jeff Ortel <[email protected]> Signed-off-by: Cherry Picker <[email protected]> Co-authored-by: Jeff Ortel <[email protected]>

rszwajko

Looks OK! Standard analysis works fine.

rszwajko · 2024-08-01T15:05:19Z

client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx

+    const ruleSetRefsFromSelectedTargets: Ref[] = fieldValues.selectedTargets
+      .map(({ ruleset }) => ruleset)
+      .filter(Boolean)
+      .map<Ref>(({ id, name }) => ({ id: id ?? 0, name: name ?? "" }));


can zero ID be used by some existing rule set? if yes then I would go with safer value.

Nope. Ids start at 1.

rszwajko · 2024-08-01T15:52:29Z

client/src/app/pages/applications/analysis-wizard/set-options.tsx

-                }
+                const updatedFormLabels = !matchingLabel
+                  ? formLabels
+                  : toggle(formLabels, matchingLabel, (tl) => tl.label);


Supports: https://issues.redhat.com/browse/MTA-3330 The way custom targets are handled by the analysis wizard has been updated. There are no visual changes. At analysis start time, a `Taskgroup` is submitted containing all of the user's choices. To better support custom targets, the `ruleSet` of any selected custom targets will no be submitted as part of the `Taskgroup.TaskData.rules`. Identifying the `ruleSet` will allow the custom target's rules to be enabled in analysis. In support of this change: - The wizard's form data now stores a `Target[]` instead of a `number[]` to make working with the selected targets more convenient across the wizard components. - Any code working with just the `targetId` has been updated to use the full `Target` object - The handlers for changes to the target labels on the wizard's options step better handle mapping the selected label changes to target card selection --------- Signed-off-by: Scott J Dickerson <[email protected]> Signed-off-by: Cherry Picker <[email protected]>

…ed rulesets (#2034) (#2042) Backport-of: #2034 Supports: https://issues.redhat.com/browse/MTA-3330 The way custom targets are handled by the analysis wizard has been updated. There are no visual changes. At analysis start time, a `Taskgroup` is submitted containing all of the user's choices. To better support custom targets, the `ruleSet` of any selected custom targets will no be submitted as part of the `Taskgroup.TaskData.rules`. Identifying the `ruleSet` will allow the custom target's rules to be enabled in analysis. In support of this change: - The wizard's form data now stores a `Target[]` instead of a `number[]` to make working with the selected targets more convenient across the wizard components. - Any code working with just the `targetId` has been updated to use the full `Target` object - The handlers for changes to the target labels on the wizard's options step better handle mapping the selected label changes to target card selection --------- Signed-off-by: Scott J Dickerson <[email protected]> Signed-off-by: Cherry Picker <[email protected]>

sjd78 requested review from ibolton336 and rszwajko as code owners August 1, 2024 01:25

jortel mentioned this pull request Aug 1, 2024

🐛 Fix Targets (Rulesets) with repositories. konveyor/tackle2-addon-analyzer#118

Merged

konveyor-ci-bot bot mentioned this pull request Aug 1, 2024

🐛 Fix Targets (Rulesets) with repositories. (#118) konveyor/tackle2-addon-analyzer#119

Merged

sjd78 added 3 commits August 1, 2024 12:24

🐛 Analysis - Update the TaskGroup to id selected rulesets

934cc39

Signed-off-by: Scott J Dickerson <[email protected]>

s/updateSelectedTargets/toggleSelectedTargets/

bebdc17

Signed-off-by: Scott J Dickerson <[email protected]>

sjd78 force-pushed the analysis_with_custom_targets branch from d478e86 to ed417ab Compare August 1, 2024 16:24

sjd78 added the cherry-pick/release-0.5 This PR should be cherry-picked to release-0.5 branch. label Aug 1, 2024

sjd78 added this to the v0.5.1 milestone Aug 1, 2024

rszwajko approved these changes Aug 1, 2024

View reviewed changes

sjd78 added 2 commits August 1, 2024 17:53

Merge branch 'main' into analysis_with_custom_targets

c05126a

Merge branch 'main' into analysis_with_custom_targets

a38bd49

sjd78 merged commit e920acb into konveyor:main Aug 2, 2024
12 checks passed

sjd78 deleted the analysis_with_custom_targets branch August 2, 2024 13:48

sjd78 mentioned this pull request Aug 2, 2024

🐛 [backport release-0.5] Analysis - Update the TaskGroup to id selected rulesets (#2034) #2042

Merged

sjd78 mentioned this pull request Aug 20, 2024

Empty custom rule allows for an empty item to be created in the targets list in analysis wizard #1076

Open

This was referenced Oct 1, 2024

operator konveyor-operator (0.6.0-alpha.1) k8s-operatorhub/community-operators#5094

Merged

operator konveyor-operator (0.6.0-alpha.1) redhat-openshift-ecosystem/community-operators-prod#5272

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 Analysis - Update the TaskGroup to id selected rulesets #2034

🐛 Analysis - Update the TaskGroup to id selected rulesets #2034

sjd78 commented Aug 1, 2024

codecov bot commented Aug 1, 2024 •

edited

Loading

rszwajko left a comment

rszwajko Aug 1, 2024

sjd78 Aug 1, 2024

rszwajko Aug 1, 2024

🐛 Analysis - Update the TaskGroup to id selected rulesets #2034

🐛 Analysis - Update the TaskGroup to id selected rulesets #2034

Conversation

sjd78 commented Aug 1, 2024

codecov bot commented Aug 1, 2024 • edited Loading

Codecov Report

rszwajko left a comment

Choose a reason for hiding this comment

rszwajko Aug 1, 2024

Choose a reason for hiding this comment

sjd78 Aug 1, 2024

Choose a reason for hiding this comment

rszwajko Aug 1, 2024

Choose a reason for hiding this comment

codecov bot commented Aug 1, 2024 •

edited

Loading