From 768c569c3bd7475856bfb89b6bab2b3ed4b348f3 Mon Sep 17 00:00:00 2001 From: Frank Niessink Date: Wed, 10 Jul 2024 10:09:05 +0200 Subject: [PATCH] Allow for leaving desired reaction times empty. Closes #9135. --- components/api_server/src/model/report.py | 29 ++++++----- .../api_server/src/routes/measurement.py | 12 ++--- components/api_server/src/routes/metric.py | 4 +- .../tests/routes/test_measurement.py | 43 +++++++++++++--- .../api_server/tests/routes/test_metric.py | 32 +++++++++--- .../frontend/src/fields/IntegerInput.js | 10 ++-- .../frontend/src/fields/IntegerInput.test.js | 12 +++++ components/frontend/src/report/ReportTitle.js | 1 + .../src/source/SourceEntityDetails.js | 51 +++++++++++++------ .../src/source/SourceEntityDetails.test.js | 37 ++++++++++++-- .../src/source/source_entity_status.js | 8 +++ components/frontend/src/utils.js | 42 +++++++++------ components/frontend/src/utils.test.js | 41 ++++++++++++++- tests/feature_tests/.vulture_ignore_list.py | 3 ++ .../src/features/measurement_entities.feature | 25 +++++++++ .../feature_tests/src/features/metric.feature | 11 ++++ tests/feature_tests/src/steps/metric.py | 16 +++++- tests/feature_tests/src/steps/report.py | 14 +++++ 18 files changed, 317 insertions(+), 74 deletions(-) diff --git a/components/api_server/src/model/report.py b/components/api_server/src/model/report.py index 74c9656538..35ebc1dae1 100644 --- a/components/api_server/src/model/report.py +++ b/components/api_server/src/model/report.py @@ -1,6 +1,9 @@ """Report model class.""" +from datetime import timedelta + from shared.model.report import Report as SharedReport +from shared.utils.date_time import now from .issue_tracker import IssueParameters, IssueTracker, IssueTrackerCredentials @@ -26,25 +29,27 @@ def issue_tracker(self) -> IssueTracker: ) return IssueTracker(url, issue_parameters, credentials) - def desired_response_time(self, status: str) -> int: - """Return the desired response time for the metric status.""" - # Note that the frontend also has these constant, in src/defaults.js. + def deadline(self, status: str) -> str | None: + """Return the deadline for metrics or measurement entities with the given status.""" + desired_response_time = self._desired_response_time(status) + return None if desired_response_time is None else str((now() + timedelta(days=desired_response_time)).date()) + + def _desired_response_time(self, status: str) -> int | None: + """Return the desired response time for the given status.""" + # Note that the frontend also has these constants, in src/defaults.js. defaults = { "debt_target_met": 60, "near_target_met": 21, "target_not_met": 7, "unknown": 3, - } - default = defaults.get(status, defaults["unknown"]) - return int(self.get("desired_response_times", {}).get(status, default)) - - def desired_measurement_entity_response_time(self, status: str) -> int | None: - """Return the desired response time for the measurement entity status.""" - defaults = { "confirmed": 180, "false_positive": 180, "wont_fix": 180, "fixed": 7, } - default = defaults.get(status) - return int(self.get("desired_response_times", {}).get(status, default)) if status in defaults else None + if status not in self.get("desired_response_times", {}): + return defaults.get(status) + try: + return int(self["desired_response_times"][status]) + except (ValueError, TypeError): # The desired response time can be empty, treat any non-integer as None + return None diff --git a/components/api_server/src/routes/measurement.py b/components/api_server/src/routes/measurement.py index af8faea1c6..e76ea5d18e 100644 --- a/components/api_server/src/routes/measurement.py +++ b/components/api_server/src/routes/measurement.py @@ -3,7 +3,6 @@ import logging import time from collections.abc import Iterator -from datetime import timedelta from typing import cast import bottle @@ -11,7 +10,6 @@ from shared.database.measurements import insert_new_measurement, latest_measurement from shared.model.measurement import Measurement -from shared.utils.date_time import now from shared.utils.functions import first from shared.utils.type import MetricId, SourceId @@ -47,11 +45,11 @@ def set_entity_attribute( description = f"{user.name()} changed the {attribute} of '{entity_description}' from '{old_value}' to '{new_value}'" entity_user_data = source.setdefault("entity_user_data", {}).setdefault(entity_key, {}) entity_user_data[attribute] = new_value - if attribute == "status": - desired_reponse_time = report.desired_measurement_entity_response_time(new_value) - end_date = str((now() + timedelta(days=desired_reponse_time)).date()) if desired_reponse_time else None - entity_user_data["status_end_date"] = end_date - description += f" and changed the status end date to '{end_date}'" + if attribute == "status" and (end_date := report.deadline(new_value)): + old_end_date = entity_user_data.get("status_end_date") + if end_date != old_end_date: + entity_user_data["status_end_date"] = end_date + description += f" and changed the status end date from '{old_end_date}' to '{end_date}'" new_measurement["delta"] = { "uuids": [report.uuid, metric.subject_uuid, metric_uuid, source_uuid], "description": description + ".", diff --git a/components/api_server/src/routes/metric.py b/components/api_server/src/routes/metric.py index 3ae472783d..0dcfdbc85b 100644 --- a/components/api_server/src/routes/metric.py +++ b/components/api_server/src/routes/metric.py @@ -1,6 +1,5 @@ """Metric routes.""" -from datetime import UTC, datetime, timedelta from typing import Any, cast import bottle @@ -157,8 +156,7 @@ def post_metric_debt(metric_uuid: MetricId, database: Database): latest = latest_measurement(database, Metric(DATA_MODEL.model_dump(), metric, metric_uuid)) # Only if the metric has at least one measurement can a technical debt target be set: new_debt_target = latest.value() if latest else None - today = datetime.now(tz=UTC).date() - new_end_date = (today + timedelta(days=report.desired_response_time("debt_target_met"))).isoformat() + new_end_date = report.deadline("debt_target_met") else: new_debt_target = None new_end_date = None diff --git a/components/api_server/tests/routes/test_measurement.py b/components/api_server/tests/routes/test_measurement.py index 275582a9d5..ce6ee77cca 100644 --- a/components/api_server/tests/routes/test_measurement.py +++ b/components/api_server/tests/routes/test_measurement.py @@ -144,22 +144,53 @@ def test_set_status_also_sets_status_end_date_if_status_has_a_desired_response_t self.assertEqual( { "description": "John Doe changed the status of 'entity title/foo/None' from '' to 'false_positive' " - f"and changed the status end date to '{deadline}'.", + f"and changed the status end date from 'None' to '{deadline}'.", "email": JOHN["email"], "uuids": [REPORT_ID, SUBJECT_ID, METRIC_ID, SOURCE_ID], }, measurement["delta"], ) - def test_set_status_resets_status_end_date_if_status_is_unconfirmed(self): - """Test that setting the status to unconfirmed also resets the end date.""" + def test_set_status_does_not_set_the_status_end_date_if_it_is_unchanged(self): + """Test that setting the status does not set the end date if it hasn't changed.""" + deadline = (now() + timedelta(days=10)).date() + self.measurement["sources"][0]["entity_user_data"] = {"entity_key": {"status_end_date": str(deadline)}} + self.report["desired_response_times"] = {"false_positive": 10} + measurement = self.set_entity_attribute("status", "false_positive") + entity = measurement["sources"][0]["entity_user_data"]["entity_key"] + self.assertEqual({"status": "false_positive", "status_end_date": str(deadline)}, entity) + self.assertEqual( + { + "description": "John Doe changed the status of 'entity title/foo/None' from '' to 'false_positive'.", + "email": JOHN["email"], + "uuids": [REPORT_ID, SUBJECT_ID, METRIC_ID, SOURCE_ID], + }, + measurement["delta"], + ) + + def test_set_status_does_not_set_status_end_date_if_desired_response_time_turned_off(self): + """Test that setting the status does not set the end date if the desired response time has been turned off.""" + self.report["desired_response_times"] = {"false_positive": None} + measurement = self.set_entity_attribute("status", "false_positive") + entity = measurement["sources"][0]["entity_user_data"]["entity_key"] + self.assertEqual({"status": "false_positive"}, entity) + self.assertEqual( + { + "description": "John Doe changed the status of 'entity title/foo/None' from '' to 'false_positive'.", + "email": JOHN["email"], + "uuids": [REPORT_ID, SUBJECT_ID, METRIC_ID, SOURCE_ID], + }, + measurement["delta"], + ) + + def test_set_status_does_not_reset_status_end_date_if_status_is_unconfirmed(self): + """Test that setting the status to unconfirmed does not reset the end date.""" measurement = self.set_entity_attribute("status", "unconfirmed") entity = measurement["sources"][0]["entity_user_data"]["entity_key"] - self.assertEqual({"status": "unconfirmed", "status_end_date": None}, entity) + self.assertEqual({"status": "unconfirmed"}, entity) self.assertEqual( { - "description": "John Doe changed the status of 'entity title/foo/None' from '' to 'unconfirmed' " - "and changed the status end date to 'None'.", + "description": "John Doe changed the status of 'entity title/foo/None' from '' to 'unconfirmed'.", "email": JOHN["email"], "uuids": [REPORT_ID, SUBJECT_ID, METRIC_ID, SOURCE_ID], }, diff --git a/components/api_server/tests/routes/test_metric.py b/components/api_server/tests/routes/test_metric.py index e13739dc00..858098baad 100644 --- a/components/api_server/tests/routes/test_metric.py +++ b/components/api_server/tests/routes/test_metric.py @@ -1,6 +1,5 @@ """Unit tests for the metric routes.""" -from datetime import UTC, datetime, timedelta from unittest.mock import Mock, patch import requests @@ -355,11 +354,33 @@ def test_turn_metric_technical_debt_on_with_existing_measurement(self, request): post_metric_debt(METRIC_ID, self.database), ) updated_report = self.updated_report() - desired_response_time = Report(self.DATA_MODEL, updated_report).desired_response_time("debt_target_met") - expected_date = datetime.now(tz=UTC).date() + timedelta(days=desired_response_time) + expected_date = Report(self.DATA_MODEL, updated_report).deadline("debt_target_met") self.assert_delta( "accepted debt from 'False' to 'True' and the debt target from 'None' to '100' and the debt end date from " - f"'None' to '{expected_date.isoformat()}' of metric 'name' of subject 'Subject' in report 'Report'", + f"'None' to '{expected_date}' of metric 'name' of subject 'Subject' in report 'Report'", + report=updated_report, + ) + self.assertTrue(updated_report["subjects"][SUBJECT_ID]["metrics"][METRIC_ID]["accept_debt"]) + + @patch("shared.model.measurement.iso_timestamp", new=Mock(return_value="2019-01-01")) + def test_turn_metric_technical_debt_on_with_existing_measurement_but_without_desired_response_time(self, request): + """Test that accepting technical debt also sets the technical debt value, but not the deadline.""" + self.report["desired_response_times"] = {"debt_target_met": None} + self.database.measurements.find_one.return_value = { + "_id": "id", + "metric_uuid": METRIC_ID, + "count": {"value": "100", "status_start": "2018-01-01"}, + "sources": [], + } + request.json = {"accept_debt": True} + self.assertDictEqual( + self.create_measurement(debt_target="100", status="debt_target_met", target="0"), + post_metric_debt(METRIC_ID, self.database), + ) + updated_report = self.updated_report() + self.assert_delta( + "accepted debt from 'False' to 'True' and the debt target from 'None' to '100' " + "of metric 'name' of subject 'Subject' in report 'Report'", report=updated_report, ) self.assertTrue(updated_report["subjects"][SUBJECT_ID]["metrics"][METRIC_ID]["accept_debt"]) @@ -369,8 +390,7 @@ def test_turn_metric_technical_debt_on_without_existing_measurement(self, reques request.json = {"accept_debt": True} self.assertDictEqual({"ok": True}, post_metric_debt(METRIC_ID, self.database)) updated_report = self.updated_report() - desired_response_time = Report(self.DATA_MODEL, updated_report).desired_response_time("debt_target_met") - expected_date = (datetime.now(tz=UTC).date() + timedelta(days=desired_response_time)).isoformat() + expected_date = Report(self.DATA_MODEL, updated_report).deadline("debt_target_met") self.assert_delta( f"accepted debt from 'False' to 'True' and the debt end date from 'None' to '{expected_date}' " "of metric 'name' of subject 'Subject' in report 'Report'", diff --git a/components/frontend/src/fields/IntegerInput.js b/components/frontend/src/fields/IntegerInput.js index c5990deb47..4278a6c4b7 100644 --- a/components/frontend/src/fields/IntegerInput.js +++ b/components/frontend/src/fields/IntegerInput.js @@ -1,4 +1,4 @@ -import { func, number, oneOfType, string } from "prop-types" +import { bool, func, number, oneOfType, string } from "prop-types" import { useState } from "react" import { ReadOnlyOrEditable } from "../context/Permissions" @@ -7,12 +7,15 @@ import { labelPropType, permissionsPropType } from "../sharedPropTypes" import { ReadOnlyInput } from "./ReadOnlyInput" function EditableIntegerInput(props) { - let { editableLabel, label, min, prefix, set_value, unit, ...otherProps } = props - const initialValue = props.value || 0 + let { allowEmpty, editableLabel, label, min, prefix, set_value, unit, ...otherProps } = props + const initialValue = props.value || (allowEmpty ? "" : 0) const [value, setValue] = useState(initialValue) const minValue = min || 0 function isValid(aValue) { + if (aValue === "") { + return allowEmpty + } if (Number.isNaN(parseInt(aValue))) { return false } @@ -69,6 +72,7 @@ function EditableIntegerInput(props) { ) } EditableIntegerInput.propTypes = { + allowEmpty: bool, editableLabel: labelPropType, label: labelPropType, max: oneOfType([number, string]), diff --git a/components/frontend/src/fields/IntegerInput.test.js b/components/frontend/src/fields/IntegerInput.test.js index d8211f37e1..ba69485bc1 100644 --- a/components/frontend/src/fields/IntegerInput.test.js +++ b/components/frontend/src/fields/IntegerInput.test.js @@ -75,6 +75,13 @@ it("does not accept an empty value", async () => { expect(setValue).toHaveBeenCalledWith("4") }) +it("accepts an empty value if an empty value is allowed", async () => { + let setValue = jest.fn() + render() + await userEvent.type(screen.getByDisplayValue(/42/), "{selectall}{backspace}{backspace}{enter}") + expect(setValue).toHaveBeenCalledWith("") +}) + it("undoes the change on escape", async () => { let setValue = jest.fn() render() @@ -107,3 +114,8 @@ it("renders missing value as 0", () => { render() expect(screen.queryAllByDisplayValue(/0/).length).toBe(1) }) + +it("renders missing value as empty if empty allowed", () => { + render() + expect(screen.queryAllByDisplayValue(/0/).length).toBe(0) +}) diff --git a/components/frontend/src/report/ReportTitle.js b/components/frontend/src/report/ReportTitle.js index 1faef400a8..3f21c8eded 100644 --- a/components/frontend/src/report/ReportTitle.js +++ b/components/frontend/src/report/ReportTitle.js @@ -68,6 +68,7 @@ function DesiredResponseTimeInput({ hoverableLabel, reload, report, status }) { const help = STATUS_DESCRIPTION[status] || SOURCE_ENTITY_STATUS_DESCRIPTION[status] return ( } requiredPermissions={[EDIT_REPORT_PERMISSION]} diff --git a/components/frontend/src/source/SourceEntityDetails.js b/components/frontend/src/source/SourceEntityDetails.js index 0f58cf8609..632babc659 100644 --- a/components/frontend/src/source/SourceEntityDetails.js +++ b/components/frontend/src/source/SourceEntityDetails.js @@ -1,4 +1,4 @@ -import { func, node, string } from "prop-types" +import { func, node, oneOf, string } from "prop-types" import { Grid, Header } from "semantic-ui-react" import { set_source_entity_attribute } from "../api/source" @@ -9,34 +9,55 @@ import { TextInput } from "../fields/TextInput" import { entityPropType, entityStatusPropType, reportPropType } from "../sharedPropTypes" import { capitalize, getDesiredResponseTime } from "../utils" import { LabelWithDate } from "../widgets/LabelWithDate" -import { SOURCE_ENTITY_STATUS_NAME } from "./source_entity_status" +import { SOURCE_ENTITY_STATUS_ACTION, SOURCE_ENTITY_STATUS_NAME } from "./source_entity_status" -function entityStatusOption(status, content, subheader) { +function entityStatusOption(status, subheader) { return { key: status, text: SOURCE_ENTITY_STATUS_NAME[status], value: status, - content:
, + content:
, } } entityStatusOption.propTypes = { status: entityStatusPropType, - content: node, subheader: node, } +function responseTimeClause(report, status, prefix = "for") { + const responseTime = getDesiredResponseTime(report, status) + return responseTime === null ? "" : ` ${prefix} ${responseTime} days` +} +responseTimeClause.propTypes = { + status: entityStatusPropType, + report: reportPropType, + prefix: oneOf(["for", "within"]), +} + function entityStatusOptions(entityType, report) { - const unconfirmedSubheader = `This ${entityType} should be reviewed to decide what to do with it.` - const confirmedSubheader = `This ${entityType} has been reviewed and should be addressed within ${getDesiredResponseTime(report, "confirmed")} days.` - const fixedSubheader = `Ignore this ${entityType} for ${getDesiredResponseTime(report, "fixed")} days because it has been fixed or will be fixed shortly.` - const falsePositiveSubheader = `Ignore this "${entityType}" for ${getDesiredResponseTime(report, "false_positive")} days because it has been incorrectly identified as ${entityType}.` - const wontFixSubheader = `Ignore this ${entityType} for ${getDesiredResponseTime(report, "wont_fix")} days because it will not be fixed.` return [ - entityStatusOption("unconfirmed", "Unconfirm", unconfirmedSubheader), - entityStatusOption("confirmed", "Confirm", confirmedSubheader), - entityStatusOption("fixed", "Resolve as fixed", fixedSubheader), - entityStatusOption("false_positive", "Resolve as false positive", falsePositiveSubheader), - entityStatusOption("wont_fix", "Resolve as won't fix", wontFixSubheader), + entityStatusOption( + "unconfirmed", + `This ${entityType} should be reviewed in order to decide what to do with it.`, + ), + entityStatusOption( + "confirmed", + `This ${entityType} has been reviewed and should be addressed${responseTimeClause(report, "confirmed", "within")}.`, + ), + entityStatusOption( + "fixed", + `Ignore this ${entityType}${responseTimeClause(report, "fixed")} because it has been fixed or will be fixed shortly.`, + ), + entityStatusOption( + "false_positive", + // If the user marks then entity status as false positive, apparently the entity type is incorrect, + // hence the false positive option has quotes around the entity type: + `Ignore this "${entityType}"${responseTimeClause(report, "false_positive")} because it has been incorrectly identified as ${entityType}.`, + ), + entityStatusOption( + "wont_fix", + `Ignore this ${entityType}${responseTimeClause(report, "wont_fix")} because it will not be fixed.`, + ), ] } entityStatusOptions.propTypes = { diff --git a/components/frontend/src/source/SourceEntityDetails.test.js b/components/frontend/src/source/SourceEntityDetails.test.js index e9a7778dbd..4fe511f270 100644 --- a/components/frontend/src/source/SourceEntityDetails.test.js +++ b/components/frontend/src/source/SourceEntityDetails.test.js @@ -7,11 +7,9 @@ import { SourceEntityDetails } from "./SourceEntityDetails" jest.mock("../api/source.js") -function reload() { - /* Dummy implementation */ -} +const reload = jest.fn -function renderSourceEntityDetails() { +function renderSourceEntityDetails(report) { render( , ) @@ -40,6 +39,36 @@ it("shows the default desired response times when the report has no desired resp }) }) +it("shows the configured desired response times", () => { + const report = { desired_response_times: { confirmed: "2", fixed: "4", false_positive: "600", wont_fix: "100" } } + renderSourceEntityDetails(report) + fireEvent.click(screen.getByText(/Unconfirmed/)) + const expectedMenuItemDescriptions = [ + "This violation has been reviewed and should be addressed within 2 days.", + "Ignore this violation for 4 days because it has been fixed or will be fixed shortly.", + 'Ignore this "violation" for 600 days because it has been incorrectly identified as violation.', + "Ignore this violation for 100 days because it will not be fixed.", + ] + expectedMenuItemDescriptions.forEach((description) => { + expect(screen.queryAllByText(description).length).toBe(1) + }) +}) + +it("shows no desired response times when the report has been configured to not have desired response times", () => { + const report = { desired_response_times: { confirmed: null, fixed: null, false_positive: null, wont_fix: null } } + renderSourceEntityDetails(report) + fireEvent.click(screen.getByText(/Unconfirmed/)) + const expectedMenuItemDescriptions = [ + "This violation has been reviewed and should be addressed.", + "Ignore this violation because it has been fixed or will be fixed shortly.", + 'Ignore this "violation" because it has been incorrectly identified as violation.', + "Ignore this violation because it will not be fixed.", + ] + expectedMenuItemDescriptions.forEach((description) => { + expect(screen.queryAllByText(description).length).toBe(1) + }) +}) + it("changes the entity status", () => { source.set_source_entity_attribute = jest.fn() renderSourceEntityDetails() diff --git a/components/frontend/src/source/source_entity_status.js b/components/frontend/src/source/source_entity_status.js index 39199aaa6a..71be9a3c72 100644 --- a/components/frontend/src/source/source_entity_status.js +++ b/components/frontend/src/source/source_entity_status.js @@ -6,6 +6,14 @@ export const SOURCE_ENTITY_STATUS_NAME = { wont_fix: "Won't fix", } +export const SOURCE_ENTITY_STATUS_ACTION = { + unconfirmed: "Unconfirm", + confirmed: "Confirm", + fixed: "Resolve as fixed", + false_positive: "Resolve as false positive", + wont_fix: "Resolve as won't fix", +} + export const SOURCE_ENTITY_STATUS_DESCRIPTION = { // Unconfirmed is missing because these descriptions are only used for the desired response times at the moment confirmed: "Confirmed means that an entity has been reviewed and should be dealt with.", diff --git a/components/frontend/src/utils.js b/components/frontend/src/utils.js index e17e29be18..8dac816ff9 100644 --- a/components/frontend/src/utils.js +++ b/components/frontend/src/utils.js @@ -114,8 +114,11 @@ export function getMetricResponseDeadline(metric, report) { deadline = new Date(metric.debt_end_date) } } else if (status in defaultDesiredResponseTimes && metric.status_start) { - deadline = new Date(metric.status_start) - deadline.setDate(deadline.getDate() + getDesiredResponseTime(report, status)) + const desiredResponseTime = getDesiredResponseTime(report, status) + if (Number.isInteger(desiredResponseTime)) { + deadline = new Date(metric.status_start) + deadline.setDate(deadline.getDate() + getDesiredResponseTime(report, status)) + } } return deadline } @@ -151,19 +154,22 @@ export function getMetricResponseOverrun(metric_uuid, metric, report, measuremen consolidatedMeasurements.forEach((measurement) => { const status = measurement?.[scale]?.status || "unknown" if (status in defaultDesiredResponseTimes) { - const desiredResponseTime = getDesiredResponseTime(report, status) * MILLISECONDS_PER_DAY - const actualResponseTime = new Date(measurement.end).getTime() - new Date(measurement.start).getTime() - const overrun = Math.max(0, actualResponseTime - desiredResponseTime) - if (overrun > 0) { - overruns.push({ - status: status, - start: measurement.start, - end: measurement.end, - desired_response_time: days(desiredResponseTime), - actual_response_time: days(actualResponseTime), - overrun: days(overrun), - }) - totalOverrun += overrun + let desiredResponseTime = getDesiredResponseTime(report, status) + if (Number.isInteger(desiredResponseTime)) { + desiredResponseTime *= MILLISECONDS_PER_DAY + const actualResponseTime = new Date(measurement.end).getTime() - new Date(measurement.start).getTime() + const overrun = Math.max(0, actualResponseTime - desiredResponseTime) + if (overrun > 0) { + overruns.push({ + status: status, + start: measurement.start, + end: measurement.end, + desired_response_time: days(desiredResponseTime), + actual_response_time: days(actualResponseTime), + overrun: days(overrun), + }) + totalOverrun += overrun + } } } }) @@ -172,7 +178,11 @@ export function getMetricResponseOverrun(metric_uuid, metric, report, measuremen export function getDesiredResponseTime(report, status) { // Precondition: status is a key of defaultDesiredResponseTimes - return report?.desired_response_times?.[status] ?? defaultDesiredResponseTimes[status] + const desiredResponseTime = report?.desired_response_times?.[status] + if (desiredResponseTime === undefined) { + return defaultDesiredResponseTimes[status] + } + return desiredResponseTime === null ? null : Number.parseInt(desiredResponseTime) } export function getMetricValue(metric, dataModel) { diff --git a/components/frontend/src/utils.test.js b/components/frontend/src/utils.test.js index 109e10e026..5a22beaefa 100644 --- a/components/frontend/src/utils.test.js +++ b/components/frontend/src/utils.test.js @@ -1,6 +1,8 @@ import { EDIT_ENTITY_PERMISSION, EDIT_REPORT_PERMISSION } from "./context/Permissions" +import { defaultDesiredResponseTimes } from "./defaults" import { capitalize, + getMetricResponseDeadline, getMetricResponseOverrun, getMetricTags, getMetricTarget, @@ -225,6 +227,34 @@ it("returns false when the user prefers light mode", () => { expect(userPrefersDarkMode("follow_os")).toBe(false) }) +it("returns the metric response deadline", () => { + expect(getMetricResponseDeadline({}, {})).toStrictEqual(null) +}) + +it("returns the metric response deadline based on the tech debt end date", () => { + const tomorrow = new Date() + tomorrow.setDate(tomorrow.getDate() + 1) + expect( + getMetricResponseDeadline({ status: "debt_target_met", debt_end_date: tomorrow.toISOString() }, {}), + ).toStrictEqual(tomorrow) +}) + +it("returns the metric response deadline based on the desired response time", () => { + const statusStart = "2024-01-01" + const expectedDeadline = new Date(statusStart) + expectedDeadline.setDate(expectedDeadline.getDate() + defaultDesiredResponseTimes.near_target_met) + expect(getMetricResponseDeadline({ status: "near_target_met", status_start: statusStart }, {})).toStrictEqual( + expectedDeadline, + ) +}) + +it("does not return a metric response deadline when the report has been configured not to have a desired response time", () => { + const report = { desired_response_times: { near_target_met: "" } } + expect(getMetricResponseDeadline({ status: "near_target_met", status_start: "2024-02-02" }, report)).toStrictEqual( + null, + ) +}) + it("returns the metric response overrun when there are no measurements", () => { expect(getMetricResponseOverrun("uuid", metric, {}, [], dataModel)).toStrictEqual({ overruns: [], @@ -257,8 +287,17 @@ it("returns the metric response overrun when there is one long measurement", () }) }) +it("returns the metric response overrun when there is one long measurement and the report has no desired response times", () => { + const report = { desired_response_times: { unknown: "" } } + const measurements = [{ metric_uuid: "uuid", start: "2000-01-01", end: "2000-01-31" }] + expect(getMetricResponseOverrun("uuid", metric, report, measurements, dataModel)).toStrictEqual({ + overruns: [], + totalOverrun: 0, + }) +}) + it("returns the metric response overrun when there is one long measurement and the report has desired response times", () => { - const report = { desired_response_times: { unknown: 10 } } + const report = { desired_response_times: { unknown: "10" } } const measurements = [{ metric_uuid: "uuid", start: "2000-01-01", end: "2000-01-31" }] expect(getMetricResponseOverrun("uuid", metric, report, measurements, dataModel)).toStrictEqual({ overruns: [ diff --git a/tests/feature_tests/.vulture_ignore_list.py b/tests/feature_tests/.vulture_ignore_list.py index 23cc82d6b8..ce74c037cc 100644 --- a/tests/feature_tests/.vulture_ignore_list.py +++ b/tests/feature_tests/.vulture_ignore_list.py @@ -52,6 +52,8 @@ do_not_accept_technical_debt # unused function (src/steps/metric.py:62) measurement_request # unused function (src/steps/metric.py:68) assert_metric_is_being_measured # unused function (src/steps/metric.py:76) +assert_metric_technical_debt_end_date_is_empty # unused function (src/steps/metric.py:83) +assert_metric_technical_debt_end_date_is_not_empty # unused function (src/steps/metric.py:90) add_notification_destination # unused function (src/steps/notification_destination.py:7) add_notification_destination_to_non_existing_report # unused function (src/steps/notification_destination.py:16) delete_notification_destination_of_non_existing_report # unused function (src/steps/notification_destination.py:24) @@ -85,6 +87,7 @@ change_non_existing_report_tracker_type # unused function (src/steps/report.py:165) get_non_existing_report_tracker_options # unused function (src/steps/report.py:172) get_non_existing_report_tracker_suggestions # unused function (src/steps/report.py:179) +set_technical_debt_desired_response_time # unused function (src/steps/report.py:186) search # unused function (src/steps/search.py:8) search_invalid # unused function (src/steps/search.py:15) check_search_results # unused function (src/steps/search.py:22) diff --git a/tests/feature_tests/src/features/measurement_entities.feature b/tests/feature_tests/src/features/measurement_entities.feature index 819d6d137a..70c53af53e 100644 --- a/tests/feature_tests/src/features/measurement_entities.feature +++ b/tests/feature_tests/src/features/measurement_entities.feature @@ -69,3 +69,28 @@ Feature: measurement entities And the client sets the status_end_date of entity 1 to "2022-02-02" And the client waits a second Then the metric status is "target_met" + + Scenario: mark an entity to a different status with the same desired response time + Given an existing metric with type "user_story_points" + And an existing source with type "azure_devops" + When the client sets the false_positive desired response time to 10 + And the client waits a second + And the client sets the wont_fix desired response time to 10 + And the client waits a second + And the client sets the confirmed desired response time to 10 + And the collector measures "120" + | key | story_points | + | 1 | 100 | + | 2 | 20 | + Then the metric status is "target_met" + When the client sets the status of entity 1 to "false_positive" + And the client waits a second + Then the metric status is "target_not_met" + When the client waits a second + And the client sets the status of entity 1 to "wont_fix" + When the client waits a second + Then the metric status is "target_not_met" + When the client waits a second + When the client sets the status of entity 1 to "confirmed" + And the client waits a second + Then the metric status is "target_met" diff --git a/tests/feature_tests/src/features/metric.feature b/tests/feature_tests/src/features/metric.feature index dc1e9aeba3..73b58bdd83 100644 --- a/tests/feature_tests/src/features/metric.feature +++ b/tests/feature_tests/src/features/metric.feature @@ -96,6 +96,17 @@ Feature: metric Then the metric status is "target_not_met" And the metric debt_target is "None" + Scenario: accept technical debt without desired technical debt response time + Given an existing metric + And an existing source + When the client sets the debt_target_met desired response time to empty + And the client accepts the technical debt + Then the metric technical debt end date is empty + When the client does not accept the technical debt + And the client sets the debt_target_met desired response time to 10 + And the client accepts the technical debt + Then the metric technical debt end date is not empty + Scenario: change metric name Given an existing metric When the client changes the metric name to "New name" diff --git a/tests/feature_tests/src/steps/metric.py b/tests/feature_tests/src/steps/metric.py index 2c9ed4bcd1..026124fb63 100644 --- a/tests/feature_tests/src/steps/metric.py +++ b/tests/feature_tests/src/steps/metric.py @@ -2,7 +2,7 @@ from datetime import UTC, datetime -from asserts import assert_equal, assert_false, assert_true +from asserts import assert_equal, assert_false, assert_is_none, assert_is_not_none, assert_true from behave import then, when from behave.runner import Context @@ -78,3 +78,17 @@ def assert_metric_is_being_measured(context: Context, being: str) -> None: """Assert that the metric is 'being' measured' or is 'not being' measured.""" metric = get_item(context, "metric") assert_equal(bool(being == "being"), metric["latest_measurement"]["measurement_requested"]) + + +@then("the metric technical debt end date is empty") +def assert_metric_technical_debt_end_date_is_empty(context: Context) -> None: + """Assert the metric technical debt end date is empty.""" + metric = get_item(context, "metric") + assert_is_none(metric.get("debt_end_date")) + + +@then("the metric technical debt end date is not empty") +def assert_metric_technical_debt_end_date_is_not_empty(context: Context) -> None: + """Assert the metric technical debt end date is not empty.""" + metric = get_item(context, "metric") + assert_is_not_none(metric["debt_end_date"]) diff --git a/tests/feature_tests/src/steps/report.py b/tests/feature_tests/src/steps/report.py index 957cb24dea..cf6f01978e 100644 --- a/tests/feature_tests/src/steps/report.py +++ b/tests/feature_tests/src/steps/report.py @@ -9,6 +9,8 @@ from behave import then, when from behave.runner import Context +from item import get_item + @when("the client gets the metric status summary") def get_report_metric_status_summary(context: Context) -> None: @@ -181,3 +183,15 @@ def get_non_existing_report_tracker_suggestions(context: Context) -> None: """Get the issue tracker suggestions of a non-existing report.""" context.uuid["report"] = report_uuid = "report-does-not-exist" context.get(f"report/{report_uuid}/issue_tracker/suggestions/query") + + +@when("the client sets the {status} desired response time to {time}") +def set_technical_debt_desired_response_time(context: Context, status: str, time: str) -> None: + """Set the technical debt desired response time of the specified status.""" + desired_response_times = get_item(context, "report").get("desired_response_times", {}) + desired_response_times[status] = None if time == "empty" else int(time) + report_uuid = context.uuid["report"] + context.post( + f"report/{report_uuid}/attribute/desired_response_times", + {"desired_response_times": desired_response_times}, + )