From ac46a357d6b665bd11f5289927312a57b013ff0f Mon Sep 17 00:00:00 2001 From: Philipp Martens Date: Tue, 25 Feb 2025 18:25:53 +0100 Subject: [PATCH 1/6] feat: add pie chart to project statistics popup showing signal count by classification --- .../ProjectStatisticsPopup.tsx | 30 ++++++++--- .../ProjectStatisticsPopup.util.ts | 54 ++++++++++++++++++- src/Frontend/enums/enums.ts | 1 + src/Frontend/types/types.ts | 4 ++ 4 files changed, 81 insertions(+), 8 deletions(-) diff --git a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx index 9c73788eb..a9b1efe0a 100644 --- a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx +++ b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx @@ -10,6 +10,7 @@ import { ProjectStatisticsPopupTitle } from '../../enums/enums'; import { closePopup } from '../../state/actions/view-actions/view-actions'; import { useAppDispatch, useAppSelector } from '../../state/hooks'; import { + getClassifications, getExternalAttributionSources, getManualAttributions, getUnresolvedExternalAttributions, @@ -27,6 +28,7 @@ import { getCriticalSignalsCount, getIncompleteAttributionsCount, getMostFrequentLicenses, + getSignalCountByClassification, getUniqueLicenseNameToAttribution, } from './ProjectStatisticsPopup.util'; @@ -41,6 +43,7 @@ export const ProjectStatisticsPopup: React.FC = () => { const manualAttributions = useAppSelector(getManualAttributions); const attributionSources = useAppSelector(getExternalAttributionSources); + const classifications = useAppSelector(getClassifications); const unresolvedExternalAttribution = useAppSelector( getUnresolvedExternalAttributions, @@ -50,12 +53,15 @@ export const ProjectStatisticsPopup: React.FC = () => { unresolvedExternalAttribution, ); - const { licenseCounts, licenseNamesWithCriticality } = - aggregateLicensesAndSourcesFromAttributions( - unresolvedExternalAttribution, - strippedLicenseNameToAttribution, - attributionSources, - ); + const { + licenseCounts, + licenseNamesWithCriticality, + licenseNamesWithClassification, + } = aggregateLicensesAndSourcesFromAttributions( + unresolvedExternalAttribution, + strippedLicenseNameToAttribution, + attributionSources, + ); const mostFrequentLicenseCountData = getMostFrequentLicenses(licenseCounts); @@ -64,6 +70,12 @@ export const ProjectStatisticsPopup: React.FC = () => { licenseNamesWithCriticality, ); + const signalCountByClassification = getSignalCountByClassification( + licenseCounts, + licenseNamesWithClassification, + classifications, + ); + const manualAttributionPropertyCounts = aggregateAttributionPropertiesFromAttributions(manualAttributions); @@ -121,6 +133,12 @@ export const ProjectStatisticsPopup: React.FC = () => { data={criticalSignalsCountData} title={ProjectStatisticsPopupTitle.CriticalSignalsCountPieChart} /> + // // SPDX-License-Identifier: Apache-2.0 -import { pickBy } from 'lodash'; +import { pickBy, toNumber } from 'lodash'; import { Attributions, + Classifications, Criticality, ExternalAttributionSources, PackageInfo, @@ -14,6 +15,7 @@ import { PieChartCriticalityNames } from '../../enums/enums'; import { AttributionCountPerSourcePerLicense, LicenseCounts, + LicenseNamesWithClassification, LicenseNamesWithCriticality, PieChartData, } from '../../types/types'; @@ -39,11 +41,13 @@ export function aggregateLicensesAndSourcesFromAttributions( ): { licenseCounts: LicenseCounts; licenseNamesWithCriticality: LicenseNamesWithCriticality; + licenseNamesWithClassification: LicenseNamesWithClassification; } { const { attributionCountPerSourcePerLicense, totalAttributionsPerLicense, licenseNamesWithCriticality, + licenseNamesWithClassification, } = getLicenseDataFromAttributionsAndSources( strippedLicenseNameToAttribution, attributions, @@ -67,7 +71,11 @@ export function aggregateLicensesAndSourcesFromAttributions( totalAttributionsPerSource, }; - return { licenseCounts, licenseNamesWithCriticality }; + return { + licenseCounts, + licenseNamesWithCriticality, + licenseNamesWithClassification, + }; } function getLicenseDataFromAttributionsAndSources( @@ -78,8 +86,10 @@ function getLicenseDataFromAttributionsAndSources( attributionCountPerSourcePerLicense: AttributionCountPerSourcePerLicense; totalAttributionsPerLicense: { [licenseName: string]: number }; licenseNamesWithCriticality: LicenseNamesWithCriticality; + licenseNamesWithClassification: LicenseNamesWithClassification; } { const licenseNamesWithCriticality: LicenseNamesWithCriticality = {}; + const licenseNamesWithClassification: LicenseNamesWithClassification = {}; const attributionCountPerSourcePerLicense: AttributionCountPerSourcePerLicense = {}; const totalAttributionsPerLicense: { [licenseName: string]: number } = {}; @@ -90,6 +100,7 @@ function getLicenseDataFromAttributionsAndSources( const { mostFrequentLicenseName, licenseCriticality, + licenseClassification, sourcesCountForLicense, } = getLicenseDataFromVariants( strippedLicenseNameToAttribution[strippedLicenseName], @@ -98,6 +109,8 @@ function getLicenseDataFromAttributionsAndSources( ); licenseNamesWithCriticality[mostFrequentLicenseName] = licenseCriticality; + licenseNamesWithClassification[mostFrequentLicenseName] = + licenseClassification; attributionCountPerSourcePerLicense[mostFrequentLicenseName] = sourcesCountForLicense; totalAttributionsPerLicense[mostFrequentLicenseName] = Object.values( @@ -111,6 +124,7 @@ function getLicenseDataFromAttributionsAndSources( attributionCountPerSourcePerLicense, totalAttributionsPerLicense, licenseNamesWithCriticality, + licenseNamesWithClassification, }; } @@ -121,6 +135,7 @@ function getLicenseDataFromVariants( ): { mostFrequentLicenseName: string; licenseCriticality: Criticality | undefined; + licenseClassification: number; sourcesCountForLicense: { [sourceNameOrTotal: string]: number; }; @@ -133,6 +148,8 @@ function getLicenseDataFromVariants( } = {}; const licenseCriticalityCounts = { high: 0, medium: 0, none: 0 }; + let licenseClassification = 0; + for (const attributionId of attributionIds) { const licenseName = attributions[attributionId].licenseName; if (licenseName) { @@ -141,6 +158,11 @@ function getLicenseDataFromVariants( const variantCriticality = attributions[attributionId].criticality; + licenseClassification = Math.max( + licenseClassification, + attributions[attributionId].classification ?? 0, + ); + licenseCriticalityCounts[variantCriticality || 'none']++; const sourceId = @@ -163,6 +185,7 @@ function getLicenseDataFromVariants( return { mostFrequentLicenseName, licenseCriticality, + licenseClassification, sourcesCountForLicense, }; } @@ -327,6 +350,33 @@ export function getCriticalSignalsCount( ); } +export function getSignalCountByClassification( + licenseCounts: LicenseCounts, + licenseNamesWithClassification: LicenseNamesWithClassification, + classifications: Classifications, +): Array { + const classificationCounts: { [classification: number]: number } = {}; + + for (const [license, attributionCount] of Object.entries( + licenseCounts.totalAttributionsPerLicense, + )) { + classificationCounts[licenseNamesWithClassification[license]] = + (classificationCounts[licenseNamesWithClassification[license]] ?? 0) + + attributionCount; + } + + return Object.keys(classifications).map((classification) => { + const classificationName = classifications[toNumber(classification)]; + const classificationCount = + classificationCounts[toNumber(classification)] ?? 0; + + return { + name: classificationName, + count: classificationCount, + }; + }); +} + export function getIncompleteAttributionsCount( attributions: Attributions, ): Array { diff --git a/src/Frontend/enums/enums.ts b/src/Frontend/enums/enums.ts index 6bc89a97b..01297d24b 100644 --- a/src/Frontend/enums/enums.ts +++ b/src/Frontend/enums/enums.ts @@ -31,6 +31,7 @@ export enum ProjectStatisticsPopupTitle { PieChartsSectionHeader = 'Pie Charts', MostFrequentLicenseCountPieChart = 'Most Frequent Licenses', CriticalSignalsCountPieChart = 'Signals by Criticality', + SignalCountByClassificationPieChart = 'Signals by Classification', IncompleteLicensesPieChart = 'Incomplete Attributions', } diff --git a/src/Frontend/types/types.ts b/src/Frontend/types/types.ts index e291242d0..05a7f9d8a 100644 --- a/src/Frontend/types/types.ts +++ b/src/Frontend/types/types.ts @@ -51,3 +51,7 @@ export interface AttributionCountPerSourcePerLicense { export interface LicenseNamesWithCriticality { [licenseName: string]: Criticality | undefined; } + +export interface LicenseNamesWithClassification { + [licenseName: string]: number; +} From a6ae64d2d91db970d3fb6c80f8908d1599dfef59 Mon Sep 17 00:00:00 2001 From: Philipp Martens Date: Wed, 26 Feb 2025 10:59:28 +0100 Subject: [PATCH 2/6] feat: properly deal with counting signals with undefined classifications --- .../ProjectStatisticsPopup.util.ts | 36 +++++++++++++------ src/Frontend/types/types.ts | 2 +- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.util.ts b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.util.ts index 47448e706..4f698310a 100644 --- a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.util.ts +++ b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.util.ts @@ -135,7 +135,7 @@ function getLicenseDataFromVariants( ): { mostFrequentLicenseName: string; licenseCriticality: Criticality | undefined; - licenseClassification: number; + licenseClassification: number | undefined; sourcesCountForLicense: { [sourceNameOrTotal: string]: number; }; @@ -148,7 +148,7 @@ function getLicenseDataFromVariants( } = {}; const licenseCriticalityCounts = { high: 0, medium: 0, none: 0 }; - let licenseClassification = 0; + let licenseClassification: number | undefined = undefined; for (const attributionId of attributionIds) { const licenseName = attributions[attributionId].licenseName; @@ -158,10 +158,16 @@ function getLicenseDataFromVariants( const variantCriticality = attributions[attributionId].criticality; - licenseClassification = Math.max( - licenseClassification, - attributions[attributionId].classification ?? 0, - ); + const variantClassification = attributions[attributionId].classification; + + if (licenseClassification === undefined) { + licenseClassification = variantClassification; + } else if (variantClassification !== undefined) { + licenseClassification = Math.max( + licenseClassification, + attributions[attributionId].classification ?? 0, + ); + } licenseCriticalityCounts[variantCriticality || 'none']++; @@ -360,12 +366,13 @@ export function getSignalCountByClassification( for (const [license, attributionCount] of Object.entries( licenseCounts.totalAttributionsPerLicense, )) { - classificationCounts[licenseNamesWithClassification[license]] = - (classificationCounts[licenseNamesWithClassification[license]] ?? 0) + - attributionCount; + // count undefined classification at index -1 in classificationCounts + const classification = licenseNamesWithClassification[license] ?? -1; + classificationCounts[classification] = + (classificationCounts[classification] ?? 0) + attributionCount; } - return Object.keys(classifications).map((classification) => { + const pieChartData = Object.keys(classifications).map((classification) => { const classificationName = classifications[toNumber(classification)]; const classificationCount = classificationCounts[toNumber(classification)] ?? 0; @@ -375,6 +382,15 @@ export function getSignalCountByClassification( count: classificationCount, }; }); + + if (classificationCounts[-1]) { + return pieChartData.concat({ + name: 'No Classification', + count: classificationCounts[-1], + }); + } + + return pieChartData; } export function getIncompleteAttributionsCount( diff --git a/src/Frontend/types/types.ts b/src/Frontend/types/types.ts index 05a7f9d8a..757ec497f 100644 --- a/src/Frontend/types/types.ts +++ b/src/Frontend/types/types.ts @@ -53,5 +53,5 @@ export interface LicenseNamesWithCriticality { } export interface LicenseNamesWithClassification { - [licenseName: string]: number; + [licenseName: string]: number | undefined; } From 5aa5c532c00f83e6f318196ee8c0c88b72914f4f Mon Sep 17 00:00:00 2001 From: Philipp Martens Date: Wed, 26 Feb 2025 11:13:32 +0100 Subject: [PATCH 3/6] feat: exclude classifications with no associated signals from pie chart --- .../ProjectStatisticsPopup.util.ts | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.util.ts b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.util.ts index 4f698310a..120594092 100644 --- a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.util.ts +++ b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.util.ts @@ -372,16 +372,18 @@ export function getSignalCountByClassification( (classificationCounts[classification] ?? 0) + attributionCount; } - const pieChartData = Object.keys(classifications).map((classification) => { - const classificationName = classifications[toNumber(classification)]; - const classificationCount = - classificationCounts[toNumber(classification)] ?? 0; - - return { - name: classificationName, - count: classificationCount, - }; - }); + const pieChartData = Object.keys(classifications) + .map((classification) => { + const classificationName = classifications[toNumber(classification)]; + const classificationCount = + classificationCounts[toNumber(classification)] ?? 0; + + return { + name: classificationName, + count: classificationCount, + }; + }) + .filter(({ count }) => count > 0); if (classificationCounts[-1]) { return pieChartData.concat({ From a4f6edae9aff9cb8e1997214b92ad40da45ab3fe Mon Sep 17 00:00:00 2001 From: Philipp Martens Date: Wed, 26 Feb 2025 11:14:29 +0100 Subject: [PATCH 4/6] test: update test for project statistics popup --- .../input/__tests__/importFromFile.test.ts | 16 +++++++-------- .../input/__tests__/parseFile.test.ts | 4 ++-- .../__tests__/ProjectStatisticsPopup.test.tsx | 20 ++++++++++++++++++- .../__tests__/load-actions.test.ts | 4 ++-- .../test-helpers/general-test-helpers.ts | 3 +++ 5 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/ElectronBackend/input/__tests__/importFromFile.test.ts b/src/ElectronBackend/input/__tests__/importFromFile.test.ts index 73c375c32..c302eacd0 100644 --- a/src/ElectronBackend/input/__tests__/importFromFile.test.ts +++ b/src/ElectronBackend/input/__tests__/importFromFile.test.ts @@ -68,8 +68,8 @@ const inputFileContent: ParsedOpossumInputFile = { }, config: { classifications: { - 0: 'UNKNOWN', - 1: 'CRITICAL', + 0: 'GOOD', + 1: 'BAD', }, }, externalAttributions: { @@ -115,8 +115,8 @@ const expectedFileContent: ParsedFileContent = { resources: { a: 1, folder: {} }, config: { classifications: { - 0: 'UNKNOWN', - 1: 'CRITICAL', + 0: 'GOOD', + 1: 'BAD', }, }, manualAttributions: { @@ -376,8 +376,8 @@ describe('Test of loading function', () => { }, config: { classifications: { - 0: 'UNKNOWN', - 1: 'CRITICAL', + 0: 'GOOD', + 1: 'BAD', }, }, externalAttributions: { @@ -438,8 +438,8 @@ describe('Test of loading function', () => { resources: { a: 1 }, config: { classifications: { - 0: 'UNKNOWN', - 1: 'CRITICAL', + 0: 'GOOD', + 1: 'BAD', }, }, manualAttributions: { diff --git a/src/ElectronBackend/input/__tests__/parseFile.test.ts b/src/ElectronBackend/input/__tests__/parseFile.test.ts index 1fa89da99..a65ea9c65 100644 --- a/src/ElectronBackend/input/__tests__/parseFile.test.ts +++ b/src/ElectronBackend/input/__tests__/parseFile.test.ts @@ -27,8 +27,8 @@ const correctInput: ParsedOpossumInputFile = { }, config: { classifications: { - 0: 'UNKNOWN', - 1: 'CRITICAL', + 0: 'GOOD', + 1: 'BAD', }, }, externalAttributions: { diff --git a/src/Frontend/Components/ProjectStatisticsPopup/__tests__/ProjectStatisticsPopup.test.tsx b/src/Frontend/Components/ProjectStatisticsPopup/__tests__/ProjectStatisticsPopup.test.tsx index 6ba6c9521..fa71ab5f4 100644 --- a/src/Frontend/Components/ProjectStatisticsPopup/__tests__/ProjectStatisticsPopup.test.tsx +++ b/src/Frontend/Components/ProjectStatisticsPopup/__tests__/ProjectStatisticsPopup.test.tsx @@ -91,6 +91,7 @@ describe('The ProjectStatisticsPopup', () => { actions: [ loadFromFile( getParsedInputFileEnrichedWithTestData({ + config: { classifications: { 0: 'GOOD', 1: 'BAD' } }, manualAttributions: testManualAttributions, externalAttributions: testExternalAttributions, }), @@ -108,6 +109,11 @@ describe('The ProjectStatisticsPopup', () => { ProjectStatisticsPopupTitle.CriticalSignalsCountPieChart, ), ).toBeInTheDocument(); + expect( + screen.getByText( + ProjectStatisticsPopupTitle.SignalCountByClassificationPieChart, + ), + ).toBeInTheDocument(); expect( screen.getAllByText( ProjectStatisticsPopupTitle.IncompleteLicensesPieChart, @@ -121,6 +127,7 @@ describe('The ProjectStatisticsPopup', () => { actions: [ loadFromFile( getParsedInputFileEnrichedWithTestData({ + config: { classifications: { 0: 'GOOD', 1: 'BAD' } }, externalAttributions: testExternalAttributions, }), ), @@ -136,6 +143,11 @@ describe('The ProjectStatisticsPopup', () => { ProjectStatisticsPopupTitle.CriticalSignalsCountPieChart, ), ).not.toBeInTheDocument(); + expect( + screen.queryByText( + ProjectStatisticsPopupTitle.SignalCountByClassificationPieChart, + ), + ).not.toBeInTheDocument(); expect( screen.getByText(ProjectStatisticsPopupTitle.IncompleteLicensesPieChart), ).toBeInTheDocument(); @@ -146,7 +158,7 @@ describe('The ProjectStatisticsPopup', () => { ).not.toHaveLength(2); }); - it('renders pie charts pie charts related to signals even if there are no attributions', () => { + it('renders pie charts related to signals even if there are no attributions', () => { const testManualAttributions: Attributions = {}; const testExternalAttributions: Attributions = { uuid_1: { @@ -170,6 +182,7 @@ describe('The ProjectStatisticsPopup', () => { actions: [ loadFromFile( getParsedInputFileEnrichedWithTestData({ + config: { classifications: { 0: 'GOOD', 1: 'BAD' } }, manualAttributions: testManualAttributions, externalAttributions: testExternalAttributions, }), @@ -186,6 +199,11 @@ describe('The ProjectStatisticsPopup', () => { ProjectStatisticsPopupTitle.CriticalSignalsCountPieChart, ), ).toBeInTheDocument(); + expect( + screen.getByText( + ProjectStatisticsPopupTitle.SignalCountByClassificationPieChart, + ), + ).toBeInTheDocument(); expect( screen.getByText(ProjectStatisticsPopupTitle.IncompleteLicensesPieChart), ).toBeInTheDocument(); diff --git a/src/Frontend/state/actions/resource-actions/__tests__/load-actions.test.ts b/src/Frontend/state/actions/resource-actions/__tests__/load-actions.test.ts index aede447cb..be9211968 100644 --- a/src/Frontend/state/actions/resource-actions/__tests__/load-actions.test.ts +++ b/src/Frontend/state/actions/resource-actions/__tests__/load-actions.test.ts @@ -49,8 +49,8 @@ const testResources: Resources = { const testConfig: ProjectConfig = { classifications: { - 0: 'UNKNOWN', - 1: 'CRITICAL', + 0: 'GOOD', + 1: 'BAD', }, }; diff --git a/src/Frontend/test-helpers/general-test-helpers.ts b/src/Frontend/test-helpers/general-test-helpers.ts index 7191858fe..96502fa6e 100644 --- a/src/Frontend/test-helpers/general-test-helpers.ts +++ b/src/Frontend/test-helpers/general-test-helpers.ts @@ -10,6 +10,7 @@ import { AttributionsToResources, ExternalAttributionSources, ParsedFileContent, + ProjectConfig, Resources, ResourcesToAttributions, } from '../../shared/shared-types'; @@ -44,6 +45,7 @@ const EMPTY_PARSED_FILE_CONTENT: ParsedFileContent = { export function getParsedInputFileEnrichedWithTestData(testData: { resources?: Resources; + config?: ProjectConfig; manualAttributions?: Attributions; resourcesToManualAttributions?: ResourcesToAttributions; externalAttributions?: Attributions; @@ -76,6 +78,7 @@ export function getParsedInputFileEnrichedWithTestData(testData: { return { ...EMPTY_PARSED_FILE_CONTENT, resources, + ...(testData.config ? { config: testData.config } : {}), manualAttributions: { attributions: testData.manualAttributions || {}, resourcesToAttributions: testResourcesToManualAttributions, From 5dbe24f4d8caf279f330e98ac98dc812407c8fb5 Mon Sep 17 00:00:00 2001 From: Philipp Martens Date: Wed, 26 Feb 2025 11:49:57 +0100 Subject: [PATCH 5/6] refactor: move text for project statistics charts to text.ts file --- .../AccordionWithPieChart.tsx | 9 ++-- .../__tests__/AccordionWithPieChart.test.tsx | 10 ++--- .../ProjectStatisticsPopup.tsx | 26 +++++++---- .../__tests__/ProjectStatisticsPopup.test.tsx | 45 ++++++++----------- src/Frontend/enums/enums.ts | 11 ----- src/shared/text.ts | 10 +++++ 6 files changed, 54 insertions(+), 57 deletions(-) diff --git a/src/Frontend/Components/AccordionWithPieChart/AccordionWithPieChart.tsx b/src/Frontend/Components/AccordionWithPieChart/AccordionWithPieChart.tsx index 1fd04c90f..fdd6dd86c 100644 --- a/src/Frontend/Components/AccordionWithPieChart/AccordionWithPieChart.tsx +++ b/src/Frontend/Components/AccordionWithPieChart/AccordionWithPieChart.tsx @@ -8,10 +8,8 @@ import MuiAccordionDetails from '@mui/material/AccordionDetails'; import MuiAccordionSummary from '@mui/material/AccordionSummary'; import MuiTypography from '@mui/material/Typography'; -import { - PieChartCriticalityNames, - ProjectStatisticsPopupTitle, -} from '../../enums/enums'; +import { text } from '../../../shared/text'; +import { PieChartCriticalityNames } from '../../enums/enums'; import { OpossumColors } from '../../shared-styles'; import { PieChartData } from '../../types/types'; import { PieChart } from '../PieChart/PieChart'; @@ -46,7 +44,8 @@ export function getColorsForPieChart( const pieChartColors: Array = []; if ( - pieChartTitle === ProjectStatisticsPopupTitle.CriticalSignalsCountPieChart + pieChartTitle === + text.projectStatisticsPopup.charts.criticalSignalsCountPieChart ) { for (const pieChartSegment of pieChartData) { switch (pieChartSegment.name) { diff --git a/src/Frontend/Components/AccordionWithPieChart/__tests__/AccordionWithPieChart.test.tsx b/src/Frontend/Components/AccordionWithPieChart/__tests__/AccordionWithPieChart.test.tsx index 604781f59..cca9f2382 100644 --- a/src/Frontend/Components/AccordionWithPieChart/__tests__/AccordionWithPieChart.test.tsx +++ b/src/Frontend/Components/AccordionWithPieChart/__tests__/AccordionWithPieChart.test.tsx @@ -2,10 +2,8 @@ // SPDX-FileCopyrightText: TNG Technology Consulting GmbH // // SPDX-License-Identifier: Apache-2.0 -import { - PieChartCriticalityNames, - ProjectStatisticsPopupTitle, -} from '../../../enums/enums'; +import { text } from '../../../../shared/text'; +import { PieChartCriticalityNames } from '../../../enums/enums'; import { OpossumColors } from '../../../shared-styles'; import { PieChartData } from '../../../types/types'; import { getColorsForPieChart } from '../AccordionWithPieChart'; @@ -34,7 +32,7 @@ describe('getColorsForPieChart', () => { const pieChartColors = getColorsForPieChart( criticalSignalsCount, - ProjectStatisticsPopupTitle.CriticalSignalsCountPieChart, + text.projectStatisticsPopup.charts.criticalSignalsCountPieChart, ); expect(pieChartColors).toEqual(expectedPieChartColors); @@ -55,7 +53,7 @@ describe('getColorsForPieChart', () => { const pieChartColors = getColorsForPieChart( sortedMostFrequentLicenses, - ProjectStatisticsPopupTitle.MostFrequentLicenseCountPieChart, + text.projectStatisticsPopup.charts.mostFrequentLicenseCountPieChart, ); expect(pieChartColors).toEqual(expectedPieChartColors); diff --git a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx index a9b1efe0a..39385c745 100644 --- a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx +++ b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx @@ -6,7 +6,6 @@ import MuiBox from '@mui/material/Box'; import MuiTypography from '@mui/material/Typography'; import { text } from '../../../shared/text'; -import { ProjectStatisticsPopupTitle } from '../../enums/enums'; import { closePopup } from '../../state/actions/view-actions/view-actions'; import { useAppDispatch, useAppSelector } from '../../state/hooks'; import { @@ -105,7 +104,8 @@ export const ProjectStatisticsPopup: React.FC = () => { manualAttributionPropertyCounts, )} title={ - ProjectStatisticsPopupTitle.AttributionPropertyCountTable + text.projectStatisticsPopup.charts + .attributionPropertyCountTable } /> { licenseCounts.totalAttributionsPerLicense } licenseNamesWithCriticality={licenseNamesWithCriticality} - title={ProjectStatisticsPopupTitle.CriticalLicensesTable} + title={text.projectStatisticsPopup.charts.criticalLicensesTable} /> {isThereAnyPieChartData - ? ProjectStatisticsPopupTitle.PieChartsSectionHeader + ? text.projectStatisticsPopup.charts.pieChartsSectionHeader : null} } diff --git a/src/Frontend/Components/ProjectStatisticsPopup/__tests__/ProjectStatisticsPopup.test.tsx b/src/Frontend/Components/ProjectStatisticsPopup/__tests__/ProjectStatisticsPopup.test.tsx index fa71ab5f4..f9408d023 100644 --- a/src/Frontend/Components/ProjectStatisticsPopup/__tests__/ProjectStatisticsPopup.test.tsx +++ b/src/Frontend/Components/ProjectStatisticsPopup/__tests__/ProjectStatisticsPopup.test.tsx @@ -7,7 +7,6 @@ import userEvent from '@testing-library/user-event'; import { Attributions } from '../../../../shared/shared-types'; import { text } from '../../../../shared/text'; -import { ProjectStatisticsPopupTitle } from '../../../enums/enums'; import { loadFromFile } from '../../../state/actions/resource-actions/load-actions'; import { getParsedInputFileEnrichedWithTestData } from '../../../test-helpers/general-test-helpers'; import { renderComponent } from '../../../test-helpers/render'; @@ -49,7 +48,7 @@ describe('The ProjectStatisticsPopup', () => { expect(screen.getByText('Reuser')).toBeInTheDocument(); }); - it('renders pie charts when there are attributions', () => { + it('renders pie charts when there are signals', () => { const testManualAttributions: Attributions = { uuid_1: { source: { @@ -101,27 +100,27 @@ describe('The ProjectStatisticsPopup', () => { expect( screen.getByText( - ProjectStatisticsPopupTitle.MostFrequentLicenseCountPieChart, + text.projectStatisticsPopup.charts.mostFrequentLicenseCountPieChart, ), ).toBeInTheDocument(); expect( screen.getByText( - ProjectStatisticsPopupTitle.CriticalSignalsCountPieChart, + text.projectStatisticsPopup.charts.criticalSignalsCountPieChart, ), ).toBeInTheDocument(); expect( screen.getByText( - ProjectStatisticsPopupTitle.SignalCountByClassificationPieChart, + text.projectStatisticsPopup.charts.signalCountByClassificationPieChart, ), ).toBeInTheDocument(); expect( screen.getAllByText( - ProjectStatisticsPopupTitle.IncompleteLicensesPieChart, + text.projectStatisticsPopup.charts.incompleteAttributionsPieChart, ), ).toHaveLength(2); }); - it('does not render pie charts when there are no attributions', () => { + it('does not render pie charts when there are no signals', () => { const testExternalAttributions: Attributions = {}; renderComponent(, { actions: [ @@ -135,27 +134,24 @@ describe('The ProjectStatisticsPopup', () => { }); expect( screen.queryByText( - ProjectStatisticsPopupTitle.MostFrequentLicenseCountPieChart, + text.projectStatisticsPopup.charts.mostFrequentLicenseCountPieChart, ), ).not.toBeInTheDocument(); expect( screen.queryByText( - ProjectStatisticsPopupTitle.CriticalSignalsCountPieChart, + text.projectStatisticsPopup.charts.criticalSignalsCountPieChart, ), ).not.toBeInTheDocument(); expect( screen.queryByText( - ProjectStatisticsPopupTitle.SignalCountByClassificationPieChart, + text.projectStatisticsPopup.charts.signalCountByClassificationPieChart, ), ).not.toBeInTheDocument(); expect( - screen.getByText(ProjectStatisticsPopupTitle.IncompleteLicensesPieChart), - ).toBeInTheDocument(); - expect( - screen.getAllByText( - ProjectStatisticsPopupTitle.IncompleteLicensesPieChart, + screen.getByText( + text.projectStatisticsPopup.charts.incompleteAttributionsPieChart, ), - ).not.toHaveLength(2); + ).toBeInTheDocument(); }); it('renders pie charts related to signals even if there are no attributions', () => { @@ -191,30 +187,27 @@ describe('The ProjectStatisticsPopup', () => { }); expect( screen.getByText( - ProjectStatisticsPopupTitle.MostFrequentLicenseCountPieChart, + text.projectStatisticsPopup.charts.mostFrequentLicenseCountPieChart, ), ).toBeInTheDocument(); expect( screen.getByText( - ProjectStatisticsPopupTitle.CriticalSignalsCountPieChart, + text.projectStatisticsPopup.charts.criticalSignalsCountPieChart, ), ).toBeInTheDocument(); expect( screen.getByText( - ProjectStatisticsPopupTitle.SignalCountByClassificationPieChart, + text.projectStatisticsPopup.charts.signalCountByClassificationPieChart, ), ).toBeInTheDocument(); expect( - screen.getByText(ProjectStatisticsPopupTitle.IncompleteLicensesPieChart), - ).toBeInTheDocument(); - expect( - screen.getAllByText( - ProjectStatisticsPopupTitle.IncompleteLicensesPieChart, + screen.getByText( + text.projectStatisticsPopup.charts.incompleteAttributionsPieChart, ), - ).not.toHaveLength(2); + ).toBeInTheDocument(); }); - it('renders tables when there are no attributions', () => { + it('renders tables when there are no attributions and no signals', () => { const testExternalAttributions: Attributions = {}; renderComponent(, { actions: [ diff --git a/src/Frontend/enums/enums.ts b/src/Frontend/enums/enums.ts index 01297d24b..6f702d2e7 100644 --- a/src/Frontend/enums/enums.ts +++ b/src/Frontend/enums/enums.ts @@ -24,17 +24,6 @@ export enum ButtonText { Delete = 'Delete', } -export enum ProjectStatisticsPopupTitle { - LicenseCountsTable = 'Signals per Sources', - AttributionPropertyCountTable = 'Attributions Overview', - CriticalLicensesTable = 'Critical Licenses', - PieChartsSectionHeader = 'Pie Charts', - MostFrequentLicenseCountPieChart = 'Most Frequent Licenses', - CriticalSignalsCountPieChart = 'Signals by Criticality', - SignalCountByClassificationPieChart = 'Signals by Classification', - IncompleteLicensesPieChart = 'Incomplete Attributions', -} - export enum PieChartCriticalityNames { HighCriticality = 'Highly critical signals', MediumCriticality = 'Medium critical signals', diff --git a/src/shared/text.ts b/src/shared/text.ts index d50ca0561..984e0af91 100644 --- a/src/shared/text.ts +++ b/src/shared/text.ts @@ -167,6 +167,16 @@ export const text = { title: 'Project Statistics', toggleStartupCheckbox: 'Show project statistics on startup', criticalLicensesSignalCountColumnName: 'Signals Count', + charts: { + licenseCountsTable: 'Signals per Sources', + attributionPropertyCountTable: 'Attributions Overview', + criticalLicensesTable: 'Critical Licenses', + pieChartsSectionHeader: 'Pie Charts', + mostFrequentLicenseCountPieChart: 'Most Frequent Licenses', + criticalSignalsCountPieChart: 'Signals by Criticality', + signalCountByClassificationPieChart: 'Signals by Classification', + incompleteAttributionsPieChart: 'Incomplete Attributions', + }, }, unsavedChangesPopup: { title: 'Unsaved Changes', From 943c40dbe222a371a74a45b0f501bf1858d3bf90 Mon Sep 17 00:00:00 2001 From: Philipp Martens Date: Wed, 26 Feb 2025 12:08:55 +0100 Subject: [PATCH 6/6] refactor: move color definition for critical signal pie chart out of general pie chart component --- .../AccordionWithPieChart.tsx | 31 ++-------- .../__tests__/AccordionWithPieChart.test.tsx | 61 ------------------- .../ProjectStatisticsPopup.tsx | 10 +++ .../ProjectStatisticsPopup.util.ts | 4 +- 4 files changed, 17 insertions(+), 89 deletions(-) delete mode 100644 src/Frontend/Components/AccordionWithPieChart/__tests__/AccordionWithPieChart.test.tsx diff --git a/src/Frontend/Components/AccordionWithPieChart/AccordionWithPieChart.tsx b/src/Frontend/Components/AccordionWithPieChart/AccordionWithPieChart.tsx index fdd6dd86c..b367619cb 100644 --- a/src/Frontend/Components/AccordionWithPieChart/AccordionWithPieChart.tsx +++ b/src/Frontend/Components/AccordionWithPieChart/AccordionWithPieChart.tsx @@ -8,8 +8,6 @@ import MuiAccordionDetails from '@mui/material/AccordionDetails'; import MuiAccordionSummary from '@mui/material/AccordionSummary'; import MuiTypography from '@mui/material/Typography'; -import { text } from '../../../shared/text'; -import { PieChartCriticalityNames } from '../../enums/enums'; import { OpossumColors } from '../../shared-styles'; import { PieChartData } from '../../types/types'; import { PieChart } from '../PieChart/PieChart'; @@ -35,35 +33,18 @@ interface AccordionProps { data: Array; title: string; defaultExpanded?: boolean; + pieChartColorMap?: { [segmentName: string]: string }; } export function getColorsForPieChart( pieChartData: Array, - pieChartTitle: string, + pieChartColorMap?: { [segmentName: string]: string }, ): Array | undefined { - const pieChartColors: Array = []; - - if ( - pieChartTitle === - text.projectStatisticsPopup.charts.criticalSignalsCountPieChart - ) { - for (const pieChartSegment of pieChartData) { - switch (pieChartSegment.name) { - case PieChartCriticalityNames.HighCriticality: - pieChartColors.push(OpossumColors.orange); - break; - case PieChartCriticalityNames.MediumCriticality: - pieChartColors.push(OpossumColors.mediumOrange); - break; - default: - pieChartColors.push(OpossumColors.darkBlue); - break; - } - } - } else { + if (pieChartColorMap === undefined) { return; } - return pieChartColors; + + return pieChartData.map(({ name }) => pieChartColorMap[name]); } export const AccordionWithPieChart: React.FC = (props) => { @@ -86,7 +67,7 @@ export const AccordionWithPieChart: React.FC = (props) => { diff --git a/src/Frontend/Components/AccordionWithPieChart/__tests__/AccordionWithPieChart.test.tsx b/src/Frontend/Components/AccordionWithPieChart/__tests__/AccordionWithPieChart.test.tsx deleted file mode 100644 index cca9f2382..000000000 --- a/src/Frontend/Components/AccordionWithPieChart/__tests__/AccordionWithPieChart.test.tsx +++ /dev/null @@ -1,61 +0,0 @@ -// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates -// SPDX-FileCopyrightText: TNG Technology Consulting GmbH -// -// SPDX-License-Identifier: Apache-2.0 -import { text } from '../../../../shared/text'; -import { PieChartCriticalityNames } from '../../../enums/enums'; -import { OpossumColors } from '../../../shared-styles'; -import { PieChartData } from '../../../types/types'; -import { getColorsForPieChart } from '../AccordionWithPieChart'; - -describe('getColorsForPieChart', () => { - it('obtains pie chart colors for critical signals pie chart', () => { - const expectedPieChartColors = [ - OpossumColors.orange, - OpossumColors.mediumOrange, - OpossumColors.darkBlue, - ]; - const criticalSignalsCount: Array = [ - { - name: PieChartCriticalityNames.HighCriticality, - count: 3, - }, - { - name: PieChartCriticalityNames.MediumCriticality, - count: 4, - }, - { - name: PieChartCriticalityNames.NoCriticality, - count: 2, - }, - ]; - - const pieChartColors = getColorsForPieChart( - criticalSignalsCount, - text.projectStatisticsPopup.charts.criticalSignalsCountPieChart, - ); - - expect(pieChartColors).toEqual(expectedPieChartColors); - }); - - it('obtains undefined pie chart colors for default case', () => { - const expectedPieChartColors = undefined; - const sortedMostFrequentLicenses: Array = [ - { - name: 'Apache License Version 2.0', - count: 3, - }, - { - name: 'The MIT License (MIT)', - count: 3, - }, - ]; - - const pieChartColors = getColorsForPieChart( - sortedMostFrequentLicenses, - text.projectStatisticsPopup.charts.mostFrequentLicenseCountPieChart, - ); - - expect(pieChartColors).toEqual(expectedPieChartColors); - }); -}); diff --git a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx index 39385c745..9478d5fe4 100644 --- a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx +++ b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx @@ -6,6 +6,8 @@ import MuiBox from '@mui/material/Box'; import MuiTypography from '@mui/material/Typography'; import { text } from '../../../shared/text'; +import { PieChartCriticalityNames } from '../../enums/enums'; +import { OpossumColors } from '../../shared-styles'; import { closePopup } from '../../state/actions/view-actions/view-actions'; import { useAppDispatch, useAppSelector } from '../../state/hooks'; import { @@ -69,6 +71,12 @@ export const ProjectStatisticsPopup: React.FC = () => { licenseNamesWithCriticality, ); + const criticalSignalsCountColors = { + [PieChartCriticalityNames.HighCriticality]: OpossumColors.orange, + [PieChartCriticalityNames.MediumCriticality]: OpossumColors.mediumOrange, + [PieChartCriticalityNames.NoCriticality]: OpossumColors.darkBlue, + }; + const signalCountByClassification = getSignalCountByClassification( licenseCounts, licenseNamesWithClassification, @@ -84,6 +92,7 @@ export const ProjectStatisticsPopup: React.FC = () => { const isThereAnyPieChartData = mostFrequentLicenseCountData.length > 0 || criticalSignalsCountData.length > 0 || + signalCountByClassification.length > 0 || incompleteAttributionsData.length > 0; function close(): void { @@ -136,6 +145,7 @@ export const ProjectStatisticsPopup: React.FC = () => { text.projectStatisticsPopup.charts .criticalSignalsCountPieChart } + pieChartColorMap={criticalSignalsCountColors} /> criticalityDataWithCount['count'] !== 0, - ); + return criticalityData.filter(({ count }) => count > 0); } export function getSignalCountByClassification(