Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Report an issue - allow user to set ticket priority #1508

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions frontend/amundsen_application/api/issue/issue.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,11 @@ def post(self) -> Response:
self.reqparse.add_argument('title', type=str, location='json')
self.reqparse.add_argument('key', type=str, location='json')
self.reqparse.add_argument('description', type=str, location='json')
self.reqparse.add_argument('priority_level', type=str, location='json')
self.reqparse.add_argument('resource_path', type=str, location='json')
args = self.reqparse.parse_args()
response = self.client.create_issue(description=args['description'],
priority_level=args['priority_level'],
table_uri=args['key'],
title=args['title'],
table_url=app.config['FRONTEND_BASE'] + args['resource_path']
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,18 @@ def get_issues(self, table_uri: str) -> IssueResults:
raise NotImplementedError # pragma: no cover

@abc.abstractmethod
def create_issue(self, table_uri: str, title: str, description: str, table_url: str) -> DataIssue:
def create_issue(self,
table_uri: str,
title: str,
description: str,
priority_level: str,
table_url: str) -> DataIssue:
"""
Given a title, description, and table key, creates a ticket in the configured project
Automatically places the table_uri in the description of the ticket.
Returns the ticket information, including URL.
:param description: user provided description for the jira ticket
:param priority_level: priority level for the ticket
:param table_uri: Table URI ie databasetype://database/table
:param title: Title of the ticket
:param table_url: Link to access the table
Expand Down
4 changes: 4 additions & 0 deletions frontend/amundsen_application/models/data_issue.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ def from_level(level: str) -> 'Optional[Priority]':

return level_to_priority.get(level)

@staticmethod
def get_jira_severity_from_level(level: str) -> str:
return Priority[level].jira_severity


class DataIssue:
def __init__(self,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,24 +57,32 @@ def get_issues(self, table_uri: str) -> IssueResults:
all_issues_url=self._task_url(table_parent_task_gid),
)

def create_issue(self, table_uri: str, title: str, description: str, table_url: str) -> DataIssue:
def create_issue(self,
table_uri: str,
title: str,
description: str,
priority_level: str,
table_url: str) -> DataIssue:
"""
Creates an issue in Asana
:param description: Description of the Asana issue
:param priority_level: priority level for the ticket
:param table_uri: Table Uri ie databasetype://database/table
:param title: Title of the Asana ticket
:param table_url: Link to access the table
:return: Metadata about the newly created issue
"""

table_parent_task_gid = self._get_parent_task_gid_for_table_uri(table_uri)
enum_value = next(opt for opt in self.priority_field_enum_options if opt['name'] == priority_level)

return self._asana_task_to_amundsen_data_issue(
self.asana_client.tasks.create_subtask_for_task(
table_parent_task_gid,
{
'name': title,
'notes': description + f'\n Table URL: {table_url}',
'custom_fields': {self.priority_field_gid: enum_value['gid']}
}
)
)
Expand Down Expand Up @@ -133,6 +141,7 @@ def _setup_custom_fields(self) -> None:

self.table_uri_field_gid = table_uri_field['gid']
self.priority_field_gid = priority_field['gid']
self.priority_field_enum_options = priority_field['enum_options']

def _get_parent_task_gid_for_table_uri(self, table_uri: str) -> str:
table_parent_tasks = list(self.asana_client.tasks.search_tasks_for_workspace(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,16 @@ def get_issues(self, table_uri: str) -> IssueResults:
logging.exception(str(e))
raise e

def create_issue(self, table_uri: str, title: str, description: str, table_url: str) -> DataIssue:
def create_issue(self,
table_uri: str,
title: str,
description: str,
priority_level: str,
table_url: str) -> DataIssue:
"""
Creates an issue in Jira
:param description: Description of the Jira issue
:param priority_level: priority level for the ticket
:param table_uri: Table Uri ie databasetype://database/table
:param title: Title of the Jira ticket
:param table_url: Link to access the table
Expand Down Expand Up @@ -109,7 +115,9 @@ def create_issue(self, table_uri: str, title: str, description: str, table_url:
f'\n Reported By: {user_email} '
f'\n Table Key: {table_uri} [PLEASE DO NOT REMOVE] '
f'\n Table URL: {table_url}'),
reporter=reporter))
priority={
'name': Priority.get_jira_severity_from_level(priority_level)
}, reporter=reporter))
return self._get_issue_properties(issue=issue)
except JIRAError as e:
logging.exception(str(e))
Expand Down
40 changes: 19 additions & 21 deletions frontend/amundsen_application/static/.betterer.results
Original file line number Diff line number Diff line change
Expand Up @@ -526,14 +526,14 @@ exports[`eslint`] = {
"js/ducks/issue/reducer.ts:1774302197": [
[119, 6, 56, "Unexpected lexical declaration in case block.", "2031834906"]
],
"js/ducks/issue/tests/index.spec.ts:2407012752": [
[136, 8, 20, "\'allIssuesUrl\' is already declared in the upper scope.", "3551613994"],
[137, 8, 13, "\'total\' is already declared in the upper scope.", "1110344606"],
[248, 10, 32, "\'allIssuesUrl\' is already declared in the upper scope.", "3851993612"],
[249, 10, 25, "\'total\' is already declared in the upper scope.", "1495122296"],
[252, 8, 33, "Use object destructuring.", "2386588093"],
[253, 8, 31, "Use object destructuring.", "507617405"],
[254, 8, 45, "Use object destructuring.", "244310461"]
"js/ducks/issue/tests/index.spec.ts:90187247": [
[140, 8, 20, "\'allIssuesUrl\' is already declared in the upper scope.", "3551613994"],
[141, 8, 13, "\'total\' is already declared in the upper scope.", "1110344606"],
[253, 10, 32, "\'allIssuesUrl\' is already declared in the upper scope.", "3851993612"],
[254, 10, 25, "\'total\' is already declared in the upper scope.", "1495122296"],
[257, 8, 33, "Use object destructuring.", "2386588093"],
[258, 8, 31, "Use object destructuring.", "507617405"],
[259, 8, 45, "Use object destructuring.", "244310461"]
],
"js/ducks/lastIndexed/sagas.ts:1498244597": [
[7, 2, 29, "\'action\' is defined but never used.", "566797395"]
Expand Down Expand Up @@ -900,19 +900,17 @@ exports[`eslint`] = {
[63, 25, 1, "\'_\' is defined but never used.", "177658"],
[66, 14, 204, "A control must be associated with a text label.", "3622841199"]
],
"js/pages/TableDetailPage/ReportTableIssue/index.tsx:90979507": [
[68, 4, 22, "Must use destructuring props assignment", "2426007440"],
[83, 11, 19, "Must use destructuring props assignment", "987121924"],
[92, 19, 22, "Must use destructuring props assignment", "2201312961"],
[98, 14, 20, "Must use destructuring props assignment", "1598284208"],
[108, 9, 17, "Must use destructuring state assignment", "4230747098"],
[111, 29, 17, "Must use destructuring state assignment", "4230747098"],
[111, 29, 10, "Use callback in setState when referencing the previous state.", "4014904506"],
[119, 15, 20, "Script URL is a form of eval.", "3959800777"],
[125, 9, 17, "Must use destructuring state assignment", "4230747098"],
[139, 16, 7, "A form label must be associated with a control.", "2729454337"],
[140, 16, 160, "A control must be associated with a text label.", "1834626670"],
[148, 16, 7, "A form label must be associated with a control.", "2729454337"]
"js/pages/TableDetailPage/ReportTableIssue/index.tsx:4007069684": [
[70, 4, 22, "Must use destructuring props assignment", "2426007440"],
[97, 19, 22, "Must use destructuring props assignment", "2201312961"],
[103, 14, 20, "Must use destructuring props assignment", "1598284208"],
[113, 9, 17, "Must use destructuring state assignment", "4230747098"],
[116, 29, 17, "Must use destructuring state assignment", "4230747098"],
[116, 29, 10, "Use callback in setState when referencing the previous state.", "4014904506"],
[130, 15, 20, "Script URL is a form of eval.", "3959800777"],
[150, 16, 7, "A form label must be associated with a control.", "2729454337"],
[151, 16, 160, "A control must be associated with a text label.", "1834626670"],
[159, 16, 7, "A form label must be associated with a control.", "2729454337"]
],
"js/pages/TableDetailPage/RequestDescriptionText/index.spec.tsx:2570104867": [
[7, 7, 11, "\'globalState\' is defined but never used.", "1130616505"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ describe('createIssue', () => {
key: 'key',
title: 'title',
description: 'description',
priority_level: 'priority_level',
resource_path: 'resource_path',
};
sendNotificationPayload = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export function createIssue(
key: payload.key,
title: payload.title,
description: payload.description,
priority_level: payload.priority_level,
resource_path: payload.resource_path,
})
.then((response: AxiosResponse<IssueApi>) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe('issue ducks', () => {
let key;
let title;
let description;
let priorityLevel;
let resourceName;
let resourcePath;
let owners;
Expand All @@ -48,6 +49,7 @@ describe('issue ducks', () => {
key = 'table';
title = 'stuff';
description = 'This is a test';
priorityLevel = 'P2';
resourceName = 'resource_name';
resourcePath = 'resource_path';
owners = ['email@email'];
Expand Down Expand Up @@ -89,6 +91,7 @@ describe('issue ducks', () => {
key,
title,
description,
priority_level: priorityLevel,
resource_path: resourcePath,
};
const notificationPayload = {
Expand All @@ -107,6 +110,7 @@ describe('issue ducks', () => {
expect(payload.createIssuePayload.key).toBe(key);
expect(payload.createIssuePayload.title).toBe(title);
expect(payload.createIssuePayload.description).toBe(description);
expect(payload.createIssuePayload.priority_level).toBe(priorityLevel);
expect(payload.createIssuePayload.resource_path).toBe(resourcePath);
expect(payload.notificationPayload.options.resource_name).toBe(
resourceName
Expand Down Expand Up @@ -186,6 +190,7 @@ describe('issue ducks', () => {
key,
title,
description,
priority_level: priorityLevel,
resource_path: resourcePath,
};
const notificationPayload = {
Expand Down Expand Up @@ -287,6 +292,7 @@ describe('issue ducks', () => {
key,
title,
description,
priority_level: priorityLevel,
resource_path: resourcePath,
};
const notificationPayload = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@ export interface CreateIssuePayload {
key: string;
title: string;
description: string;
priority_level: string;
resource_path: string;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
export const REPORT_DATA_ISSUE_TEXT = 'Report an issue';
export const PRIORITY_LABEL = 'Priority';
export const PRIORITY = { P3: 'P3', P2: 'P2', P1: 'P1', P0: 'P0' };
export const TABLE_OWNERS_NOTE =
'Please note: Table owners will also be notified via email when an issue is reported.';
export const SUBMIT = 'Submit';
kristenarmes marked this conversation as resolved.
Show resolved Hide resolved
export const CLOSE = 'Close';
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const mockFormData = {
key: 'val1',
title: 'title',
description: 'description',
priority_level: 'priority level',
resource_name: 'resource name',
resource_path: 'path',
owners: 'test@test.com',
Expand All @@ -40,6 +41,7 @@ const mockCreateIssuePayload = {
key: 'key',
title: 'title',
description: 'description',
priority_level: 'P2',
resource_path: '/table_detail/cluster/database/schema/table_name',
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

import * as React from 'react';
import { ToggleButton, ToggleButtonGroup } from 'react-bootstrap';
import { connect } from 'react-redux';
import { bindActionCreators } from 'redux';
import { GlobalState } from 'ducks/rootReducer';
Expand Down Expand Up @@ -40,6 +41,7 @@ export interface StateFromProps {

interface ReportTableIssueState {
isOpen: boolean;
issuePriority: string;
}

export type ReportTableIssueProps = StateFromProps &
Expand All @@ -53,7 +55,7 @@ export class ReportTableIssue extends React.Component<
constructor(props) {
super(props);

this.state = { isOpen: false };
this.state = { isOpen: false, issuePriority: Constants.PRIORITY.P2 };
}

submitForm = (event) => {
Expand All @@ -72,16 +74,19 @@ export class ReportTableIssue extends React.Component<

getCreateIssuePayload = (formData: FormData): CreateIssuePayload => {
const {
tableKey,
tableMetadata: { cluster, database, schema, name },
} = this.props;
const { issuePriority } = this.state;
const title = formData.get('title') as string;
const description = formData.get('description') as string;
const resourcePath = `/table_detail/${cluster}/${database}/${schema}/${name}`;

return {
title,
description,
key: this.props.tableKey,
priority_level: issuePriority,
key: tableKey,
resource_path: resourcePath,
};
};
Expand Down Expand Up @@ -112,7 +117,13 @@ export class ReportTableIssue extends React.Component<
this.setState({ isOpen: !this.state.isOpen });
};

handlePriorityChange = (event) => {
this.setState({ issuePriority: event });
};

render() {
const { isOpen, issuePriority } = this.state;

return (
<>
{/* eslint-disable jsx-a11y/anchor-is-valid */}
Expand All @@ -123,7 +134,7 @@ export class ReportTableIssue extends React.Component<
>
{Constants.REPORT_DATA_ISSUE_TEXT}
</a>
{this.state.isOpen && (
{isOpen && (
<div className="report-table-issue-modal">
<h3 className="data-issue-header">
{Constants.REPORT_DATA_ISSUE_TEXT}
Expand Down Expand Up @@ -157,9 +168,32 @@ export class ReportTableIssue extends React.Component<
{getIssueDescriptionTemplate()}
</textarea>
</div>
<button className="btn btn-primary submit" type="submit">
Submit
</button>
<label htmlFor="priority">{Constants.PRIORITY_LABEL}</label>
<div className="report-table-issue-buttons">
<ToggleButtonGroup
type="radio"
name="priority"
id="priority"
value={issuePriority}
onChange={this.handlePriorityChange}
>
<ToggleButton value={Constants.PRIORITY.P3}>
{Constants.PRIORITY.P3}
</ToggleButton>
<ToggleButton value={Constants.PRIORITY.P2}>
{Constants.PRIORITY.P2}
</ToggleButton>
<ToggleButton value={Constants.PRIORITY.P1}>
{Constants.PRIORITY.P1}
</ToggleButton>
<ToggleButton value={Constants.PRIORITY.P0}>
{Constants.PRIORITY.P0}
</ToggleButton>
</ToggleButtonGroup>
<button className="btn btn-primary submit" type="submit">
{Constants.SUBMIT}
</button>
</div>
</form>
<div className="data-owner-notification">
{Constants.TABLE_OWNERS_NOTE}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,9 @@
font-size: 10px;
margin: 5px 0 0;
}

.report-table-issue-buttons {
display: flex;
justify-content: space-between;
}
}
Loading