feat: Report an issue - allow user to set ticket priority (amundsen-i…

…o#1508) * Adding ability to set priority when reporting an issue Signed-off-by: Kristen Armes <karmes@lyft.com> * Use priority level instead of severity name Signed-off-by: Kristen Armes <karmes@lyft.com> * Use P3-P0 for priority buttons Signed-off-by: Kristen Armes <karmes@lyft.com> * Updating betterer results Signed-off-by: Kristen Armes <karmes@lyft.com> * Fix lint Signed-off-by: Kristen Armes <karmes@lyft.com> * Fixing setting of priority enum value for Asana task Signed-off-by: Kristen Armes <karmes@lyft.com> * Addressing PR comments Signed-off-by: Kristen Armes <karmes@lyft.com> * Changing submit label constant Signed-off-by: Kristen Armes <karmes@lyft.com>
vazmeee · Oct 5, 2021 · f34c953 · f34c953
1 parent 20bdb07
commit f34c953
Show file tree

Hide file tree

Showing 16 changed files with 141 additions and 33 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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']}
                 }
             )
         )
@@ -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
@@ -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
@@ -525,14 +525,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"]
@@ -903,19 +903,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
@@ -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
@@ -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>) => {

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
@@ -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
@@ -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
@@ -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/...tend/amundsen_application/static/js/pages/TableDetailPage/ReportTableIssue/index.spec.tsx b/...tend/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
@@ -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,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,
     };
   };
@@ -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}
         </a>
-        {this.state.isOpen && (
+        {isOpen && (
           <div className="report-table-issue-modal">
             <h3 className="data-issue-header">
               {Constants.REPORT_DATA_ISSUE_TEXT}
@@ -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_LABEL}
+                </button>
+              </div>
             </form>
             <div className="data-owner-notification">
               {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
@@ -27,4 +27,9 @@
     font-size: 10px;
     margin: 5px 0 0;
   }
+
+  .report-table-issue-buttons {
+    display: flex;
+    justify-content: space-between;
+  }
 }