From 36aebd285718fac17f1d59c12b136d1405b62dac Mon Sep 17 00:00:00 2001 From: Marcos Iglesias <190833+Golodhros@users.noreply.github.com> Date: Tue, 29 Sep 2020 11:55:10 -0700 Subject: [PATCH] feat: Adds sortable table detail page (#691) Signed-off-by: Marcos Iglesias Valle --- amundsen_application/static/.betterer.results | 33 ++-- .../js/components/ColumnList/index.spec.tsx | 115 ++++++++++- .../static/js/components/ColumnList/index.tsx | 87 ++++++++- .../js/components/ColumnList/styles.scss | 3 +- .../components/ColumnList/testDataBuilder.ts | 62 ++++++ .../js/components/common/Table/index.tsx | 18 +- .../static/js/config/config-default.ts | 14 +- .../static/js/config/config-types.ts | 10 +- .../static/js/config/config-utils.ts | 13 ++ .../static/js/config/index.spec.ts | 9 + .../static/js/fixtures/metadata/table.ts | 11 +- .../static/js/interfaces/Resources.ts | 10 + .../static/js/interfaces/TableMetadata.ts | 1 + .../ListSortingDropdown/index.spec.tsx | 181 ++++++++++++++++++ .../ListSortingDropdown/index.tsx | 85 ++++++++ .../ListSortingDropdown/styles.scss | 30 +++ .../TableDashboardResourceList/index.tsx | 3 +- .../js/pages/TableDetailPage/constants.ts | 2 + .../js/pages/TableDetailPage/index.spec.tsx | 1 + .../static/js/pages/TableDetailPage/index.tsx | 64 ++++++- .../js/pages/TableDetailPage/styles.scss | 12 +- 21 files changed, 711 insertions(+), 53 deletions(-) create mode 100644 amundsen_application/static/js/pages/TableDetailPage/ListSortingDropdown/index.spec.tsx create mode 100644 amundsen_application/static/js/pages/TableDetailPage/ListSortingDropdown/index.tsx create mode 100644 amundsen_application/static/js/pages/TableDetailPage/ListSortingDropdown/styles.scss diff --git a/amundsen_application/static/.betterer.results b/amundsen_application/static/.betterer.results index 9be667996..b8b70735c 100644 --- a/amundsen_application/static/.betterer.results +++ b/amundsen_application/static/.betterer.results @@ -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\'.\\n Type \'null\' is not assignable to type \'ActionCreator\'.", "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\' is not assignable to parameter of type \'DispatchFromProps\'.\\n Type \'(dispatch: any, ownProps: any) => ActionCreator\' 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"], @@ -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"] ], @@ -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"], @@ -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\'.\\n Type \'(state: GlobalState) => { dashboards: DashboardResource[]; isLoading: boolean; errorText: string | undefined; }\' is not assignable to type \'MapStateToPropsFactory\'.\\n Type \'{ dashboards: DashboardResource[]; isLoading: boolean; errorText: string | undefined; }\' is not assignable to type \'MapStateToProps\'.\\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\'.\\n Type \'(state: GlobalState) => { dashboards: DashboardResource[]; isLoading: boolean; errorText: string | undefined; }\' is not assignable to type \'MapStateToPropsFactory\'.\\n Type \'{ dashboards: DashboardResource[]; isLoading: boolean; errorText: string | undefined; }\' is not assignable to type \'MapStateToProps\'.\\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"] @@ -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> | undefined\' is not assignable to parameter of type \'Partial>\'.\\n Type \'undefined\' is not assignable to type \'Partial>\'.", "2700611480"] + "js/pages/TableDetailPage/index.spec.tsx:120018363": [ + [33, 4, 8, "Argument of type \'Partial> | undefined\' is not assignable to parameter of type \'Partial>\'.\\n Type \'undefined\' is not assignable to type \'Partial>\'.", "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 & 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 & 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"] diff --git a/amundsen_application/static/js/components/ColumnList/index.spec.tsx b/amundsen_application/static/js/components/ColumnList/index.spec.tsx index cebd852b2..f53a73765 100644 --- a/amundsen_application/static/js/components/ColumnList/index.spec.tsx +++ b/amundsen_application/static/js/components/ColumnList/index.spec.tsx @@ -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 '.'; @@ -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); @@ -46,6 +51,18 @@ const setup = (propOverrides?: Partial) => { }; 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', () => { @@ -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', () => { @@ -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(); diff --git a/amundsen_application/static/js/components/ColumnList/index.tsx b/amundsen_application/static/js/components/ColumnList/index.tsx index c173fe60e..9c63f01b8 100644 --- a/amundsen_application/static/js/components/ColumnList/index.tsx +++ b/amundsen_application/static/js/components/ColumnList/index.tsx @@ -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'; @@ -38,6 +49,7 @@ export interface ColumnListProps { database: string; editText?: string; editUrl?: string; + sortBy?: SortCriteria; } type ContentType = { @@ -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; }; @@ -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({ @@ -139,6 +198,7 @@ const ColumnList: React.FC = ({ editText, editUrl, openRequestDescriptionDialog, + sortBy = DEFAULT_SORTING, }: ColumnListProps) => { const formattedData: FormattedDataType[] = columns.map((item, index) => { const hasItemStats = !!item.stats.length; @@ -153,9 +213,11 @@ const ColumnList: React.FC = ({ 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, @@ -163,7 +225,14 @@ const ColumnList: React.FC = ({ }; }); 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[] = [ { @@ -191,13 +260,13 @@ const ColumnList: React.FC = ({ }, ]; - if (hasStats) { + if (hasUsageStat) { formattedColumns = [ ...formattedColumns, { title: 'Usage', field: 'usage', - horAlign: 'right', + horAlign: TextAlignmentValues.right, component: (usage) => (

{usage}

), @@ -212,7 +281,7 @@ const ColumnList: React.FC = ({ title: '', field: 'action', width: 80, - horAlign: 'right', + horAlign: TextAlignmentValues.right, component: (name, index) => (
= ({ return ( { + const attr = { + columns: [ + { + col_type: + 'struct', + description: null, + is_editable: true, + name: 'complex_column_name_2', + sort_order: 1, + stats: [ + { + end_epoch: 1600473600, + start_epoch: 1597881600, + stat_type: 'column_usage', + stat_val: '111', + }, + ], + }, + { + col_type: 'struct', + description: null, + is_editable: true, + name: 'complex_column_name_3', + sort_order: 2, + stats: [ + { + end_epoch: 1600473600, + start_epoch: 1597881600, + stat_type: 'test_stat', + stat_val: '000', + }, + { + end_epoch: 1600473600, + start_epoch: 1597881600, + stat_type: 'column_usage', + stat_val: '222', + }, + ], + }, + { + col_type: + 'struct>', + description: null, + is_editable: true, + name: 'complex_column_name_4', + sort_order: 3, + stats: [ + { + end_epoch: 1600473600, + start_epoch: 1597881600, + stat_type: 'column_usage', + stat_val: '333', + }, + ], + }, + ], + }; + + return new this.Klass(attr); + }; + this.withEmptyColumns = () => { const attr = { columns: [] }; diff --git a/amundsen_application/static/js/components/common/Table/index.tsx b/amundsen_application/static/js/components/common/Table/index.tsx index fadb9bb9c..3cb7c93a6 100644 --- a/amundsen_application/static/js/components/common/Table/index.tsx +++ b/amundsen_application/static/js/components/common/Table/index.tsx @@ -8,8 +8,14 @@ import { UpIcon, DownIcon } from '../SVGIcons'; import './styles.scss'; -type TextAlignmentValues = 'left' | 'right' | 'center'; +// export type SortDirection = 'asc' | 'desc'; +// export type SortCriteria = { key: string; direction: SortDirection }; +export enum TextAlignmentValues { + left = 'left', + right = 'right', + center = 'center', +} export interface TableColumn { title: string; field: string; @@ -41,7 +47,7 @@ const EXPAND_ROW_TEXT = 'Expand Row'; const DEFAULT_LOADING_ITEMS = 3; const DEFAULT_ROW_HEIGHT = 30; const EXPANDING_CELL_WIDTH = '70px'; -const DEFAULT_TEXT_ALIGNMENT = 'left'; +const DEFAULT_TEXT_ALIGNMENT = TextAlignmentValues.left; const DEFAULT_CELL_WIDTH = 'auto'; const ALIGNEMENT_TO_CLASS_MAP = { left: 'is-left-aligned', @@ -201,9 +207,9 @@ const Table: React.FC = ({ ) : null} {Object.entries(item) .filter(([key]) => fields.includes(key)) - .map(([key, value], index) => { + .map(([key, value], rowIndex) => { const columnInfo = columns.find(({ field }) => field === key); - const horAlign = columnInfo + const horAlign: TextAlignmentValues = columnInfo ? columnInfo.horAlign || DEFAULT_TEXT_ALIGNMENT : DEFAULT_TEXT_ALIGNMENT; const width = @@ -216,7 +222,7 @@ const Table: React.FC = ({ // TODO: Improve the typing of this let cellContent: React.ReactNode | typeof value = value; if (columnInfo && columnInfo.component) { - cellContent = columnInfo.component(value, index); + cellContent = columnInfo.component(value, rowIndex); } return ( @@ -224,7 +230,7 @@ const Table: React.FC = ({ className={`ams-table-cell ${getCellAlignmentClass( horAlign )}`} - key={`index:${index}`} + key={`index:${rowIndex}`} style={cellStyle} > {cellContent} diff --git a/amundsen_application/static/js/config/config-default.ts b/amundsen_application/static/js/config/config-default.ts index 65062d1d7..21deae1e9 100644 --- a/amundsen_application/static/js/config/config-default.ts +++ b/amundsen_application/static/js/config/config-default.ts @@ -1,6 +1,6 @@ import { AppConfig } from './config-types'; -import { FilterType, ResourceType } from '../interfaces'; +import { FilterType, ResourceType, SortDirection } from '../interfaces'; const configDefault: AppConfig = { badges: {}, @@ -162,6 +162,18 @@ const configDefault: AppConfig = { type: FilterType.INPUT_SELECT, }, ], + sortCriterias: { + sort_order: { + name: 'Table Default', + key: 'sort_order', + direction: SortDirection.ascending, + }, + name: { + name: 'Alphabetical', + key: 'name', + direction: SortDirection.descending, + }, + }, supportedDescriptionSources: { github: { displayName: 'Github', diff --git a/amundsen_application/static/js/config/config-types.ts b/amundsen_application/static/js/config/config-types.ts index ff68b2852..f4e31dfd2 100644 --- a/amundsen_application/static/js/config/config-types.ts +++ b/amundsen_application/static/js/config/config-types.ts @@ -1,4 +1,4 @@ -import { FilterType, ResourceType } from '../interfaces'; +import { FilterType, ResourceType, SortCriteria } from '../interfaces'; /** * AppConfig and AppConfigCustom should share the same definition, except each field in AppConfigCustom @@ -130,6 +130,13 @@ type DescriptionSourceConfig = { [id: string]: { displayName: string; iconPath: string }; }; +/** + * Shows criterias to sort tables + */ +type SortCriteriaConfig = { + [key: string]: SortCriteria; +}; + /** * Base interface for all possible ResourceConfig objects * @@ -144,6 +151,7 @@ interface BaseResourceConfig { interface TableResourceConfig extends BaseResourceConfig { supportedDescriptionSources?: DescriptionSourceConfig; + sortCriterias?: SortCriteriaConfig; } export enum BadgeStyle { diff --git a/amundsen_application/static/js/config/config-utils.ts b/amundsen_application/static/js/config/config-utils.ts index 6af7bad48..32ab8d14d 100644 --- a/amundsen_application/static/js/config/config-utils.ts +++ b/amundsen_application/static/js/config/config-utils.ts @@ -146,6 +146,19 @@ export function getCuratedTags(): string[] { return AppConfig.browse.curatedTags; } +/** + * Returns a list of table sort options + */ +export function getTableSortCriterias() { + const config = AppConfig.resourceConfig[ResourceType.table]; + + if (config && config.sortCriterias) { + return config.sortCriterias; + } + + return {}; +} + /** * Checks if nav links are active */ diff --git a/amundsen_application/static/js/config/index.spec.ts b/amundsen_application/static/js/config/index.spec.ts index e0c5a3ac7..53946a3d4 100644 --- a/amundsen_application/static/js/config/index.spec.ts +++ b/amundsen_application/static/js/config/index.spec.ts @@ -78,6 +78,15 @@ describe('getFilterConfigByResource', () => { }); }); +describe('getTableSortCriterias', () => { + it('returns the sorting criterias for tables', () => { + const expectedValue = + AppConfig.resourceConfig[ResourceType.table].sortCriterias; + + expect(ConfigUtils.getTableSortCriterias()).toBe(expectedValue); + }); +}); + describe('getBadgeConfig', () => { AppConfig.badges = { test_1: { diff --git a/amundsen_application/static/js/fixtures/metadata/table.ts b/amundsen_application/static/js/fixtures/metadata/table.ts index 355cbf9f0..827008418 100644 --- a/amundsen_application/static/js/fixtures/metadata/table.ts +++ b/amundsen_application/static/js/fixtures/metadata/table.ts @@ -18,6 +18,7 @@ export const tableMetadata: TableMetadata = { col_type: 'bigint', description: 'Test Value', is_editable: true, + sort_order: '0', name: 'ride_id', stats: [ { @@ -45,13 +46,15 @@ export const tableMetadata: TableMetadata = { description: 'ds will be the date part of requested_at ds will be the date part of requested_at ds will be the date part of requested_at ds will be the date part of requested_at ds will be the date part of requested_at ds will be the date part of requested_at ds w', is_editable: true, + sort_order: '1', name: 'ds', stats: [], }, { col_type: 'string', - description: null, + description: 'Route_id Description', is_editable: true, + sort_order: '2', name: 'route_id', stats: [ { @@ -169,7 +172,7 @@ export const tableMetadata: TableMetadata = { export const relatedDashboards: DashboardResource[] = [ { group_name: 'Test Group 1', - description: null, + description: 'Test Group 1 Description', cluster: 'gold', group_url: 'https://app.mode.com/testCompany/spaces/1234', uri: 'mode_dashboard://gold.1234/23445asb', @@ -181,7 +184,7 @@ export const relatedDashboards: DashboardResource[] = [ }, { group_name: 'Test Group 2', - description: null, + description: 'Test Group 2 Description', cluster: 'gold', group_url: 'https://app.mode.com/testCompany/spaces/345asd', uri: 'mode_dashboard://gold.345asd/asdfas001', @@ -193,7 +196,7 @@ export const relatedDashboards: DashboardResource[] = [ }, { group_name: 'Test Group 3', - description: null, + description: 'Test Group 3 Description', cluster: 'gold', group_url: 'https://app.mode.com/testCompany/spaces/casdg80', uri: 'mode_dashboard://gold.casdg80/123566', diff --git a/amundsen_application/static/js/interfaces/Resources.ts b/amundsen_application/static/js/interfaces/Resources.ts index 39a20fc35..9ce93ba40 100644 --- a/amundsen_application/static/js/interfaces/Resources.ts +++ b/amundsen_application/static/js/interfaces/Resources.ts @@ -42,6 +42,16 @@ export interface TableResource extends Resource { badges?: any[]; // TODO replace with new badges later @allisonsuarez } +export enum SortDirection { + ascending = 'asc', + descending = 'desc', +} +export interface SortCriteria { + name: string; + key: string; + direction: SortDirection; +} + export interface UserResource extends Resource, PeopleUser { type: ResourceType.user; } diff --git a/amundsen_application/static/js/interfaces/TableMetadata.ts b/amundsen_application/static/js/interfaces/TableMetadata.ts index 72b437e2e..4cef90c91 100644 --- a/amundsen_application/static/js/interfaces/TableMetadata.ts +++ b/amundsen_application/static/js/interfaces/TableMetadata.ts @@ -61,6 +61,7 @@ export interface TableColumn { description: string; is_editable: boolean; col_type: string; + sort_order: string; stats: TableColumnStats[]; } diff --git a/amundsen_application/static/js/pages/TableDetailPage/ListSortingDropdown/index.spec.tsx b/amundsen_application/static/js/pages/TableDetailPage/ListSortingDropdown/index.spec.tsx new file mode 100644 index 000000000..94e0d1341 --- /dev/null +++ b/amundsen_application/static/js/pages/TableDetailPage/ListSortingDropdown/index.spec.tsx @@ -0,0 +1,181 @@ +// Copyright Contributors to the Amundsen project. +// SPDX-License-Identifier: Apache-2.0 + +import * as React from 'react'; +import { Dropdown } from 'react-bootstrap'; +import { mount } from 'enzyme'; + +import { SortDirection } from 'interfaces'; + +import ListSortingDropdown, { ListSortingDropdownProps } from '.'; + +const DEFAULT_SORTING = { + sort_order: { + name: 'Table Default', + key: 'sort_order', + direction: SortDirection.ascending, + }, +}; +const USAGE_SORTING = { + usage: { + name: 'Usage Count', + key: 'usage', + direction: SortDirection.descending, + }, +}; + +const setup = (propOverrides?: Partial) => { + const props = { + options: {}, + ...propOverrides, + }; + const wrapper = mount( + + ); + + return { props, wrapper }; +}; + +describe('ListSortingDropdown', () => { + describe('render', () => { + it('renders without issues', () => { + expect(() => { + setup(); + }).not.toThrow(); + }); + + describe('when no options are passed', () => { + it('does not render the component', () => { + const { wrapper } = setup(); + const expected = 0; + const actual = wrapper.find('.list-sorting-dropdown').length; + + expect(actual).toEqual(expected); + }); + }); + + describe('when one option is passed', () => { + it('renders a DropDown component', () => { + const { wrapper } = setup({ options: DEFAULT_SORTING }); + const expected = 1; + const actual = wrapper.find(Dropdown).length; + + expect(actual).toEqual(expected); + }); + + it('renders one item', () => { + const { wrapper } = setup({ options: DEFAULT_SORTING }); + const expected = 1; + const actual = wrapper.find('.list-sorting-dropdown .radio-label') + .length; + + expect(actual).toEqual(expected); + }); + + it('is selected by default', () => { + const { wrapper } = setup({ options: DEFAULT_SORTING }); + const expected = true; + const actual = wrapper + .find('.list-sorting-dropdown .radio-label input') + .prop('checked'); + + expect(actual).toEqual(expected); + }); + }); + + describe('when two options are passed', () => { + it('renders a DropDown component', () => { + const { wrapper } = setup({ + options: { ...DEFAULT_SORTING, ...USAGE_SORTING }, + }); + const expected = 1; + const actual = wrapper.find(Dropdown).length; + + expect(actual).toEqual(expected); + }); + + it('renders two items', () => { + const { wrapper } = setup({ + options: { ...DEFAULT_SORTING, ...USAGE_SORTING }, + }); + const expected = 2; + const actual = wrapper.find('.list-sorting-dropdown .radio-label') + .length; + + expect(actual).toEqual(expected); + }); + + it('selects the first one by default', () => { + const { wrapper } = setup({ + options: { ...DEFAULT_SORTING, ...USAGE_SORTING }, + }); + const expected = true; + const actual = wrapper + .find('.list-sorting-dropdown .radio-label input') + .at(0) + .prop('checked'); + + expect(actual).toEqual(expected); + }); + }); + }); + + describe('lifetime', () => { + describe('when selecting an option', () => { + it('should make it the selected', () => { + const { wrapper } = setup({ + options: { ...DEFAULT_SORTING, ...USAGE_SORTING }, + }); + const expected = true; + + wrapper + .find('.list-sorting-dropdown .radio-label input') + .at(1) + .simulate('change', { target: { value: 'usage' } }); + + const actual = wrapper + .find('.list-sorting-dropdown .radio-label input') + .at(1) + .prop('checked'); + + expect(actual).toEqual(expected); + }); + + it('should call the onChange handler', () => { + const onChangeSpy = jest.fn(); + const { wrapper } = setup({ + onChange: onChangeSpy, + options: { ...DEFAULT_SORTING, ...USAGE_SORTING }, + }); + const expected = 1; + + wrapper + .find('.list-sorting-dropdown .radio-label input') + .at(1) + .simulate('change', { target: { value: 'usage' } }); + + const actual = onChangeSpy.mock.calls.length; + + expect(actual).toEqual(expected); + }); + + it('should call the onChange handler with the proper value', () => { + const onChangeSpy = jest.fn(); + const { wrapper } = setup({ + onChange: onChangeSpy, + options: { ...DEFAULT_SORTING, ...USAGE_SORTING }, + }); + const expected = ['usage']; + + wrapper + .find('.list-sorting-dropdown .radio-label input') + .at(1) + .simulate('change', { target: { value: 'usage' } }); + + const actual = onChangeSpy.mock.calls[0]; + + expect(actual).toEqual(expected); + }); + }); + }); +}); diff --git a/amundsen_application/static/js/pages/TableDetailPage/ListSortingDropdown/index.tsx b/amundsen_application/static/js/pages/TableDetailPage/ListSortingDropdown/index.tsx new file mode 100644 index 000000000..dc04ecddb --- /dev/null +++ b/amundsen_application/static/js/pages/TableDetailPage/ListSortingDropdown/index.tsx @@ -0,0 +1,85 @@ +// Copyright Contributors to the Amundsen project. +// SPDX-License-Identifier: Apache-2.0 + +import * as React from 'react'; +import { Dropdown } from 'react-bootstrap'; + +import { SortCriteria } from 'interfaces'; + +import { SORT_BY_DROPDOWN_TITLE, SORT_BY_MENU_TITLE_TEXT } from '../constants'; + +import './styles.scss'; + +type Criterias = { [key: string]: SortCriteria }; + +export interface ListSortingDropdownProps { + options: Criterias; + onChange?: (value) => void; +} + +type OptionType = string; + +const TableReportsDropdown: React.FC = ({ + options, + onChange, +}: ListSortingDropdownProps) => { + const criterias = Object.entries(options); + + if (criterias.length < 1) { + return null; + } + + const [selectedOption, setSelectedOption] = React.useState( + criterias[0][1].key + ); + const [isOpen, setOpen] = React.useState(false); + + const handleChange = (e) => { + const { value } = e.target; + + setSelectedOption(value); + setOpen(false); + if (onChange) { + onChange(value); + } + }; + + return ( + { + setOpen(!isOpen); + }} + > + + {SORT_BY_DROPDOWN_TITLE} + + +
+ {SORT_BY_MENU_TITLE_TEXT} +
+ {criterias.map(([_, { key, name }]) => ( +
  • + +
    • + ))} +     +     + ); +}; + +export default TableReportsDropdown; diff --git a/amundsen_application/static/js/pages/TableDetailPage/ListSortingDropdown/styles.scss b/amundsen_application/static/js/pages/TableDetailPage/ListSortingDropdown/styles.scss new file mode 100644 index 000000000..6b5dee1c8 --- /dev/null +++ b/amundsen_application/static/js/pages/TableDetailPage/ListSortingDropdown/styles.scss @@ -0,0 +1,30 @@ +// Copyright Contributors to the Amundsen project. +// SPDX-License-Identifier: Apache-2.0 + +@import 'typography'; + +.list-sorting-dropdown { + .list-sorting-dropdown-button { + border: 0; + } + + .list-sorting-dropdown-menu { + padding: $spacer-2; + } + + .list-sorting-dropdown-menu-title { + @extend %text-caption-w2; + + padding-bottom: $spacer-1; + } + + .list-sorting-dropdown-menu-item { + &:hover { + cursor: pointer; + } + } + + .list-sorting-dropdown-menu-item-text { + @extend %text-body-w3; + } +} diff --git a/amundsen_application/static/js/pages/TableDetailPage/TableDashboardResourceList/index.tsx b/amundsen_application/static/js/pages/TableDetailPage/TableDashboardResourceList/index.tsx index af1a1c5fd..fb3a09fa5 100644 --- a/amundsen_application/static/js/pages/TableDetailPage/TableDashboardResourceList/index.tsx +++ b/amundsen_application/static/js/pages/TableDetailPage/TableDashboardResourceList/index.tsx @@ -22,7 +22,7 @@ interface StateFromProps { errorText: string; } -export type TableDashboardResourceListProps = OwnProps & StateFromProps; +export type TableDashboardResourceListProps = StateFromProps & OwnProps; export class TableDashboardResourceList extends React.Component< TableDashboardResourceListProps @@ -54,6 +54,7 @@ export class TableDashboardResourceList extends React.Component< export const mapStateToProps = (state: GlobalState) => { const relatedDashboards = state.tableMetadata.dashboards; + return { dashboards: relatedDashboards ? relatedDashboards.dashboards : [], isLoading: relatedDashboards ? relatedDashboards.isLoading : true, diff --git a/amundsen_application/static/js/pages/TableDetailPage/constants.ts b/amundsen_application/static/js/pages/TableDetailPage/constants.ts index 2b601cbcf..5a5948bf9 100644 --- a/amundsen_application/static/js/pages/TableDetailPage/constants.ts +++ b/amundsen_application/static/js/pages/TableDetailPage/constants.ts @@ -7,3 +7,5 @@ export const FREQ_USERS_TITLE = 'Frequent Users'; export const LAST_UPDATED_TITLE = 'Last Updated'; export const OWNERS_TITLE = 'Owners'; export const TAG_TITLE = 'Tags'; +export const SORT_BY_DROPDOWN_TITLE = 'Sort by'; +export const SORT_BY_MENU_TITLE_TEXT = 'Sort by'; diff --git a/amundsen_application/static/js/pages/TableDetailPage/index.spec.tsx b/amundsen_application/static/js/pages/TableDetailPage/index.spec.tsx index 5e63d64dc..f274b27c2 100644 --- a/amundsen_application/static/js/pages/TableDetailPage/index.spec.tsx +++ b/amundsen_application/static/js/pages/TableDetailPage/index.spec.tsx @@ -17,6 +17,7 @@ import { TableDetail, TableDetailProps, MatchProps } from '.'; jest.mock('config/config-utils', () => ({ indexDashboardsEnabled: jest.fn(), + getTableSortCriterias: jest.fn(), })); const setup = ( diff --git a/amundsen_application/static/js/pages/TableDetailPage/index.tsx b/amundsen_application/static/js/pages/TableDetailPage/index.tsx index ffa1b3fa0..502fdf5bf 100644 --- a/amundsen_application/static/js/pages/TableDetailPage/index.tsx +++ b/amundsen_application/static/js/pages/TableDetailPage/index.tsx @@ -17,6 +17,7 @@ import { getDescriptionSourceDisplayName, getMaxLength, getSourceIconClass, + getTableSortCriterias, indexDashboardsEnabled, issueTrackingEnabled, notificationsEnabled, @@ -40,6 +41,7 @@ import { ResourceType, TableMetadata, RequestMetadataType, + SortCriteria, } from 'interfaces'; import DataPreviewButton from './DataPreviewButton'; @@ -57,6 +59,7 @@ import WriterLink from './WriterLink'; import TableReportsDropdown from './ResourceReportsDropdown'; import RequestDescriptionText from './RequestDescriptionText'; import RequestMetadataForm from './RequestMetadataForm'; +import ListSortingDropdown from './ListSortingDropdown'; import * as Constants from './constants'; @@ -65,8 +68,12 @@ import './styles.scss'; const SERVER_ERROR_CODE = 500; const DASHBOARDS_PER_PAGE = 10; const TABLE_SOURCE = 'table_page'; +const SORT_CRITERIAS = { + ...getTableSortCriterias(), +}; +const COLUMN_TAB_KEY = 'columns'; -export interface StateFromProps { +export interface PropsFromState { isLoading: boolean; isLoadingDashboards: boolean; numRelatedDashboards: number; @@ -92,7 +99,7 @@ export interface MatchProps { table: string; } -export type TableDetailProps = StateFromProps & +export type TableDetailProps = PropsFromState & DispatchFromProps & RouteComponentProps; @@ -105,13 +112,24 @@ const ErrorMessage = () => { ); }; +export interface StateProps { + sortedBy: SortCriteria; + currentTab: string; +} + export class TableDetail extends React.Component< - TableDetailProps & RouteComponentProps + TableDetailProps & RouteComponentProps, + StateProps > { private key: string; private didComponentMount: boolean = false; + state = { + sortedBy: SORT_CRITERIAS.sort_order, + currentTab: COLUMN_TAB_KEY, + }; + componentDidMount() { const { index, source } = getLoggingParams(this.props.location.search); @@ -125,6 +143,7 @@ export class TableDetail extends React.Component< if (this.key !== newKey) { const { index, source } = getLoggingParams(this.props.location.search); + this.key = newKey; this.props.getTableData(this.key, index, source); } @@ -147,7 +166,9 @@ export class TableDetail extends React.Component< return `${params.database}://${params.cluster}.${params.schema}/${params.table}`; } - renderProgrammaticDesc = (descriptions: ProgrammaticDescription[]) => { + renderProgrammaticDesc = ( + descriptions: ProgrammaticDescription[] | undefined + ) => { if (!descriptions) { return null; } @@ -164,6 +185,20 @@ export class TableDetail extends React.Component< )); }; + handleSortingChange = (sortValue) => { + this.toggleSort(SORT_CRITERIAS[sortValue]); + }; + + toggleSort = (sorting: SortCriteria) => { + const { sortedBy } = this.state; + + if (sorting !== sortedBy) { + this.setState({ + sortedBy: sorting, + }); + } + }; + renderTabs(editText, editUrl) { const tabInfo: TabInfo[] = []; const { @@ -172,6 +207,7 @@ export class TableDetail extends React.Component< tableData, openRequestDescriptionDialog, } = this.props; + const { sortedBy } = this.state; // Default Column content tabInfo.push({ @@ -182,6 +218,7 @@ export class TableDetail extends React.Component< database={tableData.database} editText={editText} editUrl={editUrl} + sortBy={sortedBy} /> ), key: 'columns', @@ -209,11 +246,20 @@ export class TableDetail extends React.Component< }); } - return ; + return ( + { + this.setState({ currentTab: key }); + }} + /> + ); } render() { const { isLoading, statusCode, tableData } = this.props; + const { currentTab } = this.state; let innerContent; // We want to avoid rendering the previous table's metadata before new data is fetched in componentDidMount @@ -348,6 +394,12 @@ export class TableDetail extends React.Component< )}
    + {currentTab === COLUMN_TAB_KEY && ( + + )} {this.renderTabs(editText, editUrl)}
    @@ -386,7 +438,7 @@ export const mapDispatchToProps = (dispatch: any) => { ); }; -export default connect( +export default connect( mapStateToProps, mapDispatchToProps )(TableDetail); diff --git a/amundsen_application/static/js/pages/TableDetailPage/styles.scss b/amundsen_application/static/js/pages/TableDetailPage/styles.scss index cb69223f1..f92a9e5d4 100644 --- a/amundsen_application/static/js/pages/TableDetailPage/styles.scss +++ b/amundsen_application/static/js/pages/TableDetailPage/styles.scss @@ -42,11 +42,21 @@ } .nav.nav-tabs { - margin-top: $spacer-1; + margin-top: $spacer-2; padding: 0 $spacer-2; } .tabs-component .nav.nav-tabs > li { margin: 0 $spacer-1; } + + .right-panel { + position: relative; + } + + .list-sorting-dropdown { + right: $spacer-3; + top: $spacer-2; + position: absolute; + } }