Repository: opossum-tool/OpossumUI
Pull request #2625 (2024-04-24 12:45:08)

feat: ignore hidden signals in project statistics
### Summary of changes

We reverted the changes from #2617 because the performance of the project statistics popup was too bad. This PR adds the feature back in again, but we need to fix the performance issues.

### Context and reason for change

fixes issue #2227
(cherry picked from commit b50de5ff2fb08d84a4051800d058c1ff2b0c0b2c)



Patch:
diff --git a/package.json b/package.json
index 7aed6b39..9cd3aba8 100644
--- a/package.json
+++ b/package.json
@@ -17,7 +17,7 @@
     "compare-versions": "^6.1.0",
     "dayjs": "^1.11.10",
     "electron-log": "^5.1.2",
-    "electron-settings": "^4.0.4",
+    "electron-settings": "^4.0.3",
     "fast-csv": "^5.0.1",
     "fflate": "^0.8.2",
     "js-yaml": "^4.1.0",
@@ -53,7 +53,7 @@
     "@tanstack/eslint-plugin-query": "^5.28.11",
     "@testing-library/dom": "^10.0.0",
     "@testing-library/jest-dom": "^6.4.2",
-    "@testing-library/react": "^15.0.4",
+    "@testing-library/react": "^15.0.2",
     "@testing-library/user-event": "^14.5.2",
     "@trivago/prettier-plugin-sort-imports": "^4.3.0",
     "@types/babel__core": "^7.20.5",
diff --git a/src/Frontend/Components/CriticalLicensesTable/CriticalLicensesTable.tsx b/src/Frontend/Components/CriticalLicensesTable/CriticalLicensesTable.tsx
index 3399e422..6680686e 100644
--- a/src/Frontend/Components/CriticalLicensesTable/CriticalLicensesTable.tsx
+++ b/src/Frontend/Components/CriticalLicensesTable/CriticalLicensesTable.tsx
@@ -6,16 +6,16 @@ import { sortBy } from 'lodash';
 import { ReactElement } from 'react';
 
 import { Criticality } from '../../../shared/shared-types';
+import { text } from '../../../shared/text';
 import { clickableIcon } from '../../shared-styles';
 import { LicenseNamesWithCriticality } from '../../types/types';
 import { ProjectLicensesTable } from '../ProjectLicensesTable/ProjectLicensesTable';
 
 const LICENSE_COLUMN_NAME_IN_TABLE = 'License name';
