Skip to content

Commit

Permalink
Support creating issues for specific measurement entities. Closes #5955.
Browse files Browse the repository at this point in the history
  • Loading branch information
fniessink committed Jan 26, 2024
1 parent 7505b60 commit 1cc6d6b
Show file tree
Hide file tree
Showing 16 changed files with 97 additions and 26 deletions.
16 changes: 12 additions & 4 deletions components/api_server/src/routes/metric.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,18 +187,26 @@ def add_metric_issue(metric_uuid: MetricId, database: Database):
metric, subject = report.instance_and_parents_for_uuid(metric_uuid=metric_uuid)
last_measurement = latest_successful_measurement(database, metric)
measured_value = last_measurement.value() if last_measurement else "missing"
entity_key = dict(bottle.request.json).get("entity_key")
issue_tracker = report.issue_tracker()
issue_key, error = issue_tracker.create_issue(*create_issue_text(metric, measured_value))
if error:
return {"ok": False, "error": error}
else: # pragma: no feature-test-cover # noqa: RET505
old_issue_ids = metric.get("issue_ids") or []
new_issue_ids = sorted([issue_key, *old_issue_ids])
new_issue_ids: list[str] | dict[str, list[str]]
if entity_key:
old_issue_ids = metric.get("entity_issue_ids", {})
new_issue_ids = {**old_issue_ids}
new_issue_ids.setdefault(entity_key, []).append(issue_key)
report["subjects"][subject.uuid]["metrics"][metric_uuid]["entity_issue_ids"] = new_issue_ids
else:
old_issue_ids = metric.get("issue_ids") or []
new_issue_ids = sorted([issue_key, *old_issue_ids])
report["subjects"][subject.uuid]["metrics"][metric_uuid]["issue_ids"] = new_issue_ids
description = (
f"{{user}} changed the issue_ids of metric '{metric.name}' of subject "
f"{{user}} changed the {'entity_' if entity_key else ''}issue_ids of metric '{metric.name}' of subject "
f"'{subject.name}' in report '{report.name}' from '{old_issue_ids}' to '{new_issue_ids}'."
)
report["subjects"][subject.uuid]["metrics"][metric_uuid]["issue_ids"] = new_issue_ids
insert_new_report(database, description, [report.uuid, subject.uuid, metric.uuid], report)
return {"ok": True, "issue_url": issue_tracker.browse_url(issue_key)}

Expand Down
16 changes: 16 additions & 0 deletions components/api_server/tests/routes/test_metric.py
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,12 @@ def assert_issue_inserted(self):
inserted_issue_ids = inserted_report["subjects"][SUBJECT_ID]["metrics"][METRIC_ID]["issue_ids"]
self.assertEqual(["FOO-42"], inserted_issue_ids)

def assert_entity_issue_inserted(self):
"""Check that the entity issue is inserted in the database."""
inserted_report = self.database.reports.insert_one.call_args_list[0][0][0]
inserted_entity_issue_ids = inserted_report["subjects"][SUBJECT_ID]["metrics"][METRIC_ID]["entity_issue_ids"]
self.assertEqual({"key": ["FOO-42"]}, inserted_entity_issue_ids)

@patch("bottle.request", Mock(json={"metric_url": METRIC_URL}))
def test_add_metric_issue(self, requests_post):
"""Test that an issue can be added to the issue tracker."""
Expand All @@ -592,6 +598,16 @@ def test_add_metric_issue(self, requests_post):
self.assert_issue_posted(requests_post)
self.assert_issue_inserted()

@patch("bottle.request", Mock(json={"entity_key": "key", "metric_url": METRIC_URL}))
def test_add_metric_entity_issue(self, requests_post):
"""Test that an issue can be added to the issue tracker and linked to a measurement entity."""
response = Mock()
response.json.return_value = {"key": "FOO-42"}
requests_post.return_value = response
self.assertEqual({"ok": True, "issue_url": self.ISSUE_URL}, add_metric_issue(METRIC_ID, self.database))
self.assert_issue_posted(requests_post)
self.assert_entity_issue_inserted()

