Skip to content

Commit

Permalink
feat: Adds sortable table detail page (#691)
Browse files Browse the repository at this point in the history 
Signed-off-by: Marcos Iglesias Valle <golodhros@gmail.com>
  • Loading branch information
@Golodhros
Golodhros authored Sep 29, 2020
1 parent 7a14f5f commit 36aebd2
Show file tree
Hide file tree
Showing 21 changed files with 711 additions and 53 deletions.
33 changes: 12 additions & 21 deletions amundsen_application/static/.betterer.results
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ exports[`strict null compilation`] = {
[255, 6, 11, "No overload matches this call.\\n The last overload gave the following error.\\n Type \'(() => SubmitSearchRequest) | null\' is not assignable to type \'ActionCreator<any>\'.\\n Type \'null\' is not assignable to type \'ActionCreator<any>\'.", "2296208050"],
[270, 4, 18, "No overload matches this call.\\n The last overload gave the following error.\\n Argument of type \'(dispatch: any, ownProps: any) => ActionCreator<unknown>\' is not assignable to parameter of type \'DispatchFromProps\'.\\n Type \'(dispatch: any, ownProps: any) => ActionCreator<unknown>\' is missing the following properties from type \'DispatchFromProps\': submitSearch, onInputChange, onSelectInlineResult", "2926224796"]
],
"js/components/common/Table/index.tsx:3110699653": [
[220, 22, 13, "Type \'unknown\' is not assignable to type \'ReactNode\'.\\n Type \'unknown\' is not assignable to type \'ReactPortal\'.", "971959308"]
"js/components/common/Table/index.tsx:489040313": [
[235, 22, 13, "Type \'unknown\' is not assignable to type \'ReactNode\'.\\n Type \'unknown\' is not assignable to type \'ReactPortal\'.", "971959308"]
],
"js/components/common/Tags/TagInput/index.tsx:3754832290": [
[63, 22, 6, "Type \'undefined\' is not assignable to type \'GetAllTagsRequest\'.", "1979467425"],
Expand All @@ -124,10 +124,10 @@ exports[`strict null compilation`] = {
[93, 4, 11, "Type \'Tag[]\' is not assignable to type \'never[]\'.", "255414113"],
[98, 4, 9, "Type \'Tag[]\' is not assignable to type \'never[]\'.", "3803340896"]
],
"js/config/config-utils.ts:1027600130": [
"js/config/config-utils.ts:1798174672": [
[86, 4, 25, "\'style\' is specified more than once, so this usage will be overwritten.", "1214862559"]
],
"js/config/index.spec.ts:1695312294": [
"js/config/index.spec.ts:3155903042": [
[17, 6, 61, "Object is possibly \'undefined\'.", "1496333578"],
[51, 6, 61, "Object is possibly \'undefined\'.", "1496333578"]
],
Expand Down Expand Up @@ -246,12 +246,6 @@ exports[`strict null compilation`] = {
"js/fixtures/globalState.ts:3931474038": [
[69, 4, 12, "Type \'null\' is not assignable to type \'string\'.", "124336133"]
],
"js/fixtures/metadata/table.ts:2912527294": [
[52, 6, 11, "Type \'null\' is not assignable to type \'string\'.", "1848305091"],
[171, 4, 11, "Type \'null\' is not assignable to type \'string\'.", "1848305091"],
[183, 4, 11, "Type \'null\' is not assignable to type \'string\'.", "1848305091"],
[195, 4, 11, "Type \'null\' is not assignable to type \'string\'.", "1848305091"]
],
"js/fixtures/mockRouter.ts:3563515077": [
[30, 6, 4, "Type \'null\' is not assignable to type \'{ (path: string, state?: {} | null | undefined): void; (location: LocationDescriptorObject<{} | null | undefined>): void; }\'.", "2088074939"],
[31, 6, 7, "Type \'null\' is not assignable to type \'{ (path: string, state?: {} | null | undefined): void; (location: LocationDescriptorObject<{} | null | undefined>): void; }\'.", "1364993353"],
Expand Down Expand Up @@ -329,8 +323,8 @@ exports[`strict null compilation`] = {
"js/pages/TableDetailPage/SourceLink/index.spec.tsx:4194369848": [
[42, 21, 100, "Object is possibly \'null\'.", "1316242242"]
],
"js/pages/TableDetailPage/TableDashboardResourceList/index.tsx:3147978263": [
[64, 2, 15, "No overload matches this call.\\n The last overload gave the following error.\\n Argument of type \'(state: GlobalState) => { dashboards: DashboardResource[]; isLoading: boolean; errorText: string | undefined; }\' is not assignable to parameter of type \'MapStateToPropsParam<StateFromProps, OwnProps, {}>\'.\\n Type \'(state: GlobalState) => { dashboards: DashboardResource[]; isLoading: boolean; errorText: string | undefined; }\' is not assignable to type \'MapStateToPropsFactory<StateFromProps, OwnProps, {}>\'.\\n Type \'{ dashboards: DashboardResource[]; isLoading: boolean; errorText: string | undefined; }\' is not assignable to type \'MapStateToProps<StateFromProps, OwnProps, {}>\'.\\n Type \'{ dashboards: DashboardResource[]; isLoading: boolean; errorText: string | undefined; }\' provides no match for the signature \'(state: {}, ownProps: OwnProps): StateFromProps\'.", "1389821531"]
"js/pages/TableDetailPage/TableDashboardResourceList/index.tsx:3276822301": [
[65, 2, 15, "No overload matches this call.\\n The last overload gave the following error.\\n Argument of type \'(state: GlobalState) => { dashboards: DashboardResource[]; isLoading: boolean; errorText: string | undefined; }\' is not assignable to parameter of type \'MapStateToPropsParam<StateFromProps, OwnProps, {}>\'.\\n Type \'(state: GlobalState) => { dashboards: DashboardResource[]; isLoading: boolean; errorText: string | undefined; }\' is not assignable to type \'MapStateToPropsFactory<StateFromProps, OwnProps, {}>\'.\\n Type \'{ dashboards: DashboardResource[]; isLoading: boolean; errorText: string | undefined; }\' is not assignable to type \'MapStateToProps<StateFromProps, OwnProps, {}>\'.\\n Type \'{ dashboards: DashboardResource[]; isLoading: boolean; errorText: string | undefined; }\' provides no match for the signature \'(state: {}, ownProps: OwnProps): StateFromProps\'.", "1389821531"]
],
"js/pages/TableDetailPage/TableOwnerEditor/index.spec.tsx:3400494524": [
[28, 10, 6, "Type \'{ [x: string]: { manager_id: null; manager_fullname: null; manager_email: null; profile_url: string; role_name: null; display_name: null; github_username: null; team_name: null; last_name: null; full_name: null; ... 6 more ...; user_id: string; }; }\' is not assignable to type \'OwnerDict\'.\\n Index signatures are incompatible.\\n Type \'{ manager_id: null; manager_fullname: null; manager_email: null; profile_url: string; role_name: null; display_name: null; github_username: null; team_name: null; last_name: null; full_name: null; ... 6 more ...; user_id: string; }\' is not assignable to type \'User\'.", "1719502071"]
Expand All @@ -341,16 +335,13 @@ exports[`strict null compilation`] = {
"js/pages/TableDetailPage/WatermarkLabel/index.tsx:1354016727": [
[80, 34, 3, "Argument of type \'string | null\' is not assignable to parameter of type \'string\'.\\n Type \'null\' is not assignable to type \'string\'.", "193412913"]
],
"js/pages/TableDetailPage/index.spec.tsx:3148704474": [
[32, 4, 8, "Argument of type \'Partial<Location<{} | null | undefined>> | undefined\' is not assignable to parameter of type \'Partial<Location<{} | null | undefined>>\'.\\n Type \'undefined\' is not assignable to type \'Partial<Location<{} | null | undefined>>\'.", "2700611480"]
"js/pages/TableDetailPage/index.spec.tsx:120018363": [
[33, 4, 8, "Argument of type \'Partial<Location<{} | null | undefined>> | undefined\' is not assignable to parameter of type \'Partial<Location<{} | null | undefined>>\'.\\n Type \'undefined\' is not assignable to type \'Partial<Location<{} | null | undefined>>\'.", "2700611480"]
],
"js/pages/TableDetailPage/index.tsx:3027031293": [
[160, 10, 13, "Type \'null\' is not assignable to type \'((newValue: string, onSuccess?: (() => any) | undefined, onFailure?: (() => any) | undefined) => void) | undefined\'.", "67794331"],
[199, 11, 26, "Type \'{ itemsPerPage: number; source: string; }\' is missing the following properties from type \'Readonly<Pick<TableDashboardResourceListProps, \\"source\\" | \\"isLoading\\" | \\"dashboards\\" | \\"itemsPerPage\\" | \\"errorText\\"> & OwnProps>\': isLoading, dashboards, errorText", "2224258167"],
[279, 16, 7, "Type \'string | null\' is not assignable to type \'string | undefined\'.\\n Type \'null\' is not assignable to type \'string | undefined\'.", "3817619378"],
[327, 20, 35, "Argument of type \'ProgrammaticDescription[] | undefined\' is not assignable to parameter of type \'ProgrammaticDescription[]\'.\\n Type \'undefined\' is not assignable to type \'ProgrammaticDescription[]\'.", "4249007202"],
[341, 20, 36, "Argument of type \'ProgrammaticDescription[] | undefined\' is not assignable to parameter of type \'ProgrammaticDescription[]\'.\\n Type \'undefined\' is not assignable to type \'ProgrammaticDescription[]\'.", "2770872537"],
[346, 16, 36, "Argument of type \'ProgrammaticDescription[] | undefined\' is not assignable to parameter of type \'ProgrammaticDescription[]\'.\\n Type \'undefined\' is not assignable to type \'ProgrammaticDescription[]\'.", "2776557981"]
"js/pages/TableDetailPage/index.tsx:987689418": [
[181, 10, 13, "Type \'null\' is not assignable to type \'((newValue: string, onSuccess?: (() => any) | undefined, onFailure?: (() => any) | undefined) => void) | undefined\'.", "67794331"],
[236, 11, 26, "Type \'{ itemsPerPage: number; source: string; }\' is missing the following properties from type \'Readonly<Pick<TableDashboardResourceListProps, \\"source\\" | \\"isLoading\\" | \\"dashboards\\" | \\"itemsPerPage\\" | \\"errorText\\"> & OwnProps>\': isLoading, dashboards, errorText", "2224258167"],
[325, 16, 7, "Type \'string | null\' is not assignable to type \'string | undefined\'.\\n Type \'null\' is not assignable to type \'string | undefined\'.", "3817619378"]
],
"js/utils/navigationUtils.ts:1127210474": [
[19, 50, 21, "Type \'undefined\' cannot be used as an index type.", "602535635"]
Expand Down
115 changes: 114 additions & 1 deletion amundsen_application/static/js/components/ColumnList/index.spec.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ import { Provider } from 'react-redux';
import configureStore from 'redux-mock-store';
import { mocked } from 'ts-jest/utils';

import { notificationsEnabled } from 'config/config-utils';
import { SortDirection } from 'interfaces';
import {
notificationsEnabled,
getTableSortCriterias,
} from 'config/config-utils';

import globalState from 'fixtures/globalState';
import ColumnList, { ColumnListProps } from '.';
Expand All @@ -19,6 +23,7 @@ import TestDataBuilder from './testDataBuilder';
jest.mock('config/config-utils');

const mockedNotificationsEnabled = mocked(notificationsEnabled, true);
const mockedGetTableSortCriterias = mocked(getTableSortCriterias, true);
const dataBuilder = new TestDataBuilder();
const middlewares = [];
const mockStore = configureStore(middlewares);
Expand Down Expand Up @@ -46,6 +51,18 @@ const setup = (propOverrides?: Partial<ColumnListProps>) => {
};

describe('ColumnList', () => {
mockedGetTableSortCriterias.mockReturnValue({
sort_order: {
name: 'Table Default',
key: 'sort_order',
direction: SortDirection.ascending,
},
usage: {
name: 'Usage Count',
key: 'usage',
direction: SortDirection.descending,
},
});
mockedNotificationsEnabled.mockReturnValue(true);

describe('render', () => {
Expand Down Expand Up @@ -96,6 +113,69 @@ describe('ColumnList', () => {

expect(actual).toEqual(expected);
});

describe('when usage sorting is passed', () => {
it('should sort the data by that value', () => {
const { wrapper } = setup({
columns,
sortBy: {
name: 'Usage',
key: 'usage',
direction: SortDirection.descending,
},
});
const expected = 'simple_column_name_timestamp';
const actual = wrapper
.find('.table-detail-table .ams-table-row')
.at(0)
.find('.column-name')
.text();

expect(actual).toEqual(expected);
});
});

describe('when default sorting is passed', () => {
it('should sort the data by that value', () => {
const { wrapper } = setup({
columns,
sortBy: {
name: 'Default',
key: 'sort_order',
direction: SortDirection.ascending,
},
});
const expected = 'simple_column_name_string';
const actual = wrapper
.find('.table-detail-table .ams-table-row')
.at(0)
.find('.column-name')
.text();

expect(actual).toEqual(expected);
});
});

describe('when name sorting is passed', () => {
it('should sort the data by name', () => {
const { wrapper } = setup({
columns,
sortBy: {
name: 'Name',
key: 'name',
direction: SortDirection.descending,
},
});
const expected = 'simple_column_name_bigint';
const actual = wrapper
.find('.table-detail-table .ams-table-row')
.at(0)
.find('.column-name')
.text();

expect(actual).toEqual(expected);
});
});
});

describe('when complex type columns are passed', () => {
Expand Down Expand Up @@ -152,6 +232,39 @@ describe('ColumnList', () => {
});
});

describe('when columns with serveral stats including usage are passed', () => {
const { columns } = dataBuilder.withSeveralStats().build();

it('should render the usage column', () => {
const { wrapper } = setup({ columns });
const expected = columns.length;
const actual = wrapper.find('.table-detail-table .usage-value').length;

expect(actual).toEqual(expected);
});

describe('when usage sorting is passed', () => {
it('should sort the data by that value', () => {
const { wrapper } = setup({
columns,
sortBy: {
name: 'Usage',
key: 'usage',
direction: SortDirection.ascending,
},
});
const expected = 'complex_column_name_2';
const actual = wrapper
.find('.table-detail-table .ams-table-row')
.at(0)
.find('.column-name')
.text();

expect(actual).toEqual(expected);
});
});
});

describe('when notifications are not enabled', () => {
const { columns } = dataBuilder.build();

Expand Down
87 changes: 78 additions & 9 deletions amundsen_application/static/js/components/ColumnList/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,22 @@ import { OpenRequestAction } from 'ducks/notification/types';
import EditableSection from 'components/common/EditableSection';
import Table, {
TableColumn as ReusableTableColumn,
TextAlignmentValues,
} from 'components/common/Table';

import { logAction } from 'ducks/utilMethods';
import { notificationsEnabled, getMaxLength } from 'config/config-utils';
import { TableColumn, RequestMetadataType } from 'interfaces';
import {
notificationsEnabled,
getMaxLength,
getTableSortCriterias,
} from 'config/config-utils';

import {
TableColumn,
RequestMetadataType,
SortCriteria,
SortDirection,
} from 'interfaces';

import ColumnType from './ColumnType';
import ColumnDescEditableText from './ColumnDescEditableText';
Expand All @@ -38,6 +49,7 @@ export interface ColumnListProps {
database: string;
editText?: string;
editUrl?: string;
sortBy?: SortCriteria;
}

type ContentType = {
Expand All @@ -61,12 +73,14 @@ type StatType = {
type FormattedDataType = {
content: ContentType;
type: DatatypeType;
usage: string | null;
usage: number | null;
stats: StatType | null;
action: string;
editText?: string;
editUrl?: string;
index: number;
name: string;
sort_order: string;
isEditable: boolean;
};

Expand All @@ -75,7 +89,52 @@ type ExpandedRowProps = {
index: number;
};

// TODO: Move this into the configuration once we have more info about the rest of stats
const USAGE_STAT_TYPE = 'column_usage';
const SHOW_STATS_THRESHOLD = 1;
const DEFAULT_SORTING: SortCriteria = {
name: 'Table Default',
key: 'sort_order',
direction: SortDirection.ascending,
};

const getSortingFunction = (
formattedData: FormattedDataType[],
sortBy: SortCriteria
) => {
const numberSortingFunction = (a, b) => {
return b[sortBy.key] - a[sortBy.key];
};

const stringSortingFunction = (a, b) => {
if (a[sortBy.key] && b[sortBy.key]) {
return a[sortBy.key].localeCompare(b[sortBy.key]);
}
return null;
};

if (!formattedData.length) {
return numberSortingFunction;
}

return Number.isInteger(formattedData[0][sortBy.key])
? numberSortingFunction
: stringSortingFunction;
};

const getUsageStat = (item) => {
const hasItemStats = !!item.stats.length;

if (hasItemStats) {
const usageStat = item.stats.find((s) => {
return s.stat_type === USAGE_STAT_TYPE;
});

return usageStat ? +usageStat.stat_val : null;
}

return null;
};

const handleRowExpand = (rowValues) => {
logAction({
Expand Down Expand Up @@ -139,6 +198,7 @@ const ColumnList: React.FC<ColumnListProps> = ({
editText,
editUrl,
openRequestDescriptionDialog,
sortBy = DEFAULT_SORTING,
}: ColumnListProps) => {
const formattedData: FormattedDataType[] = columns.map((item, index) => {
const hasItemStats = !!item.stats.length;
Expand All @@ -153,17 +213,26 @@ const ColumnList: React.FC<ColumnListProps> = ({
name: item.name,
database,
},
usage: hasItemStats ? item.stats[0].stat_val : '',
sort_order: item.sort_order,
usage: getUsageStat(item),
stats: hasItemStats ? item.stats[0] : null,
action: item.name,
name: item.name,
isEditable: item.is_editable,
editText,
editUrl,
index,
};
});
const statsCount = formattedData.filter((item) => !!item.stats).length;
const hasStats = statsCount >= SHOW_STATS_THRESHOLD;
const hasUsageStat =
getTableSortCriterias().usage && statsCount >= SHOW_STATS_THRESHOLD;
let formattedAndOrderedData = formattedData.sort(
getSortingFunction(formattedData, sortBy)
);
if (sortBy.direction === SortDirection.ascending) {
formattedAndOrderedData = formattedAndOrderedData.reverse();
}

let formattedColumns: ReusableTableColumn[] = [
{
Expand Down Expand Up @@ -191,13 +260,13 @@ const ColumnList: React.FC<ColumnListProps> = ({
},
];

if (hasStats) {
if (hasUsageStat) {
formattedColumns = [
...formattedColumns,
{
title: 'Usage',
field: 'usage',
horAlign: 'right',
horAlign: TextAlignmentValues.right,
component: (usage) => (
<p className="resource-type usage-value">{usage}</p>
),
Expand All @@ -212,7 +281,7 @@ const ColumnList: React.FC<ColumnListProps> = ({
title: '',
field: 'action',
width: 80,
horAlign: 'right',
horAlign: TextAlignmentValues.right,
component: (name, index) => (
<div className="actions">
<Dropdown
Expand Down Expand Up @@ -246,7 +315,7 @@ const ColumnList: React.FC<ColumnListProps> = ({
return (
<Table
columns={formattedColumns}
data={formattedData}
data={formattedAndOrderedData}
options={{
rowHeight: 72,
emptyMessage: EMPTY_MESSAGE,
Expand Down
Loading

0 comments on commit 36aebd2

Please sign in to comment.