Skip to content

Commit 6b08f2d

Browse files
fniessinkfniessink
committed
Singular and plural versions of metric units
Add singular and plural versions of the metric units to the data model and allow the user to override both via the UI. Closes #550.
1 parent e80f1ed commit 6b08f2d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+464
-141
lines changed

‎components/api_server/src/example-reports/example-report-quality-time.json‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
"name": null,
2626
"scale": "count",
2727
"unit": null,
28+
"unit_singular": null,
2829
"accept_debt": false,
2930
"debt_target": null,
3031
"direction": null,
@@ -50,6 +51,7 @@
5051
"name": null,
5152
"scale": "count",
5253
"unit": null,
54+
"unit_singular": null,
5355
"accept_debt": false,
5456
"debt_target": null,
5557
"direction": null,
@@ -75,6 +77,7 @@
7577
"name": null,
7678
"scale": "count",
7779
"unit": null,
80+
"unit_singular": null,
7881
"accept_debt": false,
7982
"debt_target": null,
8083
"direction": null,
@@ -100,6 +103,7 @@
100103
"name": null,
101104
"scale": "count",
102105
"unit": null,
106+
"unit_singular": null,
103107
"accept_debt": false,
104108
"debt_target": null,
105109
"direction": null,
@@ -125,6 +129,7 @@
125129
"name": null,
126130
"scale": "count",
127131
"unit": null,
132+
"unit_singular": null,
128133
"accept_debt": false,
129134
"debt_target": null,
130135
"direction": null,
@@ -151,6 +156,7 @@
151156
"name": null,
152157
"scale": "count",
153158
"unit": null,
159+
"unit_singular": null,
154160
"accept_debt": false,
155161
"debt_target": null,
156162
"direction": null,
@@ -176,6 +182,7 @@
176182
"name": null,
177183
"scale": "count",
178184
"unit": null,
185+
"unit_singular": null,
179186
"accept_debt": false,
180187
"debt_target": null,
181188
"direction": null,
@@ -203,6 +210,7 @@
203210
"name": null,
204211
"scale": "count",
205212
"unit": null,
213+
"unit_singular": null,
206214
"accept_debt": false,
207215
"debt_target": null,
208216
"direction": null,
@@ -230,6 +238,7 @@
230238
"name": null,
231239
"scale": "count",
232240
"unit": null,
241+
"unit_singular": null,
233242
"accept_debt": false,
234243
"debt_target": null,
235244
"direction": null,
@@ -257,6 +266,7 @@
257266
"name": null,
258267
"scale": "count",
259268
"unit": null,
269+
"unit_singular": null,
260270
"accept_debt": false,
261271
"debt_target": null,
262272
"direction": null,
@@ -282,6 +292,7 @@
282292
"name": null,
283293
"scale": "count",
284294
"unit": null,
295+
"unit_singular": null,
285296
"accept_debt": false,
286297
"debt_target": null,
287298
"direction": null,
@@ -307,6 +318,7 @@
307318
"name": null,
308319
"scale": "version_number",
309320
"unit": null,
321+
"unit_singular": null,
310322
"evaluate_targets": false,
311323
"accept_debt": false,
312324
"debt_target": null,