@patch("bottle.request", Mock(json={"metric_url": METRIC_URL}))
@patch("model.issue_tracker.requests.get")
def test_add_metric_issue_with_labels(self, requests_get, requests_post):
Expand Down
8 changes: 4 additions & 4 deletions components/collector/src/base_collectors/metric_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ def __issue_status_collectors(self) -> list[Coroutine]:
tracker_type = tracker.get("type")
has_tracker = bool(tracker_type and tracker.get("parameters", {}).get("url"))
if has_tracker and (collector_class := SourceCollector.get_subclass(tracker_type, "issue_status")):
return [
collector_class(self.__session, tracker).collect_issue_status(issue_id)
for issue_id in self._metric.get("issue_ids", [])
]
issue_ids = self._metric.get("issue_ids", [])
for entity_issue_id_list in self._metric.get("entity_issue_ids", {}).values():
issue_ids.extend(entity_issue_id_list)
return [collector_class(self.__session, tracker).collect_issue_status(issue_id) for issue_id in issue_ids]
return []

def __has_all_mandatory_parameters(self, source) -> bool:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,16 @@ async def test_issue_status(self):
response = await self.collect(get_request_json_return_value=issue_status_json)
self.assert_issue_status(response)

async def test_entity_issue_status(self):
"""Test that the issue status is returned for a measurement entity."""
self.metric["issue_ids"] = []
self.metric["entity_issue_ids"] = {"entity key": ["FOO-42"]}
issue_status_json = {
"fields": {"status": {"name": self.ISSUE_NAME, "statusCategory": {"key": "new"}}, "created": self.CREATED},
}
response = await self.collect(get_request_json_return_value=issue_status_json)
self.assert_issue_status(response)

