diff --git a/frontend/amundsen_application/api/issue/issue.py b/frontend/amundsen_application/api/issue/issue.py index d6e48b1295..7b48d82e63 100644 --- a/frontend/amundsen_application/api/issue/issue.py +++ b/frontend/amundsen_application/api/issue/issue.py @@ -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'] diff --git a/frontend/amundsen_application/base/base_issue_tracker_client.py b/frontend/amundsen_application/base/base_issue_tracker_client.py index 33c7b1c71f..46fe1d04cf 100644 --- a/frontend/amundsen_application/base/base_issue_tracker_client.py +++ b/frontend/amundsen_application/base/base_issue_tracker_client.py @@ -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 diff --git a/frontend/amundsen_application/models/data_issue.py b/frontend/amundsen_application/models/data_issue.py index 9f8197fffd..d02c205eaa 100644 --- a/frontend/amundsen_application/models/data_issue.py +++ b/frontend/amundsen_application/models/data_issue.py @@ -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, diff --git a/frontend/amundsen_application/proxy/issue_tracker_clients/asana_client.py b/frontend/amundsen_application/proxy/issue_tracker_clients/asana_client.py index 9bd16f9744..db4f8bab1d 100644 --- a/frontend/amundsen_application/proxy/issue_tracker_clients/asana_client.py +++ b/frontend/amundsen_application/proxy/issue_tracker_clients/asana_client.py @@ -57,10 +57,16 @@ 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 @@ -68,6 +74,7 @@ def create_issue(self, table_uri: str, title: str, description: str, table_url: """ 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( @@ -75,6 +82,7 @@ def create_issue(self, table_uri: str, title: str, description: str, table_url: { 'name': title, 'notes': description + f'\n Table URL: {table_url}', + 'custom_fields': {self.priority_field_gid: enum_value['gid']} } ) ) @@ -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( diff --git a/frontend/amundsen_application/proxy/issue_tracker_clients/jira_client.py b/frontend/amundsen_application/proxy/issue_tracker_clients/jira_client.py index 73a9f4242f..e9a0ed503c 100644 --- a/frontend/amundsen_application/proxy/issue_tracker_clients/jira_client.py +++ b/frontend/amundsen_application/proxy/issue_tracker_clients/jira_client.py @@ -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 @@ -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)) diff --git a/frontend/amundsen_application/static/.betterer.results b/frontend/amundsen_application/static/.betterer.results index 02a3732ce1..61374d91b6 100644 --- a/frontend/amundsen_application/static/.betterer.results +++ b/frontend/amundsen_application/static/.betterer.results @@ -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"] @@ -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:1702363396": [ + [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"], diff --git a/frontend/amundsen_application/static/js/ducks/issue/api/tests/index.spec.ts b/frontend/amundsen_application/static/js/ducks/issue/api/tests/index.spec.ts index ebaca7a634..48e6bbe105 100644 --- a/frontend/amundsen_application/static/js/ducks/issue/api/tests/index.spec.ts +++ b/frontend/amundsen_application/static/js/ducks/issue/api/tests/index.spec.ts @@ -67,6 +67,7 @@ describe('createIssue', () => { key: 'key', title: 'title', description: 'description', + priority_level: 'priority_level', resource_path: 'resource_path', }; sendNotificationPayload = { diff --git a/frontend/amundsen_application/static/js/ducks/issue/api/v0.ts b/frontend/amundsen_application/static/js/ducks/issue/api/v0.ts index d1bf30842e..cd0e616755 100644 --- a/frontend/amundsen_application/static/js/ducks/issue/api/v0.ts +++ b/frontend/amundsen_application/static/js/ducks/issue/api/v0.ts @@ -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) => { diff --git a/frontend/amundsen_application/static/js/ducks/issue/tests/index.spec.ts b/frontend/amundsen_application/static/js/ducks/issue/tests/index.spec.ts index 88cdc3120f..7960e0aae6 100644 --- a/frontend/amundsen_application/static/js/ducks/issue/tests/index.spec.ts +++ b/frontend/amundsen_application/static/js/ducks/issue/tests/index.spec.ts @@ -36,6 +36,7 @@ describe('issue ducks', () => { let key; let title; let description; + let priorityLevel; let resourceName; let resourcePath; let owners; @@ -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']; @@ -89,6 +91,7 @@ describe('issue ducks', () => { key, title, description, + priority_level: priorityLevel, resource_path: resourcePath, }; const notificationPayload = { @@ -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 @@ -186,6 +190,7 @@ describe('issue ducks', () => { key, title, description, + priority_level: priorityLevel, resource_path: resourcePath, }; const notificationPayload = { @@ -287,6 +292,7 @@ describe('issue ducks', () => { key, title, description, + priority_level: priorityLevel, resource_path: resourcePath, }; const notificationPayload = { diff --git a/frontend/amundsen_application/static/js/interfaces/Issue.ts b/frontend/amundsen_application/static/js/interfaces/Issue.ts index 0d91d874c0..5afdb8f724 100644 --- a/frontend/amundsen_application/static/js/interfaces/Issue.ts +++ b/frontend/amundsen_application/static/js/interfaces/Issue.ts @@ -11,5 +11,6 @@ export interface CreateIssuePayload { key: string; title: string; description: string; + priority_level: string; resource_path: string; } diff --git a/frontend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/constants.ts b/frontend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/constants.ts index 516221d490..6f6d01d928 100644 --- a/frontend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/constants.ts +++ b/frontend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/constants.ts @@ -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_BUTTON_LABEL = 'Submit'; export const CLOSE = 'Close'; diff --git a/frontend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/index.spec.tsx b/frontend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/index.spec.tsx index 09a79ed463..fb28a59845 100644 --- a/frontend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/index.spec.tsx +++ b/frontend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/index.spec.tsx @@ -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', @@ -40,6 +41,7 @@ const mockCreateIssuePayload = { key: 'key', title: 'title', description: 'description', + priority_level: 'P2', resource_path: '/table_detail/cluster/database/schema/table_name', }; diff --git a/frontend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/index.tsx b/frontend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/index.tsx index c45a803ba7..3d01d4a9ff 100644 --- a/frontend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/index.tsx +++ b/frontend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/index.tsx @@ -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'; @@ -40,6 +41,7 @@ export interface StateFromProps { interface ReportTableIssueState { isOpen: boolean; + issuePriority: string; } export type ReportTableIssueProps = StateFromProps & @@ -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) => { @@ -72,8 +74,10 @@ 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}`; @@ -81,7 +85,8 @@ export class ReportTableIssue extends React.Component< return { title, description, - key: this.props.tableKey, + priority_level: issuePriority, + key: tableKey, resource_path: resourcePath, }; }; @@ -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 */} @@ -123,7 +134,7 @@ export class ReportTableIssue extends React.Component< > {Constants.REPORT_DATA_ISSUE_TEXT} - {this.state.isOpen && ( + {isOpen && (

{Constants.REPORT_DATA_ISSUE_TEXT} @@ -157,9 +168,32 @@ export class ReportTableIssue extends React.Component< {getIssueDescriptionTemplate()}

- + +
+ + + {Constants.PRIORITY.P3} + + + {Constants.PRIORITY.P2} + + + {Constants.PRIORITY.P1} + + + {Constants.PRIORITY.P0} + + + +
{Constants.TABLE_OWNERS_NOTE} diff --git a/frontend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/styles.scss b/frontend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/styles.scss index 531af1980f..6a54a2c9f0 100644 --- a/frontend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/styles.scss +++ b/frontend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/styles.scss @@ -27,4 +27,9 @@ font-size: 10px; margin: 5px 0 0; } + + .report-table-issue-buttons { + display: flex; + justify-content: space-between; + } } diff --git a/frontend/tests/unit/api/issue/test_issue.py b/frontend/tests/unit/api/issue/test_issue.py index 07b67bf0d4..13c8df8c50 100644 --- a/frontend/tests/unit/api/issue/test_issue.py +++ b/frontend/tests/unit/api/issue/test_issue.py @@ -99,6 +99,7 @@ def test_create_issue_not_enabled(self) -> None: with local_app.test_client() as test: response = test.post('/api/issue/issue', data={ 'description': 'test description', + 'priority_level': 'P2', 'title': 'test title', 'key': 'key', 'resource_path': '/table_detail/cluster/database/schema/table_name' @@ -116,6 +117,7 @@ def test_create_jira_issue_missing_config(self, mock_issue_tracker_client: unitt with local_app.test_client() as test: response = test.post('/api/issue/issue', data={ 'description': 'test description', + 'priority_level': 'P2', 'title': 'test title', 'key': 'key', 'resource_path': '/table_detail/cluster/database/schema/table_name' @@ -129,6 +131,7 @@ def test_create_jira_issue_no_description(self) -> None: """ with local_app.test_client() as test: response = test.post('/api/issue/issue', data={ + 'priority_level': 'P2', 'key': 'table_key', 'title': 'test title', 'resource_path': '/table_detail/cluster/database/schema/table_name' @@ -143,6 +146,7 @@ def test_create_jira_issue_no_key(self) -> None: with local_app.test_client() as test: response = test.post('/api/issue/issue', data={ 'description': 'test description', + 'priority_level': 'P2', 'title': 'test title', 'resource_path': '/table_detail/cluster/database/schema/table_name' }) @@ -156,6 +160,7 @@ def test_create_jira_issue_no_title(self) -> None: with local_app.test_client() as test: response = test.post('/api/issue/issue', data={ 'description': 'test description', + 'priority_level': 'P2', 'key': 'table_key', 'resource_path': '/table_detail/cluster/database/schema/table_name' }) @@ -169,11 +174,26 @@ def test_create_jira_issue_no_resource_path(self) -> None: with local_app.test_client() as test: response = test.post('/api/issue/issue', data={ 'description': 'test description', + 'priority_level': 'P2', 'title': 'test title', 'key': 'key' }) self.assertEqual(response.status_code, HTTPStatus.INTERNAL_SERVER_ERROR) + def test_create_jira_issue_no_priority(self) -> None: + """ + Test request failure if resource path is missing + :return: + """ + with local_app.test_client() as test: + response = test.post('/api/issue/issue', data={ + 'description': 'test description', + 'title': 'test title', + 'key': 'key', + 'resource_path': '/table_detail/cluster/database/schema/table_name' + }) + self.assertEqual(response.status_code, HTTPStatus.INTERNAL_SERVER_ERROR) + @unittest.mock.patch('amundsen_application.api.issue.issue.get_issue_tracker_client') def test_create_jira_issue_success(self, mock_issue_tracker_client: unittest.mock.Mock) -> None: """ @@ -187,6 +207,7 @@ def test_create_jira_issue_success(self, mock_issue_tracker_client: unittest.moc content_type='multipart/form-data', data={ 'description': 'test description', + 'priority_level': 'P2', 'title': 'title', 'key': 'key', 'resource_path': '/table_detail/cluster/database/schema/table_name' diff --git a/frontend/tests/unit/issue_tracker_clients/test_jira_client.py b/frontend/tests/unit/issue_tracker_clients/test_jira_client.py index 0920b366b7..73628af54c 100644 --- a/frontend/tests/unit/issue_tracker_clients/test_jira_client.py +++ b/frontend/tests/unit/issue_tracker_clients/test_jira_client.py @@ -150,7 +150,11 @@ def test_create_returns_JIRAError(self, mock_JIRA_client: Mock) -> None: issue_tracker_password=app.config['ISSUE_TRACKER_PASSWORD'], issue_tracker_project_id=app.config['ISSUE_TRACKER_PROJECT_ID'], issue_tracker_max_results=app.config['ISSUE_TRACKER_MAX_RESULTS']) - jira_client.create_issue(description='desc', table_uri='key', title='title', table_url='http://table') + jira_client.create_issue(description='desc', + priority_level='P2', + table_uri='key', + title='title', + table_url='http://table') except JIRAError as e: self.assertTrue(type(e), type(JIRAError)) self.assertTrue(e, 'Some exception') @@ -170,6 +174,7 @@ def test_create_issue(self, mock_get_issue_properties: Mock, mock_JIRA_client: M issue_tracker_project_id=app.config['ISSUE_TRACKER_PROJECT_ID'], issue_tracker_max_results=app.config['ISSUE_TRACKER_MAX_RESULTS']) results = jira_client.create_issue(description='desc', + priority_level='P2', table_uri='key', title='title', table_url='http://table') @@ -186,4 +191,6 @@ def test_create_issue(self, mock_get_issue_properties: Mock, mock_JIRA_client: M "Reported By: test@email.com \n " "Table Key: key [PLEASE DO NOT REMOVE] \n " "Table URL: http://table"), - reporter={'name': 'test'})) + priority={ + 'name': 'Major' + }, reporter={'name': 'test'}))