From c6321020813e17fffef33a8b9a03d307233082a4 Mon Sep 17 00:00:00 2001 From: Marcos Iglesias <190833+Golodhros@users.noreply.github.com> Date: Fri, 2 Oct 2020 12:01:30 -0700 Subject: [PATCH] chore: Testing types for table input (#699) Signed-off-by: Marcos Iglesias Valle --- amundsen_application/static/.betterer.results | 3 -- .../static/.storybook/manager.ts | 2 +- .../static/js/components/ColumnList/index.tsx | 12 ++--- .../js/components/common/Table/index.spec.tsx | 23 +++++++++ .../js/components/common/Table/index.tsx | 51 +++++++++++++++---- .../components/common/Table/table.story.tsx | 2 - .../common/Table/testDataBuilder.tsx | 30 +++++++++++ 7 files changed, 100 insertions(+), 23 deletions(-) diff --git a/amundsen_application/static/.betterer.results b/amundsen_application/static/.betterer.results index 23cf8c9bf..f1e53ebf1 100644 --- a/amundsen_application/static/.betterer.results +++ b/amundsen_application/static/.betterer.results @@ -99,9 +99,6 @@ 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: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"], [66, 4, 4, "Type \'undefined\' is not assignable to type \'Tag[]\'.", "2087952548"], diff --git a/amundsen_application/static/.storybook/manager.ts b/amundsen_application/static/.storybook/manager.ts index 8b75d125d..a26d9b09d 100644 --- a/amundsen_application/static/.storybook/manager.ts +++ b/amundsen_application/static/.storybook/manager.ts @@ -6,7 +6,7 @@ addons.setConfig({ isFullscreen: false, showNav: true, showPanel: true, - panelPosition: 'bottom', + panelPosition: 'right', sidebarAnimations: true, enableShortcuts: true, isToolshown: true, diff --git a/amundsen_application/static/js/components/ColumnList/index.tsx b/amundsen_application/static/js/components/ColumnList/index.tsx index 9c63f01b8..2dfb5f80e 100644 --- a/amundsen_application/static/js/components/ColumnList/index.tsx +++ b/amundsen_application/static/js/components/ColumnList/index.tsx @@ -76,8 +76,8 @@ type FormattedDataType = { usage: number | null; stats: StatType | null; action: string; - editText?: string; - editUrl?: string; + editText: string | null; + editUrl: string | null; index: number; name: string; sort_order: string; @@ -168,8 +168,8 @@ const ExpandedRowComponent: React.FC = ( = ({ action: item.name, name: item.name, isEditable: item.is_editable, - editText, - editUrl, + editText: editText || null, + editUrl: editUrl || null, index, }; }); diff --git a/amundsen_application/static/js/components/common/Table/index.spec.tsx b/amundsen_application/static/js/components/common/Table/index.spec.tsx index f26e4daaf..8b33e0618 100644 --- a/amundsen_application/static/js/components/common/Table/index.spec.tsx +++ b/amundsen_application/static/js/components/common/Table/index.spec.tsx @@ -3,6 +3,7 @@ import * as React from 'react'; import { mount } from 'enzyme'; +import { mocked } from 'ts-jest/utils'; import Table, { TableProps } from '.'; @@ -400,6 +401,28 @@ describe('Table', () => { }); }); }); + + describe('when columns specify fields not in the data', () => { + const { columns, data } = dataBuilder.withWrongData().build(); + + beforeEach(() => { + jest.spyOn(console, 'error'); + mocked(console.error).mockImplementation(jest.fn); + }); + + afterEach(() => { + mocked(console.error).mockRestore(); + }); + + it('throws an error', () => { + expect(() => { + setup({ + data, + columns, + }); + }).toThrow(); + }); + }); }); describe('options', () => { diff --git a/amundsen_application/static/js/components/common/Table/index.tsx b/amundsen_application/static/js/components/common/Table/index.tsx index 3cb7c93a6..8410a1ff2 100644 --- a/amundsen_application/static/js/components/common/Table/index.tsx +++ b/amundsen_application/static/js/components/common/Table/index.tsx @@ -24,6 +24,12 @@ export interface TableColumn { width?: number; // sortable?: bool (false) } +type Some = string | number | boolean | symbol | bigint | object; +type ValidData = Record; // Removes the undefined values + +interface RowData { + [key: string]: Some | null; +} export interface TableOptions { tableClassName?: string; @@ -37,13 +43,25 @@ export interface TableOptions { } export interface TableProps { + data: RowData[]; columns: TableColumn[]; - data: any[]; options?: TableOptions; } +type RowStyles = { + height: string; +}; + +type EmptyRowProps = { + colspan: number; + rowStyles: RowStyles; + emptyMessage?: string; +}; + const DEFAULT_EMPTY_MESSAGE = 'No Results'; const EXPAND_ROW_TEXT = 'Expand Row'; +const INVALID_DATA_ERROR_MESSAGE = + 'Invalid data! Your data does not contain the fields specified on the columns property.'; const DEFAULT_LOADING_ITEMS = 3; const DEFAULT_ROW_HEIGHT = 30; const EXPANDING_CELL_WIDTH = '70px'; @@ -55,19 +73,26 @@ const ALIGNEMENT_TO_CLASS_MAP = { center: 'is-center-aligned', }; -type RowStyles = { - height: string; -}; - -type EmptyRowProps = { - colspan: number; - rowStyles: RowStyles; - emptyMessage?: string; -}; - const getCellAlignmentClass = (alignment: TextAlignmentValues) => ALIGNEMENT_TO_CLASS_MAP[alignment]; +const fieldIsDefined = (field, row) => row[field] !== undefined; + +const checkIfValidData = ( + data: unknown[], + fields: string[] +): data is ValidData[] => { + let isValid = true; + + for (let i = 0; i < fields.length; i++) { + if (!data.some(fieldIsDefined.bind(null, fields[i]))) { + isValid = false; + break; + } + } + return isValid; +}; + const EmptyRow: React.FC = ({ colspan, rowStyles, @@ -182,6 +207,10 @@ const Table: React.FC = ({ ); if (data.length) { + if (!checkIfValidData(data, fields)) { + throw new Error(INVALID_DATA_ERROR_MESSAGE); + } + body = data.map((item, index) => { return ( diff --git a/amundsen_application/static/js/components/common/Table/table.story.tsx b/amundsen_application/static/js/components/common/Table/table.story.tsx index e14eeb80c..7a0563fbf 100644 --- a/amundsen_application/static/js/components/common/Table/table.story.tsx +++ b/amundsen_application/static/js/components/common/Table/table.story.tsx @@ -39,8 +39,6 @@ const expandRowComponent = (rowValue, index) => ( ); -// const stories = storiesOf('Components/Table', module); - export const TableStates = () => ( <> diff --git a/amundsen_application/static/js/components/common/Table/testDataBuilder.tsx b/amundsen_application/static/js/components/common/Table/testDataBuilder.tsx index 9a6413ac1..f53dc78fe 100644 --- a/amundsen_application/static/js/components/common/Table/testDataBuilder.tsx +++ b/amundsen_application/static/js/components/common/Table/testDataBuilder.tsx @@ -44,6 +44,36 @@ function TestDataBuilder(config = {}) { ...config, }; + this.withWrongData = () => { + const attr = { + data: [ + { name: 'rowName', type: 'rowType', value: 1 }, + { name: 'rowName2', type: 'rowType2', value: 2 }, + { name: 'rowName3', type: 'rowType3', value: 3 }, + ], + columns: [ + { + title: 'Name', + field: 'name', + }, + { + title: 'Type', + field: 'type', + }, + { + title: 'Value', + field: 'value', + }, + { + title: 'Bad Field', + field: 'badfield', + }, + ], + }; + + return new this.Klass(attr); + }; + this.withUsageRow = () => { const attr = { data: [