✨ Identified risks enhancements #1573

ibolton336 · 2023-11-27T19:53:41Z

Resolves:
https://issues.redhat.com/browse/MTA-1723
https://issues.redhat.com/browse/MTA-1749
https://issues.redhat.com/browse/MTA-1754
https://issues.redhat.com/browse/MTA-1762

Resolves: #1568
Resolves: #1570
Resolves: #1569

New query to collate all apps that are inheriting a specific assessment based on underlying archetype inheritance. Use this new data to fix the applications column in the identified risks table to accurately reflect which applications have a specific risk level for a given answer.
Adds the ability to filter the applications table by the applications shown within the identified risks column.
Adds expandable functionality for rationale and mitigation text
Adds risk column for answer risk level
Link to risk filtered app inventory from landscape in reports page

Update-reports.mov

codecov · 2023-11-27T20:33:31Z

Codecov Report

Attention: 23 lines in your changes are missing coverage. Please review.

Comparison is base (835bf11) 39.55% compared to head (d057567) 39.39%.
Report is 1 commits behind head on main.

Files	Patch %	Lines
client/src/app/queries/assessments.ts	10.52%	17 Missing ⚠️
...p/hooks/table-controls/filtering/useFilterState.ts	0.00%	3 Missing ⚠️
client/src/app/utils/model-utils.tsx	25.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1573      +/-   ##
==========================================
- Coverage   39.55%   39.39%   -0.16%     
==========================================
  Files         146      146              
  Lines        4786     4810      +24     
  Branches     1143     1152       +9     
==========================================
+ Hits         1893     1895       +2     
- Misses       2879     2901      +22     
  Partials       14       14

Flag	Coverage Δ
client	`39.39% <11.53%> (-0.16%)`	⬇️
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.

sjd78

Looks ok, just a few questions/suggestions

The "fancy" query hook is really the core of this improvement. And as structured, it could be refactored to the backend doing that work at some point in future as well.

client/src/app/hooks/table-controls/filtering/useFilterState.ts

client/src/app/pages/reports/components/identified-risks-table/identified-risks-table.tsx

client/src/app/queries/assessments.ts

ibolton336 · 2023-11-29T16:36:51Z

client/src/app/components/FilterToolbar/MultiselectFilterControl.tsx

