Unverified Commit ed4e16ee authored by christina stead's avatar christina stead Committed by GitHub

Show status, priority for data issues (#412)

* update payload first commit

* fix tests

* wip

* update ui for reporting jira issues

* correctly use connected component

* fix some tests

* fix more tests, do some clean up

* update python tests

* some more cleanup

* remove white space

* remove a bit more whitespace

* update variable name

* update logging

* sort by unresolved -> resolved

* update per pr comments

* merge master

* update jira url

* Update amundsen_application/static/js/components/TableDetail/TableIssues/styles.scss
Co-Authored-By: 's avatarTamika Tannis <ttannis@lyft.com>

* fix styles
Co-authored-by: 's avatarTamika Tannis <ttannis@lyft.com>
parent e8893c1a
# JIRA SDK does not return priority beyond the name
PRIORITY_MAP = {
'Blocker': 'P0',
'Critical': 'P1',
'Major': 'P2',
'Minor': 'P3'
}
class DataIssue: class DataIssue:
def __init__(self, def __init__(self,
issue_key: str, issue_key: str,
title: str, title: str,
url: str) -> None: url: str,
status: str,
priority: str) -> None:
self.issue_key = issue_key self.issue_key = issue_key
self.title = title self.title = title
self.url = url self.url = url
self.status = status
if priority in PRIORITY_MAP:
self.priority_display_name = PRIORITY_MAP[priority]
self.priority_name = priority.lower()
else:
self.priority_display_name = None # type: ignore
self.priority_name = None # type: ignore
def serialize(self) -> dict: def serialize(self) -> dict:
return {'issue_key': self.issue_key, return {'issue_key': self.issue_key,
'title': self.title, 'title': self.title,
'url': self.url} 'url': self.url,
'status': self.status,
'priority_name': self.priority_name,
'priority_display_name': self.priority_display_name}
...@@ -5,19 +5,19 @@ from typing import List, Dict ...@@ -5,19 +5,19 @@ from typing import List, Dict
class IssueResults: class IssueResults:
def __init__(self, def __init__(self,
issues: List[DataIssue], issues: List[DataIssue],
remaining: int, total: int,
remaining_url: str) -> None: all_issues_url: str) -> None:
""" """
Returns an object representing results from an issue tracker. Returns an object representing results from an issue tracker.
:param issues: Issues in the issue tracker matching the requested table :param issues: Issues in the issue tracker matching the requested table
:param remaining: How many issues remain in the issue tracker and are not displayed :param total: How many issues in all are associated with this table
:param remaining_url: url to the remaining issues in the issue tracker :param all_issues_url: url to the all issues in the issue tracker
""" """
self.issues = issues self.issues = issues
self.remaining = remaining self.total = total
self.remaining_url = remaining_url self.all_issues_url = all_issues_url
def serialize(self) -> Dict: def serialize(self) -> Dict:
return {'issues': [issue.serialize() for issue in self.issues], return {'issues': [issue.serialize() for issue in self.issues],
'remaining': self.remaining, 'total': self.total,
'remaining_url': self.remaining_url} 'all_issues_url': self.all_issues_url}
...@@ -11,7 +11,7 @@ from amundsen_application.models.issue_results import IssueResults ...@@ -11,7 +11,7 @@ from amundsen_application.models.issue_results import IssueResults
import urllib.parse import urllib.parse
import logging import logging
SEARCH_STUB = 'text ~ "{table_key}" AND resolution = Unresolved order by createdDate DESC' SEARCH_STUB_ALL_ISSUES = 'text ~ "{table_key}" order by createdDate DESC'
# this is provided by jira as the type of a bug # this is provided by jira as the type of a bug
ISSUE_TYPE_ID = 1 ISSUE_TYPE_ID = 1
ISSUE_TYPE_NAME = 'Bug' ISSUE_TYPE_NAME = 'Bug'
...@@ -50,13 +50,13 @@ class JiraClient(BaseIssueTrackerClient): ...@@ -50,13 +50,13 @@ class JiraClient(BaseIssueTrackerClient):
:return: Metadata of matching issues :return: Metadata of matching issues
""" """
try: try:
issues = self.jira_client.search_issues(SEARCH_STUB.format( issues = self.jira_client.search_issues(SEARCH_STUB_ALL_ISSUES.format(
table_key=table_uri), table_key=table_uri),
maxResults=self.jira_max_results) maxResults=self.jira_max_results)
returned_issues = [self._get_issue_properties(issue=issue) for issue in issues] returned_issues = self._sort_issues(issues)
return IssueResults(issues=returned_issues, return IssueResults(issues=returned_issues,
remaining=self._get_remaining_issues(total=issues.total), total=issues.total,
remaining_url=self._generate_remaining_issues_url(table_uri, returned_issues)) all_issues_url=self._generate_all_issues_url(table_uri, returned_issues))
except JIRAError as e: except JIRAError as e:
logging.exception(str(e)) logging.exception(str(e))
raise e raise e
...@@ -124,18 +124,11 @@ class JiraClient(BaseIssueTrackerClient): ...@@ -124,18 +124,11 @@ class JiraClient(BaseIssueTrackerClient):
""" """
return DataIssue(issue_key=issue.key, return DataIssue(issue_key=issue.key,
title=issue.fields.summary, title=issue.fields.summary,
url=issue.permalink()) url=issue.permalink(),
status=issue.fields.status.name,
priority=issue.fields.priority.name)
def _get_remaining_issues(self, total: int) -> int: def _generate_all_issues_url(self, table_uri: str, issues: List[DataIssue]) -> str:
"""
Calculates how many issues are not being displayed, so the FE can determine whether to
display a message about issues remaining
:param total: number from the result set representing how many issues were found in all
:return: int - 0, or how many issues remain
"""
return 0 if total < self.jira_max_results else total - self.jira_max_results
def _generate_remaining_issues_url(self, table_uri: str, issues: List[DataIssue]) -> str:
""" """
Way to get the full list of jira tickets Way to get the full list of jira tickets
SDK doesn't return a query SDK doesn't return a query
...@@ -145,7 +138,22 @@ class JiraClient(BaseIssueTrackerClient): ...@@ -145,7 +138,22 @@ class JiraClient(BaseIssueTrackerClient):
""" """
if not issues or len(issues) == 0: if not issues or len(issues) == 0:
return '' return ''
# jira expects a ticket key in the query to default to, so pick the first one search_query = urllib.parse.quote(SEARCH_STUB_ALL_ISSUES.format(table_key=table_uri))
first_issue_key = issues[0].issue_key return f'{self.jira_url}/issues/?jql={search_query}'
search_query = urllib.parse.quote(SEARCH_STUB.format(table_key=table_uri))
return f'{self.jira_url}/browse/{first_issue_key}?jql={search_query}' def _sort_issues(self, issues: List[Issue]) -> List[DataIssue]:
"""
Sorts issues by resolution, first by unresolved and then by resolved. Also maps the issues to
the object used by the front end.
:param issues: Issues returned from the JIRA API
:return: List of data issues
"""
open = []
closed = []
for issue in issues:
data_issue = self._get_issue_properties(issue)
if not issue.fields.resolution:
open.append(data_issue)
else:
closed.append(data_issue)
return open + closed
...@@ -103,6 +103,9 @@ $list-group-border-radius: 0 !default; ...@@ -103,6 +103,9 @@ $list-group-border-radius: 0 !default;
// Labels // Labels
$label-primary-bg: $brand-color-3 !default; $label-primary-bg: $brand-color-3 !default;
//Priority
$priority-text-blocker: $white;
$priority-bg-color: $rose80;
// Tags // Tags
$tag-bg: $gray5; $tag-bg: $gray5;
......
...@@ -9,7 +9,6 @@ import { CreateIssueRequest } from 'ducks/issue/types'; ...@@ -9,7 +9,6 @@ import { CreateIssueRequest } from 'ducks/issue/types';
import './styles.scss'; import './styles.scss';
import { REPORT_DATA_ISSUE_TEXT, TABLE_OWNERS_NOTE } from './constants'; import { REPORT_DATA_ISSUE_TEXT, TABLE_OWNERS_NOTE } from './constants';
import { logClick } from 'ducks/utilMethods'; import { logClick } from 'ducks/utilMethods';
import { notificationsEnabled, issueTrackingEnabled } from 'config/config-utils';
import { TableMetadata, CreateIssuePayload, NotificationPayload, NotificationType } from 'interfaces'; import { TableMetadata, CreateIssuePayload, NotificationPayload, NotificationType } from 'interfaces';
export interface ComponentProps { export interface ComponentProps {
...@@ -55,6 +54,7 @@ export class ReportTableIssue extends React.Component<ReportTableIssueProps, Rep ...@@ -55,6 +54,7 @@ export class ReportTableIssue extends React.Component<ReportTableIssueProps, Rep
this.setState({isOpen: false }); this.setState({isOpen: false });
}; };
getCreateIssuePayload = (formData: FormData): CreateIssuePayload => { getCreateIssuePayload = (formData: FormData): CreateIssuePayload => {
const title = formData.get('title') as string; const title = formData.get('title') as string;
const description = formData.get('description') as string; const description = formData.get('description') as string;
...@@ -83,28 +83,19 @@ export class ReportTableIssue extends React.Component<ReportTableIssueProps, Rep ...@@ -83,28 +83,19 @@ export class ReportTableIssue extends React.Component<ReportTableIssueProps, Rep
}; };
} }
toggle = () => { toggle = (event) => {
if (!this.state.isOpen) {
logClick(event);
}
this.setState({ isOpen: !this.state.isOpen }); this.setState({ isOpen: !this.state.isOpen });
}; };
renderPipe = () => {
if (notificationsEnabled()) {
return ' | ';
}
return '';
}
render() { render() {
if (!issueTrackingEnabled()) {
return '';
}
if (this.props.isLoading) { if (this.props.isLoading) {
return <LoadingSpinner />; return <LoadingSpinner />;
} }
return ( return (
<> <>
{this.renderPipe()}
<a href="javascript:void(0)" <a href="javascript:void(0)"
className="report-table-issue-link" className="report-table-issue-link"
onClick={this.toggle} onClick={this.toggle}
......
...@@ -66,16 +66,6 @@ describe('ReportTableIssue', () => { ...@@ -66,16 +66,6 @@ describe('ReportTableIssue', () => {
} }
describe('render', () => { describe('render', () => {
beforeAll(() => {
AppConfig.issueTracking.enabled = true;
});
it('renders nothing if issueTracking not enabled', () => {
AppConfig.issueTracking.enabled = false;
const { props, wrapper } = setup({ isLoading: false });
expect(wrapper.html()).toBeFalsy();
});
it('Renders loading spinner if not ready', () => { it('Renders loading spinner if not ready', () => {
const { props, wrapper } = setup(); const { props, wrapper } = setup();
expect(wrapper.find('.loading-spinner')).toBeTruthy(); expect(wrapper.find('.loading-spinner')).toBeTruthy();
...@@ -92,7 +82,8 @@ describe('ReportTableIssue', () => { ...@@ -92,7 +82,8 @@ describe('ReportTableIssue', () => {
setStateSpy.mockClear(); setStateSpy.mockClear();
const { props, wrapper } = setup(); const { props, wrapper } = setup();
const previsOpenState = wrapper.state().isOpen; const previsOpenState = wrapper.state().isOpen;
wrapper.instance().toggle(); wrapper.instance().toggle({currentTarget: {id: 'id',
nodeName: 'button' } });
expect(setStateSpy).toHaveBeenCalledWith({ isOpen: !previsOpenState }); expect(setStateSpy).toHaveBeenCalledWith({ isOpen: !previsOpenState });
}); });
}); });
......
export const SEE_ADDITIONAL_ISSUES_TEXT = "See additional issues associated with this table"; export const NO_DATA_ISSUES_TEXT = "No associated issues";
\ No newline at end of file \ No newline at end of file
...@@ -7,15 +7,14 @@ import { Issue } from 'interfaces'; ...@@ -7,15 +7,14 @@ import { Issue } from 'interfaces';
import { getIssues } from 'ducks/issue/reducer'; import { getIssues } from 'ducks/issue/reducer';
import { logClick } from 'ducks/utilMethods'; import { logClick } from 'ducks/utilMethods';
import { GetIssuesRequest } from 'ducks/issue/types'; import { GetIssuesRequest } from 'ducks/issue/types';
import ReportTableIssue from 'components/TableDetail/ReportTableIssue';
import { NO_DATA_ISSUES_TEXT } from './constants';
import './styles.scss'; import './styles.scss';
import { issueTrackingEnabled } from 'config/config-utils';
import { SEE_ADDITIONAL_ISSUES_TEXT } from './constants';
export interface StateFromProps { export interface StateFromProps {
issues: Issue[]; issues: Issue[];
remainingIssues: number; total: number;
remainingIssuesUrl: string; allIssuesUrl: string;
} }
export interface DispatchFromProps { export interface DispatchFromProps {
...@@ -24,6 +23,7 @@ export interface DispatchFromProps { ...@@ -24,6 +23,7 @@ export interface DispatchFromProps {
export interface ComponentProps { export interface ComponentProps {
tableKey: string; tableKey: string;
tableName: string;
} }
export type TableIssueProps = StateFromProps & DispatchFromProps & ComponentProps; export type TableIssueProps = StateFromProps & DispatchFromProps & ComponentProps;
...@@ -34,57 +34,82 @@ export class TableIssues extends React.Component<TableIssueProps> { ...@@ -34,57 +34,82 @@ export class TableIssues extends React.Component<TableIssueProps> {
} }
componentDidMount() { componentDidMount() {
if (issueTrackingEnabled()) {
this.props.getIssues(this.props.tableKey); this.props.getIssues(this.props.tableKey);
} }
}
renderIssue = (issue: Issue, index: number) => { renderIssue = (issue: Issue, index: number) => {
return ( return (
<div className="issue-banner" key={`issue-${index}`}> <div className="issue-banner" key={`issue-${index}`}>
<span className={`table-issue-priority ${issue.priority_name}`}>
{issue.priority_display_name}
</span>
<a id={`table-issue-link-${index}`} className="table-issue-link" target="_blank" href={issue.url} onClick={logClick}> <a id={`table-issue-link-${index}`} className="table-issue-link" target="_blank" href={issue.url} onClick={logClick}>
<img className="icon icon-red-triangle-warning "/>
<span> <span>
{ issue.issue_key } { issue.issue_key }
</span> </span>
</a> </a>
<span className="issue-title-display-text truncated"> <span className="issue-title-display-text truncated">
<span className="issue-title-name"> <span className="issue-title-name">
"{ issue.title } { issue.title }
</span>" </span>
</span>
<span className="table-issue-status">
{issue.status}
</span> </span>
</div> </div>
); );
} }
renderMoreIssuesMessage = (count: number, url: string) => { renderMoreIssuesMessage = (count: number, url: string) => {
if (count === 0) {
return '';
}
return ( return (
<div className="issue-banner" key="more-issue-link"> <span className="table-more-issues" key="more-issue-link">
<img className="icon icon-red-triangle-warning "/>
<a id="more-issues-link" className="table-issue-more-issues" target="_blank" href={url} onClick={logClick}> <a id="more-issues-link" className="table-issue-more-issues" target="_blank" href={url} onClick={logClick}>
{ SEE_ADDITIONAL_ISSUES_TEXT } View all {count} issues
</a> </a>
|
{ this.renderReportIssueLink() }
</span>
);
}
renderReportIssueLink = () => {
return (
<div className="table-report-new-issue">
<ReportTableIssue tableKey={ this.props.tableKey } tableName={ this.props.tableName }/>
</div> </div>
); );
} }
render() { renderIssueTitle = () => {
if (!issueTrackingEnabled()) { return (
return ''; <div className="section-title title-3">
Issues
</div>
);
} }
render() {
if (this.props.issues.length === 0) { if (this.props.issues.length === 0) {
return null; return (
<div>
{this.renderIssueTitle()}
<div className="table-issues">
<div className="issue-banner">
{NO_DATA_ISSUES_TEXT}
</div>
</div>
{ this.renderReportIssueLink()}
</div>
);
} }
return ( return (
<div>
{this.renderIssueTitle()}
<div className="table-issues"> <div className="table-issues">
{ this.props.issues.map(this.renderIssue)} { this.props.issues.map(this.renderIssue)}
{ this.renderMoreIssuesMessage(this.props.remainingIssues, this.props.remainingIssuesUrl)} </div>
{ this.renderMoreIssuesMessage(this.props.total, this.props.allIssuesUrl)}
</div> </div>
); );
} }
...@@ -93,8 +118,8 @@ export class TableIssues extends React.Component<TableIssueProps> { ...@@ -93,8 +118,8 @@ export class TableIssues extends React.Component<TableIssueProps> {
export const mapStateToProps = (state: GlobalState) => { export const mapStateToProps = (state: GlobalState) => {
return { return {
issues: state.issue.issues, issues: state.issue.issues,
remainingIssues: state.issue.remainingIssues, total: state.issue.total,
remainingIssuesUrl: state.issue.remainingIssuesUrl allIssuesUrl: state.issue.allIssuesUrl
}; };
}; };
......
...@@ -9,30 +9,60 @@ ...@@ -9,30 +9,60 @@
margin: 8px 0; margin: 8px 0;
padding: 8px 16px; padding: 8px 16px;
display: flex; display: flex;
overflow: hidden; flex-direction: row;
&:first-child {
margin-top: 24px;
}
}
.icon-red-triangle-warning {
background: $red-triangle-warning;
margin: 0px 5px 0px 0px;
} }
.table-issue-link { .table-issue-link {
margin: 0px 5px 0px 0px; margin-right: $spacer-1;
min-width: fit-content; min-width: fit-content;
} }
.issue-title-name { .issue-title-name {
white-space: nowrap; white-space: nowrap;
overflow: hidden; overflow: hidden;
text-overflow: ellipsis; text-overflow: ellipsis;
max-width: fit-content;
} }
.issue-title-display-text { .issue-title-display-text {
display: flex; display: flex;
max-width: fit-content; max-width: fit-content;
} }
.table-issue-more-issues { .table-issue-status {
margin: 0px 2px 0px 2px; display: flex;
justify-content: flex-end;
flex-grow: 1.1;
min-width: fit-content;
margin-left: $spacer-2;
}
.table-issue-priority {
margin-right: $spacer-1;
border: 1px solid $rose80;
border-radius: 5px;
padding: 1px 2px;
flex: none;
} }
.blocker {
color: $priority-text-blocker;
// default is $rose80
background-color: rgba($priority-bg-color, 1);
}
.critical {
background-color: rgba($priority-bg-color, .6);
}
.major {
background-color: rgba($priority-bg-color, .3);
}
.minor {
background-color: rgba($priority-bg-color, .1);
}
}
.table-issue-more-issues {
margin-bottom: $spacer-1;
margin-right: $spacer-1;
}
.table-more-issues {
display: flex;
font-size: 16px;
}
.table-report-new-issue {
margin-bottom: $spacer-1;
margin-left: $spacer-1;
} }
...@@ -11,7 +11,9 @@ import { ...@@ -11,7 +11,9 @@ import {
mapStateToProps, mapStateToProps,
mapDispatchToProps mapDispatchToProps
} from '..'; } from '..';
import { SEE_ADDITIONAL_ISSUES_TEXT } from '../constants';
import { NO_DATA_ISSUES_TEXT } from "components/TableDetail/TableIssues/constants";
import ReportTableIssue from 'components/TableDetail/ReportTableIssue';
describe ('TableIssues', ()=> { describe ('TableIssues', ()=> {
...@@ -21,8 +23,9 @@ describe ('TableIssues', ()=> { ...@@ -21,8 +23,9 @@ describe ('TableIssues', ()=> {
const props: TableIssueProps = { const props: TableIssueProps = {
issues: [], issues: [],
tableKey: 'key', tableKey: 'key',
remainingIssues: 0, tableName: 'tableName',
remainingIssuesUrl: 'testUrl', total: 0,
allIssuesUrl: 'testUrl',
getIssues: jest.fn(), getIssues: jest.fn(),
...propOverrides ...propOverrides
}; };
...@@ -35,15 +38,9 @@ describe ('TableIssues', ()=> { ...@@ -35,15 +38,9 @@ describe ('TableIssues', ()=> {
AppConfig.issueTracking.enabled = true; AppConfig.issueTracking.enabled = true;
}); });
it('renders nothing if no issues', () => { it('renders text if no issues', () => {
const { props, wrapper } = setup({ issues: [] }); const { props, wrapper } = setup({ issues: [] });
expect(wrapper.html()).toBeFalsy(); expect(wrapper.find('.issue-banner').text()).toEqual(NO_DATA_ISSUES_TEXT);
});
it('renders nothing if issueTracking not enabled', () => {
AppConfig.issueTracking.enabled = false;
const { props, wrapper } = setup({ issues: [] });
expect(wrapper.html()).toBeFalsy();
}); });
it('renders issues if they exist', () => { it('renders issues if they exist', () => {
...@@ -51,33 +48,38 @@ describe ('TableIssues', ()=> { ...@@ -51,33 +48,38 @@ describe ('TableIssues', ()=> {
const { props, wrapper } = setup({ issues: [{ const { props, wrapper } = setup({ issues: [{
issue_key: 'issue_key', issue_key: 'issue_key',
title: 'title', title: 'title',
url: 'http://url' url: 'http://url',
status: 'Open',
priority_display_name: 'P2',
priority_name: 'major'
}]}); }]});
expect(wrapper.find('.table-issue-link').text()).toEqual('issue_key'); expect(wrapper.find('.table-issue-link').text()).toEqual('issue_key');
expect(wrapper.find('.issue-title-name').text()).toContain('title'); expect(wrapper.find('.issue-title-name').text()).toContain('title');
expect(wrapper.find('.table-issue-status').text()).toContain('Open');
expect(wrapper.find('.table-issue-priority').text()).toContain('P2');
}); });
it('renders no extra notice if no remaining issues', () => { it('renders no link to issues if no issues', () => {
const { props, wrapper } = setup({ issues: [{ const { props, wrapper } = setup({ issues: [],
issue_key: 'issue_key', total: 0,
title: 'title', allIssuesUrl: null
url: 'http://url'
}],
remainingIssues: 0,
remainingIssuesUrl: null
}); });
expect(wrapper.find('.table-issue-more-issues').length).toEqual(0); expect(wrapper.find('.table-issue-more-issues').length).toEqual(0);
}); });
it('renders extra notice if remaining issues', () => {
it('renders link if there are issues', () => {
const { props, wrapper } = setup({ issues: [{ const { props, wrapper } = setup({ issues: [{
issue_key: 'issue_key', issue_key: 'issue_key',
title: 'title', title: 'title',
url: 'http://url' url: 'http://url',
status: 'Open',
priority_display_name: 'P2',
priority_name: 'Major'
}], }],
remainingIssues: 1, total: 1,
remainingIssuesUrl: 'url' allIssuesUrl: 'url'
}); });
expect(wrapper.find('.table-issue-more-issues').text()).toEqual(SEE_ADDITIONAL_ISSUES_TEXT); expect(wrapper.find('.table-issue-more-issues').text()).toEqual('View all 1 issues');
}); });
}); });
......
...@@ -22,7 +22,6 @@ import FrequentUsers from 'components/TableDetail/FrequentUsers'; ...@@ -22,7 +22,6 @@ import FrequentUsers from 'components/TableDetail/FrequentUsers';
import LoadingSpinner from 'components/common/LoadingSpinner'; import LoadingSpinner from 'components/common/LoadingSpinner';
import LineageLink from 'components/TableDetail/LineageLink'; import LineageLink from 'components/TableDetail/LineageLink';
import OwnerEditor from 'components/TableDetail/OwnerEditor'; import OwnerEditor from 'components/TableDetail/OwnerEditor';
import ReportTableIssue from 'components/TableDetail/ReportTableIssue';
import SourceLink from 'components/TableDetail/SourceLink'; import SourceLink from 'components/TableDetail/SourceLink';
import TableDescEditableText from 'components/TableDetail/TableDescEditableText'; import TableDescEditableText from 'components/TableDetail/TableDescEditableText';
import TableHeaderBullets from 'components/TableDetail/TableHeaderBullets'; import TableHeaderBullets from 'components/TableDetail/TableHeaderBullets';
...@@ -172,8 +171,6 @@ class TableDetail extends React.Component<TableDetailProps & RouteComponentProps ...@@ -172,8 +171,6 @@ class TableDetail extends React.Component<TableDetailProps & RouteComponentProps
<main className="column-layout-1"> <main className="column-layout-1">
<section className="left-panel"> <section className="left-panel">
{} {}
<TableIssues tableKey={ this.key }/>
<EditableSection title="Description"> <EditableSection title="Description">
<TableDescEditableText <TableDescEditableText
maxLength={ AppConfig.editableText.tableDescLength } maxLength={ AppConfig.editableText.tableDescLength }
...@@ -183,8 +180,8 @@ class TableDetail extends React.Component<TableDetailProps & RouteComponentProps ...@@ -183,8 +180,8 @@ class TableDetail extends React.Component<TableDetailProps & RouteComponentProps
</EditableSection> </EditableSection>
<span> <span>
{ notificationsEnabled() && <RequestDescriptionText/> } { notificationsEnabled() && <RequestDescriptionText/> }
{ issueTrackingEnabled() && <ReportTableIssue tableKey={ this.key } tableName={ this.getDisplayName() } />}
</span> </span>
{issueTrackingEnabled() && <TableIssues tableKey={ this.key } tableName={ this.getDisplayName()}/>}
<section className="column-layout-2"> <section className="column-layout-2">
<section className="left-panel"> <section className="left-panel">
{ {
......
...@@ -8,8 +8,8 @@ export const NOTIFICATION_API_PATH = '/api/mail/v0/notification'; ...@@ -8,8 +8,8 @@ export const NOTIFICATION_API_PATH = '/api/mail/v0/notification';
export type IssuesAPI = { export type IssuesAPI = {
issues: { issues: {
issues: Issue[]; issues: Issue[];
remaining: number; total: number;
remaining_url: string; all_issues_url: string;
} }
} }
......
...@@ -50,24 +50,24 @@ export function getIssues(tableKey: string): GetIssuesRequest { ...@@ -50,24 +50,24 @@ export function getIssues(tableKey: string): GetIssuesRequest {
}; };
} }
export function getIssuesSuccess(issues: Issue[], remainingIssues?: number, remainingIssuesUrl?: string): GetIssuesResponse { export function getIssuesSuccess(issues: Issue[], total?: number, allIssuesUrl?: string): GetIssuesResponse {
return { return {
type: GetIssues.SUCCESS, type: GetIssues.SUCCESS,
payload: { payload: {
issues, issues,
remainingIssues, total,
remainingIssuesUrl allIssuesUrl
} }
} }
} }
export function getIssuesFailure(issues: Issue[], remainingIssues?: number, remainingIssuesUrl?: string): GetIssuesResponse { export function getIssuesFailure(issues: Issue[], total?: number, allIssuesUrl?: string): GetIssuesResponse {
return { return {
type: GetIssues.FAILURE, type: GetIssues.FAILURE,
payload: { payload: {
issues, issues,
remainingIssues, total,
remainingIssuesUrl allIssuesUrl
} }
} }
} }
...@@ -75,15 +75,15 @@ export function getIssuesFailure(issues: Issue[], remainingIssues?: number, rema ...@@ -75,15 +75,15 @@ export function getIssuesFailure(issues: Issue[], remainingIssues?: number, rema
/* REDUCER */ /* REDUCER */
export interface IssueReducerState { export interface IssueReducerState {
issues: Issue[], issues: Issue[],
remainingIssuesUrl: string, allIssuesUrl: string,
remainingIssues: number, total: number,
isLoading: boolean isLoading: boolean
}; };
export const initialIssuestate: IssueReducerState = { export const initialIssuestate: IssueReducerState = {
issues: [], issues: [],
remainingIssuesUrl: null, allIssuesUrl: null,
remainingIssues: 0, total: 0,
isLoading: false, isLoading: false,
}; };
......
...@@ -12,7 +12,7 @@ export function* getIssuesWorker(action: GetIssuesRequest): SagaIterator { ...@@ -12,7 +12,7 @@ export function* getIssuesWorker(action: GetIssuesRequest): SagaIterator {
let response; let response;
try { try {
response = yield call(API.getIssues, key); response = yield call(API.getIssues, key);
yield put(getIssuesSuccess(response.issues, response.remaining, response.remaining_url)); yield put(getIssuesSuccess(response.issues, response.total, response.all_issues_url));
} catch(e) { } catch(e) {
yield put(getIssuesFailure([], 0, null)); yield put(getIssuesFailure([], 0, null));
} }
......
...@@ -29,8 +29,6 @@ describe('issue ducks', () => { ...@@ -29,8 +29,6 @@ describe('issue ducks', () => {
let tableKey: string; let tableKey: string;
let issue: Issue; let issue: Issue;
let issues: Issue[]; let issues: Issue[];
let remaining: number;
let remainingUrl: string;
let key; let key;
let title; let title;
let description; let description;
...@@ -38,6 +36,8 @@ describe('issue ducks', () => { ...@@ -38,6 +36,8 @@ describe('issue ducks', () => {
let resourcePath; let resourcePath;
let owners; let owners;
let sender; let sender;
let total;
let allIssuesUrl;
beforeAll(() => { beforeAll(() => {
tableKey = 'key'; tableKey = 'key';
key = 'table', key = 'table',
...@@ -50,12 +50,15 @@ describe('issue ducks', () => { ...@@ -50,12 +50,15 @@ describe('issue ducks', () => {
issue = { issue = {
issue_key: 'issue_key', issue_key: 'issue_key',
title: 'title', title: 'title',
url: 'http://url' url: 'http://url',
status: 'Open',
priority_display_name: 'P2',
priority_name: 'Major'
}; };
issues = [issue]; issues = [issue];
remaining = 0; total = 0;
remainingUrl = 'testurl'; allIssuesUrl = 'testurl';
}); });
describe('actions', () => { describe('actions', () => {
...@@ -67,7 +70,7 @@ describe('issue ducks', () => { ...@@ -67,7 +70,7 @@ describe('issue ducks', () => {
}); });
it('getIssuesSuccess - returns the action to process success', () => { it('getIssuesSuccess - returns the action to process success', () => {
const action = getIssuesSuccess(issues, remaining, remainingUrl); const action = getIssuesSuccess(issues, total, allIssuesUrl);
expect(action.type).toBe(GetIssues.SUCCESS); expect(action.type).toBe(GetIssues.SUCCESS);
}); });
...@@ -120,17 +123,17 @@ describe('issue ducks', () => { ...@@ -120,17 +123,17 @@ describe('issue ducks', () => {
describe('reducer', () => { describe('reducer', () => {
let testState: IssueReducerState; let testState: IssueReducerState;
let remainingUrl: string; let allIssuesUrl: string;
let remaining: number; let total: number;
beforeAll(() => { beforeAll(() => {
const stateIssues: Issue[]=[]; const stateIssues: Issue[]=[];
remaining = 0; total = 0;
remainingUrl = 'testUrl'; allIssuesUrl = 'testUrl';
testState = { testState = {
total,
allIssuesUrl,
isLoading: false, isLoading: false,
issues: stateIssues, issues: stateIssues
remainingIssues: remaining,
remainingIssuesUrl: remainingUrl
}; };
}); });
...@@ -143,26 +146,26 @@ describe('issue ducks', () => { ...@@ -143,26 +146,26 @@ describe('issue ducks', () => {
expect(reducer(testState, getIssues(tableKey))).toEqual({ expect(reducer(testState, getIssues(tableKey))).toEqual({
issues: [], issues: [],
isLoading: true, isLoading: true,
remainingIssuesUrl: null, allIssuesUrl: null,
remainingIssues: 0 total: 0
}); });
}); });
it('should handle GetIssues.SUCCESS', () => { it('should handle GetIssues.SUCCESS', () => {
expect(reducer(testState, getIssuesSuccess(issues, remaining, remainingUrl))).toEqual({ expect(reducer(testState, getIssuesSuccess(issues, total, allIssuesUrl))).toEqual({
issues, issues,
isLoading: false, total,
remainingIssues: remaining, allIssuesUrl,
remainingIssuesUrl: remainingUrl isLoading: false
}); });
}); });
it('should handle GetIssues.FAILURE', () => { it('should handle GetIssues.FAILURE', () => {
expect(reducer(testState, getIssuesFailure([], 0, null))).toEqual({ expect(reducer(testState, getIssuesFailure([], 0, null))).toEqual({
total,
issues: [], issues: [],
isLoading: false, isLoading: false,
remainingIssuesUrl: null, allIssuesUrl: null
remainingIssues: remaining
}); });
}); });
...@@ -182,10 +185,10 @@ describe('issue ducks', () => { ...@@ -182,10 +185,10 @@ describe('issue ducks', () => {
} }
}; };
expect(reducer(testState, createIssue(createIssuePayload, notificationPayload))).toEqual({ expect(reducer(testState, createIssue(createIssuePayload, notificationPayload))).toEqual({
allIssuesUrl,
total,
issues: [], issues: [],
isLoading: true, isLoading: true,
remainingIssuesUrl: remainingUrl,
remainingIssues: remaining
}); });
}); });
...@@ -195,10 +198,11 @@ describe('issue ducks', () => { ...@@ -195,10 +198,11 @@ describe('issue ducks', () => {
}); });
it('should handle CreateIssue.FAILURE', () => { it('should handle CreateIssue.FAILURE', () => {
expect(reducer(testState, createIssueFailure(null))).toEqual({ issues: [], expect(reducer(testState, createIssueFailure(null))).toEqual({
isLoading: false, total,
remainingIssuesUrl: remainingUrl, allIssuesUrl,
remainingIssues: remaining issues: [],
isLoading: false
}); });
}); });
}); });
...@@ -214,21 +218,21 @@ describe('issue ducks', () => { ...@@ -214,21 +218,21 @@ describe('issue ducks', () => {
describe('getIssuesWorker', () => { describe('getIssuesWorker', () => {
let action: GetIssuesRequest; let action: GetIssuesRequest;
let remainingIssuesUrl: string; let allIssuesUrl: string;
let remainingIssues: number; let total: number;
beforeAll(() => { beforeAll(() => {
action = getIssues(tableKey); action = getIssues(tableKey);
issues = globalState.issue.issues; issues = globalState.issue.issues;
remainingIssues = globalState.issue.remainingIssues; total = globalState.issue.total;
remainingIssuesUrl = globalState.issue.remainingIssuesUrl; allIssuesUrl = globalState.issue.allIssuesUrl;
}); });
it('gets issues', () => { it('gets issues', () => {
return expectSaga(getIssuesWorker, action) return expectSaga(getIssuesWorker, action)
.provide([ .provide([
[matchers.call.fn(API.getIssues), {issues, remainingIssues, remainingIssuesUrl}], [matchers.call.fn(API.getIssues), {issues, total, allIssuesUrl}],
]) ])
.put(getIssuesSuccess(issues)) .put(getIssuesSuccess(issues, total))
.run(); .run();
}); });
......
...@@ -30,8 +30,8 @@ export interface GetIssuesResponse { ...@@ -30,8 +30,8 @@ export interface GetIssuesResponse {
type: GetIssues.SUCCESS | GetIssues.FAILURE; type: GetIssues.SUCCESS | GetIssues.FAILURE;
payload: { payload: {
issues: Issue[]; issues: Issue[];
remainingIssues: number; total: number;
remainingIssuesUrl: string; allIssuesUrl: string;
} }
}; };
......
...@@ -36,8 +36,8 @@ const globalState: GlobalState = { ...@@ -36,8 +36,8 @@ const globalState: GlobalState = {
}, },
issue: { issue: {
issues: [], issues: [],
remainingIssuesUrl: null, allIssuesUrl: null,
remainingIssues: 0, total: 0,
isLoading: true isLoading: true
}, },
notification: { notification: {
......
...@@ -2,6 +2,9 @@ export interface Issue { ...@@ -2,6 +2,9 @@ export interface Issue {
issue_key: string; issue_key: string;
title: string; title: string;
url: string; url: string;
status: string;
priority_name: string;
priority_display_name: string;
}; };
export interface CreateIssuePayload { export interface CreateIssuePayload {
......
...@@ -18,16 +18,20 @@ class IssueTest(unittest.TestCase): ...@@ -18,16 +18,20 @@ class IssueTest(unittest.TestCase):
'issue_key': 'key', 'issue_key': 'key',
'title': 'some title', 'title': 'some title',
'url': 'http://somewhere', 'url': 'http://somewhere',
'priority_name': 'Major',
'priority_display_name': 'P2'
} }
self.mock_issues = { self.mock_issues = {
'issues': [self.mock_issue] 'issues': [self.mock_issue]
} }
self.mock_data_issue = DataIssue(issue_key='key', self.mock_data_issue = DataIssue(issue_key='key',
title='title', title='title',
url='http://somewhere') url='http://somewhere',
status='open',
priority='Major')
self.expected_issues = IssueResults(issues=[self.mock_data_issue], self.expected_issues = IssueResults(issues=[self.mock_data_issue],
remaining=0, total=0,
remaining_url="http://moredata") all_issues_url="http://moredata")
# ----- Jira API Tests ---- # # ----- Jira API Tests ---- #
...@@ -76,10 +80,10 @@ class IssueTest(unittest.TestCase): ...@@ -76,10 +80,10 @@ class IssueTest(unittest.TestCase):
self.assertEqual(response.status_code, HTTPStatus.OK) self.assertEqual(response.status_code, HTTPStatus.OK)
self.assertEqual(data['issues']['issues'][0]['issue_key'], self.assertEqual(data['issues']['issues'][0]['issue_key'],
self.expected_issues.issues[0].issue_key) self.expected_issues.issues[0].issue_key)
self.assertEqual(data['issues']['remaining'], self.assertEqual(data['issues']['total'],
self.expected_issues.remaining) self.expected_issues.total)
self.assertEqual(data['issues']['remaining_url'], self.assertEqual(data['issues']['all_issues_url'],
self.expected_issues.remaining_url) self.expected_issues.all_issues_url)
mock_issue_tracker_client.return_value.get_issues.assert_called_with('table_key') mock_issue_tracker_client.return_value.get_issues.assert_called_with('table_key')
def test_create_issue_not_enabled(self) -> None: def test_create_issue_not_enabled(self) -> None:
......
...@@ -27,13 +27,18 @@ class JiraClientTest(unittest.TestCase): ...@@ -27,13 +27,18 @@ class JiraClientTest(unittest.TestCase):
self.mock_issue = { self.mock_issue = {
'issue_key': 'key', 'issue_key': 'key',
'title': 'some title', 'title': 'some title',
'url': 'http://somewhere' 'url': 'http://somewhere',
'status': 'open',
'priority_name': 'Major',
'priority_display_name': 'P2'
} }
result_list = MockJiraResultList(iterable=self.mock_issue, _total=0) result_list = MockJiraResultList(iterable=self.mock_issue, _total=0)
self.mock_jira_issues = result_list self.mock_jira_issues = result_list
self.mock_issue_instance = DataIssue(issue_key='key', self.mock_issue_instance = DataIssue(issue_key='key',
title='some title', title='some title',
url='http://somewhere') url='http://somewhere',
status='open',
priority='Major')
@unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.jira_client.JIRA') @unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.jira_client.JIRA')
def test_create_JiraClient_validates_config(self, mock_JIRA_client: Mock) -> None: def test_create_JiraClient_validates_config(self, mock_JIRA_client: Mock) -> None:
...@@ -53,7 +58,7 @@ class JiraClientTest(unittest.TestCase): ...@@ -53,7 +58,7 @@ class JiraClientTest(unittest.TestCase):
@unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.jira_client.JIRA') @unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.jira_client.JIRA')
@unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.jira_client.' @unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.jira_client.'
'JiraClient._get_remaining_issues') 'JiraClient._generate_all_issues_url')
def test_get_issues_returns_JIRAError(self, mock_remaining_issues: Mock, mock_JIRA_client: Mock) -> None: def test_get_issues_returns_JIRAError(self, mock_remaining_issues: Mock, mock_JIRA_client: Mock) -> None:
mock_JIRA_client.return_value.get_issues.side_effect = JIRAError('Some exception') mock_JIRA_client.return_value.get_issues.side_effect = JIRAError('Some exception')
mock_remaining_issues.return_value = 0 mock_remaining_issues.return_value = 0
...@@ -73,14 +78,18 @@ class JiraClientTest(unittest.TestCase): ...@@ -73,14 +78,18 @@ class JiraClientTest(unittest.TestCase):
@unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.jira_client.' @unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.jira_client.'
'JiraClient._get_issue_properties') 'JiraClient._get_issue_properties')
@unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.' @unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.'
'jira_client.JiraClient._generate_remaining_issues_url') 'jira_client.JiraClient._generate_all_issues_url')
@unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.'
'jira_client.JiraClient._sort_issues')
def test_get_issues_returns_issues(self, def test_get_issues_returns_issues(self,
mock_sort_issues: Mock,
mock_get_url: Mock, mock_get_url: Mock,
mock_get_issue_properties: Mock, mock_get_issue_properties: Mock,
mock_JIRA_client: Mock) -> None: mock_JIRA_client: Mock) -> None:
mock_JIRA_client.return_value.search_issues.return_value = self.mock_jira_issues mock_JIRA_client.return_value.search_issues.return_value = self.mock_jira_issues
mock_get_issue_properties.return_value = self.mock_issue mock_get_issue_properties.return_value = self.mock_issue
mock_get_url.return_value = 'url' mock_get_url.return_value = 'url'
mock_sort_issues.return_value = [self.mock_issue]
with app.test_request_context(): with app.test_request_context():
jira_client = JiraClient(issue_tracker_url=app.config['ISSUE_TRACKER_URL'], jira_client = JiraClient(issue_tracker_url=app.config['ISSUE_TRACKER_URL'],
issue_tracker_user=app.config['ISSUE_TRACKER_USER'], issue_tracker_user=app.config['ISSUE_TRACKER_USER'],
...@@ -90,14 +99,14 @@ class JiraClientTest(unittest.TestCase): ...@@ -90,14 +99,14 @@ class JiraClientTest(unittest.TestCase):
results = jira_client.get_issues(table_uri='key') results = jira_client.get_issues(table_uri='key')
mock_JIRA_client.assert_called mock_JIRA_client.assert_called
self.assertEqual(results.issues[0], self.mock_issue) self.assertEqual(results.issues[0], self.mock_issue)
self.assertEqual(results.remaining, self.mock_jira_issues.total) self.assertEqual(results.total, self.mock_jira_issues.total)
mock_JIRA_client.return_value.search_issues.assert_called_with( mock_JIRA_client.return_value.search_issues.assert_called_with(
'text ~ "key" AND resolution = Unresolved order by createdDate DESC', 'text ~ "key" order by createdDate DESC',
maxResults=3) maxResults=3)
@unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.jira_client.JIRA') @unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.jira_client.JIRA')
@unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.jira_client.urllib.parse.quote') @unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.jira_client.urllib.parse.quote')
def test__generate_remaining_issues_url(self, mock_url_lib: Mock, mock_JIRA_client: Mock) -> None: def test__generate_all_issues_url(self, mock_url_lib: Mock, mock_JIRA_client: Mock) -> None:
mock_url_lib.return_value = 'test' mock_url_lib.return_value = 'test'
with app.test_request_context(): with app.test_request_context():
jira_client = JiraClient(issue_tracker_url=app.config['ISSUE_TRACKER_URL'], jira_client = JiraClient(issue_tracker_url=app.config['ISSUE_TRACKER_URL'],
...@@ -105,12 +114,12 @@ class JiraClientTest(unittest.TestCase): ...@@ -105,12 +114,12 @@ class JiraClientTest(unittest.TestCase):
issue_tracker_password=app.config['ISSUE_TRACKER_PASSWORD'], issue_tracker_password=app.config['ISSUE_TRACKER_PASSWORD'],
issue_tracker_project_id=app.config['ISSUE_TRACKER_PROJECT_ID'], issue_tracker_project_id=app.config['ISSUE_TRACKER_PROJECT_ID'],
issue_tracker_max_results=app.config['ISSUE_TRACKER_MAX_RESULTS']) issue_tracker_max_results=app.config['ISSUE_TRACKER_MAX_RESULTS'])
issues = [DataIssue(issue_key='key', title='title', url='url')] issues = [DataIssue(issue_key='key', title='title', url='url', status='open', priority='Major')]
url = jira_client._generate_remaining_issues_url(table_uri="table", issues=issues) url = jira_client._generate_all_issues_url(table_uri="table", issues=issues)
self.assertEqual(url, 'test_url/browse/key?jql=test') self.assertEqual(url, 'test_url/issues/?jql=test')
@unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.jira_client.JIRA') @unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.jira_client.JIRA')
def test__generate_remaining_issues_url_no_issues(self, mock_JIRA_client: Mock) -> None: def test__generate_all_issues_url_no_issues(self, mock_JIRA_client: Mock) -> None:
with app.test_request_context(): with app.test_request_context():
jira_client = JiraClient(issue_tracker_url=app.config['ISSUE_TRACKER_URL'], jira_client = JiraClient(issue_tracker_url=app.config['ISSUE_TRACKER_URL'],
issue_tracker_user=app.config['ISSUE_TRACKER_USER'], issue_tracker_user=app.config['ISSUE_TRACKER_USER'],
...@@ -119,7 +128,7 @@ class JiraClientTest(unittest.TestCase): ...@@ -119,7 +128,7 @@ class JiraClientTest(unittest.TestCase):
issue_tracker_max_results=app.config['ISSUE_TRACKER_MAX_RESULTS']) issue_tracker_max_results=app.config['ISSUE_TRACKER_MAX_RESULTS'])
issues: List[DataIssue] issues: List[DataIssue]
issues = [] issues = []
url = jira_client._generate_remaining_issues_url(table_uri="table", issues=issues) url = jira_client._generate_all_issues_url(table_uri="table", issues=issues)
self.assertEqual(url, '') self.assertEqual(url, '')
@unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.jira_client.JIRA') @unittest.mock.patch('amundsen_application.proxy.issue_tracker_clients.jira_client.JIRA')
......
import unittest
from amundsen_application.models.data_issue import DataIssue
class DataIssueTest(unittest.TestCase):
def setUp(self) -> None:
self.issue_key = 'key'
self.title = 'title'
self.url = 'https://place'
self.status = 'open'
self.priority = 'Major'
self.maxDiff = None
def test_mapping_priority(self) -> None:
expected_priority_name = 'major'
expected_priority_display_name = 'P2'
data_issue = DataIssue(issue_key=self.issue_key,
title=self.title,
url=self.url,
status=self.status,
priority=self.priority)
self.assertEqual(data_issue.priority_display_name, expected_priority_display_name)
self.assertEqual(data_issue.priority_name, expected_priority_name)
self.assertEqual(data_issue.issue_key, self.issue_key)
self.assertEqual(data_issue.title, self.title)
self.assertEqual(data_issue.url, self.url)
self.assertEqual(data_issue.status, self.status)
def test_mapping_priorty_missing(self) -> None:
expected_priority_name = None # type: ignore
expected_priority_display_name = None # type: ignore
data_issue = DataIssue(issue_key=self.issue_key,
title=self.title,
url=self.url,
status=self.status,
priority='missing')
self.assertEqual(data_issue.priority_display_name, expected_priority_display_name)
self.assertEqual(data_issue.priority_name, expected_priority_name)
self.assertEqual(data_issue.issue_key, self.issue_key)
self.assertEqual(data_issue.title, self.title)
self.assertEqual(data_issue.url, self.url)
self.assertEqual(data_issue.status, self.status)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment