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 Dec 11, 2023
1 parent 7b90e5f commit be0c707
Show file tree
Hide file tree
Showing 17 changed files with 108 additions and 111 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 @@ -651,6 +651,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 @@ -661,6 +667,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,
}
}
85 changes: 1 addition & 84 deletions components/frontend/src/metric/MetricDebtParameters.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { render, screen, fireEvent } from '@testing-library/react';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { DataModel } from '../context/DataModel';
import { EDIT_REPORT_PERMISSION, Permissions } from '../context/Permissions';
Expand All @@ -20,18 +20,12 @@ const data_model = {
}
};

const reportWithIssueTracker = {
issue_tracker: { type: "Jira", parameters: { url: "https://jira", project_key: "KEY", issue_type: "Bug" } }
}

function renderMetricDebtParameters(
{
accept_debt = false,
scale = "count",
issue_ids = [],
report = { subjects: {} },
permissions = [EDIT_REPORT_PERMISSION],
issue_status = []
} = {}) {
render(
<Permissions.Provider value={permissions}>
Expand All @@ -43,8 +37,6 @@ function renderMetricDebtParameters(
tags: [],
accept_debt: accept_debt,
scale: scale,
issue_ids: issue_ids,
issue_status: issue_status
}
}
metric_uuid="metric_uuid"
Expand Down Expand Up @@ -84,81 +76,6 @@ it('sets the technical debt end date', async () => {
expect(fetch_server_api.fetch_server_api).toHaveBeenLastCalledWith("post", "metric/metric_uuid/attribute/debt_end_date", { debt_end_date: "2022-12-31" });
});

it('does not show an error message if the metric has no issues and no issue tracker is configured', async () => {
renderMetricDebtParameters()
expect(screen.queryAllByText(/No issue tracker configured/).length).toBe(0);
});

it('does not show an error message if the metric has no issues and an issue tracker is configured', async () => {
renderMetricDebtParameters({ report: { issue_tracker: { type: "Jira" } } })
expect(screen.queryAllByText(/No issue tracker configured/).length).toBe(0);
});

it('does not show an error message if the metric has issues and an issue tracker is configured', async () => {
renderMetricDebtParameters({ issue_ids: ["BAR-42"], report: { issue_tracker: { type: "Jira", parameters: { url: "https://jira", project_key: "KEY", issue_type: "Bug" } } } })
expect(screen.queryAllByText(/No issue tracker configured/).length).toBe(0);
});

it('shows an error message if the metric has issues but no issue tracker is configured', async () => {
renderMetricDebtParameters({ issue_ids: ["FOO-42"] })
expect(screen.queryAllByText(/No issue tracker configured/).length).toBe(1);
});

it('shows a connection error', async () => {
renderMetricDebtParameters({ issue_status: [{ issue_id: "FOO-43", connection_error: "Oops" }] })
expect(screen.queryAllByText(/Connection error/).length).toBe(1);
expect(screen.queryAllByText(/Oops/).length).toBe(1);
});

it('shows a parse error', async () => {
renderMetricDebtParameters({ issue_status: [{ issue_id: "FOO-43", parse_error: "Oops" }] })
expect(screen.queryAllByText(/Parse error/).length).toBe(1);
expect(screen.queryAllByText(/Oops/).length).toBe(1);
});

it('creates an issue', async () => {
window.open = jest.fn()
fetch_server_api.fetch_server_api = jest.fn().mockResolvedValue({ ok: true, error: "", issue_url: "https://tracker/foo-42" });
renderMetricDebtParameters({ report: reportWithIssueTracker })
fireEvent.click(screen.getByText(/Create new issue/))
expect(fetch_server_api.fetch_server_api).toHaveBeenLastCalledWith("post", "metric/metric_uuid/issue/new", { metric_url: "http://localhost/#metric_uuid" });
});

it('tries to create an issue', async () => {
fetch_server_api.fetch_server_api = jest.fn().mockResolvedValue({ ok: false, error: "Dummy", issue_url: "" });
renderMetricDebtParameters({ report: reportWithIssueTracker })
fireEvent.click(screen.getByText(/Create new issue/))
expect(fetch_server_api.fetch_server_api).toHaveBeenLastCalledWith("post", "metric/metric_uuid/issue/new", { metric_url: "http://localhost/#metric_uuid" });
});

it('does not show the create issue button if the user has no permissions', async () => {
renderMetricDebtParameters({ report: reportWithIssueTracker, permissions: [] })
expect(screen.queryAllByText(/Create new issue/).length).toBe(0)
})

it('adds an issue id', async () => {
fetch_server_api.fetch_server_api = jest.fn().mockResolvedValue({ suggestions: [{ key: "FOO-42", text: "Suggestion" }] });
renderMetricDebtParameters()
await userEvent.type(screen.getByLabelText(/Issue identifiers/), 'FOO-42{Enter}');
expect(fetch_server_api.fetch_server_api).toHaveBeenLastCalledWith("post", "metric/metric_uuid/attribute/issue_ids", { issue_ids: ["FOO-42"] });
});

it('shows issue id suggestions', async () => {
fetch_server_api.fetch_server_api = jest.fn().mockResolvedValue({ suggestions: [{ key: "FOO-42", text: "Suggestion" }] });
renderMetricDebtParameters({ report: { issue_tracker: { type: "Jira", parameters: { url: "https://jira" } } } })
await userEvent.type(screen.getByLabelText(/Issue identifiers/), 'u');
expect(screen.queryAllByText(/FOO-42: Suggestion/).length).toBe(1)
});

it('shows no issue id suggestions without a query', async () => {
fetch_server_api.fetch_server_api = jest.fn().mockResolvedValue({ suggestions: [{ key: "FOO-42", text: "Suggestion" }] });
renderMetricDebtParameters({ report: { issue_tracker: { type: "Jira", parameters: { url: "https://jira" } } } })
await userEvent.type(screen.getByLabelText(/Issue identifiers/), 's');
expect(screen.queryAllByText(/FOO-42: Suggestion/).length).toBe(1)
await userEvent.clear(screen.getByLabelText(/Issue identifiers/).firstChild);
expect(screen.queryAllByText(/FOO-42: Suggestion/).length).toBe(0)
});

it('adds a comment', async () => {
renderMetricDebtParameters()
await userEvent.type(screen.getByLabelText(/Comment/), 'Keep cool{Tab}');
Expand Down
7 changes: 4 additions & 3 deletions components/frontend/src/metric/MetricDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,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 @@ -53,7 +53,6 @@ fetchMeasurements.propTypes = {
setMeasurements: PropTypes.func
}


export function MetricDetails({
changed_fields,
isFirstMetric,
Expand All @@ -63,6 +62,7 @@ export function MetricDetails({
report_date,
reports,
report,
settings,
stopFilteringAndSorting,
subject_uuid,
visibleDetailsTabs,
Expand Down Expand Up @@ -140,7 +140,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 @@ -178,6 +178,7 @@ MetricDetails.propTypes = {
report_date: datePropType,
reports: reportsPropType,
report: reportPropType,
settings: settingsPropType,
stopFilteringAndSorting: PropTypes.func,
subject_uuid: PropTypes.string,
visibleDetailsTabs: stringsURLSearchQueryPropType
Expand Down
Loading

0 comments on commit be0c707

Please sign in to comment.