From 5aaf7bab095be4e0a40284333bb0e777365d444c Mon Sep 17 00:00:00 2001 From: Eric Date: Wed, 8 Nov 2023 21:09:06 +0000 Subject: [PATCH] Revert "design changes for loading, changed the banner, updated tests (#170) (#172)" This reverts commit 786c7c7a0d42b1e6531d3af8bfe691d60561792f. --- common/types/index.ts | 6 - common/utils/async_query_helpers.ts | 5 +- .../Main/__snapshots__/main.test.tsx.snap | 214 +++++++-------- public/components/Main/main.tsx | 6 +- .../__snapshots__/table_view.test.tsx.snap | 50 ++-- public/components/SQLPage/table_view.test.tsx | 33 ++- public/components/SQLPage/table_view.tsx | 254 ++++++------------ test/mocks/mockData.ts | 6 - 8 files changed, 228 insertions(+), 346 deletions(-) diff --git a/common/types/index.ts b/common/types/index.ts index 17208cc3..5ed212c8 100644 --- a/common/types/index.ts +++ b/common/types/index.ts @@ -87,10 +87,4 @@ export interface TreeItem { type: TreeItemType; isExpanded: boolean; values?: TreeItem[]; - isLoading?: boolean -} - -export interface isLoading { - flag: boolean; - status: string; } diff --git a/common/utils/async_query_helpers.ts b/common/utils/async_query_helpers.ts index 3731859e..689727b3 100644 --- a/common/utils/async_query_helpers.ts +++ b/common/utils/async_query_helpers.ts @@ -47,13 +47,12 @@ export const pollQueryStatus = (id: string, http: CoreStart['http'], callback) = status === 'scheduled' || status === 'waiting' ) { - callback({status: status}) setTimeout(() => pollQueryStatus(id, http, callback), POLL_INTERVAL_MS); } else if (status === 'failed') { - callback({ status: 'FAILED', results: [] }); + callback([]); } else if (status === 'success') { const results = _.get(res.data.resp, 'datarows'); - callback({ status: 'SUCCESS', results: results }); + callback(results); } }) .catch((err) => { diff --git a/public/components/Main/__snapshots__/main.test.tsx.snap b/public/components/Main/__snapshots__/main.test.tsx.snap index 3b7c236c..f34fea4e 100644 --- a/public/components/Main/__snapshots__/main.test.tsx.snap +++ b/public/components/Main/__snapshots__/main.test.tsx.snap @@ -193,7 +193,7 @@ exports[`
spec click clear button 1`] = ` >
spec click clear button 1`] = ` class="euiFlexGroup euiFlexGroup--gutterLarge euiFlexGroup--directionRow euiFlexGroup--responsive" >
-
@@ -750,7 +746,7 @@ exports[`
spec click run button, and response causes an error 1`] = ` >
spec click run button, and response causes an error 1`] = ` class="euiFlexGroup euiFlexGroup--gutterLarge euiFlexGroup--directionRow euiFlexGroup--responsive" >
-
@@ -1307,7 +1299,7 @@ exports[`
spec click run button, and response is not ok 1`] = ` >
spec click run button, and response is not ok 1`] = ` class="euiFlexGroup euiFlexGroup--gutterLarge euiFlexGroup--directionRow euiFlexGroup--responsive" >
-
@@ -1864,7 +1852,7 @@ exports[`
spec click run button, and response is ok 1`] = ` >
spec click run button, response fills null and missing values >
spec click translation button, and response is ok 1`] = ` >
spec click translation button, and response is ok 1`] = ` class="euiFlexGroup euiFlexGroup--gutterLarge euiFlexGroup--directionRow euiFlexGroup--responsive" >
-
@@ -3693,7 +3677,7 @@ exports[`
spec renders the component 1`] = ` >
{ {this.state.language === 'SQL' && ( - - + + diff --git a/public/components/SQLPage/__snapshots__/table_view.test.tsx.snap b/public/components/SQLPage/__snapshots__/table_view.test.tsx.snap index 56a16514..6b8be8c2 100644 --- a/public/components/SQLPage/__snapshots__/table_view.test.tsx.snap +++ b/public/components/SQLPage/__snapshots__/table_view.test.tsx.snap @@ -6,38 +6,34 @@ exports[`Render databases in tree fetches and displays database nodes when datas class="euiFlexGroup euiFlexGroup--gutterLarge euiFlexGroup--directionRow euiFlexGroup--responsive" >
-
diff --git a/public/components/SQLPage/table_view.test.tsx b/public/components/SQLPage/table_view.test.tsx index 976291c9..3f602918 100644 --- a/public/components/SQLPage/table_view.test.tsx +++ b/public/components/SQLPage/table_view.test.tsx @@ -3,8 +3,7 @@ import React from 'react'; import { httpClientMock } from '../../../test/mocks'; import '@testing-library/jest-dom/extend-expect'; -import { render } from '@testing-library/react'; -import { isLoading } from 'common/types'; +import { render, waitFor } from '@testing-library/react'; import { HttpResponse } from '../../../../../src/core/public'; import { mockDatabaseQuery, mockJobId, mockOpenSearchIndicies } from '../../../test/mocks/mockData'; import { TableView } from './table_view'; @@ -31,10 +30,6 @@ describe('Render databases in tree', () => { }); it('fetches and displays database nodes when datasource is s3', async () => { const client = httpClientMock; - const isLoading: isLoading = { - flag: false, - status: 'Not loading', - } client.post = jest.fn(() => { return (Promise.resolve(mockJobId) as unknown) as HttpResponse; }); @@ -42,17 +37,21 @@ describe('Render databases in tree', () => { return (Promise.resolve(mockDatabaseQuery) as unknown) as HttpResponse; }); - const asyncTest = () => { - render( - {}} - refreshTree={false} - /> - ); - }; - await asyncTest(); + const { getByText } = render( + {}} + refreshTree={false} + /> + ); + await waitFor(() => { + expect( + getByText( + 'Loading can take more than 30s. Queries can be made after the data has loaded. Any queries run before the data is loaded will be queued' + ) + ).toBeInTheDocument(); + }); expect(document.body.children[0]).toMatchSnapshot(); }); }); diff --git a/public/components/SQLPage/table_view.tsx b/public/components/SQLPage/table_view.tsx index ab4014ea..c1e0fbf6 100644 --- a/public/components/SQLPage/table_view.tsx +++ b/public/components/SQLPage/table_view.tsx @@ -11,12 +11,11 @@ import { EuiFlexItem, EuiIcon, EuiLoadingSpinner, - EuiSpacer, EuiText, EuiToolTip, EuiTreeView, } from '@elastic/eui'; -import { TreeItem, TreeItemType, isLoading } from 'common/types'; +import { TreeItem, TreeItemType } from 'common/types'; import _ from 'lodash'; import React, { useEffect, useState } from 'react'; import { CoreStart } from '../../../../../src/core/public'; @@ -45,10 +44,7 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } const [tableNames, setTableNames] = useState([]); const [selectedDatabase, setSelectedDatabase] = useState(''); const [selectedTable, setSelectedTable] = useState(null); - const [isLoadingBanner, setIsLoading] = useState({ - flag: false, - status: 'Not loading', - }); + const [isLoading, setIsLoading] = useState(false); const [indexFlyout, setIndexFlyout] = useState(<>); const [treeData, setTreeData] = useState([]); @@ -79,7 +75,7 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } let treeItem: TreeItem = { name: element, type: type, - isExpanded: false, + isExpanded: true, }; if ( @@ -87,7 +83,6 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } type != TREE_ITEM_SKIPPING_INDEX_DEFAULT_NAME ) { treeItem.values = []; - treeItem.isLoading = false; } return treeItem; }); @@ -117,6 +112,7 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } console.error(err); }); } else { + setIsLoading(true); setTableNames([]); const query = { lang: 'sql', @@ -124,36 +120,23 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } datasource: selectedItems[0]['label'], }; getJobId(query, http, (id) => { - pollQueryStatus(id, http, (data) => { - setIsLoading({ flag: true, status: data.status }); - if (data.status === 'SUCCESS') { - const fetchedDatanases = [].concat(...data.results); - setTreeData(loadTreeItem(fetchedDatanases, TREE_ITEM_DATABASE_NAME_DEFAULT_NAME)); - setIsLoading({ flag: false, status: data.status }); - } + get_async_query_results(id, http, (data) => { + data = [].concat(...data); + setTreeData(loadTreeItem(data, TREE_ITEM_DATABASE_NAME_DEFAULT_NAME)); + setIsLoading(false); }); }); } }; useEffect(() => { - setIsLoading({ - flag: false, - status: 'Not loading', - }); + setIsLoading(false); getSidebarContent(); }, [selectedItems, refreshTree]); const handleDatabaseClick = (databaseName: string) => { - setTreeData((prevTreeData) => { - return prevTreeData.map((database) => { - if (database.name === databaseName) { - return { ...database, isExpanded: true, isLoading: true }; - } - return database; - }); - }); setSelectedDatabase(databaseName); + setIsLoading(true); const query = { lang: 'sql', query: `SHOW TABLES IN ${selectedItems[0]['label']}.${databaseName}`, @@ -161,23 +144,22 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } }; getJobId(query, http, (id) => { get_async_query_results(id, http, (data) => { - if (data.status === 'SUCCESS') { - const fetchTables = data.results.map((subArray) => subArray[1]); - let values = loadTreeItem(fetchTables, TREE_ITEM_TABLE_NAME_DEFAULT_NAME); - let mvObj = loadTreeItem( - [TREE_ITEM_LOAD_MATERIALIZED_BADGE_NAME], - TREE_ITEM_LOAD_MATERIALIZED_BADGE_NAME - ); - values = [...values, ...mvObj]; - setTreeData((prevTreeData) => { - return prevTreeData.map((database) => { - if (database.name === databaseName) { - return { ...database, values: values, isExpanded: true, isLoading: false}; - } - return database; - }); + data = data.map((subArray) => subArray[1]); + let values = loadTreeItem(data, TREE_ITEM_TABLE_NAME_DEFAULT_NAME); + let mvObj = loadTreeItem( + [TREE_ITEM_LOAD_MATERIALIZED_BADGE_NAME], + TREE_ITEM_LOAD_MATERIALIZED_BADGE_NAME + ); + values = [...values, ...mvObj]; + setTreeData((prevTreeData) => { + return prevTreeData.map((database) => { + if (database.name === databaseName) { + return { ...database, values: values }; + } + return database; }); - } + }); + setIsLoading(false); }); }); }; @@ -190,31 +172,28 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } }; getJobId(coverQuery, http, (id) => { get_async_query_results(id, http, (data) => { - if (data.status === 'SUCCESS') { - const res = [].concat(data.results); - let coverIndexObj = loadTreeItem(res, TREE_ITEM_COVERING_INDEX_DEFAULT_NAME); - setTreeData((prevTreeData) => { - return prevTreeData.map((database) => { - if (database.name === selectedDatabase) { - return { - ...database, - values: database.values?.map((table) => { - if (table.name === tableName) { - return { - ...table, - values: table.values?.concat(...coverIndexObj), - isLoading: false, - isExpanded: true, - }; - } - return table; - }), - }; - } - return database; - }); + const res = [].concat(data); + let coverIndexObj = loadTreeItem(res, TREE_ITEM_COVERING_INDEX_DEFAULT_NAME); + setTreeData((prevTreeData) => { + return prevTreeData.map((database) => { + if (database.name === selectedDatabase) { + return { + ...database, + values: database.values?.map((table) => { + if (table.name === tableName) { + return { + ...table, + values: table.values?.concat(...coverIndexObj), + }; + } + return table; + }), + }; + } + return database; }); - } + }); + setIsLoading(false); }); }); }; @@ -222,25 +201,7 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } const handleButtonClick = (e: MouseEvent, tableName: string) => { e.stopPropagation(); setSelectedTable(tableName); - setTreeData((prevTreeData) => { - return prevTreeData.map((database) => { - if (database.name === selectedDatabase) { - return { - ...database, - values: database.values?.map((table) => { - if (table.name === tableName) { - return { - ...table, - isLoading: true, - }; - } - return table; - }), - }; - } - return database; - }); - }); + setIsLoading(true); const materializedViewQuery = { lang: 'sql', query: `SHOW MATERIALIZED VIEW IN ${selectedItems[0]['label']}.${selectedDatabase}`, @@ -248,56 +209,32 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } }; getJobId(materializedViewQuery, http, (id) => { get_async_query_results(id, http, (data) => { - if (data.status === 'SUCCESS') { - const fetchMaterialzedView = data.results.map((subArray) => subArray[1]); - let values = loadTreeItem(fetchMaterialzedView, TREE_ITEM_MATERIALIZED_VIEW_DEFAULT_NAME); - if (values.length === 0) { - values = [ - { - name: 'No Materialized View', - type: TREE_ITEM_BADGE_NAME, - isExpanded: false, - isLoading: false, - }, - ]; - } - setTreeData((prevTreeData) => { - return prevTreeData.map((database) => { - if (database.name === selectedDatabase) { - const updatedValues = database.values?.filter( - (item) => item.type !== TREE_ITEM_LOAD_MATERIALIZED_BADGE_NAME - ); - return { ...database, values: updatedValues?.concat(...values), isLoading: false , isExpanded: true}; - } - return database; - }); - }); + data = data.map((subArray) => subArray[1]); + let values = loadTreeItem(data, TREE_ITEM_MATERIALIZED_VIEW_DEFAULT_NAME); + if (values.length === 0) { + values = [ + { name: 'No Materialized View', type: TREE_ITEM_BADGE_NAME, isExpanded: false }, + ]; } + setTreeData((prevTreeData) => { + return prevTreeData.map((database) => { + if (database.name === selectedDatabase) { + const updatedValues = database.values?.filter( + (item) => item.type !== TREE_ITEM_LOAD_MATERIALIZED_BADGE_NAME + ); + return { ...database, values: updatedValues?.concat(...values) }; + } + return database; + }); + }); + setIsLoading(false); }); }); }; const handleTableClick = (tableName: string) => { setSelectedTable(tableName); - setTreeData((prevTreeData) => { - return prevTreeData.map((database) => { - if (database.name === selectedDatabase) { - return { - ...database, - values: database.values?.map((table) => { - if (table.name === tableName) { - return { - ...table, - isLoading: true, - }; - } - return table; - }), - }; - } - return database; - }); - }); + setIsLoading(true); const skipQuery = { lang: 'sql', query: `DESC SKIPPING INDEX ON ${selectedItems[0]['label']}.${selectedDatabase}.${tableName}`, @@ -305,7 +242,7 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } }; getJobId(skipQuery, http, (id) => { get_async_query_results(id, http, (data) => { - if (data.status === 'SUCCESS') { + if (data.length > 0) { setTreeData((prevTreeData) => { return prevTreeData.map((database) => { if (database.name === selectedDatabase) { @@ -328,8 +265,8 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } return database; }); }); - loadCoveringIndex(tableName); } + loadCoveringIndex(tableName); }); }); }; @@ -351,7 +288,6 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } Load Materialized View - {node.isLoading && }
); @@ -359,15 +295,8 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } return (
- - - - {_.truncate(node.name, { length: 50 })}{' '} - {node.isLoading && } - - - - + {_.truncate(node.name, { length: 50 })} + {' '}
); } @@ -402,7 +331,7 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } id: `${database.name}_${table.name}`, icon: table.type === TREE_ITEM_LOAD_MATERIALIZED_BADGE_NAME ? ( - null + MV ) : table.type === TREE_ITEM_BADGE_NAME ? null : ( ), @@ -410,12 +339,9 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } if (table.type !== TREE_ITEM_LOAD_MATERIALIZED_BADGE_NAME && table.values?.length === 0) { handleTableClick(table.name); } - else { - if(table.values?.length === 0) { - table.values = [{ name: 'No Indicies', type: TREE_ITEM_BADGE_NAME, isExpanded: false }]; - } + if (table.values?.length === 0) { + table.values = [{ name: 'No Indicies', type: TREE_ITEM_BADGE_NAME, isExpanded: false }]; } - }, isSelectable: true, isExpanded: table.isExpanded, @@ -440,25 +366,18 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } return ( <> - {isLoadingBanner.flag ? ( + {isLoading ? ( - Loading data
- - - Loading may take over 30 seconds - - - - - Status: {isLoadingBanner.status} - - + + Loading can take more than 30s. Queries can be made after the data has loaded. Any + queries run before the data is loaded will be queued +
@@ -471,16 +390,13 @@ export const TableView = ({ http, selectedItems, updateSQLQueries, refreshTree } )} ) : ( - - - Error loading data} - /> - - + + Error loading data} + /> + )} {indexFlyout} diff --git a/test/mocks/mockData.ts b/test/mocks/mockData.ts index 9a01e0fa..fee838f0 100644 --- a/test/mocks/mockData.ts +++ b/test/mocks/mockData.ts @@ -2364,11 +2364,5 @@ export const mockOpenSearchIndicies = { }, }; -export const mockDatabaseQuery = { - data: { - ok: true, - resp: "{ \"schema\": [ { \"name\": \"namespace\", \"type\": \"string\" } ], \"datarows\": [ [ \"default\" ] ], \"total\": 1, \"size\": 1}" - }, -};