diff --git a/components/collector/src/base_collectors/metric_collector.py b/components/collector/src/base_collectors/metric_collector.py index c1578155e7..0b0c55f720 100644 --- a/components/collector/src/base_collectors/metric_collector.py +++ b/components/collector/src/base_collectors/metric_collector.py @@ -52,7 +52,7 @@ async def collect(self) -> MetricMeasurement | None: source_measurement.source_uuid = source_uuid if issue_status_collectors: issue_statuses = await asyncio.gather(*issue_status_collectors) - return MetricMeasurement(measurements, issue_statuses) + return MetricMeasurement(self._metric, measurements, issue_statuses) def __source_collectors(self) -> list[Coroutine]: """Create the source collectors for the metric.""" diff --git a/components/collector/src/model/measurement.py b/components/collector/src/model/measurement.py index 48f68d52a7..07ffd89719 100644 --- a/components/collector/src/model/measurement.py +++ b/components/collector/src/model/measurement.py @@ -2,6 +2,8 @@ from collections.abc import Sequence +from shared.model.metric import Metric + from collector_utilities.type import URL, ErrorMessage, Value from .entity import Entities @@ -54,9 +56,11 @@ class MetricMeasurement: def __init__( self, + metric: Metric, source_measurements: Sequence[SourceMeasurement], issue_statuses: Sequence[IssueStatus], ) -> None: + self.metric = metric self.sources = source_measurements self.issue_statuses = issue_statuses self.has_error = any(source_measurement.has_error() for source_measurement in source_measurements) @@ -70,6 +74,7 @@ def as_dict(self) -> dict: "has_error": self.has_error, "metric_uuid": self.metric_uuid, "report_uuid": self.report_uuid, + "source_parameter_hash": self.metric.source_parameter_hash() } if self.issue_statuses: measurement["issue_status"] = [issue_status.as_dict() for issue_status in self.issue_statuses] diff --git a/components/collector/tests/base_collectors/test_collector.py b/components/collector/tests/base_collectors/test_collector.py index 34b7761bbd..a61a6d1aa6 100644 --- a/components/collector/tests/base_collectors/test_collector.py +++ b/components/collector/tests/base_collectors/test_collector.py @@ -94,6 +94,7 @@ def expected_source(self, **kwargs: str | None) -> dict[str, str | None | list[d def expected_measurement( self, *, + source_parameter_hash: str = "0f6df164677525409f6269ec97e4118d", metric_uuid: str = "metric_uuid", **expected_source_kwargs, ) -> dict[str, bool | list | str]: @@ -103,6 +104,7 @@ def expected_measurement( "sources": [self.expected_source(**expected_source_kwargs)], "metric_uuid": metric_uuid, "report_uuid": "report1", + "source_parameter_hash": source_parameter_hash, } async def test_fetch_successful(self): @@ -228,6 +230,7 @@ async def test_prioritize_edited_metrics(self): expected_call2 = call( self.database, self.expected_measurement( + source_parameter_hash="ebfcffc74dc017799cb4da33090a9867", metric_uuid="metric_uuid2", api_url=edited_url, landing_url=edited_url, @@ -261,6 +264,7 @@ async def test_missing_mandatory_parameter_with_default_value(self): post.assert_called_once_with( self.database, self.expected_measurement( + source_parameter_hash="8c3b464958e9ad0f20fb2e3b74c80519", value="0", entities=[], api_url="", diff --git a/components/frontend/src/measurement/MeasurementValue.js b/components/frontend/src/measurement/MeasurementValue.js index 40292bbfef..f25e19cfe8 100644 --- a/components/frontend/src/measurement/MeasurementValue.js +++ b/components/frontend/src/measurement/MeasurementValue.js @@ -1,5 +1,5 @@ import { useContext } from "react" -import { Message } from "semantic-ui-react" +import { Icon, Message } from "semantic-ui-react" import { DataModel } from "../context/DataModel" import { Label, Popup } from "../semantic_ui_react_wrappers" @@ -18,7 +18,18 @@ export function MeasurementValue({ metric, reportDate }) { const end = new Date(metric.latest_measurement.end) const now = reportDate ?? new Date() const staleMeasurementValue = now - end > MILLISECONDS_PER_HOUR // No new measurement for more than one hour means something is wrong - const trigger = staleMeasurementValue ? : {value} + let trigger + if (staleMeasurementValue) { + trigger = + } else if (metric.latest_measurement.outdated) { + trigger = ( + + ) + } else { + trigger = {value} + } return ( {staleMeasurementValue && ( @@ -28,6 +39,13 @@ export function MeasurementValue({ metric, reportDate }) { content="This may indicate a problem with Quality-time itself. Please contact a system administrator." /> )} + {metric.latest_measurement.outdated && ( + + )} {metric.status ? "The metric was last measured" : "Last measurement attempt"} diff --git a/components/frontend/src/measurement/MeasurementValue.test.js b/components/frontend/src/measurement/MeasurementValue.test.js index a6d1393684..cae8287c17 100644 --- a/components/frontend/src/measurement/MeasurementValue.test.js +++ b/components/frontend/src/measurement/MeasurementValue.test.js @@ -45,6 +45,31 @@ it("renders a value that has not been measured yet", () => { expect(screen.getAllByText(/\?/).length).toBe(1) }) +it("renders an outdated value", async () => { + render( + + + , + ) + const measurementValue = screen.getByText(/1/) + expect(measurementValue.className).toContain("yellow") + expect(measurementValue.children[0].className).toContain("loading") + await userEvent.hover(measurementValue) + await waitFor(() => { + expect(screen.queryByText(/Latest measurement out of date/)).not.toBe(null) + expect( + screen.queryByText(/The source configuration of this metric was changed after the latest measurement/), + ).not.toBe(null) + }) +}) + it("renders a minutes value", () => { render( diff --git a/components/shared_code/src/shared/model/measurement.py b/components/shared_code/src/shared/model/measurement.py index ef628580ae..5b4c65cc51 100644 --- a/components/shared_code/src/shared/model/measurement.py +++ b/components/shared_code/src/shared/model/measurement.py @@ -4,7 +4,7 @@ from abc import abstractmethod from collections.abc import Sequence -from typing import TYPE_CHECKING, ClassVar, cast +from typing import TYPE_CHECKING, ClassVar, Self, cast from packaging.version import InvalidVersion, Version @@ -294,3 +294,11 @@ def summarize(self) -> dict[str, str | dict[str, str | None]]: "start": self["start"], "end": self["end"], } + + def summarize_latest(self) -> Self: + """Return a summary of this measurement, given that it is the latest measurement.""" + if parameter_hash := self.get("source_parameter_hash"): + if parameter_hash != self.metric.source_parameter_hash(): + self["outdated"] = True + del self["source_parameter_hash"] + return self diff --git a/components/shared_code/src/shared/model/metric.py b/components/shared_code/src/shared/model/metric.py index 2e2ac4f97d..46de0afb12 100644 --- a/components/shared_code/src/shared/model/metric.py +++ b/components/shared_code/src/shared/model/metric.py @@ -6,6 +6,7 @@ from datetime import date from typing import TYPE_CHECKING, cast +from shared.utils.functions import md5_hash from shared.utils.type import ( Direction, MetricId, @@ -159,9 +160,14 @@ def summarize(self, measurements: list[Measurement], **kwargs) -> dict: summary["scale"] = self.scale() summary["status"] = self.status(latest_measurement) summary["status_start"] = latest_measurement.status_start() if latest_measurement else None - summary["latest_measurement"] = latest_measurement + summary["latest_measurement"] = latest_measurement.summarize_latest() if latest_measurement else None summary["recent_measurements"] = [measurement.summarize() for measurement in measurements] if latest_measurement: summary["issue_status"] = self.issue_statuses(latest_measurement) summary.update(kwargs) return summary + + def source_parameter_hash(self) -> str: + """Return a hash of the source parameters that can be used to check for changed parameters.""" + source_parameters = [source.get("parameters") for source in self.sources] + return md5_hash(str(source_parameters)) diff --git a/components/shared_code/tests/shared/model/test_metric.py b/components/shared_code/tests/shared/model/test_metric.py index 4dc33bd986..d32037df2e 100644 --- a/components/shared_code/tests/shared/model/test_metric.py +++ b/components/shared_code/tests/shared/model/test_metric.py @@ -25,91 +25,91 @@ def create_metric(self, **kwargs) -> Metric: """Return a metric fixture.""" return Metric(self.DATA_MODEL, {"type": "fixture_metric_type", **kwargs}, METRIC_ID) + def create_expected_summary( + self, status: str = "", measurement_timestamp: str = "", outdated: bool = False, **kwargs + ) -> dict[str, None | str | list | dict]: + """Return an expected summary.""" + if measurement_timestamp: + latest_measurement = { + "count": {"value": 1, "start": measurement_timestamp}, + "end": measurement_timestamp, + "sources": [], + "start": measurement_timestamp, + "status": status, + } + recent_measurements = [ + { + "count": {"status": status, "value": 1}, + "end": measurement_timestamp, + "start": measurement_timestamp, + }, + ] + else: + latest_measurement = None + recent_measurements = [] + summary = { + "type": "fixture_metric_type", + "scale": "count", + "status": status if status else None, + "status_start": None, + "latest_measurement": latest_measurement, + "recent_measurements": recent_measurements, + "sources": {}, + **kwargs, + } + if measurement_timestamp: + summary["issue_status"] = [] + if outdated: + summary["latest_measurement"]["outdated"] = True + return summary + def test_summarize_empty_metric(self): """Test that a minimal metric returns a summary.""" summary = self.create_metric().summarize([]) - self.assertDictEqual( - summary, - { - "type": "fixture_metric_type", - "scale": "count", - "status": None, - "status_start": None, - "latest_measurement": None, - "recent_measurements": [], - "sources": {}, - }, + self.assertDictEqual(summary, self.create_expected_summary()) + + def test_summarize_metric_with_a_measurement(self): + """Test that the metric summary includes the measurement.""" + metric = self.create_metric() + measurement_timestamp = iso_timestamp() + measurement = Measurement( + metric, + count={"value": 1, "start": measurement_timestamp}, + status="target_met", ) + summary = metric.summarize([measurement]) + self.assertDictEqual(summary, self.create_expected_summary("target_met", measurement_timestamp)) - def test_summarize_measurements(self): - """Test that a minimal metric returns a summary.""" + def test_summarize_metric_with_an_outdated_measurement(self): + """Test that the metric is marked outdated when the source parameters have changed.""" metric = self.create_metric() measurement_timestamp = iso_timestamp() measurement = Measurement( metric, count={"value": 1, "start": measurement_timestamp}, status="target_met", + source_parameter_hash="old hash", ) summary = metric.summarize([measurement]) - self.assertDictEqual( - summary, - { - "issue_status": [], - "type": "fixture_metric_type", - "scale": "count", - "status": "target_met", - "status_start": None, - "latest_measurement": { - "count": {"value": 1, "start": measurement_timestamp}, - "end": measurement_timestamp, - "sources": [], - "start": measurement_timestamp, - "status": "target_met", - }, - "recent_measurements": [ - { - "count": {"status": "target_met", "value": 1}, - "end": measurement_timestamp, - "start": measurement_timestamp, - }, - ], - "sources": {}, - }, + self.assertDictEqual(summary, self.create_expected_summary("target_met", measurement_timestamp, outdated=True)) + + def test_summarize_metric_with_an_up_to_date_measurement(self): + """Test that the metric is not marked outdated when the source parameters have not changed.""" + metric = self.create_metric() + measurement_timestamp = iso_timestamp() + measurement = Measurement( + metric, + count={"value": 1, "start": measurement_timestamp}, + status="target_met", + source_parameter_hash=metric.source_parameter_hash(), ) + summary = metric.summarize([measurement]) + self.assertDictEqual(summary, self.create_expected_summary("target_met", measurement_timestamp)) def test_summarize_metric_data(self): """Test that metric data is summarized.""" summary = self.create_metric(some_metric_property="some_value").summarize([]) - self.assertDictEqual( - summary, - { - "type": "fixture_metric_type", - "scale": "count", - "status": None, - "status_start": None, - "latest_measurement": None, - "recent_measurements": [], - "some_metric_property": "some_value", - "sources": {}, - }, - ) - - def test_summarize_metric_kwargs(self): - """Test that metric data is summarized.""" - summary = self.create_metric().summarize([], some_kw="some_arg") - self.assertDictEqual( - summary, - { - "type": "fixture_metric_type", - "scale": "count", - "status": None, - "status_start": None, - "latest_measurement": None, - "recent_measurements": [], - "some_kw": "some_arg", - "sources": {}, - }, - ) + self.assertDictEqual(summary, self.create_expected_summary(some_metric_property="some_value")) def test_debt_end_date(self): """Test debt end date.""" @@ -151,3 +151,7 @@ def test_not_equal(self): def test_not_equal_to_string(self): """Test that a metric is not equal to a string.""" self.assertNotEqual(self.create_metric(), "foo") + + def test_source_parameter_hash(self): + """Test the source parameter hash.""" + self.assertEqual("d751713988987e9331980363e24189ce", self.create_metric().source_parameter_hash())