async def test_issue_status_doing(self):
"""Test that the issue status is returned."""
issue_status_json = {
Expand Down
4 changes: 2 additions & 2 deletions components/frontend/src/api/metric.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ export function set_metric_debt(metric_uuid, value, reload) {
fetch_server_api('post', `metric/${metric_uuid}/debt`, { "accept_debt": value }).then(reload)
}

export function add_metric_issue(metric_uuid, reload) {
const payload = {metric_url: `${window.location}#${metric_uuid}`}
export function add_metric_issue(entityKey, metric_uuid, reload) {
const payload = {metric_url: `${window.location}#${metric_uuid}`, entity_key: entityKey}
return fetch_server_api('post', `metric/${metric_uuid}/issue/new`, payload).then((json) => {
if (json.ok) {
window.open(json.issue_url)
Expand Down
5 changes: 3 additions & 2 deletions components/frontend/src/issue/IssueStatus.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,15 @@ IssuesWithTracker.propTypes = {
settings: settingsPropType
}

export function IssueStatus({ metric, issueTrackerMissing, settings }) {
const issueIds = getMetricIssueIds(metric)
export function IssueStatus({ entityKey, metric, issueTrackerMissing, settings }) {
const issueIds = getMetricIssueIds(metric, entityKey)
if (issueTrackerMissing && issueIds.length > 0) {
return <IssuesWithoutTracker issueIds={issueIds} />
}
return <IssuesWithTracker issueIds={issueIds} metric={metric} settings={settings} />
}
IssueStatus.propTypes = {
entityKey: PropTypes.string,
issueTrackerMissing: PropTypes.bool,
metric: metricPropType,
settings: settingsPropType
Expand Down
13 changes: 8 additions & 5 deletions components/frontend/src/issue/IssuesRows.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,22 @@ import { ErrorMessage } from '../errorMessage';
import { getMetricIssueIds } from '../utils';
import { metricPropType, reportPropType } from '../sharedPropTypes';

function CreateIssueButton({ issueTrackerConfigured, issueTrackerInstruction, metric_uuid, target, reload }) {
function CreateIssueButton({ entityKey, issueTrackerConfigured, issueTrackerInstruction, metric_uuid, target, reload }) {
return (
<ActionButton
action='Create new'
disabled={!issueTrackerConfigured}
fluid
icon='plus'
item_type='issue'
onClick={() => add_metric_issue(metric_uuid, reload)}
onClick={() => add_metric_issue(entityKey, metric_uuid, reload)}
popup={<>Create a new issue for this {target} in the configured issue tracker and add its identifier to the tracked issue identifiers.{issueTrackerInstruction}</>}
position='top center'
/>
)
}
CreateIssueButton.propTypes = {
entityKey: PropTypes.string,
issueTrackerConfigured: PropTypes.bool,
issueTrackerInstruction: PropTypes.node,
metric_uuid: PropTypes.string,
Expand Down Expand Up @@ -77,11 +78,12 @@ IssueIdentifiers.propTypes = {
reload: PropTypes.func
}

export function IssuesRows({ metric, metric_uuid, reload, report, target }) {
export function IssuesRows({ entityKey, metric, metric_uuid, reload, report, target }) {
const parameters = report?.issue_tracker?.parameters;
const issueTrackerConfigured = Boolean(report?.issue_tracker?.type && parameters?.url && parameters?.project_key && parameters?.issue_type);
const issueTrackerInstruction = issueTrackerConfigured ? null : <p>Please configure an issue tracker by expanding the report title, selecting the 'Issue tracker' tab, and configuring an issue tracker.</p>;
const issueIdentifiersProps = {
entityKey: entityKey,
issueTrackerInstruction: issueTrackerInstruction,
metric: metric,
metric_uuid: metric_uuid,
Expand All @@ -103,6 +105,7 @@ export function IssuesRows({ metric, metric_uuid, reload, report, target }) {
<>
<Grid.Column width={3} verticalAlign="bottom">
<CreateIssueButton
entityKey={entityKey}
issueTrackerConfigured={issueTrackerConfigured}
issueTrackerInstruction={issueTrackerInstruction}
metric_uuid={metric_uuid}
Expand All @@ -117,7 +120,7 @@ export function IssuesRows({ metric, metric_uuid, reload, report, target }) {
}
/>
</Grid.Row>
{(getMetricIssueIds(metric).length > 0 && !issueTrackerConfigured) &&
{(getMetricIssueIds(metric, entityKey).length > 0 && !issueTrackerConfigured) &&
<Grid.Row>
<Grid.Column width={16}>
<ErrorMessage title="No issue tracker configured" message={issueTrackerInstruction} />
Expand Down Expand Up @@ -156,4 +159,4 @@ IssuesRows.propTypes = {
reload: PropTypes.func,
report: reportPropType,
target: PropTypes.string,
}
}
7 changes: 4 additions & 3 deletions components/frontend/src/metric/MetricDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { MetricConfigurationParameters } from './MetricConfigurationParameters';
import { MetricDebtParameters } from './MetricDebtParameters';
import { MetricTypeHeader } from './MetricTypeHeader';
import { TrendGraph } from './TrendGraph';
import { datePropType, reportPropType, reportsPropType, stringsPropType, stringsURLSearchQueryPropType} from '../sharedPropTypes';
import { datePropType, reportPropType, reportsPropType, settingsPropType, stringsPropType, stringsURLSearchQueryPropType } from '../sharedPropTypes';

function Buttons({ isFirstMetric, isLastMetric, metric_uuid, reload, stopFilteringAndSorting }) {
return (
Expand Down Expand Up @@ -54,7 +54,6 @@ fetchMeasurements.propTypes = {
setMeasurements: PropTypes.func
}


export function MetricDetails({
changed_fields,
isFirstMetric,
Expand All @@ -64,6 +63,7 @@ export function MetricDetails({
report_date,
reports,
report,
settings,
stopFilteringAndSorting,
subject_uuid,
expandedItems,
Expand Down Expand Up @@ -142,7 +142,7 @@ export function MetricDetails({
const source_name = get_source_name(report_source, dataModel);
panes.push({
menuItem: <Menu.Item key={source.source_uuid}><FocusableTab>{source_name}</FocusableTab></Menu.Item>,
render: () => <Tab.Pane><SourceEntities report={report} metric={metric} metric_uuid={metric_uuid} source={source} reload={measurementsReload} /></Tab.Pane>
render: () => <Tab.Pane><SourceEntities report={report} metric={metric} metric_uuid={metric_uuid} source={source} reload={measurementsReload} settings={settings} /></Tab.Pane>
});
});
}
Expand Down Expand Up @@ -174,6 +174,7 @@ MetricDetails.propTypes = {
report_date: datePropType,
reports: reportsPropType,
report: reportPropType,
settings: settingsPropType,
stopFilteringAndSorting: PropTypes.func,
subject_uuid: PropTypes.string,
expandedItems: stringsURLSearchQueryPropType
Expand Down
10 changes: 8 additions & 2 deletions components/frontend/src/source/SourceEntities.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Button, Icon, Popup, Table } from '../semantic_ui_react_wrappers';
import { SourceEntity } from './SourceEntity';
import { capitalize } from '../utils';
import { DataModel } from '../context/DataModel';
import { metricPropType, reportPropType, sourcePropType } from '../sharedPropTypes';
import { metricPropType, reportPropType, settingsPropType, sourcePropType } from '../sharedPropTypes';

export function alignment(attributeType, attributeAlignment) {
if (attributeAlignment === "left" || attributeAlignment === "right") {
Expand All @@ -18,7 +18,7 @@ alignment.propTypes = {
attributeAligment: PropTypes.string
}

export function SourceEntities({ metric, metric_uuid, reload, report, source }) {
export function SourceEntities({ metric, metric_uuid, reload, report, settings, source }) {
const dataModel = useContext(DataModel)
const [hideIgnoredEntities, setHideIgnoredEntities] = useState(false);
const [sortColumn, setSortColumn] = useState(null);
Expand Down Expand Up @@ -73,6 +73,9 @@ export function SourceEntities({ metric, metric_uuid, reload, report, source })
>
{capitalize(entity_name)} first seen
</Table.HeaderCell>
<Table.HeaderCell>
Issues
</Table.HeaderCell>
{entity_attributes.map((entity_attribute) =>
<Table.HeaderCell
key={entity_attribute.key}
Expand Down Expand Up @@ -110,9 +113,11 @@ export function SourceEntities({ metric, metric_uuid, reload, report, source })
entity_name={entity_name}
hide_ignored_entities={hideIgnoredEntities}
key={entity.key}
metric={metric}
metric_uuid={metric_uuid}
reload={reload}
report={report}
settings={settings}
status={source.entity_user_data?.[entity.key]?.status ?? "unconfirmed"}
status_end_date={source.entity_user_data?.[entity.key]?.status_end_date ?? ""}
rationale={source.entity_user_data?.[entity.key]?.rationale ?? ""}
Expand All @@ -134,5 +139,6 @@ SourceEntities.propTypes = {
metric_uuid: PropTypes.string,
reload: PropTypes.func,
report: reportPropType,
settings: settingsPropType,
source: sourcePropType
}
11 changes: 10 additions & 1 deletion components/frontend/src/source/SourceEntity.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import React, { useState } from 'react';
import PropTypes from 'prop-types';
import { Table } from 'semantic-ui-react';
import { IssueStatus } from '../issue/IssueStatus';
import { TableRowWithDetails } from '../widgets/TableRowWithDetails';
import { TimeAgoWithDate } from '../widgets/TimeAgoWithDate';
import { SourceEntityDetails } from './SourceEntityDetails';
import { SourceEntityAttribute } from './SourceEntityAttribute';
import { source_entity_status_name } from './source_entity_status';
import { alignment } from './SourceEntities';
import { entityAttributesPropType, entityPropType, entityStatusPropType, reportPropType } from '../sharedPropTypes';
import { entityAttributesPropType, entityPropType, entityStatusPropType, metricPropType, reportPropType, settingsPropType } from '../sharedPropTypes';
import "./SourceEntity.css";

function entityCanBeIgnored(status, statusEndDateString) {
Expand All @@ -23,6 +24,7 @@ entityCanBeIgnored.propTypes = {

export function SourceEntity(
{
metric,
metric_uuid,
source_uuid,
hide_ignored_entities,
Expand All @@ -32,6 +34,7 @@ export function SourceEntity(
rationale,
reload,
report,
settings,
status,
status_end_date
}
Expand All @@ -53,6 +56,7 @@ export function SourceEntity(
}
const details = <SourceEntityDetails
entity={entity}
metric={metric}
metric_uuid={metric_uuid}
name={entity_name}
rationale={rationale}
Expand All @@ -68,6 +72,9 @@ export function SourceEntity(
<Table.Cell style={style}>{status === "unconfirmed" ? "" : status_end_date}</Table.Cell>
<Table.Cell style={style}>{status === "unconfirmed" ? "" : rationale}</Table.Cell>
<Table.Cell style={style}>{entity.first_seen ? <TimeAgoWithDate dateFirst date={entity.first_seen} /> : ""}</Table.Cell>
<Table.Cell>
<IssueStatus entityKey={entity.key} metric={metric} issueTrackerMissing={!report.issue_tracker} settings={settings} />
</Table.Cell>
{entity_attributes.map((entity_attribute) =>
<Table.Cell
key={entity_attribute.key}
Expand All @@ -80,6 +87,7 @@ export function SourceEntity(
);
}
SourceEntity.propTypes = {
metric: metricPropType,
metric_uuid: PropTypes.string,
source_uuid: PropTypes.string,
hide_ignored_entities: PropTypes.bool,
Expand All @@ -89,6 +97,7 @@ SourceEntity.propTypes = {
rationale: PropTypes.string,
reload: PropTypes.func,
report: reportPropType,
settings: settingsPropType,
status: entityStatusPropType,
status_end_date: PropTypes.string
}
2 changes: 2 additions & 0 deletions components/frontend/src/source/SourceEntity.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ function renderSourceEntity(
entity_attributes={[{ key: "attr1" }, { key: "attr2", color: { bad: "warning" } }]}
entity_name="entity"
hide_ignored_entities={hide_ignored_entities}
metric={{}}
rationale={rationale}
report={{report_uuid: "report_uuid"}}
status={status}
status_end_date={status_end_date}
/>
Expand Down
6 changes: 5 additions & 1 deletion components/frontend/src/source/SourceEntityDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import { capitalize } from '../utils';
import { source_entity_status_name as status_name } from './source_entity_status';
import { EDIT_ENTITY_PERMISSION } from '../context/Permissions';
import { LabelWithDate } from '../widgets/LabelWithDate';
import { entityPropType, entityStatusPropType, reportPropType } from '../sharedPropTypes';
import { IssuesRows } from '../issue/IssuesRows';
import { entityPropType, entityStatusPropType, metricPropType, reportPropType } from '../sharedPropTypes';

function entityStatusOption(status, text, content, subheader) {
return {
Expand Down Expand Up @@ -46,6 +47,7 @@ entityStatusOptions.propTypes = {
export function SourceEntityDetails(
{
entity,
metric,
metric_uuid,
name,
rationale,
Expand Down Expand Up @@ -96,11 +98,13 @@ export function SourceEntityDetails(
/>
</Grid.Column>
</Grid.Row>
<IssuesRows entityKey={entity.key} metric={metric} metric_uuid={metric_uuid} reload={reload} report={report} target={name} />
</Grid>
);
}
SourceEntityDetails.propTypes = {
entity: entityPropType,
metric: metricPropType,
metric_uuid: PropTypes.string,
name: PropTypes.string,
rationale: PropTypes.string,
Expand Down
2 changes: 2 additions & 0 deletions components/frontend/src/source/SourceEntityDetails.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ function renderSourceEntityDetails() {
render(
<Permissions.Provider value={[EDIT_ENTITY_PERMISSION]}>
<SourceEntityDetails
metric={{}}
metric_uuid="metric_uuid"
source_uuid="source_uuid"
entity={{ key: "key" }}
report={{ report_uuid: "report_uuid" }}
status="unconfirmed"
name="violation"
reload={reload}
Expand Down
Loading

0 comments on commit 1cc6d6b

Please sign in to comment.