From c0b31ed4b19e3b8c6702d4436f259c8b84bf5228 Mon Sep 17 00:00:00 2001 From: Frank Niessink Date: Wed, 2 Oct 2019 15:07:50 +0200 Subject: [PATCH 1/2] Store measurement values for each scale that a metric supports so that the graphs show correct information when the user changes the metric scale. Fixes #637. --- components/frontend/src/metric/Measurement.js | 17 +++++------ .../frontend/src/metric/MeasurementDetails.js | 2 +- components/frontend/src/metric/TrendGraph.js | 9 +++--- .../frontend/src/metric/TrendSparkline.js | 7 +++-- .../server/src/database/measurements.py | 7 +++-- components/server/src/database/reports.py | 25 +++++++++-------- components/server/src/routes/report.py | 3 +- components/server/src/utilities/type.py | 5 ---- .../unittests/routes/test_measurement.py | 18 ++++++------ .../tests/unittests/routes/test_report.py | 28 +++++++++++-------- docs/CHANGELOG.md | 3 +- 11 files changed, 65 insertions(+), 59 deletions(-) delete mode 100644 components/server/src/utilities/type.py diff --git a/components/frontend/src/metric/Measurement.js b/components/frontend/src/metric/Measurement.js index 423ffaf5e0..4d85411789 100644 --- a/components/frontend/src/metric/Measurement.js +++ b/components/frontend/src/metric/Measurement.js @@ -8,10 +8,13 @@ import { Tag } from '../widgets/Tag'; import { TableRowWithDetails } from '../widgets/TableRowWithDetails'; export function Measurement(props) { + const metric = props.report.subjects[props.subject_uuid].metrics[props.metric_uuid]; + const metric_type = props.datamodel.metrics[metric.type]; + const metric_scale = metric.scale || metric_type.default_scale || "count"; var latest_measurement, start, end, value, status, sources, measurement_timestring; if (props.measurements.length === 0) { latest_measurement = null; - value = null; + value = "?"; status = null; sources = []; start = new Date(); @@ -20,8 +23,8 @@ export function Measurement(props) { } else { latest_measurement = props.measurements[props.measurements.length - 1]; sources = latest_measurement.sources; - value = latest_measurement.value; - status = latest_measurement.status; + value = latest_measurement[metric_scale].value || "?"; + status = latest_measurement[metric_scale].status || null; start = new Date(latest_measurement.start); end = new Date(latest_measurement.end); measurement_timestring = latest_measurement.end; @@ -30,15 +33,12 @@ export function Measurement(props) { target_met: 'check', near_target_met: 'warning', debt_target_met: 'money', target_not_met: 'x', null: 'question' }[status]; - const metric = props.report.subjects[props.subject_uuid].metrics[props.metric_uuid]; - const metric_type = props.datamodel.metrics[metric.type]; const target = metric.accept_debt ? metric.debt_target : metric.target; const metric_direction = {"<": "≦", ">": "≧"}[metric.direction || metric_type.direction]; const positive = status === "target_met"; const active = status === "debt_target_met"; const negative = status === "target_not_met"; const warning = status === "near_target_met"; - const metric_scale = metric.scale || metric_type.default_scale || "count"; const metric_unit_prefix = metric_scale === "percentage" ? "% " : " "; const metric_unit = `${metric_unit_prefix}${metric.unit || metric_type.unit}`; const metric_name = metric.name || metric_type.name; @@ -57,6 +57,7 @@ export function Measurement(props) { readOnly={props.readOnly} reload={props.reload} report={props.report} + scale={metric_scale} stop_sort={props.stop_sort} subject_uuid={props.subject_uuid} unit={metric_unit} @@ -67,14 +68,14 @@ export function Measurement(props) { {metric_name} - measurement.end >= week_ago_string)} /> + measurement.end >= week_ago_string)} scale={metric_scale} /> {(value === null ? '?' : value) + metric_unit}} + trigger={{value + metric_unit}} flowing hoverable> Measured ({start.toLocaleString()} - {end.toLocaleString()}) diff --git a/components/frontend/src/metric/MeasurementDetails.js b/components/frontend/src/metric/MeasurementDetails.js index 0d02d267a1..2445d2bd83 100644 --- a/components/frontend/src/metric/MeasurementDetails.js +++ b/components/frontend/src/metric/MeasurementDetails.js @@ -64,7 +64,7 @@ export function MeasurementDetails(props) { { menuItem: {'Trend'}, render: () => - + } ); diff --git a/components/frontend/src/metric/TrendGraph.js b/components/frontend/src/metric/TrendGraph.js index f368ca7efd..037db0d1c7 100644 --- a/components/frontend/src/metric/TrendGraph.js +++ b/components/frontend/src/metric/TrendGraph.js @@ -12,12 +12,13 @@ export function TrendGraph(props) { let max_y = 10; for (var i = 0; i < props.measurements.length; i++) { const measurement = props.measurements[i]; - const m = measurement.value !== null ? Number(measurement.value) : null; - if (m !== null && m > max_y) { max_y = m } + const value = measurement[props.scale].value || null; + const y = value !== null ? Number(value) : null; + if (y !== null && y > max_y) { max_y = y } const x1 = new Date(measurement.start); const x2 = new Date(measurement.end); - measurements.push({ y: m, x: x1 }); - measurements.push({ y: m, x: x2 }); + measurements.push({ y: y, x: x1 }); + measurements.push({ y: y, x: x2 }); } const axisStyle = { axisLabel: { padding: 30, fontSize: 11 }, tickLabels: { fontSize: 8 } }; return ( diff --git a/components/frontend/src/metric/TrendSparkline.js b/components/frontend/src/metric/TrendSparkline.js index 5181201fef..b7f8f23dd9 100644 --- a/components/frontend/src/metric/TrendSparkline.js +++ b/components/frontend/src/metric/TrendSparkline.js @@ -5,11 +5,12 @@ export function TrendSparkline(props) { let measurements = []; for (var i = 0; i < props.measurements.length; i++) { const measurement = props.measurements[i]; - const m = measurement.value !== null ? Number(measurement.value) : null; + const value = measurement[props.scale].value || null; + const y = value !== null ? Number(value) : null; const x1 = new Date(measurement.start); const x2 = new Date(measurement.end); - measurements.push({y: m, x: x1}); - measurements.push({y: m, x: x2}); + measurements.push({y: y, x: x1}); + measurements.push({y: y, x: x2}); } return ( diff --git a/components/server/src/database/measurements.py b/components/server/src/database/measurements.py index 2719210977..52989a7071 100644 --- a/components/server/src/database/measurements.py +++ b/components/server/src/database/measurements.py @@ -52,10 +52,11 @@ def insert_new_measurement(database: Database, measurement, metric=None): database, measurement["report_uuid"], measurement["metric_uuid"]) if metric is None else metric datamodel = latest_datamodel(database) metric_type = datamodel["metrics"][metric["type"]] - scale = metric.get("scale") or metric_type["default_scale"] direction = metric.get("direction") or metric_type["direction"] - measurement["value"] = calculate_measurement_value(measurement["sources"], metric["addition"], scale, direction) - measurement["status"] = determine_measurement_status(database, metric, measurement["value"]) + for scale in metric_type["scales"]: + value = calculate_measurement_value(measurement["sources"], metric["addition"], scale, direction) + status = determine_measurement_status(database, metric, value) + measurement[scale] = dict(value=value, status=status) measurement["start"] = measurement["end"] = iso_timestamp() # Mark this measurement as the most recent one: measurement["last"] = True diff --git a/components/server/src/database/reports.py b/components/server/src/database/reports.py index 891ee6c9fd..61847455ed 100644 --- a/components/server/src/database/reports.py +++ b/components/server/src/database/reports.py @@ -6,7 +6,7 @@ from pymongo.database import Database from utilities.functions import iso_timestamp -from utilities.type import Summary +from .datamodels import latest_datamodel def latest_reports(database: Database, max_iso_timestamp: str = ""): @@ -39,21 +39,22 @@ def summarize_report(database: Database, report) -> None: from .measurements import last_measurements # pylint:disable=cyclic-import status_color_mapping = dict( target_met="green", debt_target_met="grey", near_target_met="yellow", target_not_met="red") - summary = dict(red=0, green=0, yellow=0, grey=0, white=0) - summary_by_subject: Dict[str, Summary] = dict() - summary_by_tag: Dict[str, Summary] = dict() + report["summary"] = dict(red=0, green=0, yellow=0, grey=0, white=0) + report["summary_by_subject"] = dict() + report["summary_by_tag"] = dict() last_measurements_by_metric_uuid = {m["metric_uuid"]: m for m in last_measurements(database, report["report_uuid"])} + datamodel = latest_datamodel(database) for subject_uuid, subject in report.get("subjects", {}).items(): for metric_uuid, metric in subject.get("metrics", {}).items(): - last_measurement = last_measurements_by_metric_uuid.get(metric_uuid) - color = status_color_mapping.get(last_measurement["status"], "white") if last_measurement else "white" - summary[color] += 1 - summary_by_subject.setdefault(subject_uuid, dict(red=0, green=0, yellow=0, grey=0, white=0))[color] += 1 + last_measurement = last_measurements_by_metric_uuid.get(metric_uuid, dict()) + scale = metric.get("scale") or datamodel["metrics"][metric["type"]].get("default_scale", "count") + status = last_measurement.get(scale, {}).get("status", last_measurement.get("status", None)) + color = status_color_mapping.get(status, "white") + report["summary"][color] += 1 + report["summary_by_subject"].setdefault( + subject_uuid, dict(red=0, green=0, yellow=0, grey=0, white=0))[color] += 1 for tag in metric["tags"]: - summary_by_tag.setdefault(tag, dict(red=0, green=0, yellow=0, grey=0, white=0))[color] += 1 - report["summary"] = summary - report["summary_by_subject"] = summary_by_subject - report["summary_by_tag"] = summary_by_tag + report["summary_by_tag"].setdefault(tag, dict(red=0, green=0, yellow=0, grey=0, white=0))[color] += 1 def latest_report(database: Database, report_uuid: str): diff --git a/components/server/src/routes/report.py b/components/server/src/routes/report.py index aa6d790b68..6f7624214d 100644 --- a/components/server/src/routes/report.py +++ b/components/server/src/routes/report.py @@ -138,8 +138,7 @@ def post_metric_attribute(report_uuid: str, metric_uuid: str, metric_attribute: description=f"{sessions.user(database)} changed the {metric_attribute} of metric '{data.metric_name}' of " f"subject '{data.subject_name}' in report '{data.report_name}' from '{old_value}' to '{value}'.") insert_new_report(database, data.report) - if metric_attribute in ( - "accept_debt", "debt_target", "debt_end_date", "direction", "near_target", "scale", "target"): + if metric_attribute in ("accept_debt", "debt_target", "debt_end_date", "direction", "near_target", "target"): latest = latest_measurement(database, metric_uuid) if latest: return insert_new_measurement(database, latest, data.metric) diff --git a/components/server/src/utilities/type.py b/components/server/src/utilities/type.py deleted file mode 100644 index 16827f5853..0000000000 --- a/components/server/src/utilities/type.py +++ /dev/null @@ -1,5 +0,0 @@ -"""Quality-time specific types.""" - -from typing import Dict - -Summary = Dict[str, int] diff --git a/components/server/tests/unittests/routes/test_measurement.py b/components/server/tests/unittests/routes/test_measurement.py index 9fb812bdc3..f22079059f 100644 --- a/components/server/tests/unittests/routes/test_measurement.py +++ b/components/server/tests/unittests/routes/test_measurement.py @@ -35,7 +35,8 @@ def setUp(self): sources=dict(source_uuid=dict())))))) self.database.reports.find_one.return_value = report self.database.reports.distinct.return_value = ["report_uuid"] - self.database.datamodels.find_one.return_value = dict(_id="", metrics=dict(metric_type=dict(direction="<"))) + self.database.datamodels.find_one.return_value = dict( + _id="", metrics=dict(metric_type=dict(direction="<", scales=["count"]))) def set_measurement_id(measurement): measurement["_id"] = "measurement_id" @@ -48,8 +49,8 @@ def test_first_measurement(self, request): self.database.measurements.find_one.return_value = None request.json = dict(report_uuid="report_uuid", metric_uuid="metric_uuid", sources=[]) new_measurement = dict( - _id="measurement_id", report_uuid="report_uuid", metric_uuid="metric_uuid", sources=[], value=None, - status=None, start="2019-01-01", end="2019-01-01", last=True) + _id="measurement_id", report_uuid="report_uuid", metric_uuid="metric_uuid", sources=[], + count=dict(value=None, status=None), start="2019-01-01", end="2019-01-01", last=True) self.assertEqual(new_measurement, post_measurement(self.database)) self.database.measurements.insert_one.assert_called_once() @@ -67,8 +68,8 @@ def test_changed_measurement_value(self, request): sources = [dict(value="1", total=None, parse_error=None, connection_error=None, entities=[])] request.json = dict(report_uuid="report_uuid", metric_uuid="metric_uuid", sources=sources) new_measurement = dict( - _id="measurement_id", report_uuid="report_uuid", metric_uuid="metric_uuid", status="near_target_met", - start="2019-01-01", end="2019-01-01", value="1", last=True, sources=sources) + _id="measurement_id", report_uuid="report_uuid", metric_uuid="metric_uuid", last=True, + count=dict(status="near_target_met", value="1"), start="2019-01-01", end="2019-01-01", sources=sources) self.assertEqual(new_measurement, post_measurement(self.database)) self.database.measurements.insert_one.assert_called_once() @@ -80,8 +81,8 @@ def test_changed_measurement_entities(self, request): sources = [dict(value="1", total=None, parse_error=None, connection_error=None, entities=[dict(key="b")])] request.json = dict(report_uuid="report_uuid", metric_uuid="metric_uuid", sources=sources) new_measurement = dict( - _id="measurement_id", report_uuid="report_uuid", metric_uuid="metric_uuid", status="near_target_met", - start="2019-01-01", end="2019-01-01", value="1", last=True, sources=sources) + _id="measurement_id", report_uuid="report_uuid", metric_uuid="metric_uuid", last=True, + count=dict(status="near_target_met", value="1"), start="2019-01-01", end="2019-01-01", sources=sources) self.assertEqual(new_measurement, post_measurement(self.database)) self.database.measurements.insert_one.assert_called_once() @@ -128,7 +129,8 @@ def insert_one(new_measurement): type="metric_type", target="0", near_target="10", debt_target="0", accept_debt=False, scale="count", addition="sum", direction="<", tags=[]))))) database.datamodels = Mock() - database.datamodels.find_one.return_value = dict(_id=123, metrics=dict(metric_type=dict(direction="<"))) + database.datamodels.find_one.return_value = dict( + _id=123, metrics=dict(metric_type=dict(direction="<", scales=["count"]))) with patch("bottle.request", Mock(json=dict(attribute="value"))): measurement = set_entity_attribute("metric_uuid", "source_uuid", "entity_key", "attribute", database) entity = measurement["sources"][0]["entity_user_data"]["entity_key"] diff --git a/components/server/tests/unittests/routes/test_report.py b/components/server/tests/unittests/routes/test_report.py index 5f6efffad8..14de07fc3e 100644 --- a/components/server/tests/unittests/routes/test_report.py +++ b/components/server/tests/unittests/routes/test_report.py @@ -75,10 +75,10 @@ def setUp(self): self.database.datamodels.find_one.return_value = dict( _id="id", metrics=dict( - old_type=dict(name="Old type"), + old_type=dict(name="Old type", scales=["count"]), new_type=dict( - default_scale="count", addition="sum", direction="<", target="0", near_target="1", tags=[], - sources=["source_type"]))) + scales=["count"], default_scale="count", addition="sum", direction="<", target="0", near_target="1", + tags=[], sources=["source_type"]))) def test_post_metric_name(self, request): """Test that the metric name can be changed.""" @@ -125,8 +125,8 @@ def set_measurement_id(measurement): request.json = dict(target="10") self.assertEqual( dict( - _id="measurement_id", end="2019-01-01", sources=[], start="2019-01-01", status=None, value=None, - metric_uuid="metric_uuid", last=True), + _id="measurement_id", end="2019-01-01", sources=[], start="2019-01-01", + count=dict(status=None, value=None), metric_uuid="metric_uuid", last=True), post_metric_attribute("report_uuid", "metric_uuid", "target", self.database)) self.assertEqual( dict(report_uuid="report_uuid", subject_uuid="subject_uuid", metric_uuid="metric_uuid", @@ -146,8 +146,8 @@ def set_measurement_id(measurement): request.json = dict(debt_end_date="2019-06-07") self.assertEqual( dict( - _id="measurement_id", end="2019-01-01", sources=[], start="2019-01-01", last=True, status=None, - metric_uuid="metric_uuid", value=None), + _id="measurement_id", end="2019-01-01", sources=[], start="2019-01-01", last=True, + metric_uuid="metric_uuid", count=dict(value=None, status=None)), post_metric_attribute("report_uuid", "metric_uuid", "debt_end_date", self.database)) self.assertEqual( dict(report_uuid="report_uuid", subject_uuid="subject_uuid", metric_uuid="metric_uuid", @@ -413,14 +413,14 @@ def test_get_metrics(self): """Test that the metrics can be retrieved and deleted reports are skipped.""" report = dict( _id="id", report_uuid="report_uuid", - subjects=dict(subject_uuid=dict(metrics=dict(metric_uuid=dict(tags=[]))))) + subjects=dict(subject_uuid=dict(metrics=dict(metric_uuid=dict(type="metric_type", tags=[]))))) self.database.reports_overviews.find_one.return_value = dict(_id="id", title="Reports", subtitle="") self.database.reports.distinct.return_value = ["report_uuid", "deleted_report"] self.database.reports.find_one.side_effect = [report, dict(deleted=True)] self.database.measurements.find.return_value = [dict( _id="id", metric_uuid="metric_uuid", status="red", sources=[dict(source_uuid="source_uuid", parse_error=None, connection_error=None, value="42")])] - self.assertEqual(dict(metric_uuid=dict(tags=[])), get_metrics(self.database)) + self.assertEqual(dict(metric_uuid=dict(type="metric_type", tags=[])), get_metrics(self.database)) def test_delete_metric(self): """Test that the metric can be deleted.""" @@ -476,6 +476,8 @@ def test_add_report(self): def test_get_report(self): """Test that a report can be retrieved.""" + self.database.datamodels.find_one.return_value = dict( + _id="id", metrics=dict(metric_type=dict(default_scale="count"))) self.database.reports_overviews.find_one.return_value = dict(_id="id", title="Reports", subtitle="") self.database.measurements.find.return_value = [ dict( @@ -513,6 +515,8 @@ def test_post_reports_attribute(self, request): def test_get_tag_report(self, request): """Test that a tag report can be retrieved.""" date_time = request.report_date = iso_timestamp() + self.database.datamodels.find_one.return_value = dict( + _id="id", metrics=dict(metric_type=dict(default_scale="count"))) self.database.reports.find_one.return_value = None self.database.measurements.find.return_value = [] self.database.reports.distinct.return_value = ["report_uuid"] @@ -522,8 +526,8 @@ def test_get_tag_report(self, request): subject_without_metrics=dict(metrics=dict()), subject_uuid=dict( metrics=dict( - metric_with_tag=dict(tags=["tag"]), - metric_without_tag=dict(tags=["other tag"]))))) + metric_with_tag=dict(type="metric_type", tags=["tag"]), + metric_without_tag=dict(type="metric_type", tags=["other tag"]))))) self.assertEqual( dict( summary=dict(red=0, green=0, yellow=0, grey=0, white=1), @@ -531,5 +535,5 @@ def test_get_tag_report(self, request): summary_by_subject=dict(subject_uuid=dict(red=0, green=0, yellow=0, grey=0, white=1)), title='Report for tag "tag"', subtitle="Note: tag reports are read-only", report_uuid="tag-tag", timestamp=date_time, subjects=dict( - subject_uuid=dict(metrics=dict(metric_with_tag=dict(tags=["tag"]))))), + subject_uuid=dict(metrics=dict(metric_with_tag=dict(type="metric_type", tags=["tag"]))))), get_tag_report("tag", self.database)) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index da093155a3..29df558c5d 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -13,7 +13,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed -- Allow for specifying which test results (skipped, failed, errored, and/or passed) to count when using SonarQube as source for the "Tests" metric. Fixes [#634](https://github.com/ICTU/quality-time/issues/634). +- Allow for specifying which test results (skipped, failed, errored, and/or passed) to count when using SonarQube as source for the "Tests" metric. Fixes [#634](https://github.com/ICTU/quality-time/issues/634). +- Store measurement values for each scale that a metric supports so that the graphs show correct information when the user changes the metric scale. Fixes [#637](https://github.com/ICTU/quality-time/issues/637). ## [0.10.2] - [2019-09-26] From 6150591dd5d88df38ecacb907085cb4099136f67 Mon Sep 17 00:00:00 2001 From: Frank Niessink Date: Wed, 2 Oct 2019 15:21:42 +0200 Subject: [PATCH 2/2] Fix unit tests. --- components/frontend/src/metric/TrendGraph.test.js | 2 +- components/frontend/src/metric/TrendSparkline.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/components/frontend/src/metric/TrendGraph.test.js b/components/frontend/src/metric/TrendGraph.test.js index b68b07302c..3512ea8a34 100644 --- a/components/frontend/src/metric/TrendGraph.test.js +++ b/components/frontend/src/metric/TrendGraph.test.js @@ -10,6 +10,6 @@ it('renders without crashing', () => { it('renders measurements without crashing', () => { const div = document.createElement('div'); - ReactDOM.render(, div); + ReactDOM.render(, div); ReactDOM.unmountComponentAtNode(div); }); \ No newline at end of file diff --git a/components/frontend/src/metric/TrendSparkline.test.js b/components/frontend/src/metric/TrendSparkline.test.js index 7739fa77d8..c1b43269a5 100644 --- a/components/frontend/src/metric/TrendSparkline.test.js +++ b/components/frontend/src/metric/TrendSparkline.test.js @@ -10,6 +10,6 @@ it('renders without crashing', () => { it('renders measurements without crashing', () => { const div = document.createElement('div'); - ReactDOM.render(, div); + ReactDOM.render(, div); ReactDOM.unmountComponentAtNode(div); }); \ No newline at end of file