@@ -138,8 +138,6 @@ export const MultiselectFilterControl = <TItem,>({
    const input = textInput?.toLowerCase();

    return renderSelectOptions((optionProps, groupName) => {
-      if (!input) return false;


Needed to remove this to fix the select options not rendering on first render after converting the app name filter to multiselect for the app inventory table.

…tions Signed-off-by: ibolton336 <ibolton@redhat.com>

Port over initial filter values change Properly serialize & deserialize filters to match server side implementation Signed-off-by: ibolton336 <ibolton@redhat.com>

Signed-off-by: ibolton336 <ibolton@redhat.com>

… same question Signed-off-by: ibolton336 <ibolton@redhat.com>

Signed-off-by: ibolton336 <ibolton@redhat.com>

sjd78

Handful of minor non-blocking comments, but looks really good. Everything works in a dev env.

I can see a followup request for filtering on the identified risks table. Trying to look at a single assessment if multiple exist, especially if the assessment you want to look at is not alphabetically first, will be annoying. (Well that is assuming that mere users get annoyed at the same things I do...). To be enhanced in a future PR.

sjd78 · 2023-11-30T20:27:01Z

client/src/app/queries/assessments.ts

+        queryFn: () =>
+          archetypeId ? getArchetypeById(archetypeId) : undefined,
+        enabled: !!archetypeId,


With the filtering in place, archetypeId will always be a real number, so this can be simplified:

Suggested change

queryFn: () =>

archetypeId ? getArchetypeById(archetypeId) : undefined,

enabled: !!archetypeId,

queryFn: () => getArchetypeById(archetypeId),

(not blocking)

sjd78 · 2023-11-30T20:57:29Z

client/src/app/pages/reports/reports.tsx

                    <TextContent>
-                      <Text component="h3">{t("terms.currentLandscape")}</Text>
+                      <Flex>


The screen shots have the questionnaire dropdown top right in the donut chart card. The code has the drop down next to the "Current landscape" title itself. Not a big difference, but I want to make sure it is currently in the correct place.

Screenshot:

sjd78 · 2023-11-30T21:01:02Z

client/src/app/pages/reports/reports.tsx

-                <Card isExpanded={isRiskCardOpen}>
-                  <CardHeader
-                    onExpand={() => setIsRiskCardOpen((current) => !current)}
-                  >
                    <CardTitle>


Since the expandy card is simply just always opened now, the extra layout around the header text doesn't make sense.

(not blocking)

sjd78 · 2023-11-30T21:07:13Z

client/src/app/pages/review/review-page.tsx

+            <AssessmentLandscape
+              questionnaire={null}
+              assessmentRefs={
+                application?.assessments || archetype?.assessments
+              }
+            />
+            <IdentifiedRisksTable
+              assessmentRefs={
+                application?.assessments || archetype?.assessments
+              }


Wrap in a CardBody so the padding matches the report page.

The landscape looks ok, but the risks table is taking up 100% width of the card.

Review page:

Reports page:

Followup changes to konveyor#1573: - Simplify the assessment risk query - Simplify the header for reports / identified risk card header - Fixup layout of the review page assessment summary card Signed-off-by: Scott J Dickerson <sdickers@redhat.com>

Followup changes to #1573: - Simplify the assessment risk query - Simplify the header for reports / identified risk card header - Fixup layout of the review page assessment summary card Signed-off-by: Scott J Dickerson <sdickers@redhat.com>

ibolton336 changed the title ~~Identified risks enhancements~~ ✨ Identified risks enhancements Nov 27, 2023

ibolton336 marked this pull request as ready for review November 27, 2023 20:05

ibolton336 force-pushed the identified-risks-enhancements branch from 1ff3205 to 7a6667b Compare November 27, 2023 20:22

ibolton336 force-pushed the identified-risks-enhancements branch 2 times, most recently from 4f44b49 to 561117e Compare November 27, 2023 23:28

sjd78 requested changes Nov 28, 2023

View reviewed changes

ibolton336 force-pushed the identified-risks-enhancements branch from c01bf98 to 554ce14 Compare November 28, 2023 21:06

ibolton336 marked this pull request as draft November 28, 2023 21:08

ibolton336 marked this pull request as ready for review November 29, 2023 16:35

ibolton336 commented Nov 29, 2023

View reviewed changes

ibolton336 force-pushed the identified-risks-enhancements branch from 7d4d9d8 to e3ce6bc Compare November 29, 2023 16:48

ibolton336 added 13 commits November 29, 2023 14:04

✨ Enhance identified-risks table to include archetype apps in calcula…

e8666b5

…tions Signed-off-by: ibolton336 <ibolton@redhat.com>

Update count to reflect apps inheriting assessments from archetypes

4ca6b66

Port over initial filter values change Properly serialize & deserialize filters to match server side implementation Signed-off-by: ibolton336 <ibolton@redhat.com>

Add expandable for rationale and mitigation

4d40495

Signed-off-by: ibolton336 <ibolton@redhat.com>

Add risk column to table with icon

10b6ec3

Signed-off-by: ibolton336 <ibolton@redhat.com>

Add identified risks table to review page

b7bcf7d

Signed-off-by: ibolton336 <ibolton@redhat.com>

Add landscape to review page

beb6f4d

Signed-off-by: ibolton336 <ibolton@redhat.com>

remove old cards

e2b647b

Signed-off-by: ibolton336 <ibolton@redhat.com>

Add new application landscape logic

720ebc0

Signed-off-by: ibolton336 <ibolton@redhat.com>

Add nullish coalescing and fix risk id leading to dupe table rows for…

e07e0ba

… same question Signed-off-by: ibolton336 <ibolton@redhat.com>

Refactor fetch assessments with archetype applications query

1e95f91

Signed-off-by: ibolton336 <ibolton@redhat.com>

Naming fix and get() usage improvement

09756be

Signed-off-by: ibolton336 <ibolton@redhat.com>

Consolidate donut usage

487d980

Signed-off-by: ibolton336 <ibolton@redhat.com>

Cleanup assessmentRefs filter logic

64c5b56

Signed-off-by: ibolton336 <ibolton@redhat.com>

ibolton336 force-pushed the identified-risks-enhancements branch from 9712c12 to 8a2c498 Compare November 29, 2023 19:04

Add risks link from report

dd86b64

Signed-off-by: ibolton336 <ibolton@redhat.com>

ibolton336 force-pushed the identified-risks-enhancements branch from 8a2c498 to dd86b64 Compare November 29, 2023 19:05

ibolton336 added 2 commits November 29, 2023 14:21

Move dropdown to right of title as requested by UXD

294aba7

Signed-off-by: ibolton336 <ibolton@redhat.com>

Update filter values

b083796

Signed-off-by: ibolton336 <ibolton@redhat.com>

Merge branch 'main' into identified-risks-enhancements

d057567

sjd78 approved these changes Nov 30, 2023

View reviewed changes

sjd78 merged commit 85ac7b4 into konveyor:main Nov 30, 2023
10 checks passed

sjd78 mentioned this pull request Nov 30, 2023

🐛 Refactoring to follow up PR #1573 comments #1578

Merged

This was referenced Dec 11, 2023

operator konveyor-operator (0.3.0-rc.1) k8s-operatorhub/community-operators#3613

Merged

operator konveyor-operator (0.3.0-rc.1) redhat-openshift-ecosystem/community-operators-prod#3714

Merged

This was referenced Jan 23, 2024

operator konveyor-operator (0.3.0) k8s-operatorhub/community-operators#3793

Merged

operator konveyor-operator (0.3.0) redhat-openshift-ecosystem/community-operators-prod#3927

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Identified risks enhancements #1573

✨ Identified risks enhancements #1573

ibolton336 commented Nov 27, 2023 •

edited by sjd78

Loading

codecov bot commented Nov 27, 2023 •

edited

Loading

sjd78 left a comment •

edited

Loading

ibolton336 Nov 29, 2023

sjd78 left a comment

sjd78 Nov 30, 2023

sjd78 Nov 30, 2023 •

edited

Loading

sjd78 Nov 30, 2023 •

edited

Loading

sjd78 Nov 30, 2023

✨ Identified risks enhancements #1573

✨ Identified risks enhancements #1573

Conversation

ibolton336 commented Nov 27, 2023 • edited by sjd78 Loading

codecov bot commented Nov 27, 2023 • edited Loading

Codecov Report

sjd78 left a comment • edited Loading

Choose a reason for hiding this comment

ibolton336 Nov 29, 2023

Choose a reason for hiding this comment

sjd78 left a comment

Choose a reason for hiding this comment

sjd78 Nov 30, 2023

Choose a reason for hiding this comment

sjd78 Nov 30, 2023 • edited Loading

Choose a reason for hiding this comment

sjd78 Nov 30, 2023 • edited Loading

Choose a reason for hiding this comment

sjd78 Nov 30, 2023

Choose a reason for hiding this comment

ibolton336 commented Nov 27, 2023 •

edited by sjd78

Loading

codecov bot commented Nov 27, 2023 •

edited

Loading

sjd78 left a comment •

edited

Loading

sjd78 Nov 30, 2023 •

edited

Loading

sjd78 Nov 30, 2023 •

edited

Loading