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 Nov 17, 2023
1 parent c7c5abe commit f36f439
Show file tree
Hide file tree
Showing 16 changed files with 313 additions and 224 deletions.
15 changes: 11 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,25 @@ 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])
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
70 changes: 43 additions & 27 deletions components/api_server/tests/routes/test_metric.py
Original file line number Diff line number Diff line change
Expand Up @@ -588,11 +588,13 @@ def test_delete_metric(self):
)


@patch("bottle.request", Mock(json={"metric_url": "https://quality_time/metric42"}))
@patch("model.issue_tracker.requests.post")
class MetricIssueTest(DataModelTestCase):
"""Unit tests for metric issue routes."""

METRIC_URL = "https://quality_time/metric42"
ISSUE_URL = "https://tracker/browse/FOO-42"

def setUp(self):
"""Extend to set up the report fixture."""
super().setUp()
Expand Down Expand Up @@ -637,23 +639,45 @@ def setUp(self):
"from Source.\nPlease go to https://zap for more details.\n",
},
}
self.issue_api = "https://tracker/rest/api/2/issue"
self.issue_url = "https://tracker/browse/FOO-42"

def assert_issue_posted(self, method):
"""Check that method is called to post the issue data to the issue tracker."""
issue_api = "https://tracker/rest/api/2/issue"
method.assert_called_once_with(issue_api, auth=None, headers={}, json=self.expected_json, timeout=10)

