Skip to content

Commit

Permalink
Request a metric to be measured ASAP.
Browse files Browse the repository at this point in the history
Allow for requesting a metric to be measured as soon as possible.

Closes #920.
  • Loading branch information
fniessink committed Jun 24, 2024
1 parent d0ad23f commit 3e69fe9
Show file tree
Hide file tree
Showing 15 changed files with 192 additions and 16 deletions.
14 changes: 12 additions & 2 deletions components/collector/src/database/measurements.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@ def create_measurement(database: Database, measurement_data: dict) -> None:
measurement.copy_entity_first_seen_timestamps(latest_successful)
measurement.copy_entity_user_data(latest if latest_successful is None else latest_successful)
measurement.update_measurement() # Update the scales so we can compare the two measurements
if measurement.equals(latest):
# If the new measurement is equal to the previous one, merge them together
if measurement.equals(latest) and no_measurement_requested_after(latest):
# If the new measurement is equal to the latest one and no measurement update was requested, merge the
# two measurements together. Don't merge the measurements together when a measurement was requested,
# even if the new measurement value did not change, so that the frontend gets a new measurement count
# via the number of measurements server-sent events endpoint and knows the requested measurement was done.
update_measurement_end(database, latest["_id"])
return
insert_new_measurement(database, measurement)
Expand All @@ -38,3 +41,10 @@ def create_measurement(database: Database, measurement_data: dict) -> None:
def update_measurement_end(database: Database, measurement_id: MeasurementId) -> None: # pragma: no feature-test-cover
"""Set the end date and time of the measurement to the current date and time."""
database.measurements.update_one(filter={"_id": measurement_id}, update={"$set": {"end": iso_timestamp()}})


def no_measurement_requested_after(measurement: Measurement) -> bool:
"""Return whether a measurement was requested later than the end of this measurement."""
if measurement_request_timestamp := measurement.metric.get("measurement_requested"):
return bool(measurement_request_timestamp < measurement["end"])
return True
9 changes: 9 additions & 0 deletions components/collector/tests/database/test_measurements.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,12 @@ def test_copy_first_seen_timestamps(self):
"2023-07-18",
next(self.database.measurements.find())["sources"][0]["entities"][0]["first_seen"],
)

def test_create_measurement_after_measurement_was_requested(self):
"""Test that a new measurement is added after a measurement request, even if the measurement is unchanged."""
self.database["reports"].insert_one(create_report(report_uuid=REPORT_ID))
create_measurement(self.database, self.measurement_data())
self.database["reports"].update_one({"report_uuid": REPORT_ID}, {"$set": {"last": False}})
self.database["reports"].insert_one(create_report(report_uuid=REPORT_ID, measurement_requested="3000-01-01"))
create_measurement(self.database, self.measurement_data())
self.assertEqual(2, len(list(self.database.measurements.find())))
4 changes: 4 additions & 0 deletions components/collector/tests/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ def create_report(title: str = "Title", report_uuid: str = "report1", **kwargs)
metric_id: MetricId = METRIC_ID
metric_type = "dependencies"
source_type = "pip"
measurement_requested = None

for key, value in kwargs.items():
match key:
Expand All @@ -32,6 +33,8 @@ def create_report(title: str = "Title", report_uuid: str = "report1", **kwargs)
metric_type = value
case "source_type":
source_type = value
case "measurement_requested":
measurement_requested = value
case _:
raise ValueError

Expand All @@ -53,6 +56,7 @@ def create_report(title: str = "Title", report_uuid: str = "report1", **kwargs)
"parameters": {"url": "https://url", "password": "password"},
},
},
"measurement_requested": measurement_requested,
},
}

Expand Down
16 changes: 14 additions & 2 deletions components/frontend/src/measurement/MeasurementValue.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@ import { Icon } from "semantic-ui-react"
import { DataModel } from "../context/DataModel"
import { Label, Popup } from "../semantic_ui_react_wrappers"
import { datePropType, metricPropType } from "../sharedPropTypes"
import { formatMetricValue, getMetricScale, getMetricValue, MILLISECONDS_PER_HOUR } from "../utils"
import {
formatMetricValue,
getMetricScale,
getMetricValue,
isMeasurementRequested,
MILLISECONDS_PER_HOUR,
} from "../utils"
import { TimeAgoWithDate } from "../widgets/TimeAgoWithDate"
import { WarningMessage } from "../widgets/WarningMessage"

