Skip to content

Commit

Permalink
fix: Update metrics calculations and related UI components (#1172)
Browse files Browse the repository at this point in the history
* Address missing db persisted calculation for exclusions (this wasn't impacting the frontend directly because the calls were being done again on the reports pages)

* Revise aria-label with progress bar to avoid double speech

* Update getMetrics to stop rounding up and use utilities

* Remove Math.round usage on candidate test plans page

* Update shared/calculations usage

* Include migration and better handling of finalizedTestResultsResolver.js

* Add tests for getMetrics

* Add tests for getMetrics

* Add tests for getMetrics

* Reduce noisy aria-labels

* Additional clarifying comments
  • Loading branch information
howard-e authored Jul 25, 2024
1 parent d1c409c commit e4242bc
Show file tree
Hide file tree
Showing 11 changed files with 3,552 additions and 26 deletions.
16 changes: 8 additions & 8 deletions client/components/CandidateReview/TestPlans/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
import ClippedProgressBar from '@components/common/ClippedProgressBar';
import { convertDateToString } from '@client/utils/formatter';
import './TestPlans.css';
import { calculations } from 'shared';

const FullHeightContainer = styled(Container)`
min-height: calc(100vh - 64px);
Expand Down Expand Up @@ -486,13 +487,12 @@ const TestPlans = ({ testPlanVersions }) => {
obj.mustAssertionsFailedCount,
0
),
totalSupportPercent:
Math.round(
allMetrics.reduce(
(acc, obj) => acc + obj.supportPercent,
0
) / allMetrics.length
) || 0
totalSupportPercent: calculations.trimDecimals(
allMetrics.reduce(
(acc, obj) => acc + obj.supportPercent,
0
) / allMetrics.length
)
};

// Make sure issues are unique
Expand Down Expand Up @@ -550,10 +550,10 @@ const TestPlans = ({ testPlanVersions }) => {
<CenteredTd>
<Link
to={`/candidate-test-plan/${testPlanVersion.id}/${atId}`}
aria-label={`${metrics.totalSupportPercent}%`}
>
<ClippedProgressBar
progress={metrics.totalSupportPercent}
label={`${metrics.totalSupportPercent}% completed`}
clipped
/>
</Link>
Expand Down
4 changes: 1 addition & 3 deletions client/components/Reports/SummarizeTestPlanReports.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,10 @@ const SummarizeTestPlanReports = ({ testPlanVersions }) => {
`/report/${testPlanVersion.id}` +
`/targets/${testPlanReport.id}`
}
aria-label={`${metrics.supportPercent}%`}
>
<ClippedProgressBar
progress={metrics.supportPercent}
label={`${getTestPlanTargetTitle(
testPlanTarget
)}, ${metrics.supportPercent}% completed`}
/>
</Link>
</td>
Expand Down
12 changes: 3 additions & 9 deletions client/components/common/ClippedProgressBar/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,11 @@ import React from 'react';
import PropTypes from 'prop-types';
import './ClippedProgressBar.css';

const ProgressBar = ({
progress = 0,
label = '',
clipped = true,
decorative
}) => {
const ProgressBar = ({ progress = 0, clipped = true, decorative }) => {
return (
<>
{clipped ? (
<div className="progress clipped" aria-label={label}>
<div className="progress clipped">
<div
className="front"
style={{
Expand All @@ -23,7 +18,7 @@ const ProgressBar = ({
<div className="back">{decorative ? null : `${progress}%`}</div>
</div>
) : (
<div className="progress" aria-label={label}>
<div className="progress">
<div
className="progress-bar bg-info"
style={{
Expand All @@ -40,7 +35,6 @@ const ProgressBar = ({

ProgressBar.propTypes = {
progress: PropTypes.number,
label: PropTypes.string,
clipped: PropTypes.bool,
decorative: PropTypes.bool
};
Expand Down
64 changes: 64 additions & 0 deletions server/migrations/20240724212706-updateMetrics.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
'use strict';

const { getMetrics } = require('shared');
const populateData = require('../services/PopulatedData/populateData');
const runnableTestsResolver = require('../resolvers/TestPlanReport/runnableTestsResolver');
const finalizedTestResultsResolver = require('../resolvers/TestPlanReport/finalizedTestResultsResolver');
const {
updateTestPlanReportById
} = require('../models/services/TestPlanReportService');
const getGraphQLContext = require('../graphql-context');

/** @type {import('sequelize-cli').Migration} */
module.exports = {
async up(queryInterface, Sequelize) {
return queryInterface.sequelize.transaction(async transaction => {
const context = getGraphQLContext({ req: { transaction } });

const testPlanReports = await queryInterface.sequelize.query(
`select distinct on ("TestPlanReport".id) "TestPlanReport".id, metrics
from "TestPlanReport"
join public."TestPlanRun" testPlanRun on "TestPlanReport".id = testPlanRun."testPlanReportId"
where jsonb_array_length(testPlanRun."testResults") > 0;`,
{
type: Sequelize.QueryTypes.SELECT,
transaction
}
);

for (const testPlanReport of testPlanReports) {
const { testPlanReport: testPlanReportPopulated } = await populateData(
{ testPlanReportId: testPlanReport.id },
{ context }
);
const runnableTests = runnableTestsResolver(
testPlanReportPopulated,
null,
context
);
const finalizedTestResults = await finalizedTestResultsResolver(
testPlanReportPopulated,
null,
context
);
const metrics = getMetrics({
testPlanReport: {
...testPlanReportPopulated,
finalizedTestResults,
runnableTests
}
});
await updateTestPlanReportById({
id: testPlanReportPopulated.id,
values: {
metrics: {
...testPlanReportPopulated.metrics,
...metrics
}
},
transaction
});
}
});
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ const finalizedTestResultsResolver = async (testPlanReport, _, context) => {
// Return the primary test plan run, otherwise pick the first TestPlanRun found.
const testPlanRun =
testPlanReport.testPlanRuns.find(({ isPrimary }) => isPrimary) ||
testPlanReport.testPlanRuns.find(testPlanRun =>
testPlanRun.testResults?.some(testResult => !!testResult.completedAt)
) ||
testPlanReport.testPlanRuns[0];

return testResultsResolver(
Expand Down
26 changes: 26 additions & 0 deletions shared/calculations.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* @param {number} value
* @param {number} total
* @param {boolean} ignoreError - to account for cases where having a NaN is "expected"
* @returns {number}
*/
const calculatePercentage = (value, total, { ignoreError = true } = {}) => {
if (!ignoreError && total === 0) {
throw new Error("Unable to divide. 'total' cannot be 0.");
}
return (value / total) * 100;
};

const trimDecimals = (number, decimals = 0) => {
if (decimals === undefined || decimals <= 0) {
return Math.floor(number);
} else {
let factor = Math.pow(10, decimals);
return Math.floor(number * factor) / factor;
}
};

module.exports = {
calculatePercentage,
trimDecimals
};
34 changes: 29 additions & 5 deletions shared/getMetrics.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const convertAssertionPriority = require('./convertAssertionPriority');
const { calculatePercentage, trimDecimals } = require('./calculations');

const sum = arr => arr?.reduce((total, item) => total + item, 0) || 0;

Expand Down Expand Up @@ -46,11 +47,32 @@ const countAssertions = ({
if (isClient) {
all = scenarioResult?.[`${priority.toLowerCase()}AssertionResults`] || [];
} else {
all = scenarioResult.assertionResults.filter(
a =>
all = scenarioResult.assertionResults.filter(a => {
// Same as `server/resolvers/ScenarioResult/assertionResultsResolver.js`
if (a.assertion?.assertionExceptions?.length) {
const scenarioSettings = scenarioResult.scenario.settings;
const scenarioCommandId =
scenarioResult.scenario.commandIds.join(' ');

const foundException = a.assertion.assertionExceptions.find(
exception =>
exception.settings === scenarioSettings &&
exception.commandId === scenarioCommandId
);

if (foundException) {
return (
convertAssertionPriority(foundException.priority) ===
convertAssertionPriority(priority)
);
}
}

return (
convertAssertionPriority(a.assertion.priority) ===
convertAssertionPriority(priority)
);
);
});
}
if (passedOnly) return all.filter(each => each.passed).length;
return all.length;
Expand Down Expand Up @@ -236,9 +258,11 @@ const getMetrics = ({
supportLevel = 'FULL';
}

const supportPercent = Math.round(
(mustAssertionsPassedCount / mustAssertionsCount) * 100
const percentage = calculatePercentage(
mustAssertionsPassedCount + shouldAssertionsPassedCount,
mustAssertionsCount + shouldAssertionsCount
);
const supportPercent = trimDecimals(percentage);

const assertionsPassedCount =
mustAssertionsPassedCount +
Expand Down
4 changes: 3 additions & 1 deletion shared/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const calculations = require('./calculations');
const convertAssertionPriority = require('./convertAssertionPriority');
const getMetrics = require('./getMetrics');
module.exports = { convertAssertionPriority, getMetrics };

module.exports = { calculations, convertAssertionPriority, getMetrics };
Loading

0 comments on commit e4242bc

Please sign in to comment.