def assert_issue_inserted(self):
"""Check that the issue is inserted in the database."""
inserted_report = self.database.reports.insert_one.call_args_list[0][0][0]
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."""
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))
requests_post.assert_called_once_with(
self.issue_api,
auth=None,
headers={},
json=self.expected_json,
timeout=10,
)
self.assertEqual({"ok": True, "issue_url": self.ISSUE_URL}, add_metric_issue(METRIC_ID, self.database))
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):
"""Test that an issue can be added to the issue tracker."""
Expand All @@ -668,16 +692,12 @@ def test_add_metric_issue_with_labels(self, requests_get, requests_post):
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.assertEqual({"ok": True, "issue_url": self.ISSUE_URL}, add_metric_issue(METRIC_ID, self.database))
self.expected_json["fields"]["labels"] = ["label", "label_with_spaces"]
requests_post.assert_called_once_with(
self.issue_api,
auth=None,
headers={},
json=self.expected_json,
timeout=10,
)
self.assert_issue_posted(requests_post)
self.assert_issue_inserted()

@patch("bottle.request", Mock(json={"metric_url": METRIC_URL}))
@disable_logging
@patch("model.issue_tracker.requests.get")
def test_add_metric_issue_with_epic_link(self, requests_get, requests_post):
Expand All @@ -697,16 +717,12 @@ def test_add_metric_issue_with_epic_link(self, requests_get, requests_post):
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.assertEqual({"ok": True, "issue_url": self.ISSUE_URL}, add_metric_issue(METRIC_ID, self.database))
self.expected_json["fields"]["epic_link_field_id"] = "FOO-420"
requests_post.assert_called_once_with(
self.issue_api,
auth=None,
headers={},
json=self.expected_json,
timeout=10,
)
self.assert_issue_posted(requests_post)
self.assert_issue_inserted()

@patch("bottle.request", Mock(json={"metric_url": METRIC_URL}))
@disable_logging
def test_add_metric_issue_failure(self, requests_post):
"""Test that an error message is returned if an issue cannot be added to the issue tracker."""
Expand Down
5 changes: 3 additions & 2 deletions components/frontend/src/api/metric.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ 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) {
return fetch_server_api('post', `metric/${metric_uuid}/issue/new`, {metric_url: `${window.location}#${metric_uuid}`}).then((json) => {
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)
} else {
Expand Down
111 changes: 111 additions & 0 deletions components/frontend/src/issue/IssuesRows.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import React, { useState } from 'react';
import { Grid } from 'semantic-ui-react';
import { set_metric_attribute, add_metric_issue } from '../api/metric';
import { get_report_issue_tracker_suggestions } from '../api/report';
import { EDIT_REPORT_PERMISSION, ReadOnlyOrEditable } from '../context/Permissions';
import { MultipleChoiceInput } from '../fields/MultipleChoiceInput';
import { ActionButton } from '../widgets/Button';
import { LabelWithHelp } from '../widgets/LabelWithHelp';
import { ErrorMessage } from '../errorMessage';
import { get_metric_issue_ids } from '../utils';

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(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'
/>
)
}

function IssueIdentifiers({ issue_tracker_instruction, metric, metric_uuid, report_uuid, target, reload }) {
const issueStatusHelp = (
<>
<p>Identifiers of issues in the configured issue tracker that track the progress of fixing this {target}.</p>
<p>When the issues have all been resolved, or the technical debt end date has passed, whichever happens first, the technical debt should be resolved and the technical debt target is no longer evaluated.</p>
{issue_tracker_instruction}
</>
)
const [suggestions, setSuggestions] = useState([]);
const labelId = `issue-identifiers-label-${metric_uuid}`
const issue_ids = get_metric_issue_ids(metric);
return (
<MultipleChoiceInput
aria-labelledby={labelId}
allowAdditions
onSearchChange={(query) => {
if (query) {
get_report_issue_tracker_suggestions(report_uuid, query).then((suggestionsResponse) => {
const suggestionOptions = suggestionsResponse.suggestions.map((s) => ({ key: s.key, text: `${s.key}: ${s.text}`, value: s.key }))
setSuggestions(suggestionOptions)
})
} else {
setSuggestions([])
}
}}
requiredPermissions={[EDIT_REPORT_PERMISSION]}
label={<LabelWithHelp labelId={labelId} label="Issue identifiers" help={issueStatusHelp} />}
options={suggestions}
set_value={(value) => set_metric_attribute(metric_uuid, "issue_ids", value, reload)}
value={issue_ids}
key={issue_ids} // Make sure the multiple choice input is rerendered when the issue ids change
/>
)
}

export function IssuesRows({ entityKey, metric, metric_uuid, reload, report, target }) {
const parameters = report?.issue_tracker?.parameters;
const issueTrackerConfigured = 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>;
return (
<>
<Grid.Row>
<ReadOnlyOrEditable
requiredPermissions={[EDIT_REPORT_PERMISSION]}
readOnlyComponent={
<Grid.Column width={16}>
<IssueIdentifiers issue_tracker_instruction={issueTrackerInstruction} metric={metric} metric_uuid={metric_uuid} report_uuid={report.report_uuid} target={target ?? "metric"} reload={reload} />
</Grid.Column>
}
editableComponent={
<>
< Grid.Column width={3} verticalAlign="bottom">
<CreateIssueButton entityKey={entityKey} issueTrackerConfigured={issueTrackerConfigured} issueTrackerInstruction={issueTrackerInstruction} metric_uuid={metric_uuid} target={target ?? "metric"} reload={reload} />
</Grid.Column>
<Grid.Column width={13}>
<IssueIdentifiers issue_tracker_instruction={issueTrackerInstruction} metric={metric} metric_uuid={metric_uuid} report_uuid={report.report_uuid} target={target ?? "metric"} reload={reload} />
</Grid.Column>
</>
}
/>
</Grid.Row>
{(get_metric_issue_ids(metric).length > 0 && !issueTrackerConfigured) &&
<Grid.Row>
<Grid.Column width={16}>
<ErrorMessage title="No issue tracker configured" message={issueTrackerInstruction} />
</Grid.Column>
</Grid.Row>
}
{(metric.issue_status ?? []).filter((issue_status => issue_status.connection_error)).map((issue_status) =>
<Grid.Row key={issue_status.issue_id}>
<Grid.Column width={16}>
<ErrorMessage key={issue_status.issue_id} title={"Connection error while retrieving " + issue_status.issue_id} message={issue_status.connection_error} />
</Grid.Column>
</Grid.Row>
)}
{(metric.issue_status ?? []).filter((issue_status => issue_status.parse_error)).map((issue_status) =>
<Grid.Row key={issue_status.issue_id}>
<Grid.Column width={16}>
<ErrorMessage key={issue_status.issue_id} title={"Parse error while processing " + issue_status.issue_id} message={issue_status.parse_error} />
</Grid.Column>
</Grid.Row>
)}
</>
)
}
112 changes: 112 additions & 0 deletions components/frontend/src/issue/IssuesRows.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import React from 'react';
import { fireEvent, render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { EDIT_REPORT_PERMISSION, Permissions } from '../context/Permissions';
import { IssuesRows } from './IssuesRows';
import * as fetch_server_api from '../api/fetch_server_api';

jest.mock("../api/fetch_server_api.js")

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

function renderIssuesRow(
{
issue_ids = [],
report = { subjects: {} },
permissions = [EDIT_REPORT_PERMISSION],
issue_status = []
} = {}) {
render(
<Permissions.Provider value={permissions}>
<IssuesRows
metric={
{
type: "violations",
issue_ids: issue_ids,
issue_status: issue_status
}
}
metric_uuid="metric_uuid"
reload={jest.fn}
report={report}
/>
</Permissions.Provider>
);
}

it('does not show an error message if the metric has no issues and no issue tracker is configured', () => {
renderIssuesRow()
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', () => {
renderIssuesRow({ 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', () => {
renderIssuesRow({ 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', () => {
renderIssuesRow({ issue_ids: ["FOO-42"] })
expect(screen.queryAllByText(/No issue tracker configured/).length).toBe(1);
});

it('shows a connection error', () => {
renderIssuesRow({ 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', () => {
renderIssuesRow({ 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', () => {
window.open = jest.fn()
fetch_server_api.fetch_server_api = jest.fn().mockResolvedValue({ ok: true, error: "", issue_url: "https://tracker/foo-42" });
renderIssuesRow({ 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', () => {
fetch_server_api.fetch_server_api = jest.fn().mockResolvedValue({ ok: false, error: "Dummy", issue_url: "" });
renderIssuesRow({ 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', () => {
renderIssuesRow({ 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" }] });
renderIssuesRow()
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" }] });
renderIssuesRow({ 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" }] });
renderIssuesRow({ 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)
});
Loading

0 comments on commit f36f439

Please sign in to comment.