Expand Down Expand Up @@ -42,8 +48,9 @@ export function MeasurementValue({ metric, reportDate }) {
const now = reportDate ?? new Date()
const stale = now - end > MILLISECONDS_PER_HOUR // No new measurement for more than one hour means something is wrong
const outdated = metric.latest_measurement.outdated ?? false
const requested = isMeasurementRequested(metric)
return (
<Popup trigger={measurementValueLabel(stale, outdated, value)} flowing hoverable>
<Popup trigger={measurementValueLabel(stale, outdated || requested, value)} flowing hoverable>
<WarningMessage
showIf={stale}
header="This metric was not recently measured"
Expand All @@ -54,6 +61,11 @@ export function MeasurementValue({ metric, reportDate }) {
header="Latest measurement out of date"
content="The source configuration of this metric was changed after the latest measurement."
/>
<WarningMessage
showIf={requested}
header="Measurement requested"
content="An update of the latest measurement was requested by a user."
/>
<TimeAgoWithDate date={metric.latest_measurement.end}>
{metric.status ? "The metric was last measured" : "Last measurement attempt"}
</TimeAgoWithDate>
Expand Down
33 changes: 33 additions & 0 deletions components/frontend/src/measurement/MeasurementValue.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { MeasurementValue } from "./MeasurementValue"

function renderMeasurementValue({
latest_measurement = {},
measurement_requested = null,
reportDate = null,
scale = "count",
status = null,
Expand All @@ -17,6 +18,7 @@ function renderMeasurementValue({
<MeasurementValue
metric={{
latest_measurement: latest_measurement,
measurement_requested: measurement_requested,
scale: scale,
status: status,
type: type,
Expand Down Expand Up @@ -64,6 +66,37 @@ it("renders an outdated value", async () => {
})
})

it("renders a value for which a measurement was requested", async () => {
const now = new Date().toISOString()
renderMeasurementValue({
latest_measurement: { count: { value: 1 }, start: now, end: now },
measurement_requested: now,
})
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(/Measurement requested/)).not.toBe(null)
expect(screen.queryByText(/An update of the latest measurement was requested by a user/)).not.toBe(null)
})
})

it("renders a value for which a measurement was requested, but which is now up to date", async () => {
const now = new Date().toISOString()
renderMeasurementValue({
latest_measurement: { count: { value: 1 }, start: now, end: now },
measurement_requested: "2024-01-01T00:00:00",
})
const measurementValue = screen.getByText(/1/)
expect(measurementValue.className).not.toContain("yellow")
await userEvent.hover(measurementValue)
await waitFor(() => {
expect(screen.queryByText(/Measurement requested/)).toBe(null)
expect(screen.queryByText(/An update of the latest measurement was requested by a user/)).toBe(null)
})
})

it("renders a minutes value", () => {
renderMeasurementValue({
type: "duration",
Expand Down
30 changes: 27 additions & 3 deletions components/frontend/src/metric/MetricDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,44 @@ import { EDIT_REPORT_PERMISSION, ReadOnlyOrEditable } from "../context/Permissio
import { Label, Tab } from "../semantic_ui_react_wrappers"
import {
datePropType,
metricPropType,
reportPropType,
reportsPropType,
stringsPropType,
stringsURLSearchQueryPropType,
} from "../sharedPropTypes"
import { SourceEntities } from "../source/SourceEntities"
import { Sources } from "../source/Sources"
import { getMetricScale, getSourceName } from "../utils"
import { DeleteButton, PermLinkButton, ReorderButtonGroup } from "../widgets/Button"
import { getMetricScale, getSourceName, isMeasurementRequested } from "../utils"
import { ActionButton, DeleteButton, PermLinkButton, ReorderButtonGroup } from "../widgets/Button"
import { FocusableTab } from "../widgets/FocusableTab"
import { showMessage } from "../widgets/toast"
import { MetricConfigurationParameters } from "./MetricConfigurationParameters"
import { MetricDebtParameters } from "./MetricDebtParameters"
import { MetricTypeHeader } from "./MetricTypeHeader"
import { TrendGraph } from "./TrendGraph"

function Buttons({ isFirstMetric, isLastMetric, metric_uuid, reload, stopFilteringAndSorting, url }) {
function RequestMeasurementButton({ metric, metric_uuid, reload }) {
const measurementRequested = isMeasurementRequested(metric)
return (
<ActionButton
action="Measure"
disabled={measurementRequested}
icon="refresh"
itemType="metric"
loading={measurementRequested}
onClick={() => set_metric_attribute(metric_uuid, "measurement_requested", new Date().toISOString(), reload)}
popup={`Measure this metric as soon as possible`}
/>
)
}
RequestMeasurementButton.propTypes = {
metric: metricPropType,
metric_uuid: string,
reload: func,
}

function Buttons({ isFirstMetric, isLastMetric, metric, metric_uuid, reload, stopFilteringAndSorting, url }) {
return (
<ReadOnlyOrEditable
requiredPermissions={[EDIT_REPORT_PERMISSION]}
Expand All @@ -44,6 +65,7 @@ function Buttons({ isFirstMetric, isLastMetric, metric_uuid, reload, stopFilteri
}}
/>
<PermLinkButton itemType="metric" url={url} />
<RequestMeasurementButton metric={metric} metric_uuid={metric_uuid} reload={reload} />
<DeleteButton itemType="metric" onClick={() => delete_metric(metric_uuid, reload)} />
</div>
}
Expand All @@ -53,6 +75,7 @@ function Buttons({ isFirstMetric, isLastMetric, metric_uuid, reload, stopFilteri
Buttons.propTypes = {
isFirstMetric: bool,
isLastMetric: bool,
metric: metricPropType,
metric_uuid: string,
reload: func,
stopFilteringAndSorting: func,
Expand Down Expand Up @@ -234,6 +257,7 @@ export function MetricDetails({
panes={panes}
/>
<Buttons
metric={metric}
metric_uuid={metric_uuid}
isFirstMetric={isFirstMetric}
isLastMetric={isLastMetric}
Expand Down
10 changes: 10 additions & 0 deletions components/frontend/src/metric/MetricDetails.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,3 +176,13 @@ it("deletes the metric", async () => {
fireEvent.click(screen.getByText(/Delete metric/))
expect(fetch_server_api.fetch_server_api).toHaveBeenCalledWith("delete", "metric/metric_uuid", {})
})

it("measures the metric", async () => {
await renderMetricDetails()
fireEvent.click(screen.getByText(/Measure metric/))
expect(fetch_server_api.fetch_server_api).toHaveBeenCalledWith(
"post",
"metric/metric_uuid/attribute/measurement_requested",
expect.objectContaining({}), // Ignore the attribute value, it's new Date().toISOString()
)
})
10 changes: 10 additions & 0 deletions components/frontend/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,16 @@ export function getMetricUnit(metric, dataModel) {
return metric.unit || dataModel.metrics[metric.type].unit || ""
}

export function isMeasurementRequested(metric) {
if (metric.measurement_requested) {
if (metric.latest_measurement) {
return new Date(metric.measurement_requested) >= new Date(metric.latest_measurement.end)
}
return true
}
return false
}

export function getMetricResponseDeadline(metric, report) {
let deadline = null
const status = metric.status || "unknown"
Expand Down
9 changes: 9 additions & 0 deletions components/frontend/src/utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
getSubjectType,
getSubjectTypeMetrics,
getUserPermissions,
isMeasurementRequested,
niceNumber,
nrMetricsInReport,
nrMetricsInReports,
Expand Down Expand Up @@ -450,3 +451,11 @@ it("sums numbers", () => {
expect(sum({ a: 1 })).toBe(1)
expect(sum({ a: 1, b: 2 })).toBe(3)
})

it("returns whether a measurement is requested for the metric", () => {
expect(isMeasurementRequested({})).toBe(false)
expect(isMeasurementRequested({ measurement_requested: "2000-01-01" })).toBe(true)
const latest = { end: "2024-01-01" }
expect(isMeasurementRequested({ measurement_requested: "2023-01-01", latest_measurement: latest })).toBe(false)
expect(isMeasurementRequested({ measurement_requested: "2025-01-01", latest_measurement: latest })).toBe(true)
})
2 changes: 2 additions & 0 deletions components/shared_code/src/shared/model/measurement.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,8 @@ def summarize_latest(self) -> Self:
if parameter_hash != self.metric.source_parameter_hash():
self["outdated"] = True
del self["source_parameter_hash"]
if measurement_requested := self.metric.get("measurement_requested"):
self["measurement_requested"] = measurement_requested >= self["end"]
return self

def source_parameter_hash(self) -> str:
Expand Down
23 changes: 23 additions & 0 deletions components/shared_code/tests/shared/model/test_measurement.py
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,29 @@ def test_summarize_with_non_default_start_date(self):
measurement.summarize(),
)

def test_summarize_latest_with_old_measurement_request(self):
"""Test that the measurement includes the measurement requested flag when a measurement was requested."""
now = datetime.now(tz=UTC).replace(microsecond=0)
just_now = (now - timedelta(seconds=1)).isoformat()
metric = self.metric(measurement_requested=just_now)
measurement = self.measurement(metric, start=now.isoformat(), end=now.isoformat())
self.assertFalse(measurement.summarize_latest()["measurement_requested"])

def test_summarize_latest_with_new_measurement_request(self):
"""Test that the measurement includes the measurement requested flag when a measurement was requested."""
now = datetime.now(tz=UTC).replace(microsecond=0)
just_now = (now - timedelta(seconds=1)).isoformat()
metric = self.metric(measurement_requested=now.isoformat())
measurement = self.measurement(metric, start=just_now, end=just_now)
self.assertTrue(measurement.summarize_latest()["measurement_requested"])

def test_summarize_latest_without_measurement_request(self):
"""Test that the measurement has no measurement requested flag when no measurement was requested."""
now = datetime.now(tz=UTC).replace(microsecond=0).isoformat()
metric = self.metric()
measurement = self.measurement(metric, start=now, end=now)
self.assertNotIn("measurement_requested", measurement.summarize_latest())


class CalculateMeasurementValueTest(MeasurementTestCase):
"""Unit tests for calculating the measurement value from one or more source measurements."""
Expand Down
1 change: 1 addition & 0 deletions docs/src/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ If your currently installed *Quality-time* version is v4.10.0 or older, please r
### Added

- Allow for hiding the legend card via the Settings panel.
- Allow for requesting a metric to be measured as soon as possible. Closes [#920](https://github.com/ICTU/quality-time/issues/920).
- In the UI, while a source parameter of a metric have been changed and the metric has not been measured with the new parameter yet, show a spinner. Closes [#3134](https://github.com/ICTU/quality-time/issues/3134).
- Explain the difference between security warnings and violations in a new FAQ section of the documentation. Closes [#7797](https://github.com/ICTU/quality-time/issues/7797).
- When adding a metric to a subject, add an option to hide metric types already used. Closes [#7992](https://github.com/ICTU/quality-time/issues/7992).
Expand Down
19 changes: 10 additions & 9 deletions tests/feature_tests/.vulture_ignore_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,16 @@
_.report_date # unused attribute (src/steps/measurement.py:138)
_.report_date # unused attribute (src/steps/measurement.py:140)
check_reports_overview_measurements # unused function (src/steps/measurement.py:144)
assert_issue_status # unused function (src/steps/metric.py:10)
assert_issue_id_suggestions # unused function (src/steps/metric.py:21)
retrieve_issue_tracker_options # unused function (src/steps/metric.py:28)
assert_issue_tracker_options # unused function (src/steps/metric.py:34)
create_new_issue # unused function (src/steps/metric.py:40)
new_issue_response # unused function (src/steps/metric.py:46)
accept_technical_debt # unused function (src/steps/metric.py:54)
do_not_accept_technical_debt # unused function (src/steps/metric.py:60)
assert_metric_lastest_measurement_is_outdated # unused function (src/steps/metric.py:66)
assert_issue_status # unused function (src/steps/metric.py:12)
assert_issue_id_suggestions # unused function (src/steps/metric.py:23)
retrieve_issue_tracker_options # unused function (src/steps/metric.py:30)
assert_issue_tracker_options # unused function (src/steps/metric.py:36)
create_new_issue # unused function (src/steps/metric.py:42)
new_issue_response # unused function (src/steps/metric.py:48)
accept_technical_debt # unused function (src/steps/metric.py:56)
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)
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)
Expand Down
11 changes: 11 additions & 0 deletions tests/feature_tests/src/features/metric.feature
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,14 @@ Feature: metric
Given an existing metric
When the client changes the metric comment to "Text<script>alert('Danger')</script>"
Then the metric comment is "Text"

Scenario: request measurement
Given an existing metric
And an existing source
And the collector has measured "100"
When the client requests the metric to be measured
Then the metric is being measured
When the client waits a second
And the collector measures "100"
And the client waits a second
Then the metric is not being measured
17 changes: 17 additions & 0 deletions tests/feature_tests/src/steps/metric.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
"""Feature tests for metric specific attributes."""

from datetime import UTC, datetime

from asserts import assert_equal, assert_false, assert_true
from behave import then, when
from behave.runner import Context
Expand Down Expand Up @@ -61,3 +63,18 @@ def accept_technical_debt(context: Context) -> None:
def do_not_accept_technical_debt(context: Context) -> None:
"""Change the technical debt of the metric, including debt target and end date."""
context.post(f"metric/{context.uuid['metric']}/debt", json={"accept_debt": False})


@when("the client requests the metric to be measured")
def measurement_request(context: Context) -> None:
"""Request a metric to be measured."""
now = datetime.now(tz=UTC).replace(microsecond=0).isoformat()
attribute_endpoint = f"metric/{context.uuid['metric']}/attribute/measurement_requested"
context.post(attribute_endpoint, json={"measurement_requested": now})


@then("the metric is {being} measured")
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"])

0 comments on commit 3e69fe9

Please sign in to comment.