‎components/api_server/src/example-reports/example-report.json‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
"name": null,
2727
"scale": "count",
2828
"unit": null,
29+
"unit_singular": null,
2930
"accept_debt": false,
3031
"debt_target": null,
3132
"direction": null,
@@ -56,6 +57,7 @@
5657
"name": "Percentage test code",
5758
"scale": "percentage",
5859
"unit": null,
60+
"unit_singular": null,
5961
"accept_debt": false,
6062
"debt_target": null,
6163
"direction": ">",
@@ -118,6 +120,7 @@
118120
"name": null,
119121
"scale": "count",
120122
"unit": null,
123+
"unit_singular": null,
121124
"accept_debt": false,
122125
"debt_target": null,
123126
"direction": null,
@@ -180,6 +183,7 @@
180183
"name": null,
181184
"scale": "count",
182185
"unit": null,
186+
"unit_singular": null,
183187
"accept_debt": false,
184188
"debt_target": null,
185189
"direction": null,
@@ -238,6 +242,7 @@
238242
"name": null,
239243
"scale": "count",
240244
"unit": null,
245+
"unit_singular": null,
241246
"accept_debt": false,
242247
"debt_target": null,
243248
"direction": null,
@@ -283,6 +288,7 @@
283288
"name": null,
284289
"scale": "count",
285290
"unit": null,
291+
"unit_singular": null,
286292
"accept_debt": false,
287293
"debt_target": null,
288294
"direction": null,
@@ -328,6 +334,7 @@
328334
"name": null,
329335
"scale": "count",
330336
"unit": null,
337+
"unit_singular": null,
331338
"accept_debt": false,
332339
"debt_target": null,
333340
"direction": null,
@@ -373,6 +380,7 @@
373380
"name": null,
374381
"scale": "count",
375382
"unit": null,
383+
"unit_singular": null,
376384
"accept_debt": false,
377385
"debt_target": null,
378386
"direction": null,
@@ -440,6 +448,7 @@
440448
"name": null,
441449
"scale": "count",
442450
"unit": null,
451+
"unit_singular": null,
443452
"accept_debt": false,
444453
"debt_target": null,
445454
"direction": null,
@@ -574,6 +583,7 @@
574583
"name": null,
575584
"scale": "count",
576585
"unit": null,
586+
"unit_singular": null,
577587
"accept_debt": false,
578588
"debt_target": null,
579589
"direction": null,
@@ -640,6 +650,7 @@
640650
"name": null,
641651
"scale": "count",
642652
"unit": null,
653+
"unit_singular": null,
643654
"accept_debt": false,
644655
"debt_target": null,
645656
"direction": null,
@@ -662,6 +673,7 @@
662673
"name": "Time since last security test",
663674
"scale": "count",
664675
"unit": null,
676+
"unit_singular": null,
665677
"accept_debt": false,
666678
"debt_target": null,
667679
"direction": null,
@@ -689,6 +701,7 @@
689701
"name": null,
690702
"scale": "count",
691703
"unit": null,
704+
"unit_singular": null,
692705
"accept_debt": false,
693706
"debt_target": null,
694707
"direction": null,
@@ -753,6 +766,7 @@
753766
"name": "Number of performance tests",
754767
"scale": "count",
755768
"unit": null,
769+
"unit_singular": null,
756770
"accept_debt": false,
757771
"debt_target": null,
758772
"direction": null,
@@ -817,6 +831,7 @@
817831
"name": null,
818832
"scale": "count",
819833
"unit": null,
834+
"unit_singular": null,
820835
"accept_debt": false,
821836
"debt_target": null,
822837
"direction": null,
@@ -842,6 +857,7 @@
842857
"name": null,
843858
"scale": "percentage",
844859
"unit": null,
860+
"unit_singular": null,
845861
"accept_debt": false,
846862
"debt_target": null,
847863
"direction": null,
@@ -867,6 +883,7 @@
867883
"name": null,
868884
"scale": "percentage",
869885
"unit": null,
886+
"unit_singular": null,
870887
"accept_debt": false,
871888
"debt_target": null,
872889
"direction": null,
@@ -904,6 +921,7 @@
904921
"name": null,
905922
"scale": "count",
906923
"unit": null,
924+
"unit_singular": null,
907925
"accept_debt": false,
908926
"debt_target": null,
909927
"direction": null,
@@ -929,6 +947,7 @@
929947
"name": "Time since last performance test",
930948
"scale": "count",
931949
"unit": null,
950+
"unit_singular": null,
932951
"accept_debt": false,
933952
"debt_target": null,
934953
"direction": null,
@@ -982,6 +1001,7 @@
9821001
"name": null,
9831002
"scale": "count",
9841003
"unit": null,
1004+
"unit_singular": null,
9851005
"accept_debt": false,
9861006
"debt_target": null,
9871007
"direction": null,
@@ -1008,6 +1028,7 @@
10081028
"name": "Time since last Axe-core report",
10091029
"scale": "count",
10101030
"unit": null,
1031+
"unit_singular": null,
10111032
"accept_debt": false,
10121033
"debt_target": null,
10131034
"direction": null,
@@ -1033,6 +1054,7 @@
10331054
"name": null,
10341055
"scale": "version_number",
10351056
"unit": null,
1057+
"unit_singular": null,
10361058
"accept_debt": false,
10371059
"debt_target": null,
10381060
"direction": null,

‎components/api_server/src/model/defaults.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ def default_metric_attributes(metric_type: str):
2020
"name": None,
2121
"scale": metric.default_scale,
2222
"unit": None,
23+
"unit_singular": None,
2324
"evaluate_targets": metric.evaluate_targets,
2425
"accept_debt": False,
2526
"debt_target": None,

