From 19640085cab9bab67ca323f3a50d509ff6636f41 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Wed, 18 Oct 2023 16:59:49 -0700 Subject: [PATCH] fixed create table async query bug (#158) (#163) * avoid displaying results if no scheme is returned on success for async query * if there is no schema, will handle case by displaying success/failure regardless * success callout when no schema returned * added failure page to async * fixed some test errors and added snapshots * update to failure portion * included tests of loading and failure states --------- (cherry picked from commit 55d6352b7f35895e26e4deed427b8b3ce75e7ad3) Signed-off-by: Paul Sebastian Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- common/types/index.ts | 2 +- public/components/Main/main.tsx | 37 +- .../QueryResults/AsyncQueryBody.tsx | 32 -- .../QueryResults/QueryResults.test.tsx | 244 +++++++++++--- .../components/QueryResults/QueryResults.tsx | 9 +- .../__snapshots__/QueryResults.test.tsx.snap | 315 ++++++++++++++++++ .../QueryResults/async_query_body.tsx | 76 +++++ 7 files changed, 621 insertions(+), 94 deletions(-) delete mode 100644 public/components/QueryResults/AsyncQueryBody.tsx create mode 100644 public/components/QueryResults/async_query_body.tsx diff --git a/common/types/index.ts b/common/types/index.ts index e669ce49..a941b29e 100644 --- a/common/types/index.ts +++ b/common/types/index.ts @@ -79,4 +79,4 @@ export interface CreateAccelerationForm { formErrors: FormErrorsType; } -export type AsyncQueryLoadingStatus = 'SUCCESS' | 'FAILED' | 'RUNNING' | 'SCHEDULED' | 'CANCELED'; +export type AsyncQueryLoadingStatus = 'SUCCESS' | 'FAILED' | 'RUNNING' | 'SCHEDULED' | 'CANCELLED'; \ No newline at end of file diff --git a/public/components/Main/main.tsx b/public/components/Main/main.tsx index ae28ff67..7e8382d8 100644 --- a/public/components/Main/main.tsx +++ b/public/components/Main/main.tsx @@ -15,6 +15,7 @@ import { EuiPanel, EuiSpacer, EuiText, + EuiCallOut, } from '@elastic/eui'; import { IHttpResponse } from 'angular'; import _ from 'lodash'; @@ -108,8 +109,10 @@ interface MainState { selectedDatasource: EuiComboBoxOptionOption[]; asyncLoading: boolean; asyncLoadingStatus: AsyncQueryLoadingStatus; + asyncQueryError: string; asyncJobId: string; isAccelerationFlyoutOpened: boolean; + isCallOutVisible: boolean; } const SUCCESS_MESSAGE = 'Success'; @@ -246,8 +249,10 @@ export class Main extends React.Component { selectedDatasource: [{ label: 'OpenSearch' }], asyncLoading: false, asyncLoadingStatus: 'SUCCESS', + asyncQueryError: '', asyncJobId: '', isAccelerationFlyoutOpened: false, + isCallOutVisible: false, }; this.httpClient = this.props.httpClient; this.updateSQLQueries = _.debounce(this.updateSQLQueries, 250).bind(this); @@ -403,6 +408,9 @@ export class Main extends React.Component { queryResultsCSV: [], queryResultsTEXT: [], searchQuery: '', + asyncLoading: false, + asyncLoadingStatus: 'SUCCESS', + isCallOutVisible: false, }, () => console.log('Successfully updated the states') ); // added callback function to handle async issues @@ -474,6 +482,7 @@ export class Main extends React.Component { asyncLoading: true, asyncLoadingStatus: 'SCHEDULED', asyncJobId: queryId, + isCallOutVisible: false, }); this.callGetStartPolling(queries); const interval = setInterval(() => { @@ -513,7 +522,7 @@ export class Main extends React.Component { this.setState({ queries: queries, queryResults: [result], - queryResultsTable: resultTable, + queryResultsTable: result.data['schema'].length > 0 ? resultTable : [], selectedTabId: getDefaultTabId([result]), selectedTabName: getDefaultTabLabel([result], queries[0]), messages: this.getMessage(resultTable), @@ -524,6 +533,7 @@ export class Main extends React.Component { searchQuery: '', asyncLoading: false, asyncLoadingStatus: status, + isCallOutVisible: !(result.data['schema'].length > 0), }); } else if (_.isEqual(status, 'FAILED') || _.isEqual(status, 'CANCELLED')) { this.setState({ @@ -535,6 +545,7 @@ export class Main extends React.Component { className: 'error-message', }, ], + asyncQueryError: result.data['error'], }); } else { this.setState({ @@ -760,6 +771,9 @@ export class Main extends React.Component { selectedTabId: MESSAGE_TAB_LABEL, selectedTabName: MESSAGE_TAB_LABEL, itemIdToExpandedRowMap: {}, + asyncLoading: false, + asyncLoadingStatus: 'SUCCESS', + isCallOutVisible: false, }); }; @@ -890,8 +904,8 @@ export class Main extends React.Component { getText={this.getText} isResultFullScreen={this.state.isResultFullScreen} setIsResultFullScreen={this.setIsResultFullScreen} - asyncLoading={this.state.asyncLoading} asyncLoadingStatus={this.state.asyncLoadingStatus} + asyncQueryError={this.state.asyncQueryError} cancelAsyncQuery={this.cancelAsyncQuery} selectedDatasource={this.state.selectedDatasource} /> @@ -956,6 +970,23 @@ export class Main extends React.Component {
{page}
+ {this.state.isCallOutVisible && ( + <> + + this.setState({ + isCallOutVisible: false, + }) + } + /> + + + )}
{ getText={this.getText} isResultFullScreen={this.state.isResultFullScreen} setIsResultFullScreen={this.setIsResultFullScreen} - asyncLoading={this.state.asyncLoading} asyncLoadingStatus={this.state.asyncLoadingStatus} + asyncQueryError={this.state.asyncQueryError} cancelAsyncQuery={this.cancelAsyncQuery} selectedDatasource={this.state.selectedDatasource} /> diff --git a/public/components/QueryResults/AsyncQueryBody.tsx b/public/components/QueryResults/AsyncQueryBody.tsx deleted file mode 100644 index eb28ab85..00000000 --- a/public/components/QueryResults/AsyncQueryBody.tsx +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import { EuiFlexGroup, EuiText, EuiLoadingSpinner, EuiButton } from '@elastic/eui'; -import { AsyncQueryLoadingStatus } from '../../../common/types'; -import React from 'react'; - -interface AsyncQueryBodyProps { - asyncLoading: boolean; - asyncLoadingStatus: AsyncQueryLoadingStatus; - cancelAsyncQuery: () => void; -} - -export function AsyncQueryBody(props: AsyncQueryBodyProps) { - const { asyncLoading, asyncLoadingStatus, cancelAsyncQuery } = props; - - // TODO: implement query failure display - // TODO: implement query cancellation - - return ( - - - -

Query running

- - status: {asyncLoadingStatus} - Cancel - - ); -} diff --git a/public/components/QueryResults/QueryResults.test.tsx b/public/components/QueryResults/QueryResults.test.tsx index 336bf0a8..2e5bafa5 100644 --- a/public/components/QueryResults/QueryResults.test.tsx +++ b/public/components/QueryResults/QueryResults.test.tsx @@ -3,20 +3,20 @@ * SPDX-License-Identifier: Apache-2.0 */ - -import React from "react"; -import "regenerator-runtime"; -import "mutationobserver-shim"; -import "@testing-library/jest-dom/extend-expect"; -import { render, fireEvent, configure } from "@testing-library/react"; -import { mockQueryResults, mockQueries } from "../../../test/mocks/mockData"; -import { MESSAGE_TAB_LABEL } from "../../utils/constants"; -import QueryResults from "./QueryResults"; -import { Tab, ItemIdToExpandedRowMap, ResponseDetail, QueryResult } from "../Main/main"; +import React from 'react'; +import 'regenerator-runtime'; +import 'mutationobserver-shim'; +import '@testing-library/jest-dom/extend-expect'; +import { render, fireEvent, configure } from '@testing-library/react'; +import { mockQueryResults, mockQueries } from '../../../test/mocks/mockData'; +import { MESSAGE_TAB_LABEL } from '../../utils/constants'; +import QueryResults from './QueryResults'; +import { Tab, ItemIdToExpandedRowMap, ResponseDetail, QueryResult } from '../Main/main'; configure({ testIdAttribute: 'data-test-subj' }); -function renderSQLQueryResults(mockQueryResults: ResponseDetail[], +function renderSQLQueryResults( + mockQueryResults: ResponseDetail[], mockQueries: string[] = [], mockSearchQuery: string = '', onSelectedTabIdChange: (tab: Tab) => void, @@ -26,12 +26,12 @@ function renderSQLQueryResults(mockQueryResults: ResponseDetail[], getJdbc: (queries: string[]) => void, getCsv: (queries: string[]) => void, getText: (queries: string[]) => void, - setIsResultFullScreen: (isFullScreen: boolean) => void) { - + setIsResultFullScreen: (isFullScreen: boolean) => void +) { return { ...render( [], getText={getText} isResultFullScreen={false} setIsResultFullScreen={setIsResultFullScreen} + asyncLoadingStatus="SUCCESS" + asyncQueryError="" + selectedDatasource={[{ label: 'OpenSearch' }]} + cancelAsyncQuery={() => {}} /> ), }; } -describe(" spec", () => { - it("renders the component with no data", async () => { - (window as any).HTMLElement.prototype.scrollBy = function () { }; +describe(' spec', () => { + it('renders the component with no data', async () => { + (window as any).HTMLElement.prototype.scrollBy = function () {}; expect(document.body.children[0]).toMatchSnapshot(); }); }); -describe(" spec", () => { +describe(' spec', () => { const onSelectedTabIdChange = jest.fn(); const onQueryChange = jest.fn(); const updateExpandedMap = jest.fn(); - const mockSearchQuery = ""; + const mockSearchQuery = ''; const getRawResponse = jest.fn(); const getJdbc = jest.fn(); const getCsv = jest.fn(); @@ -77,10 +81,26 @@ describe(" spec", () => { const setIsResultFullScreen = jest.fn(); (window as any).HTMLElement.prototype.scrollBy = jest.fn(); - it("renders the component with mock query results", async () => { - const { getAllByRole, getByText, getAllByText, getAllByTestId, getAllByLabelText } = - renderSQLQueryResults(mockQueryResults, mockQueries, mockSearchQuery, onSelectedTabIdChange, onQueryChange, - updateExpandedMap, getRawResponse, getJdbc, getCsv, getText, setIsResultFullScreen); + it('renders the component with mock query results', async () => { + const { + getAllByRole, + getByText, + getAllByText, + getAllByTestId, + getAllByLabelText, + } = renderSQLQueryResults( + mockQueryResults, + mockQueries, + mockSearchQuery, + onSelectedTabIdChange, + onQueryChange, + updateExpandedMap, + getRawResponse, + getJdbc, + getCsv, + getText, + setIsResultFullScreen + ); expect(document.body.children[0]).toMatchSnapshot(); @@ -101,16 +121,27 @@ describe(" spec", () => { // It tests pagination await fireEvent.click(getAllByLabelText('Page 2 of 2')[0]); await fireEvent.click(getAllByText('Rows per page', { exact: false })[0]); - expect(getByText("10 rows")); - expect(getByText("20 rows")); - expect(getByText("50 rows")); - expect(getByText("100 rows")); - await fireEvent.click(getByText("20 rows")); + expect(getByText('10 rows')); + expect(getByText('20 rows')); + expect(getByText('50 rows')); + expect(getByText('100 rows')); + await fireEvent.click(getByText('20 rows')); }); - it("renders the component to test tabs down arrow", async () => { - const { getAllByTestId } = renderSQLQueryResults(mockQueryResults, mockQueries, mockSearchQuery, onSelectedTabIdChange, - onQueryChange, updateExpandedMap, getRawResponse, getJdbc, getCsv, getText, setIsResultFullScreen); + it('renders the component to test tabs down arrow', async () => { + const { getAllByTestId } = renderSQLQueryResults( + mockQueryResults, + mockQueries, + mockSearchQuery, + onSelectedTabIdChange, + onQueryChange, + updateExpandedMap, + getRawResponse, + getJdbc, + getCsv, + getText, + setIsResultFullScreen + ); expect(document.body.children[0]).toMatchSnapshot(); @@ -118,11 +149,10 @@ describe(" spec", () => { expect(getAllByTestId('slide-down')); await fireEvent.click(getAllByTestId('slide-down')[0]); }); - }); - -function renderPPLQueryResults(mockQueryResults: ResponseDetail[], +function renderPPLQueryResults( + mockQueryResults: ResponseDetail[], mockQueries: string[] = [], mockSearchQuery: string = '', onSelectedTabIdChange: (tab: Tab) => void, @@ -132,12 +162,12 @@ function renderPPLQueryResults(mockQueryResults: ResponseDetail[], getJdbc: (queries: string[]) => void, getCsv: (queries: string[]) => void, getText: (queries: string[]) => void, - setIsResultFullScreen: (isFullScreen: boolean) => void) { - + setIsResultFullScreen: (isFullScreen: boolean) => void +) { return { ...render( [], getText={getText} isResultFullScreen={false} setIsResultFullScreen={setIsResultFullScreen} + asyncLoadingStatus="SUCCESS" + asyncQueryError="" + selectedDatasource={[{ label: 'OpenSearch' }]} + cancelAsyncQuery={() => {}} /> ), }; } -describe(" spec", () => { - it("renders the component with no data", async () => { - (window as any).HTMLElement.prototype.scrollBy = function () { }; +describe(' spec', () => { + it('renders the component with no data', async () => { + (window as any).HTMLElement.prototype.scrollBy = function () {}; expect(document.body.children[0]).toMatchSnapshot(); }); }); -describe(" spec", () => { +describe(' spec', () => { const onSelectedTabIdChange = jest.fn(); const onQueryChange = jest.fn(); const updateExpandedMap = jest.fn(); - const mockSearchQuery = ""; + const mockSearchQuery = ''; const getRawResponse = jest.fn(); const getJdbc = jest.fn(); const getCsv = jest.fn(); @@ -184,10 +218,26 @@ describe(" spec", () => { const setIsResultFullScreen = jest.fn(); (window as any).HTMLElement.prototype.scrollBy = jest.fn(); - it("renders the component with mock query results", async () => { - const { getAllByRole, getByText, getAllByText, getAllByTestId, getAllByLabelText } = - renderPPLQueryResults(mockQueryResults, mockQueries, mockSearchQuery, onSelectedTabIdChange, onQueryChange, - updateExpandedMap, getRawResponse, getJdbc, getCsv, getText, setIsResultFullScreen); + it('renders the component with mock query results', async () => { + const { + getAllByRole, + getByText, + getAllByText, + getAllByTestId, + getAllByLabelText, + } = renderPPLQueryResults( + mockQueryResults, + mockQueries, + mockSearchQuery, + onSelectedTabIdChange, + onQueryChange, + updateExpandedMap, + getRawResponse, + getJdbc, + getCsv, + getText, + setIsResultFullScreen + ); expect(document.body.children[0]).toMatchSnapshot(); @@ -208,16 +258,27 @@ describe(" spec", () => { // It tests pagination await fireEvent.click(getAllByLabelText('Page 2 of 2')[0]); await fireEvent.click(getAllByText('Rows per page', { exact: false })[0]); - expect(getByText("10 rows")); - expect(getByText("20 rows")); - expect(getByText("50 rows")); - expect(getByText("100 rows")); - await fireEvent.click(getByText("20 rows")); + expect(getByText('10 rows')); + expect(getByText('20 rows')); + expect(getByText('50 rows')); + expect(getByText('100 rows')); + await fireEvent.click(getByText('20 rows')); }); - it("renders the component to test tabs down arrow", async () => { - const { getAllByTestId } = renderPPLQueryResults(mockQueryResults, mockQueries, mockSearchQuery, onSelectedTabIdChange, - onQueryChange, updateExpandedMap, getRawResponse, getJdbc, getCsv, getText, setIsResultFullScreen); + it('renders the component to test tabs down arrow', async () => { + const { getAllByTestId } = renderPPLQueryResults( + mockQueryResults, + mockQueries, + mockSearchQuery, + onSelectedTabIdChange, + onQueryChange, + updateExpandedMap, + getRawResponse, + getJdbc, + getCsv, + getText, + setIsResultFullScreen + ); expect(document.body.children[0]).toMatchSnapshot(); @@ -225,5 +286,80 @@ describe(" spec", () => { expect(getAllByTestId('slide-down')); await fireEvent.click(getAllByTestId('slide-down')[0]); }); +}); +describe(' spec', () => { + it('renders async query loading component', async () => { + const asyncTest = () => { + render( + {}} + itemIdToExpandedRowMap={{}} + onQueryChange={() => {}} + updateExpandedMap={() => {}} + searchQuery={''} + tabsOverflow={true} + getJson={() => {}} + getJdbc={() => {}} + getCsv={() => {}} + getText={() => {}} + isResultFullScreen={false} + setIsResultFullScreen={() => {}} + asyncLoadingStatus="RUNNING" + asyncQueryError="" + selectedDatasource={[{ label: 'mys3' }]} + cancelAsyncQuery={() => {}} + /> + ); + }; + await asyncTest(); + expect(document.body.children[0]).toMatchSnapshot(); + }); + + it('renders async query failure component', async () => { + const asyncTest = () => { + render( + {}} + itemIdToExpandedRowMap={{}} + onQueryChange={() => {}} + updateExpandedMap={() => {}} + searchQuery={''} + tabsOverflow={true} + getJson={() => {}} + getJdbc={() => {}} + getCsv={() => {}} + getText={() => {}} + isResultFullScreen={false} + setIsResultFullScreen={() => {}} + asyncLoadingStatus="FAILED" + asyncQueryError="custom error" + selectedDatasource={[{ label: 'mys3' }]} + cancelAsyncQuery={() => {}} + /> + ); + }; + await asyncTest(); + expect(document.body.children[0]).toMatchSnapshot(); + }); }); diff --git a/public/components/QueryResults/QueryResults.tsx b/public/components/QueryResults/QueryResults.tsx index 58d204e8..55f4f2ce 100644 --- a/public/components/QueryResults/QueryResults.tsx +++ b/public/components/QueryResults/QueryResults.tsx @@ -45,7 +45,7 @@ import { ResponseDetail, Tab, } from '../Main/main'; -import { AsyncQueryBody } from './AsyncQueryBody'; +import { AsyncQueryBody } from './async_query_body'; import QueryResultsBody from './QueryResultsBody'; interface QueryResultsProps { @@ -71,8 +71,8 @@ interface QueryResultsProps { getText: (queries: string[]) => void; isResultFullScreen: boolean; setIsResultFullScreen: (isFullScreen: boolean) => void; - asyncLoading: boolean; asyncLoadingStatus: AsyncQueryLoadingStatus; + asyncQueryError: string; cancelAsyncQuery: () => void; selectedDatasource: EuiComboBoxOptionOption[]; } @@ -335,7 +335,8 @@ class QueryResults extends React.Component
- {!this.props.asyncLoading ? ( + {this.props.asyncLoadingStatus === 'SUCCESS' || + this.props.asyncLoadingStatus === 'CANCELLED' ? ( <> {this.props.queryResults.length === 0 ? ( // show no results message instead of the results table when there are no results @@ -443,9 +444,9 @@ class QueryResults extends React.Component diff --git a/public/components/QueryResults/__snapshots__/QueryResults.test.tsx.snap b/public/components/QueryResults/__snapshots__/QueryResults.test.tsx.snap index 5c99ca41..270ed879 100644 --- a/public/components/QueryResults/__snapshots__/QueryResults.test.tsx.snap +++ b/public/components/QueryResults/__snapshots__/QueryResults.test.tsx.snap @@ -1,5 +1,233 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[` spec renders async query failure component 1`] = ` +
+
+
+
+
+
+

+ Results +

+
+
+
+ +
+
+
+
+
+
+
+ +
+

+ Query failed +

+
+
+ The query failed to execute and the operation could not be complete. +
+ +
+
+
+
+
+
+`; + +exports[` spec renders async query loading component 1`] = ` +
+
+
+
+
+
+

+ Results +

+
+
+
+ +
+
+
+
+
+
+
+ +
+

+ Query running +

+
+
+ Status: + RUNNING +
+ +
+
+
+
+
+
+`; + exports[` spec renders the component with no data 1`] = `undefined`; exports[` spec renders the component with no data 2`] = `undefined`; @@ -328,6 +556,52 @@ exports[` spec renders the component to test tabs down
+
+
+
+
+ +
+
+
+
spec renders the component with mock query re
+
+
+
+
+ +
+
+
+
void; + asyncQueryError: string; +} + +export function AsyncQueryBody(props: AsyncQueryBodyProps) { + const { asyncLoadingStatus, cancelAsyncQuery, asyncQueryError } = props; + const [isModalVisible, setIsModalVisible] = useState(false); + + const closeModal = () => setIsModalVisible(false); + const showModal = () => setIsModalVisible(true); + + let modal; + if (isModalVisible) { + modal = ( + + + Error + + {asyncQueryError} + + + + Close + + + + ); + } + + return ( + + {asyncLoadingStatus == 'FAILED' ? ( + <> + + +

Query failed

+ + The query failed to execute and the operation could not be complete. + showModal()}>View error details + {modal} + + ) : ( + <> + + +

Query running

+ + Status: {asyncLoadingStatus} + Cancel + + )} + + ); +}