Unverified Commit 99e2cda9 authored by Tamika Tannis's avatar Tamika Tannis Committed by GitHub

fix: Separate related dashboards component & request (#494)

* Separate related dashboards component & request

* Code clenaup

* Tests

* Fix some tests
parent 1af81d57
......@@ -656,7 +656,7 @@ def get_related_dashboard_metadata(table_key: str) -> Response:
def _get_related_dashboards_metadata(*, url: str) -> Dict[str, Any]:
results_dict = {
'dashboards': {},
'dashboards': [],
'msg': '',
}
......@@ -674,7 +674,7 @@ def _get_related_dashboards_metadata(*, url: str) -> Dict[str, Any]:
results_dict['status_code'] = status_code
if status_code != HTTPStatus.OK:
message = 'Encountered error: Related Dashboard Metadata request failed'
message = f'Encountered {status_code} Error: Related dashboard metadata request failed'
results_dict['msg'] = message
logging.error(message)
return results_dict
......
import * as React from 'react';
import { shallow } from 'enzyme';
import ResourceList from 'components/common/ResourceList';
import ShimmeringResourceLoader from 'components/common/ShimmeringResourceLoader';
import { dashboardSummary } from 'fixtures/metadata/dashboard';
import globalState from 'fixtures/globalState';
import {
TableDashboardResourceList,
TableDashboardResourceListProps,
mapStateToProps,
} from '.';
const setup = (propOverrides?: Partial<TableDashboardResourceListProps>) => {
const props = {
itemsPerPage: 10,
source: 'test',
dashboards: [],
isLoading: false,
errorText: 'test',
...propOverrides,
};
const wrapper = shallow<TableDashboardResourceList>(
<TableDashboardResourceList {...props} />
);
return { props, wrapper };
};
describe('TableDashboardResourceList', () => {
describe('render', () => {
it('should render without problems', () => {
expect(() => {
setup();
}).not.toThrow();
});
it('should render the resource loader', () => {
const { props, wrapper } = setup({ isLoading: true });
const element = wrapper.find(ShimmeringResourceLoader);
expect(element.length).toEqual(1);
expect(element.props().numItems).toBe(props.itemsPerPage);
});
it('should render the resource list', () => {
const { props, wrapper } = setup();
const element = wrapper.find(ResourceList);
expect(element.length).toEqual(1);
expect(element.props().allItems).toBe(props.dashboards);
expect(element.props().emptyText).toBe(props.errorText);
expect(element.props().itemsPerPage).toBe(props.itemsPerPage);
expect(element.props().source).toBe(props.source);
});
});
});
describe('mapStateToProps', () => {
describe('when dashboards do not exist on tableMetadata state', () => {
let result;
beforeAll(() => {
result = mapStateToProps(globalState);
});
it('sets dashboards to default', () => {
expect(result.dashboards).toEqual([]);
});
it('sets isLoading to default', () => {
expect(result.isLoading).toBe(true);
});
it('sets errorText to default', () => {
expect(result.errorText).toEqual('');
});
});
describe('when dashboards exist on tableMetadata state', () => {
let result;
let testState;
beforeAll(() => {
testState = {
dashboards: {
dashboards: [dashboardSummary],
isLoading: false,
errorText: '',
},
};
result = mapStateToProps({
...globalState,
tableMetadata: {
...globalState.tableMetadata,
...testState,
},
});
});
it('sets dashboards to default', () => {
expect(result.dashboards).toEqual(testState.dashboards.dashboards);
});
});
});
import * as React from 'react';
import { connect } from 'react-redux';
import ResourceList from 'components/common/ResourceList';
import ShimmeringResourceLoader from 'components/common/ShimmeringResourceLoader';
import { GlobalState } from 'ducks/rootReducer';
import { DashboardResource } from 'interfaces';
interface OwnProps {
itemsPerPage: number;
source: string;
}
interface StateFromProps {
dashboards: DashboardResource[];
isLoading: boolean;
errorText: string;
}
export type TableDashboardResourceListProps = OwnProps & StateFromProps;
export class TableDashboardResourceList extends React.Component<
TableDashboardResourceListProps
> {
render() {
const {
dashboards,
errorText,
isLoading,
itemsPerPage,
source,
} = this.props;
let content = <ShimmeringResourceLoader numItems={itemsPerPage} />;
if (!isLoading) {
content = (
<ResourceList
allItems={dashboards}
emptyText={errorText || undefined}
itemsPerPage={itemsPerPage}
source={source}
/>
);
}
return content;
}
}
export const mapStateToProps = (state: GlobalState) => {
const relatedDashboards = state.tableMetadata.dashboards;
return {
dashboards: relatedDashboards ? relatedDashboards.dashboards : [],
isLoading: relatedDashboards ? relatedDashboards.isLoading : true,
errorText: relatedDashboards ? relatedDashboards.errorMessage : '',
};
};
export default connect<StateFromProps, {}, OwnProps>(
mapStateToProps,
null
)(TableDashboardResourceList);
import * as React from 'react';
import * as History from 'history';
import { mount } from 'enzyme';
import { mount, shallow } from 'enzyme';
import { mocked } from 'ts-jest/utils';
import { getMockRouterProps } from '../../fixtures/mockRouter';
import { tableMetadata } from '../../fixtures/metadata/table';
import { getMockRouterProps } from 'fixtures/mockRouter';
import { tableMetadata } from 'fixtures/metadata/table';
import LoadingSpinner from '../common/LoadingSpinner';
import LoadingSpinner from 'components/common/LoadingSpinner';
import TabsComponent from 'components/common/TabsComponent';
import { indexDashboardsEnabled } from 'config/config-utils';
import { TableDetail, TableDetailProps, MatchProps } from '.';
jest.mock('config/config-utils', () => ({
indexDashboardsEnabled: jest.fn(),
}));
const setup = (
propOverrides?: Partial<TableDetailProps>,
location?: Partial<History.Location>
......@@ -24,6 +31,8 @@ const setup = (
);
const props = {
isLoading: false,
isLoadingDashboards: false,
numRelatedDashboards: 0,
statusCode: 200,
tableData: tableMetadata,
getTableData: jest.fn(),
......@@ -36,6 +45,24 @@ const setup = (
};
describe('TableDetail', () => {
describe('renderTabs', () => {
let wrapper;
beforeAll(() => {
wrapper = setup().wrapper;
});
it('renders one tab when dashboards are not enabled', () => {
mocked(indexDashboardsEnabled).mockImplementation(() => false);
const content = shallow(<div>{wrapper.instance().renderTabs()}</div>);
expect(content.find(TabsComponent).props().tabs.length).toEqual(1);
});
it('renders two tabs when dashboards are enabled', () => {
mocked(indexDashboardsEnabled).mockImplementation(() => true);
const content = shallow(<div>{wrapper.instance().renderTabs()}</div>);
expect(content.find(TabsComponent).props().tabs.length).toEqual(2);
});
});
describe('render', () => {
it('should render without problems', () => {
expect(() => {
......
......@@ -16,15 +16,15 @@ import TabsComponent from 'components/common/TabsComponent';
import EditableText from 'components/common/EditableText';
import LoadingSpinner from 'components/common/LoadingSpinner';
import Flag from 'components/common/Flag';
import ResourceList from 'components/common/ResourceList';
import DataPreviewButton from 'components/TableDetail/DataPreviewButton';
import ColumnList from 'components/TableDetail/ColumnList';
import DataPreviewButton from 'components/TableDetail/DataPreviewButton';
import ExploreButton from 'components/TableDetail/ExploreButton';
import FrequentUsers from 'components/TableDetail/FrequentUsers';
import LineageLink from 'components/TableDetail/LineageLink';
import OwnerEditor from 'components/TableDetail/OwnerEditor';
import SourceLink from 'components/TableDetail/SourceLink';
import TableDashboardResourceList from 'components/TableDetail/TableDashboardResourceList';
import TableDescEditableText from 'components/TableDetail/TableDescEditableText';
import TableHeaderBullets from 'components/TableDetail/TableHeaderBullets';
import TableIssues from 'components/TableDetail/TableIssues';
......@@ -37,6 +37,7 @@ import EditableSection from 'components/common/EditableSection';
import {
getSourceIconClass,
indexDashboardsEnabled,
issueTrackingEnabled,
notificationsEnabled,
} from 'config/config-utils';
......@@ -57,6 +58,8 @@ const TABLE_SOURCE = 'table_page';
export interface StateFromProps {
isLoading: boolean;
isLoadingDashboards: boolean;
numRelatedDashboards: number;
statusCode?: number;
tableData: TableMetadata;
}
......@@ -138,18 +141,25 @@ export class TableDetail extends React.Component<
title: `Columns (${this.props.tableData.columns.length})`,
});
// Dashboard content
tabInfo.push({
content: (
<ResourceList
allItems={this.props.tableData.dashboards}
itemsPerPage={DASHBOARDS_PER_PAGE}
source={TABLE_SOURCE}
/>
),
key: 'dashboards',
title: `Dashboards (${this.props.tableData.dashboards.length})`,
});
if (indexDashboardsEnabled()) {
const loadingTitle = (
<div className="tab-title">
Dashboards <LoadingSpinner />
</div>
);
tabInfo.push({
content: (
<TableDashboardResourceList
itemsPerPage={DASHBOARDS_PER_PAGE}
source={TABLE_SOURCE}
/>
),
key: 'dashboards',
title: this.props.isLoadingDashboards
? loadingTitle
: `Dashboards (${this.props.numRelatedDashboards})`,
});
}
return <TabsComponent tabs={tabInfo} defaultTab="columns" />;
}
......@@ -305,6 +315,12 @@ export const mapStateToProps = (state: GlobalState) => {
isLoading: state.tableMetadata.isLoading,
statusCode: state.tableMetadata.statusCode,
tableData: state.tableMetadata.tableData,
numRelatedDashboards: state.tableMetadata.dashboards
? state.tableMetadata.dashboards.dashboards.length
: 0,
isLoadingDashboards: state.tableMetadata.dashboards
? state.tableMetadata.dashboards.isLoading
: true,
};
};
......
......@@ -22,4 +22,14 @@
}
}
}
.tab-title {
display: flex;
.loading-spinner {
height: 20px;
margin: 0 0 0 $spacer-1/2;
width: 20px;
}
}
}
......@@ -23,7 +23,8 @@
.empty-message {
display: flex;
justify-content: center;
margin-top: $spacer-4;
margin: $spacer-4 $spacer-4 0;
word-break: break-all;
}
}
......
......@@ -37,6 +37,7 @@ describe('helpers', () => {
};
mockRelatedDashboardsResponseData = {
dashboards: relatedDashboards,
msg: '',
};
mockResponseData = {
tableData: tableResponseData,
......@@ -80,10 +81,7 @@ describe('helpers', () => {
describe('getTableDataFromResponseData', () => {
it('uses the filterFromObj method', () => {
Helpers.getTableDataFromResponseData(
mockResponseData,
mockRelatedDashboardsResponseData
);
Helpers.getTableDataFromResponseData(mockResponseData);
expect(filterFromObjSpy).toHaveBeenCalledWith(tableResponseData, [
'owners',
......@@ -94,20 +92,8 @@ describe('helpers', () => {
describe('produces the final TableMetadata information', () => {
it('contains the columns key', () => {
const expected = 0;
const actual = Helpers.getTableDataFromResponseData(
mockResponseData,
mockRelatedDashboardsResponseData
).columns.length;
expect(actual).toEqual(expected);
});
it('contains the dashboards key', () => {
const expected = 3;
const actual = Helpers.getTableDataFromResponseData(
mockResponseData,
mockRelatedDashboardsResponseData
).dashboards.length;
const actual = Helpers.getTableDataFromResponseData(mockResponseData)
.columns.length;
expect(actual).toEqual(expected);
});
......
......@@ -33,18 +33,15 @@ export function getRelatedDashboardSlug(key: string): string {
}
/**
* Parses the response for table metadata and the related dashboard information to create a TableMetadata object
* Parses the response for table metadata information to create a TableMetadata object
*/
export function getTableDataFromResponseData(
responseData: API.TableDataAPI,
relatedDashboardsData: API.RelatedDashboardDataAPI
responseData: API.TableDataAPI
): TableMetadata {
const mergedTableData = {
...filterFromObj(responseData.tableData, ['owners', 'tags']),
...filterFromObj(relatedDashboardsData, ['msg', 'status_code']),
};
return mergedTableData as TableMetadata;
return filterFromObj(responseData.tableData, [
'owners',
'tags',
]) as TableMetadata;
}
/**
......
......@@ -11,6 +11,7 @@ import {
} from 'interfaces';
/** HELPERS **/
import { indexDashboardsEnabled } from 'config/config-utils';
import {
getTableQueryParams,
getRelatedDashboardSlug,
......@@ -23,7 +24,6 @@ import {
export const API_PATH = '/api/metadata/v0';
// TODO: Consider created shared interfaces for ducks so we can reuse MessageAPI everywhere else
type MessageAPI = { msg: string };
export type TableData = TableMetadata & {
......@@ -34,37 +34,53 @@ export type DescriptionAPI = { description: string } & MessageAPI;
export type LastIndexedAPI = { timestamp: string } & MessageAPI;
export type PreviewDataAPI = { previewData: PreviewData } & MessageAPI;
export type TableDataAPI = { tableData: TableData } & MessageAPI;
export type RelatedDashboardDataAPI = { dashboards: DashboardResource[] };
export type RelatedDashboardDataAPI = {
dashboards: DashboardResource[];
} & MessageAPI;
export function getTableData(
tableKey: string,
index?: string,
source?: string
) {
const tableQueryParams = getTableQueryParams(tableKey, index, source);
const tableURL = `${API_PATH}/table?${tableQueryParams}`;
const tableRequest = axios.get<TableDataAPI>(tableURL);
return tableRequest.then((tableResponse: AxiosResponse<TableDataAPI>) => ({
data: getTableDataFromResponseData(tableResponse.data),
owners: getTableOwnersFromResponseData(tableResponse.data),
tags: tableResponse.data.tableData.tags,
statusCode: tableResponse.status,
}));
}
export function getTableDashboards(tableKey: string) {
if (!indexDashboardsEnabled()) {
return Promise.resolve({ dashboards: [] });
}
const relatedDashboardsSlug: string = getRelatedDashboardSlug(tableKey);
const relatedDashboardsURL: string = `${API_PATH}/table/${relatedDashboardsSlug}/dashboards`;
const relatedDashboardsRequest = axios.get<RelatedDashboardDataAPI>(
relatedDashboardsURL
);
const tableQueryParams = getTableQueryParams(tableKey, index, source);
const tableURL = `${API_PATH}/table?${tableQueryParams}`;
const tableRequest = axios.get<TableDataAPI>(tableURL);
return Promise.all([tableRequest, relatedDashboardsRequest]).then(
([tableResponse, relatedDashboardsResponse]: [
AxiosResponse<TableDataAPI>,
AxiosResponse<RelatedDashboardDataAPI>
]) => ({
data: getTableDataFromResponseData(
tableResponse.data,
relatedDashboardsResponse.data
),
owners: getTableOwnersFromResponseData(tableResponse.data),
tags: tableResponse.data.tableData.tags,
statusCode: tableResponse.status,
})
);
return relatedDashboardsRequest
.then(
(relatedDashboardsResponse: AxiosResponse<RelatedDashboardDataAPI>) => ({
dashboards: relatedDashboardsResponse.data.dashboards,
})
)
.catch((e: AxiosError<RelatedDashboardDataAPI>) => {
const { response } = e;
let msg = '';
if (response && response.data && response.data.msg) {
msg = response.data.msg;
}
const status = response ? response.status : null;
return Promise.reject({ msg, dashboards: [] });
});
}
export function getTableDescription(tableData: TableMetadata) {
......
import {
DashboardResource,
OwnerDict,
PreviewData,
PreviewQueryParams,
......@@ -11,6 +12,8 @@ import {
GetTableData,
GetTableDataRequest,
GetTableDataResponse,
GetTableDashboards,
GetTableDashboardsResponse,
GetTableDescription,
GetTableDescriptionRequest,
GetTableDescriptionResponse,
......@@ -78,6 +81,19 @@ export function getTableDataSuccess(
};
}
export function getTableDashboardsResponse(
dashboards: DashboardResource[],
errorMessage: string = ''
): GetTableDashboardsResponse {
return {
type: GetTableDashboards.RESPONSE,
payload: {
dashboards,
errorMessage,
},
};
}
export function getTableDescription(
onSuccess?: () => any,
onFailure?: () => any
......@@ -227,6 +243,11 @@ export function getPreviewDataSuccess(
/* REDUCER */
export interface TableMetadataReducerState {
dashboards?: {
isLoading: boolean;
dashboards: DashboardResource[];
errorMessage?: string;
};
isLoading: boolean;
lastIndexed: number;
preview: {
......@@ -247,7 +268,6 @@ export const initialTableDataState: TableMetadata = {
badges: [],
cluster: '',
columns: [],
dashboards: [],
database: '',
is_editable: false,
is_view: false,
......@@ -278,14 +298,17 @@ export default function reducer(
action
): TableMetadataReducerState {
switch (action.type) {
case GetTableData.REQUEST:
case GetTableDashboards.RESPONSE:
return {
...state,
isLoading: true,
preview: initialPreviewState,
tableData: initialTableDataState,
tableOwners: tableOwnersReducer(state.tableOwners, action),
dashboards: {
isLoading: false,
dashboards: action.payload.dashboards,
errorMessage: action.payload.errorMessage,
},
};
case GetTableData.REQUEST:
return initialState;
case GetTableData.FAILURE:
return {
...state,
......
......@@ -4,6 +4,7 @@ import { call, put, select, takeEvery, takeLatest } from 'redux-saga/effects';
import * as API from './api/v0';
import {
getTableDashboardsResponse,
getTableDataFailure,
getTableDataSuccess,
getTableDescriptionFailure,
......@@ -34,8 +35,8 @@ import {
} from './types';
export function* getTableDataWorker(action: GetTableDataRequest): SagaIterator {
const { key, searchIndex, source } = action.payload;
try {
const { key, searchIndex, source } = action.payload;
const { data, owners, statusCode, tags } = yield call(
API.getTableData,
key,
......@@ -43,6 +44,13 @@ export function* getTableDataWorker(action: GetTableDataRequest): SagaIterator {
source
);
yield put(getTableDataSuccess(data, owners, statusCode, tags));
try {
const { dashboards } = yield call(API.getTableDashboards, key);
yield put(getTableDashboardsResponse(dashboards));
} catch (error) {
yield put(getTableDashboardsResponse([], error.msg));
}
} catch (e) {
yield put(getTableDataFailure());
}
......
......@@ -10,6 +10,7 @@ import {
User,
} from 'interfaces';
import { dashboardSummary } from 'fixtures/metadata/dashboard';
import globalState from 'fixtures/globalState';
import * as API from '../api/v0';
......@@ -18,6 +19,7 @@ import reducer, {
getTableData,
getTableDataFailure,
getTableDataSuccess,
getTableDashboardsResponse,
getTableDescription,
getTableDescriptionFailure,
getTableDescriptionSuccess,
......@@ -296,6 +298,24 @@ describe('tableMetadata ducks', () => {
expect(reducer(testState, { type: 'INVALID.ACTION' })).toEqual(testState);
});
it('should handle GetTableDashboards.RESPONSE', () => {
const mockDashboards = [dashboardSummary];
const mockMessage = 'test';
expect(
reducer(
testState,
getTableDashboardsResponse(mockDashboards, mockMessage)
)
).toEqual({
...testState,
dashboards: {
isLoading: false,
dashboards: mockDashboards,
errorMessage: mockMessage,
},
});
});
it('should handle GetTableDescription.FAILURE', () => {
expect(
reducer(testState, getTableDescriptionFailure(expectedData))
......@@ -382,6 +402,9 @@ describe('tableMetadata ducks', () => {
statusCode: expectedStatus,
tags: expectedTags,
};
const mockDashboardsResult = {
dashboards: [dashboardSummary],
};
testSaga(
getTableDataWorker,
getTableData(testKey, testIndex, testSource)
......@@ -398,10 +421,14 @@ describe('tableMetadata ducks', () => {
)
)
.next()
.call(API.getTableDashboards, testKey)
.next(mockDashboardsResult)
.put(getTableDashboardsResponse(mockDashboardsResult.dashboards))
.next()
.isDone();
});
it('handles request error', () => {
it('handles request error on getTableData', () => {
testSaga(getTableDataWorker, getTableData(testKey))
.next()
.throw(new Error())
......
import {
DashboardResource,
OwnerDict,
PreviewData,
PreviewQueryParams,
......@@ -30,6 +31,17 @@ export interface GetTableDataResponse {
};
}
export enum GetTableDashboards {
RESPONSE = 'amundsen/tableMetadata/GET_TABLE_DASHBOARDS_RESPONSE',
}
export interface GetTableDashboardsResponse {
type: GetTableDashboards.RESPONSE;
payload: {
dashboards: DashboardResource[];
errorMessage: string;
};
}
export enum GetTableDescription {
REQUEST = 'amundsen/tableMetadata/GET_TABLE_DESCRIPTION_REQUEST',
SUCCESS = 'amundsen/tableMetadata/GET_TABLE_DESCRIPTION_SUCCESS',
......
......@@ -158,7 +158,6 @@ const globalState: GlobalState = {
badges: [],
cluster: '',
columns: [],
dashboards: [],
database: '',
is_editable: false,
is_view: false,
......
......@@ -93,7 +93,6 @@ export const tableMetadata: TableMetadata = {
],
},
],
dashboards: [],
database: 'hive',
description:
'One row per ride request, showing all stages of the ride funnel. ',
......
import { UpdateMethod } from './Enums';
import { User } from './User';
import { Badge } from './Tags';
import { DashboardResource } from './Resources';
interface PartitionData {
is_partitioned: boolean;
......@@ -79,7 +78,6 @@ export interface TableMetadata {
badges: Badge[];
cluster: string;
columns: TableColumn[];
dashboards: DashboardResource[];
database: string;
is_editable: boolean;
is_view: boolean;
......
......@@ -1036,8 +1036,8 @@ class MetadataTest(unittest.TestCase):
)
self.assertEqual(response.status_code, HTTPStatus.BAD_REQUEST)
expected = {
'dashboards': {},
'msg': 'Encountered error: Related Dashboard Metadata request failed',
'dashboards': [],
'msg': 'Encountered 400 Error: Related dashboard metadata request failed',
'status_code': 400
}
self.assertEqual(response.json, expected)
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