‎components/api_server/src/routes/metric.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,11 +217,12 @@ def create_issue_text(metric: Metric, measured_value: Value) -> tuple[str, str]:
217217
# for the text formatting notation used by Jira API version 2.
218218
source_urls = [f"[{url}|{url}]" for url in [get_source_url(source) for source in metric.sources] if url]
219219
percentage = "%" if metric.scale() == "percentage" else ""
220-
issue_summary = f"Fix {measured_value}{percentage} {metric.unit} from {source_names}"
220+
unit = metric.unit_singular if measured_value in ("1", "1.0") else metric.unit
221+
issue_summary = f"Fix {measured_value}{percentage} {unit} from {source_names}"
221222
source_url_str = f"\nPlease go to {', '.join(source_urls)} for more details." if source_urls else ""
222223
issue_description = (
223224
f"The metric [{metric.name}|{metric_url}] in Quality-time reports "
224-
f"{measured_value}{percentage} {metric.unit} from {source_names}.{source_url_str}\n"
225+
f"{measured_value}{percentage} {unit} from {source_names}.{source_url_str}\n"
225226
)
226227
return issue_summary, issue_description
227228

‎components/api_server/tests/model/test_actions.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,14 @@ class MoveItemToIndexTest(unittest.TestCase):
134134
def setUp(self):
135135
"""Override to set up the subject under test."""
136136
self.data_model = {
137-
"metrics": {"security_warnings": {"name": "Security warnings", "unit": "warnings", "tags": []}},
137+
"metrics": {
138+
"security_warnings": {
139+
"name": "Security warnings",
140+
"unit": "warnings",
141+
"unit_singular": "warning",
142+
"tags": [],
143+
}
144+
},
138145
"subjects": {"software": {"name": "Software"}},
139146
}
140147

‎components/api_server/tests/model/test_defaults.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ def test_default_metric_attributes(self):
3636
"sources": {},
3737
"tags": ["ci"],
3838
"unit": None,
39+
"unit_singular": None,
3940
},
4041
default_metric_attributes("software_version"),
4142
)

‎components/api_server/tests/routes/test_metric.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,7 @@ def setUp(self):
585585
"type": "violations",
586586
"name": "name",
587587
"unit": Unit.VIOLATIONS,
588+
"unit_singular": Unit.VIOLATION,
588589
"sources": self.sources,
589590
},
590591
},
@@ -632,6 +633,20 @@ def test_add_metric_issue(self, requests_post):
632633
self.assert_issue_posted(requests_post)
633634
self.assert_issue_inserted()
634635

636+
@patch("bottle.request", Mock(json={"metric_url": METRIC_URL}))
637+
def test_add_metric_issue_when_measurement_is_one(self, requests_post):
638+
"""Test that the singular unit is used if the measurement is one."""
639+
self.measurement["count"]["value"] = "1"
640+
self.expected_json["fields"]["summary"] = "Fix 1 violation from Source"
641+
description = self.expected_json["fields"]["description"]
642+
self.expected_json["fields"]["description"] = description.replace("42 violations", "1 violation")
643+
response = Mock()
644+
response.json.return_value = {"key": "FOO-42"}
645+
requests_post.return_value = response
646+
self.assertEqual({"ok": True, "issue_url": self.ISSUE_URL}, add_metric_issue(METRIC_ID, self.database))
647+
self.assert_issue_posted(requests_post)
648+
self.assert_issue_inserted()
649+
635650
@patch("bottle.request", Mock(json={"metric_url": METRIC_URL}))
636651
def test_add_metric_issue_with_landing_url(self, requests_post):
637652
"""Test that the metric landing URL is used if available."""

‎components/collector/src/source_collectors/quality_time/metrics.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ def __parse_entity(self, entity: Entity, metric, landing_url: URL) -> Entity:
7777
status_start = self.__metric_status_start(metric.get("status_start", ""))
7878
entity["status_start_date"] = status_start.isoformat() if status_start else ""
7979
entity["unit"] = metric.get("unit") or DATA_MODEL.metrics[metric["type"]].unit.value
80+
entity["unit_singular"] = metric.get("unit_singular") or DATA_MODEL.metrics[metric["type"]].unit_singular.value
8081
entity["measurement"] = value
8182
direction = str(metric.get("direction") or DATA_MODEL.metrics[metric["type"]].direction.value)
8283
direction = {"<": "≦", ">": "≧"}.get(direction, direction)

‎components/collector/tests/fixtures.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ def create_report(title: str = "Title", report_uuid: str = "report1", **kwargs)
4646
"target": "0",
4747
"accept_debt": False,
4848
"scale": "count",
49-
"unit": "foo",
5049
"webhook": "www.webhook.com",
5150
"scales": ["count", "percentage"],
5251
"sources": {

‎components/collector/tests/source_collectors/quality_time/test_metrics.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ def setUp(self):
2525
"measurement": "20",
2626
"target": "≦ 2",
2727
"unit": "violations",
28+
"unit_singular": "violation",
2829
"status": "target_not_met",
2930
"status_start_date": "2020-05-23T07:53:17+00:00",
3031
},

0 commit comments

Comments
 (0)