-const COUNT_COLUMN_NAME_IN_TABLE = 'Signals Count';
 const FOOTER_TITLE = 'Total';
 const TABLE_COLUMN_NAMES = [
   LICENSE_COLUMN_NAME_IN_TABLE,
-  COUNT_COLUMN_NAME_IN_TABLE,
+  text.projectStatisticsPopup.criticalLicensesSignalCountColumnName,
 ];
 
 const classes = {
@@ -93,7 +93,9 @@ export function CriticalLicensesTable(
           ({ licenseName, totalNumberOfAttributions }) => [
             licenseName,
             {
-              [COUNT_COLUMN_NAME_IN_TABLE]: totalNumberOfAttributions,
+              [text.projectStatisticsPopup
+                .criticalLicensesSignalCountColumnName]:
+                totalNumberOfAttributions,
             },
           ],
         ),
diff --git a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx
index db1e4544..c55ebebb 100644
--- a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx
+++ b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx
@@ -11,9 +11,9 @@ import { ProjectStatisticsPopupTitle } from '../../enums/enums';
 import { closePopup } from '../../state/actions/view-actions/view-actions';
 import { useAppDispatch, useAppSelector } from '../../state/hooks';
 import {
-  getExternalAttributions,
   getExternalAttributionSources,
   getManualAttributions,
+  getUnresolvedExternalAttributions,
 } from '../../state/selectors/resource-selectors';
 import { useUserSetting } from '../../state/variables/use-user-setting';
 import { AccordionWithPieChart } from '../AccordionWithPieChart/AccordionWithPieChart';
@@ -41,15 +41,19 @@ export function ProjectStatisticsPopup(): ReactElement {
   const dispatch = useAppDispatch();
 
   const manualAttributions = useAppSelector(getManualAttributions);
-  const externalAttributions = useAppSelector(getExternalAttributions);
   const attributionSources = useAppSelector(getExternalAttributionSources);
 
-  const strippedLicenseNameToAttribution =
-    getUniqueLicenseNameToAttribution(externalAttributions);
+  const unresolvedExternalAttribution = useAppSelector(
+    getUnresolvedExternalAttributions,
+  );
+
+  const strippedLicenseNameToAttribution = getUniqueLicenseNameToAttribution(
+    unresolvedExternalAttribution,
+  );
 
   const { licenseCounts, licenseNamesWithCriticality } =
     aggregateLicensesAndSourcesFromAttributions(
-      externalAttributions,
+      unresolvedExternalAttribution,
       strippedLicenseNameToAttribution,
       attributionSources,
     );
diff --git a/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNode.tsx b/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNode.tsx
index 570d771c..5c1d93ac 100644
--- a/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNode.tsx
+++ b/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNode.tsx
@@ -86,6 +86,7 @@ export function ResourcesTreeNode({ node, nodeId, nodeName }: TreeNode) {
         nodeId,
         resourcesToExternalAttributions,
         externalAttributions,
+        resolvedExternalAttributions,
       )}
       isAttributionBreakpoint={attributionBreakpoints.has(nodeId)}
       showFolderIcon={canHaveChildren && !filesWithChildren.has(nodeId)}
diff --git a/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNode.util.ts b/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNode.util.ts
index 18fa095e..caf63410 100644
--- a/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNode.util.ts
+++ b/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNode.util.ts
@@ -16,11 +16,19 @@ export function getCriticality(
   nodeId: string,
   resourcesToExternalAttributions: ResourcesToAttributions,
   externalAttributions: Attributions,
+  resolvedExternalAttributions: Set<string>,
 ): Criticality | undefined {
-  if (hasExternalAttribution(nodeId, resourcesToExternalAttributions)) {
+  if (
+    hasUnresolvedExternalAttribution(
+      nodeId,
+      resourcesToExternalAttributions,
+      resolvedExternalAttributions,
+    )
+  ) {
     const attributionsForResource = resourcesToExternalAttributions[nodeId];
     for (const attributionId of attributionsForResource) {
       if (
+        !resolvedExternalAttributions.has(attributionId) &&
         externalAttributions[attributionId].criticality === Criticality.High
       ) {
         return Criticality.High;
@@ -29,6 +37,7 @@ export function getCriticality(
 
     for (const attributionId of attributionsForResource) {
       if (
+        !resolvedExternalAttributions.has(attributionId) &&
         externalAttributions[attributionId].criticality === Criticality.Medium
       ) {
         return Criticality.Medium;
diff --git a/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNodeLabel/ResourcesTreeNodeLabel.tsx b/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNodeLabel/ResourcesTreeNodeLabel.tsx
index 12e137bc..50fdd7b6 100644
--- a/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNodeLabel/ResourcesTreeNodeLabel.tsx
+++ b/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNodeLabel/ResourcesTreeNodeLabel.tsx
@@ -93,7 +93,7 @@ export function ResourcesTreeNodeLabel(props: Props): ReactElement {
       >
         {props.labelText}
       </MuiTypography>
-      {props.hasExternalAttribution &&
+      {props.hasUnresolvedExternalAttribution &&
         (props.criticality ? (
           <CriticalityIcon
             criticality={props.criticality}
diff --git a/src/Frontend/state/selectors/resource-selectors.ts b/src/Frontend/state/selectors/resource-selectors.ts
index 386cc97d..23771942 100644
--- a/src/Frontend/state/selectors/resource-selectors.ts
+++ b/src/Frontend/state/selectors/resource-selectors.ts
@@ -50,6 +50,15 @@ export function getResolvedExternalAttributions(state: State): Set<string> {
   return state.resourceState.resolvedExternalAttributions;
 }
 
+export function getUnresolvedExternalAttributions(state: State): Attributions {
+  return Object.fromEntries(
+    Object.entries(state.resourceState.externalData.attributions).filter(
+      ([attributionId]) =>
+        !state.resourceState.resolvedExternalAttributions.has(attributionId),
+    ),
+  );
+}
+
 export function getExpandedIds(state: State): Array<string> {
   return state.resourceState.expandedIds;
 }
diff --git a/src/shared/text.ts b/src/shared/text.ts
index b35d6a64..02151661 100644
--- a/src/shared/text.ts
+++ b/src/shared/text.ts
@@ -102,6 +102,7 @@ export const text = {
   projectStatisticsPopup: {
     title: 'Project Statistics',
     toggleStartupCheckbox: 'Show project statistics on startup',
+    criticalLicensesSignalCountColumnName: 'Signals Count',
   },
   unsavedChangesPopup: {
     title: 'Unsaved Changes',
diff --git a/yarn.lock b/yarn.lock
index ea112d49..dd4b1f93 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -3300,9 +3300,9 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@testing-library/react@npm:^15.0.4":
-  version: 15.0.4
-  resolution: "@testing-library/react@npm:15.0.4"
+"@testing-library/react@npm:^15.0.2":
+  version: 15.0.2
+  resolution: "@testing-library/react@npm:15.0.2"
   dependencies:
     "@babel/runtime": "npm:^7.12.5"
     "@testing-library/dom": "npm:^10.0.0"
@@ -3310,7 +3310,7 @@ __metadata:
   peerDependencies:
     react: ^18.0.0
     react-dom: ^18.0.0
-  checksum: 10/c4362b753408e43a4527a5bdeda447fcb0d37d1764591c2e8a2be12001081d6ccb618904a8f3e328c35a9a37b3561fed3e520e780cf318bd68031c51ad7873e4
+  checksum: 10/6327e465733cec9455792acee7781d1437f7494ddbb976ca72e07446daaf8e7fe5dadd27d05690f09e8d4adce651984ed9aeca4ede9b87bf69b172f450085ede
   languageName: node
   linkType: hard
 
@@ -6010,16 +6010,16 @@ __metadata:
   languageName: node
   linkType: hard
 
-"electron-settings@npm:^4.0.4":
-  version: 4.0.4
-  resolution: "electron-settings@npm:4.0.4"
+"electron-settings@npm:^4.0.3":
+  version: 4.0.3
+  resolution: "electron-settings@npm:4.0.3"
   dependencies:
     lodash: "npm:^4.17.21"
     mkdirp: "npm:^1.0.4"
     write-file-atomic: "npm:^3.0.3"
   peerDependencies:
     electron: ">= 2"
-  checksum: 10/a7fe4615f4cbef6a257828cc4f61abcf66d07a87f35eaf38fc7051572a9650ee1e0846f5ba4c12de9c782235fcfc7cd2c72d73c65e37b7ceaf1810a66dbc5976
+  checksum: 10/7e212861c98196213a3ad4e4118b8c7276232604c9682c385b07222631e8d6b684570cc9a915ae63c7abc8a04688c0e4efbc797ea370c6840925baa71229bf4c
   languageName: node
   linkType: hard
 
@@ -9797,7 +9797,7 @@ __metadata:
     "@tanstack/react-query": "npm:^5.32.0"
     "@testing-library/dom": "npm:^10.0.0"
     "@testing-library/jest-dom": "npm:^6.4.2"
-    "@testing-library/react": "npm:^15.0.4"
+    "@testing-library/react": "npm:^15.0.2"
     "@testing-library/user-event": "npm:^14.5.2"
     "@trivago/prettier-plugin-sort-imports": "npm:^4.3.0"
     "@types/babel__core": "npm:^7.20.5"
@@ -9825,7 +9825,7 @@ __metadata:
     electron-builder: "npm:^24.13.3"
     electron-log: "npm:^5.1.2"
     electron-playwright-helpers: "npm:^1.7.1"
-    electron-settings: "npm:^4.0.4"
+    electron-settings: "npm:^4.0.3"
     eslint: "npm:^8.57.0"
     eslint-config-prettier: "npm:^9.1.0"
     eslint-plugin-filenames: "npm:^1.3.2"


Test patch:
diff --git a/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNodeLabel/__tests__/ResourcesTreeNodeLabel.test.tsx b/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNodeLabel/__tests__/ResourcesTreeNodeLabel.test.tsx
index 0751ab22..1e45d43e 100644
--- a/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNodeLabel/__tests__/ResourcesTreeNodeLabel.test.tsx
+++ b/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNodeLabel/__tests__/ResourcesTreeNodeLabel.test.tsx
@@ -80,7 +80,7 @@ describe('ResourcesTreeNodeLabel', () => {
     ).toBeInTheDocument();
   });
 
-  it('renders a folder with resolved signal and icon', () => {
+  it('renders a folder with resolved signal but without criticality icon', () => {
     render(
       <ResourcesTreeNodeLabel
         labelText={'Test label'}
@@ -99,7 +99,6 @@ describe('ResourcesTreeNodeLabel', () => {
     );
 
     expect(screen.getByText('Test label')).toBeInTheDocument();
-    expect(screen.getByLabelText('Criticality icon')).toBeInTheDocument();
     expect(
       screen.getByLabelText('Directory icon without information'),
     ).toBeInTheDocument();
diff --git a/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/__tests__/ResourcesTreeNode.util.test.ts b/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/__tests__/ResourcesTreeNode.util.test.ts
index 3f7a0fc7..9e508f8f 100644
--- a/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/__tests__/ResourcesTreeNode.util.test.ts
+++ b/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/__tests__/ResourcesTreeNode.util.test.ts
@@ -38,6 +38,7 @@ describe('ResourcesTreeNode', () => {
         nodeId,
         resourcesToExternalAttributions,
         externalAttributions,
+        new Set(),
       );
       expect(criticality).toEqual(expectedCriticalities[nodeId]);
     }
diff --git a/src/Frontend/Components/ResourceBrowser/ResourcesTree/__tests__/ResourcesTree.test.tsx b/src/Frontend/Components/ResourceBrowser/ResourcesTree/__tests__/ResourcesTree.test.tsx
index e14ec6b1..83a425bd 100644
--- a/src/Frontend/Components/ResourceBrowser/ResourcesTree/__tests__/ResourcesTree.test.tsx
+++ b/src/Frontend/Components/ResourceBrowser/ResourcesTree/__tests__/ResourcesTree.test.tsx
@@ -206,7 +206,7 @@ describe('ResourcesTree', () => {
     act(() => {
       store.dispatch(addResolvedExternalAttributions([testUuid]));
     });
-    expectIconToExist(screen, 'Criticality icon', 'src', true);
+    expectIconToExist(screen, 'Criticality icon', 'src', false);
     expectResourceIconLabelToBe(
       screen,
       'src',
diff --git a/src/e2e-tests/__tests__/project-statistics.test.ts b/src/e2e-tests/__tests__/project-statistics.test.ts
new file mode 100644
index 00000000..d1f91b57
--- /dev/null
+++ b/src/e2e-tests/__tests__/project-statistics.test.ts
@@ -0,0 +1,71 @@
+// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates
+// SPDX-FileCopyrightText: TNG Technology Consulting GmbH <https://www.tngtech.com>
+//
+// SPDX-License-Identifier: Apache-2.0
+import { Criticality } from '../../shared/shared-types';
+import { faker, test } from '../utils';
+
+const [resourceName1] = faker.opossum.resourceNames({ count: 1 });
+const [attributionId1, packageInfo1] = faker.opossum.rawAttribution({
+  criticality: Criticality.Medium,
+});
+const [attributionId3, packageInfo3] = faker.opossum.rawAttribution({
+  criticality: Criticality.Medium,
+});
+test.use({
+  data: {
+    inputData: faker.opossum.inputData({
+      resources: faker.opossum.resources({
+        [resourceName1]: 1,
+      }),
+      externalAttributions: faker.opossum.rawAttributions({
+        [attributionId1]: packageInfo1,
+        [attributionId3]: packageInfo3,
+      }),
+      resourcesToAttributions: faker.opossum.resourcesToAttributions({
+        [faker.opossum.filePath(resourceName1)]: [
+          attributionId1,
+          attributionId3,
+        ],
+      }),
+    }),
+    outputData: faker.opossum.outputData({}),
+  },
+});
+
+test('opens, displays, and closes project statistics', async ({
+  menuBar,
+  projectStatisticsPopup,
+}) => {
+  await menuBar.openProjectStatistics();
+  await projectStatisticsPopup.assert.titleIsVisible();
+
+  await projectStatisticsPopup.closeButton.click();
+  await projectStatisticsPopup.assert.titleIsHidden();
+});
+
+test('hidden signals are ignored for project statistics', async ({
+  menuBar,
+  projectStatisticsPopup,
+  resourcesTree,
+  signalsPanel,
+  attributionDetails,
+}) => {
+  await menuBar.openProjectStatistics();
+  await projectStatisticsPopup.assert.titleIsVisible();
+
+  await projectStatisticsPopup.assert.criticalLicenseCount(2);
+
+  await projectStatisticsPopup.closeButton.click();
+  await resourcesTree.goto(resourceName1);
+
+  await signalsPanel.packageCard.assert.isVisible(packageInfo1);
+  await signalsPanel.packageCard.assert.isVisible(packageInfo3);
+
+  await signalsPanel.packageCard.click(packageInfo3);
+  await attributionDetails.deleteButton.click();
+  await signalsPanel.packageCard.assert.isHidden(packageInfo3);
+
+  await menuBar.openProjectStatistics();
+  await projectStatisticsPopup.assert.criticalLicenseCount(1);
+});
diff --git a/src/e2e-tests/page-objects/ProjectStatisticsPopup.ts b/src/e2e-tests/page-objects/ProjectStatisticsPopup.ts
index 13aadaa9..624bdfbc 100644
--- a/src/e2e-tests/page-objects/ProjectStatisticsPopup.ts
+++ b/src/e2e-tests/page-objects/ProjectStatisticsPopup.ts
@@ -4,15 +4,28 @@
 // SPDX-License-Identifier: Apache-2.0
 import { expect, type Locator, type Page } from '@playwright/test';
 
+import { text } from '../../shared/text';
+
 export class ProjectStatisticsPopup {
   private readonly node: Locator;
   readonly title: Locator;
   readonly closeButton: Locator;
+  readonly totalCriticalLicensesCount: Locator;
 
   constructor(window: Page) {
     this.node = window.getByLabel('project statistics');
-    this.title = this.node.getByText('Project Statistics');
+    this.title = this.node.getByRole('heading').getByText('Project Statistics');
     this.closeButton = this.node.getByRole('button', { name: 'Close' });
+    const signalsCount = window.getByText(
+      text.projectStatisticsPopup.criticalLicensesSignalCountColumnName,
+    );
+    this.totalCriticalLicensesCount = this.node
+      .getByRole('table')
+      .filter({ has: signalsCount })
+      .getByRole('row')
+      .last()
+      .getByRole('cell')
+      .last();
   }
 
   public assert = {
@@ -22,5 +35,10 @@ export class ProjectStatisticsPopup {
     titleIsHidden: async (): Promise<void> => {
       await expect(this.title).toBeHidden();
     },
+    criticalLicenseCount: async (count: number): Promise<void> => {
+      await expect(this.totalCriticalLicensesCount).toContainText(
+        count.toString(),
+      );
+    },
   };
 }