You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by hu...@apache.org on 2022/10/07 17:14:09 UTC

[superset] branch 2022.39.1 created (now 2af1000492)

This is an automated email from the ASF dual-hosted git repository.

hugh pushed a change to branch 2022.39.1
in repository https://gitbox.apache.org/repos/asf/superset.git


      at 2af1000492 fix conflict with cherry 882bfb67a

This branch includes the following new commits:

     new cc8c7d4506 Fixing tags package
     new 5a181c5c31 fix: Dataset duplication fatal error (#21358)
     new 1dbfc6503d fix(sqllab): SqlEditorLeftBar listening to database changes (#21628)
     new d0de438ff4 fix: catch error when masking encrypted extra is none (#21570)
     new 2c452903f3 Revert "chore: refactor SqlEditor to functional component (#21320)"
     new 22fac9e7e6 fix: Allow clickhouse dbs with timestamps to visualize queries (#21446)
     new 08cd63eb2b fix: add logging to alerts and reports to find non-triggering issues (#21684)
     new 777e22f720 chore: add 4xx error codes where applicable (#21627)
     new a169ed1225 Revert "chore: refactor ResultSet to functional component (#21186)"
     new 98fcab48df fix: Race conditions with setupExtensions (#21647)
     new 42304a4302 fix: add `get_column` function for Query obj (#21691)
     new e77978db7d fix(database): Handle String errors in DatabaseModal (#21709)
     new 2af1000492 fix conflict with cherry 882bfb67a

The 13 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[superset] 04/13: fix: catch error when masking encrypted extra is none (#21570)

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch 2022.39.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit d0de438ff458b92d1817d32928d8564218ff4f55
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Sun Oct 2 19:49:01 2022 -0700

    fix: catch error when masking encrypted extra is none (#21570)
    
    (cherry picked from commit ef78ec6b30ece829e6fcf0a73d35dac343dcd70c)
---
 superset/db_engine_specs/base.py                  |  6 ++-
 superset/db_engine_specs/bigquery.py              | 16 +++++--
 superset/db_engine_specs/gsheets.py               | 16 +++++--
 superset/models/core.py                           | 12 ++---
 tests/unit_tests/db_engine_specs/test_bigquery.py | 54 ++++++++++++++++++++++-
 tests/unit_tests/db_engine_specs/test_gsheets.py  | 45 ++++++++++++++++++-
 6 files changed, 132 insertions(+), 17 deletions(-)

diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py
index 5a8354fd78..d80625dc0c 100644
--- a/superset/db_engine_specs/base.py
+++ b/superset/db_engine_specs/base.py
@@ -1618,7 +1618,7 @@ class BaseEngineSpec:  # pylint: disable=too-many-public-methods
         return user.username if user else None
 
     @classmethod
-    def mask_encrypted_extra(cls, encrypted_extra: str) -> str:
+    def mask_encrypted_extra(cls, encrypted_extra: Optional[str]) -> Optional[str]:
         """
         Mask ``encrypted_extra``.
 
@@ -1631,7 +1631,9 @@ class BaseEngineSpec:  # pylint: disable=too-many-public-methods
 
     # pylint: disable=unused-argument
     @classmethod
-    def unmask_encrypted_extra(cls, old: str, new: str) -> str:
+    def unmask_encrypted_extra(
+        cls, old: Optional[str], new: Optional[str]
+    ) -> Optional[str]:
         """
         Remove masks from ``encrypted_extra``.
 
diff --git a/superset/db_engine_specs/bigquery.py b/superset/db_engine_specs/bigquery.py
index 4016c8f08f..38b1f92023 100644
--- a/superset/db_engine_specs/bigquery.py
+++ b/superset/db_engine_specs/bigquery.py
@@ -396,10 +396,13 @@ class BigQueryEngineSpec(BaseEngineSpec):
         raise ValidationError("Invalid service credentials")
 
     @classmethod
-    def mask_encrypted_extra(cls, encrypted_extra: str) -> str:
+    def mask_encrypted_extra(cls, encrypted_extra: Optional[str]) -> Optional[str]:
+        if encrypted_extra is None:
+            return encrypted_extra
+
         try:
             config = json.loads(encrypted_extra)
-        except json.JSONDecodeError:
+        except (json.JSONDecodeError, TypeError):
             return encrypted_extra
 
         try:
@@ -410,14 +413,19 @@ class BigQueryEngineSpec(BaseEngineSpec):
         return json.dumps(config)
 
     @classmethod
-    def unmask_encrypted_extra(cls, old: str, new: str) -> str:
+    def unmask_encrypted_extra(
+        cls, old: Optional[str], new: Optional[str]
+    ) -> Optional[str]:
         """
         Reuse ``private_key`` if available and unchanged.
         """
+        if old is None or new is None:
+            return new
+
         try:
             old_config = json.loads(old)
             new_config = json.loads(new)
-        except json.JSONDecodeError:
+        except (TypeError, json.JSONDecodeError):
             return new
 
         if "credentials_info" not in new_config:
diff --git a/superset/db_engine_specs/gsheets.py b/superset/db_engine_specs/gsheets.py
index 3ee284788c..3562e0d0b1 100644
--- a/superset/db_engine_specs/gsheets.py
+++ b/superset/db_engine_specs/gsheets.py
@@ -140,10 +140,13 @@ class GSheetsEngineSpec(SqliteEngineSpec):
         raise ValidationError("Invalid service credentials")
 
     @classmethod
-    def mask_encrypted_extra(cls, encrypted_extra: str) -> str:
+    def mask_encrypted_extra(cls, encrypted_extra: Optional[str]) -> Optional[str]:
+        if encrypted_extra is None:
+            return encrypted_extra
+
         try:
             config = json.loads(encrypted_extra)
-        except json.JSONDecodeError:
+        except (TypeError, json.JSONDecodeError):
             return encrypted_extra
 
         try:
@@ -154,14 +157,19 @@ class GSheetsEngineSpec(SqliteEngineSpec):
         return json.dumps(config)
 
     @classmethod
-    def unmask_encrypted_extra(cls, old: str, new: str) -> str:
+    def unmask_encrypted_extra(
+        cls, old: Optional[str], new: Optional[str]
+    ) -> Optional[str]:
         """
         Reuse ``private_key`` if available and unchanged.
         """
+        if old is None or new is None:
+            return new
+
         try:
             old_config = json.loads(old)
             new_config = json.loads(new)
-        except json.JSONDecodeError:
+        except (TypeError, json.JSONDecodeError):
             return new
 
         if "service_account_info" not in new_config:
diff --git a/superset/models/core.py b/superset/models/core.py
index 8c1b58171d..ca15137935 100755
--- a/superset/models/core.py
+++ b/superset/models/core.py
@@ -248,7 +248,7 @@ class Database(
         return sqlalchemy_url.get_backend_name()
 
     @property
-    def masked_encrypted_extra(self) -> str:
+    def masked_encrypted_extra(self) -> Optional[str]:
         return self.db_engine_spec.mask_encrypted_extra(self.encrypted_extra)
 
     @property
@@ -261,10 +261,12 @@ class Database(
         masked_encrypted_extra = db_engine_spec.mask_encrypted_extra(
             self.encrypted_extra
         )
-        try:
-            encrypted_config = json.loads(masked_encrypted_extra)
-        except (TypeError, json.JSONDecodeError):
-            encrypted_config = {}
+        encrypted_config = {}
+        if masked_encrypted_extra is not None:
+            try:
+                encrypted_config = json.loads(masked_encrypted_extra)
+            except (TypeError, json.JSONDecodeError):
+                pass
 
         try:
             # pylint: disable=useless-suppression
diff --git a/tests/unit_tests/db_engine_specs/test_bigquery.py b/tests/unit_tests/db_engine_specs/test_bigquery.py
index 13f7fd3150..8c33489faf 100644
--- a/tests/unit_tests/db_engine_specs/test_bigquery.py
+++ b/tests/unit_tests/db_engine_specs/test_bigquery.py
@@ -186,9 +186,61 @@ def test_unmask_encrypted_extra() -> None:
         }
     )
 
-    assert json.loads(BigQueryEngineSpec.unmask_encrypted_extra(old, new)) == {
+    assert json.loads(str(BigQueryEngineSpec.unmask_encrypted_extra(old, new))) == {
         "credentials_info": {
             "project_id": "yellow-unicorn-314419",
             "private_key": "SECRET",
         },
     }
+
+
+def test_unmask_encrypted_extra_when_empty() -> None:
+    """
+    Test that a None value works for ``encrypted_extra``.
+    """
+    from superset.db_engine_specs.bigquery import BigQueryEngineSpec
+
+    old = None
+    new = json.dumps(
+        {
+            "credentials_info": {
+                "project_id": "yellow-unicorn-314419",
+                "private_key": "XXXXXXXXXX",
+            },
+        }
+    )
+
+    assert json.loads(str(BigQueryEngineSpec.unmask_encrypted_extra(old, new))) == {
+        "credentials_info": {
+            "project_id": "yellow-unicorn-314419",
+            "private_key": "XXXXXXXXXX",
+        },
+    }
+
+
+def test_unmask_encrypted_extra_when_new_is_empty() -> None:
+    """
+    Test that a None value works for ``encrypted_extra``.
+    """
+    from superset.db_engine_specs.bigquery import BigQueryEngineSpec
+
+    old = json.dumps(
+        {
+            "credentials_info": {
+                "project_id": "black-sanctum-314419",
+                "private_key": "SECRET",
+            },
+        }
+    )
+    new = None
+
+    assert BigQueryEngineSpec.unmask_encrypted_extra(old, new) is None
+
+
+def test_mask_encrypted_extra_when_empty() -> None:
+    """
+    Test that the encrypted extra will return a none value if the field is empty.
+    """
+    from superset.db_engine_specs.bigquery import BigQueryEngineSpec
+
+    assert BigQueryEngineSpec.mask_encrypted_extra(None) is None
diff --git a/tests/unit_tests/db_engine_specs/test_gsheets.py b/tests/unit_tests/db_engine_specs/test_gsheets.py
index 219f259c1f..a226653ed5 100644
--- a/tests/unit_tests/db_engine_specs/test_gsheets.py
+++ b/tests/unit_tests/db_engine_specs/test_gsheets.py
@@ -228,9 +228,52 @@ def test_unmask_encrypted_extra() -> None:
         }
     )
 
-    assert json.loads(GSheetsEngineSpec.unmask_encrypted_extra(old, new)) == {
+    assert json.loads(str(GSheetsEngineSpec.unmask_encrypted_extra(old, new))) == {
         "service_account_info": {
             "project_id": "yellow-unicorn-314419",
             "private_key": "SECRET",
         },
     }
+
+
+def test_unmask_encrypted_extra_when_old_is_none() -> None:
+    """
+    Test that a None value works for ``encrypted_extra``.
+    """
+    from superset.db_engine_specs.gsheets import GSheetsEngineSpec
+
+    old = None
+    new = json.dumps(
+        {
+            "service_account_info": {
+                "project_id": "yellow-unicorn-314419",
+                "private_key": "XXXXXXXXXX",
+            },
+        }
+    )
+
+    assert json.loads(str(GSheetsEngineSpec.unmask_encrypted_extra(old, new))) == {
+        "service_account_info": {
+            "project_id": "yellow-unicorn-314419",
+            "private_key": "XXXXXXXXXX",
+        },
+    }
+
+
+def test_unmask_encrypted_extra_when_new_is_none() -> None:
+    """
+    Test that a None value works for ``encrypted_extra``.
+    """
+    from superset.db_engine_specs.gsheets import GSheetsEngineSpec
+
+    old = json.dumps(
+        {
+            "service_account_info": {
+                "project_id": "yellow-unicorn-314419",
+                "private_key": "XXXXXXXXXX",
+            },
+        }
+    )
+    new = None
+
+    assert GSheetsEngineSpec.unmask_encrypted_extra(old, new) is None


[superset] 03/13: fix(sqllab): SqlEditorLeftBar listening to database changes (#21628)

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch 2022.39.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 1dbfc6503d5645c08bf7ec763c1aeaa5e9184894
Author: Antonio Rivero Martinez <38...@users.noreply.github.com>
AuthorDate: Wed Sep 28 20:22:09 2022 -0300

    fix(sqllab): SqlEditorLeftBar listening to database changes (#21628)
    
    (cherry picked from commit 71bf2673071d5db6688fbaefd4457aeeae3464bb)
---
 .../SqlEditorLeftBar/SqlEditorLeftBar.test.jsx     | 89 ++++++++++++++--------
 .../SqlLab/components/SqlEditorLeftBar/index.tsx   |  2 +-
 2 files changed, 59 insertions(+), 32 deletions(-)

diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx
index 9d32454899..5f9b140c12 100644
--- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx
+++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx
@@ -19,7 +19,7 @@
 import React from 'react';
 import configureStore from 'redux-mock-store';
 import fetchMock from 'fetch-mock';
-import { render, screen } from 'spec/helpers/testing-library';
+import { render, screen, act } from 'spec/helpers/testing-library';
 import userEvent from '@testing-library/user-event';
 import { Provider } from 'react-redux';
 import '@testing-library/jest-dom/extend-expect';
@@ -28,7 +28,6 @@ import SqlEditorLeftBar from 'src/SqlLab/components/SqlEditorLeftBar';
 import {
   table,
   initialState,
-  databases,
   defaultQueryEditor,
   mockedActions,
 } from 'src/SqlLab/fixtures';
@@ -37,7 +36,11 @@ const mockedProps = {
   actions: mockedActions,
   tables: [table],
   queryEditor: defaultQueryEditor,
-  database: databases,
+  database: {
+    id: 1,
+    database_name: 'main',
+    backend: 'mysql',
+  },
   height: 0,
 };
 const middlewares = [thunk];
@@ -46,15 +49,13 @@ const store = mockStore(initialState);
 
 fetchMock.get('glob:*/api/v1/database/*/schemas/?*', { result: [] });
 fetchMock.get('glob:*/superset/tables/**', {
-  json: {
-    options: [
-      {
-        label: 'ab_user',
-        value: 'ab_user',
-      },
-    ],
-    tableLength: 1,
-  },
+  options: [
+    {
+      label: 'ab_user',
+      value: 'ab_user',
+    },
+  ],
+  tableLength: 1,
 });
 
 describe('Left Panel Expansion', () => {
@@ -74,9 +75,7 @@ describe('Left Panel Expansion', () => {
       initialState,
     });
 
-    expect(
-      await screen.findByText(/select database or type database name/i),
-    ).toBeInTheDocument();
+    expect(await screen.findByText(/Database/i)).toBeInTheDocument();
     expect(
       screen.queryAllByTestId('table-element').length,
     ).toBeGreaterThanOrEqual(1);
@@ -94,30 +93,27 @@ describe('Left Panel Expansion', () => {
     const schemaSelect = screen.getByRole('combobox', {
       name: 'Select schema or type schema name',
     });
-    const dropdown = screen.getByText(/Select table or type table name/i);
-    const abUser = screen.getByText(/ab_user/i);
+    const dropdown = screen.getByText(/Table/i);
+    const abUser = screen.queryAllByText(/ab_user/i);
 
-    expect(
-      await screen.findByText(/select database or type database name/i),
-    ).toBeInTheDocument();
-    expect(dbSelect).toBeInTheDocument();
-    expect(schemaSelect).toBeInTheDocument();
-    expect(dropdown).toBeInTheDocument();
-    expect(abUser).toBeInTheDocument();
-    expect(
-      container.querySelector('.ant-collapse-content-active'),
-    ).toBeInTheDocument();
+    act(async () => {
+      expect(await screen.findByText(/Database/i)).toBeInTheDocument();
+      expect(dbSelect).toBeInTheDocument();
+      expect(schemaSelect).toBeInTheDocument();
+      expect(dropdown).toBeInTheDocument();
+      expect(abUser).toHaveLength(2);
+      expect(
+        container.querySelector('.ant-collapse-content-active'),
+      ).toBeInTheDocument();
+    });
   });
 
   test('should toggle the table when the header is clicked', async () => {
     const collapseMock = jest.fn();
     render(
       <SqlEditorLeftBar
+        {...mockedProps}
         actions={{ ...mockedActions, collapseTable: collapseMock }}
-        tables={[table]}
-        queryEditor={defaultQueryEditor}
-        database={databases}
-        height={0}
       />,
       {
         useRedux: true,
@@ -130,4 +126,35 @@ describe('Left Panel Expansion', () => {
     userEvent.click(header);
     expect(collapseMock).toHaveBeenCalled();
   });
+
+  test('When changing database the table list must be updated', async () => {
+    const { rerender } = render(<SqlEditorLeftBar {...mockedProps} />, {
+      useRedux: true,
+      initialState,
+    });
+
+    await act(async () => {
+      expect(await screen.findByText(/main/i)).toBeInTheDocument();
+      expect(await screen.findByText(/ab_user/i)).toBeInTheDocument();
+    });
+    rerender(
+      <SqlEditorLeftBar
+        {...mockedProps}
+        actions={{ ...mockedActions }}
+        database={{
+          id: 2,
+          database_name: 'new_db',
+          backend: 'postgresql',
+        }}
+        queryEditor={{ ...mockedProps.queryEditor, schema: 'new_schema' }}
+        tables={[{ ...mockedProps.tables[0], dbId: 2, name: 'new_table' }]}
+      />,
+      {
+        useRedux: true,
+        initialState,
+      },
+    );
+    expect(await screen.findByText(/new_db/i)).toBeInTheDocument();
+    expect(await screen.findByText(/new_table/i)).toBeInTheDocument();
+  });
 });
diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx
index b3c1bc9f63..1a24edc657 100644
--- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx
+++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx
@@ -138,7 +138,7 @@ export default function SqlEditorLeftBar({
       setUserSelected(userSelected);
       setItem(LocalStorageKeys.db, null);
     } else setUserSelected(database);
-  }, []);
+  }, [database]);
 
   useEffect(() => {
     queryEditorRef.current = queryEditor;


[superset] 02/13: fix: Dataset duplication fatal error (#21358)

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch 2022.39.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 5a181c5c315a230ce05a41fe5986fe90d22ce3e2
Author: Reese <10...@users.noreply.github.com>
AuthorDate: Tue Sep 27 16:02:37 2022 -0400

    fix: Dataset duplication fatal error (#21358)
    
    (cherry picked from commit e3ddd0bdd5f8f976e0a1733f6da29e33a2545c27)
---
 superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx
index 27750eab32..df2f54cb0f 100644
--- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx
+++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx
@@ -701,7 +701,7 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
 
     SupersetClient.post({
       endpoint: `/api/v1/dataset/duplicate`,
-      postPayload: {
+      jsonPayload: {
         base_model_id: datasetCurrentlyDuplicating?.id,
         table_name: newDatasetName,
       },


[superset] 09/13: Revert "chore: refactor ResultSet to functional component (#21186)"

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch 2022.39.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit a169ed12258694200452482da59273c7adcf8070
Author: Joe Li <jo...@preset.io>
AuthorDate: Tue Oct 4 10:41:02 2022 -0700

    Revert "chore: refactor ResultSet to functional component (#21186)"
    
    This reverts commit f6032956780380bdac1e3ae950687ae53eabf2e1.
---
 .../components/ExploreCtasResultsButton/index.tsx  |  25 +-
 .../components/ExploreResultsButton/index.tsx      |   1 -
 .../src/SqlLab/components/QueryTable/index.tsx     |   1 +
 .../SqlLab/components/ResultSet/ResultSet.test.jsx | 219 ++++++++
 .../SqlLab/components/ResultSet/ResultSet.test.tsx | 216 --------
 .../src/SqlLab/components/ResultSet/index.tsx      | 573 +++++++++++----------
 .../src/SqlLab/components/SouthPane/index.tsx      |   3 +
 .../src/components/FilterableTable/index.tsx       |   3 +-
 .../src/components/ProgressBar/index.tsx           |   2 +-
 9 files changed, 542 insertions(+), 501 deletions(-)

diff --git a/superset-frontend/src/SqlLab/components/ExploreCtasResultsButton/index.tsx b/superset-frontend/src/SqlLab/components/ExploreCtasResultsButton/index.tsx
index ac9e8b2fb4..fbcdc15bc5 100644
--- a/superset-frontend/src/SqlLab/components/ExploreCtasResultsButton/index.tsx
+++ b/superset-frontend/src/SqlLab/components/ExploreCtasResultsButton/index.tsx
@@ -17,19 +17,19 @@
  * under the License.
  */
 import React from 'react';
-import { useSelector, useDispatch } from 'react-redux';
-import { t, JsonObject } from '@superset-ui/core';
-import {
-  createCtasDatasource,
-  addInfoToast,
-  addDangerToast,
-} from 'src/SqlLab/actions/sqlLab';
+import { useSelector } from 'react-redux';
+import { t } from '@superset-ui/core';
 import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
 import Button from 'src/components/Button';
 import { exploreChart } from 'src/explore/exploreUtils';
 import { SqlLabRootState } from 'src/SqlLab/types';
 
 interface ExploreCtasResultsButtonProps {
+  actions: {
+    createCtasDatasource: Function;
+    addInfoToast: Function;
+    addDangerToast: Function;
+  };
   table: string;
   schema?: string | null;
   dbId: number;
@@ -37,15 +37,16 @@ interface ExploreCtasResultsButtonProps {
 }
 
 const ExploreCtasResultsButton = ({
+  actions,
   table,
   schema,
   dbId,
   templateParams,
 }: ExploreCtasResultsButtonProps) => {
+  const { createCtasDatasource, addInfoToast, addDangerToast } = actions;
   const errorMessage = useSelector(
     (state: SqlLabRootState) => state.sqlLab.errorMessage,
   );
-  const dispatch = useDispatch<(dispatch: any) => Promise<JsonObject>>();
 
   const buildVizOptions = {
     datasourceName: table,
@@ -55,7 +56,7 @@ const ExploreCtasResultsButton = ({
   };
 
   const visualize = () => {
-    dispatch(createCtasDatasource(buildVizOptions))
+    createCtasDatasource(buildVizOptions)
       .then((data: { table_id: number }) => {
         const formData = {
           datasource: `${data.table_id}__table`,
@@ -66,14 +67,12 @@ const ExploreCtasResultsButton = ({
           all_columns: [],
           row_limit: 1000,
         };
-        dispatch(
-          addInfoToast(t('Creating a data source and creating a new tab')),
-        );
+        addInfoToast(t('Creating a data source and creating a new tab'));
         // open new window for data visualization
         exploreChart(formData);
       })
       .catch(() => {
-        dispatch(addDangerToast(errorMessage || t('An error occurred')));
+        addDangerToast(errorMessage || t('An error occurred'));
       });
   };
 
diff --git a/superset-frontend/src/SqlLab/components/ExploreResultsButton/index.tsx b/superset-frontend/src/SqlLab/components/ExploreResultsButton/index.tsx
index 4ab7777736..24d5e8686f 100644
--- a/superset-frontend/src/SqlLab/components/ExploreResultsButton/index.tsx
+++ b/superset-frontend/src/SqlLab/components/ExploreResultsButton/index.tsx
@@ -39,7 +39,6 @@ const ExploreResultsButton = ({
       onClick={onClick}
       disabled={!allowsSubquery}
       tooltip={t('Explore the result set in the data exploration view')}
-      data-test="explore-results-button"
     >
       <InfoTooltipWithTrigger
         icon="line-chart"
diff --git a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx
index d7ef5ed51c..54edb7f97e 100644
--- a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx
+++ b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx
@@ -245,6 +245,7 @@ const QueryTable = ({
                   showSql
                   user={user}
                   query={query}
+                  actions={actions}
                   height={400}
                   displayLimit={displayLimit}
                   defaultQueryLimit={1000}
diff --git a/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.jsx b/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.jsx
new file mode 100644
index 0000000000..c04e236133
--- /dev/null
+++ b/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.jsx
@@ -0,0 +1,219 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { shallow } from 'enzyme';
+import { styledMount } from 'spec/helpers/theming';
+import { render, screen } from 'spec/helpers/testing-library';
+import { Provider } from 'react-redux';
+import sinon from 'sinon';
+import Alert from 'src/components/Alert';
+import ProgressBar from 'src/components/ProgressBar';
+import Loading from 'src/components/Loading';
+import configureStore from 'redux-mock-store';
+import thunk from 'redux-thunk';
+import fetchMock from 'fetch-mock';
+import FilterableTable from 'src/components/FilterableTable';
+import ExploreResultsButton from 'src/SqlLab/components/ExploreResultsButton';
+import ResultSet from 'src/SqlLab/components/ResultSet';
+import ErrorMessageWithStackTrace from 'src/components/ErrorMessage/ErrorMessageWithStackTrace';
+import {
+  cachedQuery,
+  failedQueryWithErrorMessage,
+  failedQueryWithErrors,
+  queries,
+  runningQuery,
+  stoppedQuery,
+  initialState,
+  user,
+  queryWithNoQueryLimit,
+} from 'src/SqlLab/fixtures';
+
+const mockStore = configureStore([thunk]);
+const store = mockStore(initialState);
+const clearQuerySpy = sinon.spy();
+const fetchQuerySpy = sinon.spy();
+const reRunQuerySpy = sinon.spy();
+const mockedProps = {
+  actions: {
+    clearQueryResults: clearQuerySpy,
+    fetchQueryResults: fetchQuerySpy,
+    reRunQuery: reRunQuerySpy,
+  },
+  cache: true,
+  query: queries[0],
+  height: 140,
+  database: { allows_virtual_table_explore: true },
+  user,
+  defaultQueryLimit: 1000,
+};
+const stoppedQueryProps = { ...mockedProps, query: stoppedQuery };
+const runningQueryProps = { ...mockedProps, query: runningQuery };
+const fetchingQueryProps = {
+  ...mockedProps,
+  query: {
+    dbId: 1,
+    cached: false,
+    ctas: false,
+    id: 'ryhHUZCGb',
+    progress: 100,
+    state: 'fetching',
+    startDttm: Date.now() - 500,
+  },
+};
+const cachedQueryProps = { ...mockedProps, query: cachedQuery };
+const failedQueryWithErrorMessageProps = {
+  ...mockedProps,
+  query: failedQueryWithErrorMessage,
+};
+const failedQueryWithErrorsProps = {
+  ...mockedProps,
+  query: failedQueryWithErrors,
+};
+const newProps = {
+  query: {
+    cached: false,
+    resultsKey: 'new key',
+    results: {
+      data: [{ a: 1 }],
+    },
+  },
+};
+fetchMock.get('glob:*/api/v1/dataset?*', { result: [] });
+
+test('is valid', () => {
+  expect(React.isValidElement(<ResultSet {...mockedProps} />)).toBe(true);
+});
+
+test('renders a Table', () => {
+  const wrapper = shallow(<ResultSet {...mockedProps} />);
+  expect(wrapper.find(FilterableTable)).toExist();
+});
+
+describe('componentDidMount', () => {
+  const propsWithError = {
+    ...mockedProps,
+    query: { ...queries[0], errorMessage: 'Your session timed out' },
+  };
+  let spy;
+  beforeEach(() => {
+    reRunQuerySpy.resetHistory();
+    spy = sinon.spy(ResultSet.prototype, 'componentDidMount');
+  });
+  afterEach(() => {
+    spy.restore();
+  });
+  it('should call reRunQuery if timed out', () => {
+    shallow(<ResultSet {...propsWithError} />);
+    expect(reRunQuerySpy.callCount).toBe(1);
+  });
+
+  it('should not call reRunQuery if no error', () => {
+    shallow(<ResultSet {...mockedProps} />);
+    expect(reRunQuerySpy.callCount).toBe(0);
+  });
+});
+
+describe('UNSAFE_componentWillReceiveProps', () => {
+  const wrapper = shallow(<ResultSet {...mockedProps} />);
+  let spy;
+  beforeEach(() => {
+    clearQuerySpy.resetHistory();
+    fetchQuerySpy.resetHistory();
+    spy = sinon.spy(ResultSet.prototype, 'UNSAFE_componentWillReceiveProps');
+  });
+  afterEach(() => {
+    spy.restore();
+  });
+  it('should update cached data', () => {
+    wrapper.setProps(newProps);
+
+    expect(wrapper.state().data).toEqual(newProps.query.results.data);
+    expect(clearQuerySpy.callCount).toBe(1);
+    expect(clearQuerySpy.getCall(0).args[0]).toEqual(newProps.query);
+    expect(fetchQuerySpy.callCount).toBe(1);
+    expect(fetchQuerySpy.getCall(0).args[0]).toEqual(newProps.query);
+  });
+});
+
+test('should render success query', () => {
+  const wrapper = shallow(<ResultSet {...mockedProps} />);
+  const filterableTable = wrapper.find(FilterableTable);
+  expect(filterableTable.props().data).toBe(mockedProps.query.results.data);
+  expect(wrapper.find(ExploreResultsButton)).toExist();
+});
+test('should render empty results', () => {
+  const props = {
+    ...mockedProps,
+    query: { ...mockedProps.query, results: { data: [] } },
+  };
+  const wrapper = styledMount(
+    <Provider store={store}>
+      <ResultSet {...props} />
+    </Provider>,
+  );
+  expect(wrapper.find(FilterableTable)).not.toExist();
+  expect(wrapper.find(Alert)).toExist();
+  expect(wrapper.find(Alert).render().text()).toBe(
+    'The query returned no data',
+  );
+});
+
+test('should render cached query', () => {
+  const wrapper = shallow(<ResultSet {...cachedQueryProps} />);
+  const cachedData = [{ col1: 'a', col2: 'b' }];
+  wrapper.setState({ data: cachedData });
+  const filterableTable = wrapper.find(FilterableTable);
+  expect(filterableTable.props().data).toBe(cachedData);
+});
+
+test('should render stopped query', () => {
+  const wrapper = shallow(<ResultSet {...stoppedQueryProps} />);
+  expect(wrapper.find(Alert)).toExist();
+});
+
+test('should render running/pending/fetching query', () => {
+  const wrapper = shallow(<ResultSet {...runningQueryProps} />);
+  expect(wrapper.find(ProgressBar)).toExist();
+});
+
+test('should render fetching w/ 100 progress query', () => {
+  const wrapper = shallow(<ResultSet {...fetchingQueryProps} />);
+  expect(wrapper.find(Loading)).toExist();
+});
+
+test('should render a failed query with an error message', () => {
+  const wrapper = shallow(<ResultSet {...failedQueryWithErrorMessageProps} />);
+  expect(wrapper.find(ErrorMessageWithStackTrace)).toExist();
+});
+
+test('should render a failed query with an errors object', () => {
+  const wrapper = shallow(<ResultSet {...failedQueryWithErrorsProps} />);
+  expect(wrapper.find(ErrorMessageWithStackTrace)).toExist();
+});
+
+test('renders if there is no limit in query.results but has queryLimit', () => {
+  render(<ResultSet {...mockedProps} />, { useRedux: true });
+  expect(screen.getByRole('grid')).toBeInTheDocument();
+});
+
+test('renders if there is a limit in query.results but not queryLimit', () => {
+  const props = { ...mockedProps, query: queryWithNoQueryLimit };
+  render(<ResultSet {...props} />, { useRedux: true });
+  expect(screen.getByRole('grid')).toBeInTheDocument();
+});
diff --git a/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx b/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx
deleted file mode 100644
index 2818ed279c..0000000000
--- a/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx
+++ /dev/null
@@ -1,216 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-import React from 'react';
-import { render, screen, waitFor } from 'spec/helpers/testing-library';
-import configureStore from 'redux-mock-store';
-import { Store } from 'redux';
-import thunk from 'redux-thunk';
-import fetchMock from 'fetch-mock';
-import ResultSet from 'src/SqlLab/components/ResultSet';
-import {
-  cachedQuery,
-  failedQueryWithErrorMessage,
-  failedQueryWithErrors,
-  queries,
-  runningQuery,
-  stoppedQuery,
-  initialState,
-  user,
-  queryWithNoQueryLimit,
-} from 'src/SqlLab/fixtures';
-
-const mockedProps = {
-  cache: true,
-  query: queries[0],
-  height: 140,
-  database: { allows_virtual_table_explore: true },
-  user,
-  defaultQueryLimit: 1000,
-};
-const stoppedQueryProps = { ...mockedProps, query: stoppedQuery };
-const runningQueryProps = { ...mockedProps, query: runningQuery };
-const fetchingQueryProps = {
-  ...mockedProps,
-  query: {
-    dbId: 1,
-    cached: false,
-    ctas: false,
-    id: 'ryhHUZCGb',
-    progress: 100,
-    state: 'fetching',
-    startDttm: Date.now() - 500,
-  },
-};
-const cachedQueryProps = { ...mockedProps, query: cachedQuery };
-const failedQueryWithErrorMessageProps = {
-  ...mockedProps,
-  query: failedQueryWithErrorMessage,
-};
-const failedQueryWithErrorsProps = {
-  ...mockedProps,
-  query: failedQueryWithErrors,
-};
-const newProps = {
-  query: {
-    cached: false,
-    resultsKey: 'new key',
-    results: {
-      data: [{ a: 1 }],
-    },
-  },
-};
-fetchMock.get('glob:*/api/v1/dataset?*', { result: [] });
-
-const middlewares = [thunk];
-const mockStore = configureStore(middlewares);
-const setup = (props?: any, store?: Store) =>
-  render(<ResultSet {...props} />, {
-    useRedux: true,
-    ...(store && { store }),
-  });
-
-describe('ResultSet', () => {
-  it('renders a Table', async () => {
-    const { getByTestId } = setup(mockedProps, mockStore(initialState));
-    const table = getByTestId('table-container');
-    expect(table).toBeInTheDocument();
-  });
-
-  it('should render success query', async () => {
-    const { queryAllByText, getByTestId } = setup(
-      mockedProps,
-      mockStore(initialState),
-    );
-
-    const table = getByTestId('table-container');
-    expect(table).toBeInTheDocument();
-
-    const firstColumn = queryAllByText(
-      mockedProps.query.results?.columns[0].name ?? '',
-    )[0];
-    const secondColumn = queryAllByText(
-      mockedProps.query.results?.columns[1].name ?? '',
-    )[0];
-    expect(firstColumn).toBeInTheDocument();
-    expect(secondColumn).toBeInTheDocument();
-
-    const exploreButton = getByTestId('explore-results-button');
-    expect(exploreButton).toBeInTheDocument();
-  });
-
-  it('should render empty results', async () => {
-    const props = {
-      ...mockedProps,
-      query: { ...mockedProps.query, results: { data: [] } },
-    };
-    await waitFor(() => {
-      setup(props, mockStore(initialState));
-    });
-
-    const alert = screen.getByRole('alert');
-    expect(alert).toBeInTheDocument();
-    expect(alert).toHaveTextContent('The query returned no data');
-  });
-
-  it('should call reRunQuery if timed out', async () => {
-    const store = mockStore(initialState);
-    const propsWithError = {
-      ...mockedProps,
-      query: { ...queries[0], errorMessage: 'Your session timed out' },
-    };
-
-    setup(propsWithError, store);
-    expect(store.getActions()).toHaveLength(1);
-    expect(store.getActions()[0].query.errorMessage).toEqual(
-      'Your session timed out',
-    );
-    expect(store.getActions()[0].type).toEqual('START_QUERY');
-  });
-
-  it('should not call reRunQuery if no error', async () => {
-    const store = mockStore(initialState);
-    setup(mockedProps, store);
-    expect(store.getActions()).toEqual([]);
-  });
-
-  it('should render cached query', async () => {
-    const store = mockStore(initialState);
-    const { rerender } = setup(cachedQueryProps, store);
-
-    // @ts-ignore
-    rerender(<ResultSet {...newProps} />);
-    expect(store.getActions()).toHaveLength(1);
-    expect(store.getActions()[0].query.results).toEqual(
-      cachedQueryProps.query.results,
-    );
-    expect(store.getActions()[0].type).toEqual('CLEAR_QUERY_RESULTS');
-  });
-
-  it('should render stopped query', async () => {
-    await waitFor(() => {
-      setup(stoppedQueryProps, mockStore(initialState));
-    });
-
-    const alert = screen.getByRole('alert');
-    expect(alert).toBeInTheDocument();
-  });
-
-  it('should render running/pending/fetching query', async () => {
-    const { getByTestId } = setup(runningQueryProps, mockStore(initialState));
-    const progressBar = getByTestId('progress-bar');
-    expect(progressBar).toBeInTheDocument();
-  });
-
-  it('should render fetching w/ 100 progress query', async () => {
-    const { getByRole, getByText } = setup(
-      fetchingQueryProps,
-      mockStore(initialState),
-    );
-    const loading = getByRole('status');
-    expect(loading).toBeInTheDocument();
-    expect(getByText('fetching')).toBeInTheDocument();
-  });
-
-  it('should render a failed query with an error message', async () => {
-    await waitFor(() => {
-      setup(failedQueryWithErrorMessageProps, mockStore(initialState));
-    });
-
-    expect(screen.getByText('Database error')).toBeInTheDocument();
-    expect(screen.getByText('Something went wrong')).toBeInTheDocument();
-  });
-
-  it('should render a failed query with an errors object', async () => {
-    await waitFor(() => {
-      setup(failedQueryWithErrorsProps, mockStore(initialState));
-    });
-    expect(screen.getByText('Database error')).toBeInTheDocument();
-  });
-
-  it('renders if there is no limit in query.results but has queryLimit', async () => {
-    const { getByRole } = setup(mockedProps, mockStore(initialState));
-    expect(getByRole('grid')).toBeInTheDocument();
-  });
-
-  it('renders if there is a limit in query.results but not queryLimit', async () => {
-    const props = { ...mockedProps, query: queryWithNoQueryLimit };
-    const { getByRole } = setup(props, mockStore(initialState));
-    expect(getByRole('grid')).toBeInTheDocument();
-  });
-});
diff --git a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx
index 78387f6dc4..27913d7fc4 100644
--- a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx
+++ b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx
@@ -16,14 +16,12 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React, { useState, useEffect, useCallback } from 'react';
-import { useDispatch } from 'react-redux';
+import React from 'react';
 import ButtonGroup from 'src/components/ButtonGroup';
 import Alert from 'src/components/Alert';
 import Button from 'src/components/Button';
 import shortid from 'shortid';
 import { styled, t, QueryResponse } from '@superset-ui/core';
-import { usePrevious } from 'src/hooks/usePrevious';
 import ErrorMessageWithStackTrace from 'src/components/ErrorMessage/ErrorMessageWithStackTrace';
 import {
   ISaveableDatasource,
@@ -42,14 +40,7 @@ import FilterableTable, {
 import CopyToClipboard from 'src/components/CopyToClipboard';
 import { addDangerToast } from 'src/components/MessageToasts/actions';
 import { prepareCopyToClipboardTabularData } from 'src/utils/common';
-import {
-  CtasEnum,
-  clearQueryResults,
-  addQueryEditor,
-  fetchQueryResults,
-  reFetchQueryResults,
-  reRunQuery,
-} from 'src/SqlLab/actions/sqlLab';
+import { CtasEnum } from 'src/SqlLab/actions/sqlLab';
 import { URL_PARAMS } from 'src/constants';
 import ExploreCtasResultsButton from '../ExploreCtasResultsButton';
 import ExploreResultsButton from '../ExploreResultsButton';
@@ -63,7 +54,9 @@ enum LIMITING_FACTOR {
   NOT_LIMITED = 'NOT_LIMITED',
 }
 
-export interface ResultSetProps {
+interface ResultSetProps {
+  showControls?: boolean;
+  actions: Record<string, any>;
   cache?: boolean;
   csv?: boolean;
   database?: Record<string, any>;
@@ -77,9 +70,17 @@ export interface ResultSetProps {
   defaultQueryLimit: number;
 }
 
+interface ResultSetState {
+  searchText: string;
+  showExploreResultsButton: boolean;
+  data: Record<string, any>[];
+  showSaveDatasetModal: boolean;
+  alertIsOpen: boolean;
+}
+
 const ResultlessStyles = styled.div`
   position: relative;
-  min-height: ${({ theme }) => theme.gridUnit * 25}px;
+  min-height: 100px;
   [role='alert'] {
     margin-top: ${({ theme }) => theme.gridUnit * 2}px;
   }
@@ -99,8 +100,8 @@ const MonospaceDiv = styled.div`
 `;
 
 const ReturnedRows = styled.div`
-  font-size: ${({ theme }) => theme.typography.sizes.s}px;
-  line-height: ${({ theme }) => theme.gridUnit * 6}px;
+  font-size: 13px;
+  line-height: 24px;
 `;
 
 const ResultSetControls = styled.div`
@@ -120,84 +121,115 @@ const LimitMessage = styled.span`
   margin-left: ${({ theme }) => theme.gridUnit * 2}px;
 `;
 
-const ResultSet = ({
-  cache = false,
-  csv = true,
-  database = {},
-  displayLimit,
-  height,
-  query,
-  search = true,
-  showSql = false,
-  visualize = true,
-  user,
-  defaultQueryLimit,
-}: ResultSetProps) => {
-  const [searchText, setSearchText] = useState('');
-  const [cachedData, setCachedData] = useState<Record<string, unknown>[]>([]);
-  const [showSaveDatasetModal, setShowSaveDatasetModal] = useState(false);
-  const [alertIsOpen, setAlertIsOpen] = useState(false);
-
-  const dispatch = useDispatch();
-
-  const reRunQueryIfSessionTimeoutErrorOnMount = useCallback(() => {
-    if (
-      query.errorMessage &&
-      query.errorMessage.indexOf('session timed out') > 0
-    ) {
-      dispatch(reRunQuery(query));
-    }
-  }, []);
+export default class ResultSet extends React.PureComponent<
+  ResultSetProps,
+  ResultSetState
+> {
+  static defaultProps = {
+    cache: false,
+    csv: true,
+    database: {},
+    search: true,
+    showSql: false,
+    visualize: true,
+  };
 
-  useEffect(() => {
-    // only do this the first time the component is rendered/mounted
-    reRunQueryIfSessionTimeoutErrorOnMount();
-  }, [reRunQueryIfSessionTimeoutErrorOnMount]);
+  constructor(props: ResultSetProps) {
+    super(props);
+    this.state = {
+      searchText: '',
+      showExploreResultsButton: false,
+      data: [],
+      showSaveDatasetModal: false,
+      alertIsOpen: false,
+    };
+    this.changeSearch = this.changeSearch.bind(this);
+    this.fetchResults = this.fetchResults.bind(this);
+    this.popSelectStar = this.popSelectStar.bind(this);
+    this.reFetchQueryResults = this.reFetchQueryResults.bind(this);
+    this.toggleExploreResultsButton =
+      this.toggleExploreResultsButton.bind(this);
+  }
 
-  const fetchResults = (query: QueryResponse) => {
-    dispatch(fetchQueryResults(query, displayLimit));
-  };
+  async componentDidMount() {
+    // only do this the first time the component is rendered/mounted
+    this.reRunQueryIfSessionTimeoutErrorOnMount();
+  }
 
-  const prevQuery = usePrevious(query);
-  useEffect(() => {
-    if (cache && query.cached && query?.results?.data?.length > 0) {
-      setCachedData(query.results.data);
-      dispatch(clearQueryResults(query));
+  UNSAFE_componentWillReceiveProps(nextProps: ResultSetProps) {
+    // when new results comes in, save them locally and clear in store
+    if (
+      this.props.cache &&
+      !nextProps.query.cached &&
+      nextProps.query.results &&
+      nextProps.query.results.data &&
+      nextProps.query.results.data.length > 0
+    ) {
+      this.setState({ data: nextProps.query.results.data }, () =>
+        this.clearQueryResults(nextProps.query),
+      );
     }
     if (
-      query.resultsKey &&
-      prevQuery?.resultsKey &&
-      query.resultsKey !== prevQuery.resultsKey
+      nextProps.query.resultsKey &&
+      nextProps.query.resultsKey !== this.props.query.resultsKey
     ) {
-      fetchResults(query);
+      this.fetchResults(nextProps.query);
     }
-  }, [query, cache]);
+  }
 
-  const calculateAlertRefHeight = (alertElement: HTMLElement | null) => {
+  calculateAlertRefHeight = (alertElement: HTMLElement | null) => {
     if (alertElement) {
-      setAlertIsOpen(true);
+      this.setState({ alertIsOpen: true });
     } else {
-      setAlertIsOpen(false);
+      this.setState({ alertIsOpen: false });
     }
   };
 
-  const popSelectStar = (tempSchema: string | null, tempTable: string) => {
+  clearQueryResults(query: QueryResponse) {
+    this.props.actions.clearQueryResults(query);
+  }
+
+  popSelectStar(tempSchema: string | null, tempTable: string) {
     const qe = {
       id: shortid.generate(),
       name: tempTable,
       autorun: false,
-      dbId: query.dbId,
+      dbId: this.props.query.dbId,
       sql: `SELECT * FROM ${tempSchema ? `${tempSchema}.` : ''}${tempTable}`,
     };
-    dispatch(addQueryEditor(qe));
-  };
+    this.props.actions.addQueryEditor(qe);
+  }
 
-  const changeSearch = (event: React.ChangeEvent<HTMLInputElement>) => {
-    setSearchText(event.target.value);
-  };
+  toggleExploreResultsButton() {
+    this.setState(prevState => ({
+      showExploreResultsButton: !prevState.showExploreResultsButton,
+    }));
+  }
+
+  changeSearch(event: React.ChangeEvent<HTMLInputElement>) {
+    this.setState({ searchText: event.target.value });
+  }
+
+  fetchResults(query: QueryResponse) {
+    this.props.actions.fetchQueryResults(query, this.props.displayLimit);
+  }
 
-  const createExploreResultsOnClick = async () => {
-    const { results } = query;
+  reFetchQueryResults(query: QueryResponse) {
+    this.props.actions.reFetchQueryResults(query);
+  }
+
+  reRunQueryIfSessionTimeoutErrorOnMount() {
+    const { query } = this.props;
+    if (
+      query.errorMessage &&
+      query.errorMessage.indexOf('session timed out') > 0
+    ) {
+      this.props.actions.reRunQuery(query);
+    }
+  }
+
+  createExploreResultsOnClick = async () => {
+    const { results } = this.props.query;
 
     if (results?.query_id) {
       const key = await postFormData(results.query_id, 'query', {
@@ -216,14 +248,16 @@ const ResultSet = ({
     }
   };
 
-  const renderControls = () => {
-    if (search || visualize || csv) {
-      let { data } = query.results;
-      if (cache && query.cached) {
-        data = cachedData;
+  renderControls() {
+    if (this.props.search || this.props.visualize || this.props.csv) {
+      let { data } = this.props.query.results;
+      if (this.props.cache && this.props.query.cached) {
+        ({ data } = this.state);
       }
-      const { columns } = query.results;
+      const { columns } = this.props.query.results;
       // Added compute logic to stop user from being able to Save & Explore
+      const { showSaveDatasetModal } = this.state;
+      const { query } = this.props;
 
       const datasource: ISaveableDatasource = {
         columns: query.results.columns as ISimpleColumn[],
@@ -238,7 +272,7 @@ const ResultSet = ({
         <ResultSetControls>
           <SaveDatasetModal
             visible={showSaveDatasetModal}
-            onHide={() => setShowSaveDatasetModal(false)}
+            onHide={() => this.setState({ showSaveDatasetModal: false })}
             buttonTextOnSave={t('Save & Explore')}
             buttonTextOnOverwrite={t('Overwrite & Explore')}
             modalDescription={t(
@@ -247,13 +281,14 @@ const ResultSet = ({
             datasource={datasource}
           />
           <ResultSetButtons>
-            {visualize && database?.allows_virtual_table_explore && (
-              <ExploreResultsButton
-                database={database}
-                onClick={createExploreResultsOnClick}
-              />
-            )}
-            {csv && (
+            {this.props.visualize &&
+              this.props.database?.allows_virtual_table_explore && (
+                <ExploreResultsButton
+                  database={this.props.database}
+                  onClick={this.createExploreResultsOnClick}
+                />
+              )}
+            {this.props.csv && (
               <Button buttonSize="small" href={`/superset/csv/${query.id}`}>
                 <i className="fa fa-file-text-o" /> {t('Download to CSV')}
               </Button>
@@ -270,11 +305,11 @@ const ResultSet = ({
               hideTooltip
             />
           </ResultSetButtons>
-          {search && (
+          {this.props.search && (
             <input
               type="text"
-              onChange={changeSearch}
-              value={searchText}
+              onChange={this.changeSearch}
+              value={this.state.searchText}
               className="form-control input-sm"
               disabled={columns.length > MAX_COLUMNS_FOR_TABLE}
               placeholder={
@@ -288,14 +323,14 @@ const ResultSet = ({
       );
     }
     return <div />;
-  };
+  }
 
-  const renderRowsReturned = () => {
-    const { results, rows, queryLimit, limitingFactor } = query;
+  renderRowsReturned() {
+    const { results, rows, queryLimit, limitingFactor } = this.props.query;
     let limitMessage;
     const limitReached = results?.displayLimitReached;
     const limit = queryLimit || results.query.limit;
-    const isAdmin = !!user?.roles?.Admin;
+    const isAdmin = !!this.props.user?.roles?.Admin;
     const rowsCount = Math.min(rows || 0, results?.data?.length || 0);
 
     const displayMaxRowsReachedMessage = {
@@ -313,10 +348,10 @@ const ResultSet = ({
       ),
     };
     const shouldUseDefaultDropdownAlert =
-      limit === defaultQueryLimit &&
+      limit === this.props.defaultQueryLimit &&
       limitingFactor === LIMITING_FACTOR.DROPDOWN;
 
-    if (limitingFactor === LIMITING_FACTOR.QUERY && csv) {
+    if (limitingFactor === LIMITING_FACTOR.QUERY && this.props.csv) {
       limitMessage = t(
         'The number of rows displayed is limited to %(rows)d by the query',
         { rows },
@@ -351,11 +386,11 @@ const ResultSet = ({
           </span>
         )}
         {!limitReached && shouldUseDefaultDropdownAlert && (
-          <div ref={calculateAlertRefHeight}>
+          <div ref={this.calculateAlertRefHeight}>
             <Alert
               type="warning"
               message={t('%(rows)d rows returned', { rows })}
-              onClose={() => setAlertIsOpen(false)}
+              onClose={() => this.setState({ alertIsOpen: false })}
               description={t(
                 'The number of rows displayed is limited to %s by the dropdown.',
                 rows,
@@ -364,10 +399,10 @@ const ResultSet = ({
           </div>
         )}
         {limitReached && (
-          <div ref={calculateAlertRefHeight}>
+          <div ref={this.calculateAlertRefHeight}>
             <Alert
               type="warning"
-              onClose={() => setAlertIsOpen(false)}
+              onClose={() => this.setState({ alertIsOpen: false })}
               message={t('%(rows)d rows returned', { rows: rowsCount })}
               description={
                 isAdmin
@@ -379,191 +414,193 @@ const ResultSet = ({
         )}
       </ReturnedRows>
     );
-  };
-
-  const limitReached = query?.results?.displayLimitReached;
-  let sql;
-  let exploreDBId = query.dbId;
-  if (database?.explore_database_id) {
-    exploreDBId = database.explore_database_id;
   }
 
-  let trackingUrl;
-  if (
-    query.trackingUrl &&
-    query.state !== 'success' &&
-    query.state !== 'fetching'
-  ) {
-    trackingUrl = (
-      <Button
-        className="sql-result-track-job"
-        buttonSize="small"
-        href={query.trackingUrl}
-        target="_blank"
-      >
-        {query.state === 'running' ? t('Track job') : t('See query details')}
-      </Button>
-    );
-  }
-
-  if (showSql) {
-    sql = <HighlightedSql sql={query.sql} />;
-  }
-
-  if (query.state === 'stopped') {
-    return <Alert type="warning" message={t('Query was stopped')} />;
-  }
+  render() {
+    const { query } = this.props;
+    const limitReached = query?.results?.displayLimitReached;
+    let sql;
+    let exploreDBId = query.dbId;
+    if (this.props.database && this.props.database.explore_database_id) {
+      exploreDBId = this.props.database.explore_database_id;
+    }
+    let trackingUrl;
+    if (
+      query.trackingUrl &&
+      query.state !== 'success' &&
+      query.state !== 'fetching'
+    ) {
+      trackingUrl = (
+        <Button
+          className="sql-result-track-job"
+          buttonSize="small"
+          href={query.trackingUrl}
+          target="_blank"
+        >
+          {query.state === 'running' ? t('Track job') : t('See query details')}
+        </Button>
+      );
+    }
 
-  if (query.state === 'failed') {
-    return (
-      <ResultlessStyles>
-        <ErrorMessageWithStackTrace
-          title={t('Database error')}
-          error={query?.errors?.[0]}
-          subtitle={<MonospaceDiv>{query.errorMessage}</MonospaceDiv>}
-          copyText={query.errorMessage || undefined}
-          link={query.link}
-          source="sqllab"
-        />
-        {trackingUrl}
-      </ResultlessStyles>
-    );
-  }
+    if (this.props.showSql) sql = <HighlightedSql sql={query.sql} />;
 
-  if (query.state === 'success' && query.ctas) {
-    const { tempSchema, tempTable } = query;
-    let object = 'Table';
-    if (query.ctas_method === CtasEnum.VIEW) {
-      object = 'View';
+    if (query.state === 'stopped') {
+      return <Alert type="warning" message={t('Query was stopped')} />;
     }
-    return (
-      <div>
-        <Alert
-          type="info"
-          message={
-            <>
-              {t(object)} [
-              <strong>
-                {tempSchema ? `${tempSchema}.` : ''}
-                {tempTable}
-              </strong>
-              ] {t('was created')} &nbsp;
-              <ButtonGroup>
-                <Button
-                  buttonSize="small"
-                  className="m-r-5"
-                  onClick={() => popSelectStar(tempSchema, tempTable)}
-                >
-                  {t('Query in a new tab')}
-                </Button>
-                <ExploreCtasResultsButton
-                  table={tempTable}
-                  schema={tempSchema}
-                  dbId={exploreDBId}
-                />
-              </ButtonGroup>
-            </>
-          }
-        />
-      </div>
-    );
-  }
-
-  if (query.state === 'success' && query.results) {
-    const { results } = query;
-    // Accounts for offset needed for height of ResultSetRowsReturned component if !limitReached
-    const rowMessageHeight = !limitReached ? 32 : 0;
-    // Accounts for offset needed for height of Alert if this.state.alertIsOpen
-    const alertContainerHeight = 70;
-    // We need to calculate the height of this.renderRowsReturned()
-    // if we want results panel to be proper height because the
-    // FilterTable component needs an explicit height to render
-    // react-virtualized Table component
-    const rowsHeight = alertIsOpen
-      ? height - alertContainerHeight
-      : height - rowMessageHeight;
-    let data;
-    if (cache && query.cached) {
-      data = cachedData;
-    } else if (results?.data) {
-      ({ data } = results);
+    if (query.state === 'failed') {
+      return (
+        <ResultlessStyles>
+          <ErrorMessageWithStackTrace
+            title={t('Database error')}
+            error={query?.errors?.[0]}
+            subtitle={<MonospaceDiv>{query.errorMessage}</MonospaceDiv>}
+            copyText={query.errorMessage || undefined}
+            link={query.link}
+            source="sqllab"
+          />
+          {trackingUrl}
+        </ResultlessStyles>
+      );
     }
-    if (data && data.length > 0) {
-      const expandedColumns = results.expanded_columns
-        ? results.expanded_columns.map(col => col.name)
-        : [];
+    if (query.state === 'success' && query.ctas) {
+      const { tempSchema, tempTable } = query;
+      let object = 'Table';
+      if (query.ctas_method === CtasEnum.VIEW) {
+        object = 'View';
+      }
       return (
-        <>
-          {renderControls()}
-          {renderRowsReturned()}
-          {sql}
-          <FilterableTable
-            data={data}
-            orderedColumnKeys={results.columns.map(col => col.name)}
-            height={rowsHeight}
-            filterText={searchText}
-            expandedColumns={expandedColumns}
+        <div>
+          <Alert
+            type="info"
+            message={
+              <>
+                {t(object)} [
+                <strong>
+                  {tempSchema ? `${tempSchema}.` : ''}
+                  {tempTable}
+                </strong>
+                ] {t('was created')} &nbsp;
+                <ButtonGroup>
+                  <Button
+                    buttonSize="small"
+                    className="m-r-5"
+                    onClick={() => this.popSelectStar(tempSchema, tempTable)}
+                  >
+                    {t('Query in a new tab')}
+                  </Button>
+                  <ExploreCtasResultsButton
+                    // @ts-ignore Redux types are difficult to work with, ignoring for now
+                    actions={this.props.actions}
+                    table={tempTable}
+                    schema={tempSchema}
+                    dbId={exploreDBId}
+                  />
+                </ButtonGroup>
+              </>
+            }
           />
-        </>
+        </div>
       );
     }
-    if (data && data.length === 0) {
-      return <Alert type="warning" message={t('The query returned no data')} />;
+    if (query.state === 'success' && query.results) {
+      const { results } = query;
+      // Accounts for offset needed for height of ResultSetRowsReturned component if !limitReached
+      const rowMessageHeight = !limitReached ? 32 : 0;
+      // Accounts for offset needed for height of Alert if this.state.alertIsOpen
+      const alertContainerHeight = 70;
+      // We need to calculate the height of this.renderRowsReturned()
+      // if we want results panel to be propper height because the
+      // FilterTable component nedds an explcit height to render
+      // react-virtualized Table component
+      const height = this.state.alertIsOpen
+        ? this.props.height - alertContainerHeight
+        : this.props.height - rowMessageHeight;
+      let data;
+      if (this.props.cache && query.cached) {
+        ({ data } = this.state);
+      } else if (results && results.data) {
+        ({ data } = results);
+      }
+      if (data && data.length > 0) {
+        const expandedColumns = results.expanded_columns
+          ? results.expanded_columns.map(col => col.name)
+          : [];
+        return (
+          <>
+            {this.renderControls()}
+            {this.renderRowsReturned()}
+            {sql}
+            <FilterableTable
+              data={data}
+              orderedColumnKeys={results.columns.map(col => col.name)}
+              height={height}
+              filterText={this.state.searchText}
+              expandedColumns={expandedColumns}
+            />
+          </>
+        );
+      }
+      if (data && data.length === 0) {
+        return (
+          <Alert type="warning" message={t('The query returned no data')} />
+        );
+      }
     }
-  }
-
-  if (query.cached || (query.state === 'success' && !query.results)) {
-    if (query.isDataPreview) {
-      return (
-        <Button
-          buttonSize="small"
-          buttonStyle="primary"
-          onClick={() =>
-            dispatch(
-              reFetchQueryResults({
+    if (query.cached || (query.state === 'success' && !query.results)) {
+      if (query.isDataPreview) {
+        return (
+          <Button
+            buttonSize="small"
+            buttonStyle="primary"
+            onClick={() =>
+              this.reFetchQueryResults({
                 ...query,
                 isDataPreview: true,
-              }),
-            )
-          }
-        >
-          {t('Fetch data preview')}
-        </Button>
-      );
+              })
+            }
+          >
+            {t('Fetch data preview')}
+          </Button>
+        );
+      }
+      if (query.resultsKey) {
+        return (
+          <Button
+            buttonSize="small"
+            buttonStyle="primary"
+            onClick={() => this.fetchResults(query)}
+          >
+            {t('Refetch results')}
+          </Button>
+        );
+      }
     }
-    if (query.resultsKey) {
-      return (
-        <Button
-          buttonSize="small"
-          buttonStyle="primary"
-          onClick={() => fetchResults(query)}
-        >
-          {t('Refetch results')}
-        </Button>
+    let progressBar;
+    if (query.progress > 0) {
+      progressBar = (
+        <ProgressBar
+          percent={parseInt(query.progress.toFixed(0), 10)}
+          striped
+        />
       );
     }
-  }
+    const progressMsg =
+      query && query.extra && query.extra.progress
+        ? query.extra.progress
+        : null;
 
-  let progressBar;
-  if (query.progress > 0) {
-    progressBar = (
-      <ProgressBar percent={parseInt(query.progress.toFixed(0), 10)} striped />
+    return (
+      <ResultlessStyles>
+        <div>{!progressBar && <Loading position="normal" />}</div>
+        {/* show loading bar whenever progress bar is completed but needs time to render */}
+        <div>{query.progress === 100 && <Loading position="normal" />}</div>
+        <QueryStateLabel query={query} />
+        <div>
+          {progressMsg && <Alert type="success" message={progressMsg} />}
+        </div>
+        <div>{query.progress !== 100 && progressBar}</div>
+        {trackingUrl && <div>{trackingUrl}</div>}
+      </ResultlessStyles>
     );
   }
-
-  const progressMsg = query?.extra?.progress ?? null;
-
-  return (
-    <ResultlessStyles>
-      <div>{!progressBar && <Loading position="normal" />}</div>
-      {/* show loading bar whenever progress bar is completed but needs time to render */}
-      <div>{query.progress === 100 && <Loading position="normal" />}</div>
-      <QueryStateLabel query={query} />
-      <div>{progressMsg && <Alert type="success" message={progressMsg} />}</div>
-      <div>{query.progress !== 100 && progressBar}</div>
-      {trackingUrl && <div>{trackingUrl}</div>}
-    </ResultlessStyles>
-  );
-};
-
-export default ResultSet;
+}
diff --git a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx
index 2be88f6fe2..ddcd972f98 100644
--- a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx
+++ b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx
@@ -164,8 +164,10 @@ export default function SouthPane({
       if (Date.now() - latestQuery.startDttm <= LOCALSTORAGE_MAX_QUERY_AGE_MS) {
         results = (
           <ResultSet
+            showControls
             search
             query={latestQuery}
+            actions={actions}
             user={user}
             height={innerTabContentHeight + EXTRA_HEIGHT_RESULTS}
             database={databases[latestQuery.dbId]}
@@ -197,6 +199,7 @@ export default function SouthPane({
           query={query}
           visualize={false}
           csv={false}
+          actions={actions}
           cache
           user={user}
           height={innerTabContentHeight}
diff --git a/superset-frontend/src/components/FilterableTable/index.tsx b/superset-frontend/src/components/FilterableTable/index.tsx
index 16ae37e671..8324e93b90 100644
--- a/superset-frontend/src/components/FilterableTable/index.tsx
+++ b/superset-frontend/src/components/FilterableTable/index.tsx
@@ -578,7 +578,7 @@ const FilterableTable = ({
 
     // fix height of filterable table
     return (
-      <StyledFilterableTable data-test="grid-container">
+      <StyledFilterableTable>
         <ScrollSync>
           {({ onScroll, scrollLeft }) => (
             <>
@@ -659,7 +659,6 @@ const FilterableTable = ({
     return (
       <StyledFilterableTable
         className="filterable-table-container"
-        data-test="table-container"
         ref={container}
       >
         {fitted && (
diff --git a/superset-frontend/src/components/ProgressBar/index.tsx b/superset-frontend/src/components/ProgressBar/index.tsx
index ba69fc90c6..93b4315e52 100644
--- a/superset-frontend/src/components/ProgressBar/index.tsx
+++ b/superset-frontend/src/components/ProgressBar/index.tsx
@@ -27,7 +27,7 @@ export interface ProgressBarProps extends ProgressProps {
 
 // eslint-disable-next-line @typescript-eslint/no-unused-vars
 const ProgressBar = styled(({ striped, ...props }: ProgressBarProps) => (
-  <AntdProgress data-test="progress-bar" {...props} />
+  <AntdProgress {...props} />
 ))`
   line-height: 0;
   position: static;


[superset] 12/13: fix(database): Handle String errors in DatabaseModal (#21709)

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch 2022.39.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit e77978db7d04538fa38acc7a7f5b53320e1ef9d2
Author: Antonio Rivero Martinez <38...@users.noreply.github.com>
AuthorDate: Thu Oct 6 15:01:22 2022 -0300

    fix(database): Handle String errors in DatabaseModal (#21709)
    
    (cherry picked from commit 97273f59f867a9b329370b903e3616c24b43a5bc)
---
 .../data/database/DatabaseModal/index.test.jsx     | 91 ++++++++++++++++++++++
 .../CRUD/data/database/DatabaseModal/index.tsx     |  7 +-
 2 files changed, 97 insertions(+), 1 deletion(-)

diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx
index 2724dcce25..4fb5d60cdd 100644
--- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx
+++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx
@@ -26,6 +26,7 @@ import {
   cleanup,
   act,
 } from 'spec/helpers/testing-library';
+import * as hooks from 'src/views/CRUD/hooks';
 import DatabaseModal from './index';
 
 const dbProps = {
@@ -1269,4 +1270,94 @@ describe('DatabaseModal', () => {
       expect(schemasForFileUploadText).not.toBeInTheDocument();
     });
   });
+
+  describe('DatabaseModal w errors as objects', () => {
+    jest.mock('src/views/CRUD/hooks', () => ({
+      ...jest.requireActual('src/views/CRUD/hooks'),
+      useSingleViewResource: jest.fn(),
+    }));
+    const useSingleViewResourceMock = jest.spyOn(
+      hooks,
+      'useSingleViewResource',
+    );
+
+    useSingleViewResourceMock.mockReturnValue({
+      state: {
+        loading: false,
+        resource: null,
+        error: { _schema: 'Test Error With Object' },
+      },
+      fetchResource: jest.fn(),
+      createResource: jest.fn(),
+      updateResource: jest.fn(),
+      clearError: jest.fn(),
+    });
+
+    const renderAndWait = async () => {
+      const mounted = act(async () => {
+        render(<DatabaseModal {...dbProps} dbEngine="PostgreSQL" />, {
+          useRedux: true,
+        });
+      });
+
+      return mounted;
+    };
+
+    beforeEach(async () => {
+      await renderAndWait();
+    });
+
+    test('Error displays when it is an object', async () => {
+      const step2of3text = screen.getByText(/step 2 of 3/i);
+      const errorSection = screen.getByText(/Database Creation Error/i);
+      expect(step2of3text).toBeVisible();
+      expect(errorSection).toBeVisible();
+    });
+  });
+
+  describe('DatabaseModal w errors as strings', () => {
+    jest.mock('src/views/CRUD/hooks', () => ({
+      ...jest.requireActual('src/views/CRUD/hooks'),
+      useSingleViewResource: jest.fn(),
+    }));
+    const useSingleViewResourceMock = jest.spyOn(
+      hooks,
+      'useSingleViewResource',
+    );
+
+    useSingleViewResourceMock.mockReturnValue({
+      state: {
+        loading: false,
+        resource: null,
+        error: 'Test Error With String',
+      },
+      fetchResource: jest.fn(),
+      createResource: jest.fn(),
+      updateResource: jest.fn(),
+      clearError: jest.fn(),
+    });
+
+    const renderAndWait = async () => {
+      const mounted = act(async () => {
+        render(<DatabaseModal {...dbProps} dbEngine="PostgreSQL" />, {
+          useRedux: true,
+        });
+      });
+
+      return mounted;
+    };
+
+    beforeEach(async () => {
+      await renderAndWait();
+    });
+
+    test('Error displays when it is a string', async () => {
+      const step2of3text = screen.getByText(/step 2 of 3/i);
+      const errorTitleMessage = screen.getByText(/Database Creation Error/i);
+      const errorMessage = screen.getByText(/Test Error With String/i);
+      expect(step2of3text).toBeVisible();
+      expect(errorTitleMessage).toBeVisible();
+      expect(errorMessage).toBeVisible();
+    });
+  });
 });
diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
index 9fb55246ce..de5d750ebd 100644
--- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
+++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
@@ -1144,7 +1144,12 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
   const errorAlert = () => {
     let alertErrors: string[] = [];
     if (!isEmpty(dbErrors)) {
-      alertErrors = typeof dbErrors === 'object' ? Object.values(dbErrors) : [];
+      alertErrors =
+        typeof dbErrors === 'object'
+          ? Object.values(dbErrors)
+          : typeof dbErrors === 'string'
+          ? [dbErrors]
+          : [];
     } else if (!isEmpty(validationErrors)) {
       alertErrors =
         validationErrors?.error_type === 'GENERIC_DB_ENGINE_ERROR'


[superset] 06/13: fix: Allow clickhouse dbs with timestamps to visualize queries (#21446)

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch 2022.39.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 22fac9e7e6bbeff08a35ed370b30cae57be6916d
Author: Hugh A. Miles II <hu...@gmail.com>
AuthorDate: Mon Oct 3 14:22:20 2022 -0400

    fix: Allow clickhouse dbs with timestamps to visualize queries (#21446)
    
    (cherry picked from commit 4d0c2ba6ef3f8ca7479cf46383ddac9470aa3329)
---
 superset/models/helpers.py | 41 +++++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index c81d268a1e..da526b559c 100644
--- a/superset/models/helpers.py
+++ b/superset/models/helpers.py
@@ -1570,26 +1570,31 @@ class ExploreMixin:  # pylint: disable=too-many-public-methods
                 groupby_all_columns[timestamp.name] = timestamp
 
             # Use main dttm column to support index with secondary dttm columns.
-            if (
-                db_engine_spec.time_secondary_columns
-                and self.main_dttm_col in self.dttm_cols
-                and self.main_dttm_col != dttm_col.column_name
-            ):
-                if isinstance(self.main_dttm_col, dict):
-                    time_filters.append(
-                        self.get_time_filter(
-                            self.main_dttm_col,
-                            from_dttm,
-                            to_dttm,
-                        )
-                    )
+            if db_engine_spec.time_secondary_columns:
+                if isinstance(dttm_col, dict):
+                    dttm_col_name = dttm_col.get("column_name")
                 else:
-                    time_filters.append(
-                        columns_by_name[self.main_dttm_col].get_time_filter(
-                            from_dttm,
-                            to_dttm,
+                    dttm_col_name = dttm_col.column_name
+
+                if (
+                    self.main_dttm_col in self.dttm_cols
+                    and self.main_dttm_col != dttm_col_name
+                ):
+                    if isinstance(self.main_dttm_col, dict):
+                        time_filters.append(
+                            self.get_time_filter(
+                                self.main_dttm_col,
+                                from_dttm,
+                                to_dttm,
+                            )
+                        )
+                    else:
+                        time_filters.append(
+                            columns_by_name[self.main_dttm_col].get_time_filter(
+                                from_dttm,
+                                to_dttm,
+                            )
                         )
-                    )
 
             if isinstance(dttm_col, dict):
                 time_filters.append(self.get_time_filter(dttm_col, from_dttm, to_dttm))


[superset] 08/13: chore: add 4xx error codes where applicable (#21627)

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch 2022.39.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 777e22f720c5455fe726e618669155a5c70e0ddd
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Mon Oct 3 15:19:47 2022 -0700

    chore: add 4xx error codes where applicable (#21627)
    
    (cherry picked from commit 4417c6e3e27d61d174d137c5afae4f4fb723382c)
---
 superset/reports/commands/exceptions.py           | 18 ++++++++++++++++--
 superset/reports/commands/execute.py              | 23 ++++++++++++-----------
 tests/integration_tests/reports/commands_tests.py |  5 +++--
 3 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/superset/reports/commands/exceptions.py b/superset/reports/commands/exceptions.py
index 1f52aab8d8..c087cf95e0 100644
--- a/superset/reports/commands/exceptions.py
+++ b/superset/reports/commands/exceptions.py
@@ -119,6 +119,7 @@ class DashboardNotSavedValidationError(ValidationError):
 
 
 class ReportScheduleInvalidError(CommandInvalidError):
+    status = 422
     message = _("Report Schedule parameters are invalid.")
 
 
@@ -135,6 +136,7 @@ class ReportScheduleUpdateFailedError(CreateFailedError):
 
 
 class ReportScheduleNotFoundError(CommandException):
+    status = 404
     message = _("Report Schedule not found.")
 
 
@@ -163,10 +165,12 @@ class ReportScheduleExecuteUnexpectedError(CommandException):
 
 
 class ReportSchedulePreviousWorkingError(CommandException):
+    status = 429
     message = _("Report Schedule is still working, refusing to re-compute.")
 
 
 class ReportScheduleWorkingTimeoutError(CommandException):
+    status = 408
     message = _("Report Schedule reached a working timeout.")
 
 
@@ -188,20 +192,22 @@ class ReportScheduleCreationMethodUniquenessValidationError(CommandException):
 
 
 class AlertQueryMultipleRowsError(CommandException):
-
+    status = 422
     message = _("Alert query returned more than one row.")
 
 
 class AlertValidatorConfigError(CommandException):
-
+    status = 422
     message = _("Alert validator config error.")
 
 
 class AlertQueryMultipleColumnsError(CommandException):
+    status = 422
     message = _("Alert query returned more than one column.")
 
 
 class AlertQueryInvalidTypeError(CommandException):
+    status = 422
     message = _("Alert query returned a non-number value.")
 
 
@@ -210,30 +216,37 @@ class AlertQueryError(CommandException):
 
 
 class AlertQueryTimeout(CommandException):
+    status = 408
     message = _("A timeout occurred while executing the query.")
 
 
 class ReportScheduleScreenshotTimeout(CommandException):
+    status = 408
     message = _("A timeout occurred while taking a screenshot.")
 
 
 class ReportScheduleCsvTimeout(CommandException):
+    status = 408
     message = _("A timeout occurred while generating a csv.")
 
 
 class ReportScheduleDataFrameTimeout(CommandException):
+    status = 408
     message = _("A timeout occurred while generating a dataframe.")
 
 
 class ReportScheduleAlertGracePeriodError(CommandException):
+    status = 429
     message = _("Alert fired during grace period.")
 
 
 class ReportScheduleAlertEndGracePeriodError(CommandException):
+    status = 429
     message = _("Alert ended grace period.")
 
 
 class ReportScheduleNotificationError(CommandException):
+    status = 429
     message = _("Alert on grace period")
 
 
@@ -250,6 +263,7 @@ class ReportScheduleUnexpectedError(CommandException):
 
 
 class ReportScheduleForbiddenError(ForbiddenError):
+    status = 403
     message = _("Changing this report is forbidden")
 
 
diff --git a/superset/reports/commands/execute.py b/superset/reports/commands/execute.py
index 1fbdba1c25..52ff1cd4e7 100644
--- a/superset/reports/commands/execute.py
+++ b/superset/reports/commands/execute.py
@@ -42,7 +42,6 @@ from superset.reports.commands.exceptions import (
     ReportScheduleDataFrameTimeout,
     ReportScheduleExecuteUnexpectedError,
     ReportScheduleNotFoundError,
-    ReportScheduleNotificationError,
     ReportSchedulePreviousWorkingError,
     ReportScheduleScreenshotFailedError,
     ReportScheduleScreenshotTimeout,
@@ -399,9 +398,9 @@ class BaseReportState:
         """
         Sends a notification to all recipients
 
-        :raises: ReportScheduleNotificationError
+        :raises: NotificationError
         """
-        notification_errors = []
+        notification_errors: List[NotificationError] = []
         for recipient in recipients:
             notification = create_notification(recipient, notification_content)
             try:
@@ -415,15 +414,17 @@ class BaseReportState:
                     notification.send()
             except NotificationError as ex:
                 # collect notification errors but keep processing them
-                notification_errors.append(str(ex))
+                notification_errors.append(ex)
         if notification_errors:
-            raise ReportScheduleNotificationError(";".join(notification_errors))
+            # raise errors separately so that we can utilize error status codes
+            for error in notification_errors:
+                raise error
 
     def send(self) -> None:
         """
         Creates the notification content and sends them to all recipients
 
-        :raises: ReportScheduleNotificationError
+        :raises: NotificationError
         """
         notification_content = self._get_notification_content()
         self._send(notification_content, self._report_schedule.recipients)
@@ -432,7 +433,7 @@ class BaseReportState:
         """
         Creates and sends a notification for an error, to all recipients
 
-        :raises: ReportScheduleNotificationError
+        :raises: NotificationError
         """
         header_data = self._get_log_data()
         header_data["error_text"] = message
@@ -527,7 +528,7 @@ class ReportNotTriggeredErrorState(BaseReportState):
                     return
             self.send()
             self.update_report_schedule_and_log(ReportState.SUCCESS)
-        except CommandException as first_ex:
+        except Exception as first_ex:
             self.update_report_schedule_and_log(
                 ReportState.ERROR, error_message=str(first_ex)
             )
@@ -543,7 +544,7 @@ class ReportNotTriggeredErrorState(BaseReportState):
                         ReportState.ERROR,
                         error_message=REPORT_SCHEDULE_ERROR_NOTIFICATION_MARKER,
                     )
-                except CommandException as second_ex:
+                except Exception as second_ex:  # pylint: disable=broad-except
                     self.update_report_schedule_and_log(
                         ReportState.ERROR, error_message=str(second_ex)
                     )
@@ -600,7 +601,7 @@ class ReportSuccessState(BaseReportState):
                 if not AlertCommand(self._report_schedule).run():
                     self.update_report_schedule_and_log(ReportState.NOOP)
                     return
-            except CommandException as ex:
+            except Exception as ex:
                 self.send_error(
                     f"Error occurred for {self._report_schedule.type}:"
                     f" {self._report_schedule.name}",
@@ -615,7 +616,7 @@ class ReportSuccessState(BaseReportState):
         try:
             self.send()
             self.update_report_schedule_and_log(ReportState.SUCCESS)
-        except CommandException as ex:
+        except Exception as ex:  # pylint: disable=broad-except
             self.update_report_schedule_and_log(
                 ReportState.ERROR, error_message=str(ex)
             )
diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py
index 12b0a19592..02e5ce8bb4 100644
--- a/tests/integration_tests/reports/commands_tests.py
+++ b/tests/integration_tests/reports/commands_tests.py
@@ -39,10 +39,10 @@ from superset.reports.commands.exceptions import (
     ReportScheduleCsvFailedError,
     ReportScheduleCsvTimeout,
     ReportScheduleNotFoundError,
-    ReportScheduleNotificationError,
     ReportSchedulePreviousWorkingError,
     ReportScheduleScreenshotFailedError,
     ReportScheduleScreenshotTimeout,
+    ReportScheduleUnexpectedError,
     ReportScheduleWorkingTimeoutError,
 )
 from superset.reports.commands.execute import AsyncExecuteReportScheduleCommand
@@ -113,6 +113,7 @@ def assert_log(state: str, error_message: Optional[str] = None):
 
     if state == ReportState.ERROR:
         # On error we send an email
+        print(logs)
         assert len(logs) == 3
     else:
         assert len(logs) == 2
@@ -1321,7 +1322,7 @@ def test_email_dashboard_report_fails(
     screenshot_mock.return_value = SCREENSHOT_FILE
     email_mock.side_effect = SMTPException("Could not connect to SMTP XPTO")
 
-    with pytest.raises(ReportScheduleNotificationError):
+    with pytest.raises(ReportScheduleUnexpectedError):
         AsyncExecuteReportScheduleCommand(
             TEST_ID, create_report_email_dashboard.id, datetime.utcnow()
         ).run()


[superset] 11/13: fix: add `get_column` function for Query obj (#21691)

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch 2022.39.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 42304a4302d46a8f9e7a9c397cf96d77fdfa3d87
Author: Hugh A. Miles II <hu...@gmail.com>
AuthorDate: Wed Oct 5 18:25:44 2022 -0400

    fix: add `get_column` function for Query obj (#21691)
    
    (cherry picked from commit 51c54b3c9bc69273bb5da004b8f9a7ae202de8fd)
---
 superset/common/query_context_processor.py |  3 ++-
 superset/models/sql_lab.py                 | 15 +++++++++++----
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/superset/common/query_context_processor.py b/superset/common/query_context_processor.py
index 98aaecebd9..01259ede1d 100644
--- a/superset/common/query_context_processor.py
+++ b/superset/common/query_context_processor.py
@@ -266,7 +266,8 @@ class QueryContextProcessor:
             # Query datasource didn't support `get_column`
             and hasattr(datasource, "get_column")
             and (col := datasource.get_column(label))
-            and col.is_dttm
+            # todo(hugh) standardize column object in Query datasource
+            and (col.get("is_dttm") if isinstance(col, dict) else col.is_dttm)
         )
         dttm_cols = [
             DateColumn(
diff --git a/superset/models/sql_lab.py b/superset/models/sql_lab.py
index 408bc708df..f75973ad17 100644
--- a/superset/models/sql_lab.py
+++ b/superset/models/sql_lab.py
@@ -51,7 +51,6 @@ from superset.models.helpers import (
 )
 from superset.sql_parse import CtasMethod, ParsedQuery, Table
 from superset.sqllab.limiting_factor import LimitingFactor
-from superset.superset_typing import ResultSetColumnType
 from superset.utils.core import GenericDataType, QueryStatus, user_label
 
 if TYPE_CHECKING:
@@ -183,7 +182,7 @@ class Query(
         return list(ParsedQuery(self.sql).tables)
 
     @property
-    def columns(self) -> List[ResultSetColumnType]:
+    def columns(self) -> List[Dict[str, Any]]:
         bool_types = ("BOOL",)
         num_types = (
             "DOUBLE",
@@ -217,7 +216,7 @@ class Query(
             computed_column["column_name"] = col.get("name")
             computed_column["groupby"] = True
             columns.append(computed_column)
-        return columns  # type: ignore
+        return columns
 
     @property
     def data(self) -> Dict[str, Any]:
@@ -288,7 +287,7 @@ class Query(
     def main_dttm_col(self) -> Optional[str]:
         for col in self.columns:
             if col.get("is_dttm"):
-                return col.get("column_name")  # type: ignore
+                return col.get("column_name")
         return None
 
     @property
@@ -332,6 +331,14 @@ class Query(
     def tracking_url(self, value: str) -> None:
         self.tracking_url_raw = value
 
+    def get_column(self, column_name: Optional[str]) -> Optional[Dict[str, Any]]:
+        if not column_name:
+            return None
+        for col in self.columns:
+            if col.get("column_name") == column_name:
+                return col
+        return None
+
 
 class SavedQuery(Model, AuditMixinNullable, ExtraJSONMixin, ImportExportMixin):
     """ORM model for SQL query"""


[superset] 13/13: fix conflict with cherry 882bfb67a

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch 2022.39.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 2af1000492999ec4461b816560d975dd5117855b
Author: Hugh A. Miles II <hu...@gmail.com>
AuthorDate: Thu Oct 6 22:57:12 2022 -0400

    fix conflict with cherry 882bfb67a
---
 .../CRUD/data/database/DatabaseModal/index.tsx     | 17 +++++++-
 superset/databases/schemas.py                      |  5 +++
 superset/db_engine_specs/gsheets.py                | 19 +++++++--
 tests/unit_tests/db_engine_specs/test_gsheets.py   | 48 ++++++++++++++++++----
 4 files changed, 76 insertions(+), 13 deletions(-)

diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
index de5d750ebd..a2ca444752 100644
--- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
+++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
@@ -569,6 +569,18 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
     // Clone DB object
     const dbToUpdate = JSON.parse(JSON.stringify(db || {}));
 
+    if (dbToUpdate.catalog) {
+      // convert catalog to fit /validate_parameters endpoint
+      dbToUpdate.catalog = Object.assign(
+        {},
+        ...dbToUpdate.catalog.map((x: { name: string; value: string }) => ({
+          [x.name]: x.value,
+        })),
+      );
+    } else {
+      dbToUpdate.catalog = {};
+    }
+
     if (dbToUpdate.configuration_method === CONFIGURATION_METHOD.DYNAMIC_FORM) {
       // Validate DB before saving
       const errors = await getValidation(dbToUpdate, true);
@@ -734,7 +746,10 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
       });
     }
 
-    setDB({ type: ActionType.addTableCatalogSheet });
+    if (database_name === 'Google Sheets') {
+      // only create a catalog if the DB is Google Sheets
+      setDB({ type: ActionType.addTableCatalogSheet });
+    }
   };
 
   const renderAvailableSelector = () => (
diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py
index 201e35cbfc..dafd1ba7fc 100644
--- a/superset/databases/schemas.py
+++ b/superset/databases/schemas.py
@@ -315,6 +315,11 @@ class DatabaseValidateParametersSchema(Schema):
         values=fields.Raw(allow_none=True),
         description="DB-specific parameters for configuration",
     )
+    catalog = fields.Dict(
+        keys=fields.String(),
+        values=fields.Raw(allow_none=True),
+        description="Gsheets specific column for managing label to sheet urls",
+    )
     database_name = fields.String(
         description=database_name_description,
         allow_none=True,
diff --git a/superset/db_engine_specs/gsheets.py b/superset/db_engine_specs/gsheets.py
index 3562e0d0b1..b42b26c3bc 100644
--- a/superset/db_engine_specs/gsheets.py
+++ b/superset/db_engine_specs/gsheets.py
@@ -55,6 +55,11 @@ class GSheetsParametersSchema(Schema):
 
 class GSheetsParametersType(TypedDict):
     service_account_info: str
+    catalog: Optional[Dict[str, str]]
+
+
+class GSheetsPropertiesType(TypedDict):
+    parameters: GSheetsParametersType
     catalog: Dict[str, str]
 
 
@@ -208,9 +213,18 @@ class GSheetsEngineSpec(SqliteEngineSpec):
     @classmethod
     def validate_parameters(
         cls,
-        parameters: GSheetsParametersType,
+        properties: GSheetsParametersType,
     ) -> List[SupersetError]:
         errors: List[SupersetError] = []
+
+        # backwards compatible just incase people are send data
+        # via parameters for validation
+        parameters = properties.get("parameters", {})
+        if parameters and parameters.get("catalog"):
+            table_catalog = parameters.get("catalog", {})
+        else:
+            table_catalog = properties.get("catalog", {})
+
         encrypted_credentials = parameters.get("service_account_info") or "{}"
 
         # On create the encrypted credentials are a string,
@@ -218,8 +232,6 @@ class GSheetsEngineSpec(SqliteEngineSpec):
         if isinstance(encrypted_credentials, str):
             encrypted_credentials = json.loads(encrypted_credentials)
 
-        table_catalog = parameters.get("catalog", {})
-
         if not table_catalog:
             # Allowing users to submit empty catalogs
             errors.append(
@@ -245,6 +257,7 @@ class GSheetsEngineSpec(SqliteEngineSpec):
         )
         conn = engine.connect()
         idx = 0
+
         for name, url in table_catalog.items():
 
             if not name:
diff --git a/tests/unit_tests/db_engine_specs/test_gsheets.py b/tests/unit_tests/db_engine_specs/test_gsheets.py
index a226653ed5..4a0092b4f7 100644
--- a/tests/unit_tests/db_engine_specs/test_gsheets.py
+++ b/tests/unit_tests/db_engine_specs/test_gsheets.py
@@ -33,11 +33,38 @@ class ProgrammingError(Exception):
 def test_validate_parameters_simple() -> None:
     from superset.db_engine_specs.gsheets import (
         GSheetsEngineSpec,
-        GSheetsParametersType,
+        GSheetsPropertiesType,
     )
 
-    parameters: GSheetsParametersType = {
-        "service_account_info": "",
+    properties: GSheetsPropertiesType = {
+        "parameters": {
+            "service_account_info": "",
+            "catalog": {},
+        },
+        "catalog": {},
+    }
+    errors = GSheetsEngineSpec.validate_parameters(properties)
+    assert errors == [
+        SupersetError(
+            message="Sheet name is required",
+            error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR,
+            level=ErrorLevel.WARNING,
+            extra={"catalog": {"idx": 0, "name": True}},
+        ),
+    ]
+
+
+def test_validate_parameters_simple_with_in_root_catalog() -> None:
+    from superset.db_engine_specs.gsheets import (
+        GSheetsEngineSpec,
+        GSheetsPropertiesType,
+    )
+
+    properties: GSheetsPropertiesType = {
+        "parameters": {
+            "service_account_info": "",
+            "catalog": {},
+        },
         "catalog": {},
     }
     errors = GSheetsEngineSpec.validate_parameters(parameters)
@@ -56,7 +83,7 @@ def test_validate_parameters_catalog(
 ) -> None:
     from superset.db_engine_specs.gsheets import (
         GSheetsEngineSpec,
-        GSheetsParametersType,
+        GSheetsPropertiesType,
     )
 
     g = mocker.patch("superset.db_engine_specs.gsheets.g")
@@ -71,8 +98,8 @@ def test_validate_parameters_catalog(
         ProgrammingError("Unsupported table: https://www.google.com/"),
     ]
 
-    parameters: GSheetsParametersType = {
-        "service_account_info": "",
+    properties: GSheetsPropertiesType = {
+        "parameters": {"service_account_info": "", "catalog": None},
         "catalog": {
             "private_sheet": "https://docs.google.com/spreadsheets/d/1/edit",
             "public_sheet": "https://docs.google.com/spreadsheets/d/1/edit#gid=1",
@@ -146,7 +173,7 @@ def test_validate_parameters_catalog_and_credentials(
 ) -> None:
     from superset.db_engine_specs.gsheets import (
         GSheetsEngineSpec,
-        GSheetsParametersType,
+        GSheetsPropertiesType,
     )
 
     g = mocker.patch("superset.db_engine_specs.gsheets.g")
@@ -161,8 +188,11 @@ def test_validate_parameters_catalog_and_credentials(
         ProgrammingError("Unsupported table: https://www.google.com/"),
     ]
 
-    parameters: GSheetsParametersType = {
-        "service_account_info": "",
+    properties: GSheetsPropertiesType = {
+        "parameters": {
+            "service_account_info": "",
+            "catalog": None,
+        },
         "catalog": {
             "private_sheet": "https://docs.google.com/spreadsheets/d/1/edit",
             "public_sheet": "https://docs.google.com/spreadsheets/d/1/edit#gid=1",


[superset] 01/13: Fixing tags package

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch 2022.39.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit cc8c7d450626765a854315c1c6b201004b687278
Author: Craig <cr...@craigrueda.com>
AuthorDate: Mon Sep 26 13:44:25 2022 -0700

    Fixing tags package
---
 superset/tags/__init__.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/superset/tags/__init__.py b/superset/tags/__init__.py
new file mode 100644
index 0000000000..13a83393a9
--- /dev/null
+++ b/superset/tags/__init__.py
@@ -0,0 +1,16 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.


[superset] 07/13: fix: add logging to alerts and reports to find non-triggering issues (#21684)

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch 2022.39.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 08cd63eb2bfdd9891a7065ae16eeadc9614d7f5b
Author: Phillip Kelley-Dotson <pk...@yahoo.com>
AuthorDate: Mon Oct 3 13:24:32 2022 -0700

    fix: add logging to alerts and reports to find non-triggering issues (#21684)
    
    (cherry picked from commit 84c3cf66ea0858f7dd7ae1a1fca7260cec076bf6)
---
 superset/reports/commands/execute.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/superset/reports/commands/execute.py b/superset/reports/commands/execute.py
index 90c5040cc1..1fbdba1c25 100644
--- a/superset/reports/commands/execute.py
+++ b/superset/reports/commands/execute.py
@@ -436,6 +436,7 @@ class BaseReportState:
         """
         header_data = self._get_log_data()
         header_data["error_text"] = message
+        logger.info("header_data info %s", header_data)
         notification_content = NotificationContent(
             name=name, text=message, header_data=header_data
         )


[superset] 05/13: Revert "chore: refactor SqlEditor to functional component (#21320)"

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch 2022.39.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 2c452903f3922d99b408c71d27cda51368c067b2
Author: Joe Li <jo...@preset.io>
AuthorDate: Mon Oct 3 10:36:11 2022 -0700

    Revert "chore: refactor SqlEditor to functional component (#21320)"
    
    This reverts commit 2224ebecfe7af0a4fa3736c33d697f6f41a07e46.
---
 .../src/SqlLab/components/SqlEditor/index.jsx      | 888 ++++++++++++---------
 superset-frontend/src/SqlLab/constants.ts          |   6 -
 2 files changed, 496 insertions(+), 398 deletions(-)

diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx
index d6947c1ba5..c48594d304 100644
--- a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx
+++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx
@@ -18,15 +18,16 @@
  */
 /* eslint-disable jsx-a11y/anchor-is-valid */
 /* eslint-disable jsx-a11y/no-static-element-interactions */
-import React, { useState, useEffect, useMemo, useRef } from 'react';
+import React from 'react';
 import { CSSTransition } from 'react-transition-group';
-import { useDispatch, useSelector } from 'react-redux';
+import { connect } from 'react-redux';
+import { bindActionCreators } from 'redux';
 import PropTypes from 'prop-types';
 import Split from 'react-split';
-import { t, styled, useTheme } from '@superset-ui/core';
+import { t, styled, withTheme } from '@superset-ui/core';
 import debounce from 'lodash/debounce';
 import throttle from 'lodash/throttle';
-import Modal from 'src/components/Modal';
+import StyledModal from 'src/components/Modal';
 import Mousetrap from 'mousetrap';
 import Button from 'src/components/Button';
 import Timer from 'src/components/Timer';
@@ -47,6 +48,7 @@ import {
   queryEditorSetAndSaveSql,
   queryEditorSetTemplateParams,
   runQueryFromSqlEditor,
+  runQuery,
   saveQuery,
   addSavedQueryToTabState,
   scheduleQuery,
@@ -60,12 +62,6 @@ import {
   SQL_EDITOR_GUTTER_MARGIN,
   SQL_TOOLBAR_HEIGHT,
   SQL_EDITOR_LEFTBAR_WIDTH,
-  SQL_EDITOR_PADDING,
-  INITIAL_NORTH_PERCENT,
-  INITIAL_SOUTH_PERCENT,
-  SET_QUERY_EDITOR_SQL_DEBOUNCE_MS,
-  VALIDATION_DEBOUNCE_MS,
-  WINDOW_RESIZE_THROTTLE_MS,
 } from 'src/SqlLab/constants';
 import {
   getItem,
@@ -87,6 +83,13 @@ import RunQueryActionButton from '../RunQueryActionButton';
 import { newQueryTabName } from '../../utils/newQueryTabName';
 import QueryLimitSelect from '../QueryLimitSelect';
 
+const SQL_EDITOR_PADDING = 10;
+const INITIAL_NORTH_PERCENT = 30;
+const INITIAL_SOUTH_PERCENT = 70;
+const SET_QUERY_EDITOR_SQL_DEBOUNCE_MS = 2000;
+const VALIDATION_DEBOUNCE_MS = 600;
+const WINDOW_RESIZE_THROTTLE_MS = 100;
+
 const appContainer = document.getElementById('app');
 const bootstrapData = JSON.parse(
   appContainer.getAttribute('data-bootstrap') || '{}',
@@ -129,7 +132,7 @@ const StyledToolbar = styled.div`
 const StyledSidebar = styled.div`
   flex: 0 0 ${({ width }) => width}px;
   width: ${({ width }) => width}px;
-  padding: ${({ theme, hide }) => (hide ? 0 : theme.gridUnit * 2.5)}px;
+  padding: ${({ hide }) => (hide ? 0 : 10)}px;
   border-right: 1px solid
     ${({ theme, hide }) =>
       hide ? 'transparent' : theme.colors.grayscale.light2};
@@ -137,10 +140,13 @@ const StyledSidebar = styled.div`
 
 const propTypes = {
   actions: PropTypes.object.isRequired,
+  database: PropTypes.object,
+  latestQuery: PropTypes.object,
   tables: PropTypes.array.isRequired,
   editorQueries: PropTypes.array.isRequired,
   dataPreviewQueries: PropTypes.array.isRequired,
   queryEditor: PropTypes.object.isRequired,
+  hideLeftBar: PropTypes.bool,
   defaultQueryLimit: PropTypes.number.isRequired,
   maxRow: PropTypes.number.isRequired,
   displayLimit: PropTypes.number.isRequired,
@@ -148,97 +154,158 @@ const propTypes = {
   scheduleQueryWarning: PropTypes.string,
 };
 
-const SqlEditor = ({
-  actions,
-  tables,
-  editorQueries,
-  dataPreviewQueries,
-  queryEditor,
-  defaultQueryLimit,
-  maxRow,
-  displayLimit,
-  saveQueryWarning,
-  scheduleQueryWarning = null,
-}) => {
-  const theme = useTheme();
-  const dispatch = useDispatch();
-
-  const { database, latestQuery, hideLeftBar } = useSelector(
-    ({ sqlLab: { unsavedQueryEditor, databases, queries } }) => {
-      let { dbId, latestQueryId, hideLeftBar } = queryEditor;
-      if (unsavedQueryEditor.id === queryEditor.id) {
-        dbId = unsavedQueryEditor.dbId || dbId;
-        latestQueryId = unsavedQueryEditor.latestQueryId || latestQueryId;
-        hideLeftBar = unsavedQueryEditor.hideLeftBar || hideLeftBar;
-      }
-      return {
-        database: databases[dbId],
-        latestQuery: queries[latestQueryId],
-        hideLeftBar,
-      };
-    },
-  );
+const defaultProps = {
+  database: null,
+  latestQuery: null,
+  hideLeftBar: false,
+  scheduleQueryWarning: null,
+};
 
-  const queryEditors = useSelector(({ sqlLab }) => sqlLab.queryEditors);
+class SqlEditor extends React.PureComponent {
+  constructor(props) {
+    super(props);
+    this.state = {
+      autorun: props.queryEditor.autorun,
+      ctas: '',
+      northPercent: props.queryEditor.northPercent || INITIAL_NORTH_PERCENT,
+      southPercent: props.queryEditor.southPercent || INITIAL_SOUTH_PERCENT,
+      autocompleteEnabled: getItem(
+        LocalStorageKeys.sqllab__is_autocomplete_enabled,
+        true,
+      ),
+      showCreateAsModal: false,
+      createAs: '',
+      showEmptyState: false,
+    };
+    this.sqlEditorRef = React.createRef();
+    this.northPaneRef = React.createRef();
+
+    this.elementStyle = this.elementStyle.bind(this);
+    this.onResizeStart = this.onResizeStart.bind(this);
+    this.onResizeEnd = this.onResizeEnd.bind(this);
+    this.canValidateQuery = this.canValidateQuery.bind(this);
+    this.runQuery = this.runQuery.bind(this);
+    this.setEmptyState = this.setEmptyState.bind(this);
+    this.stopQuery = this.stopQuery.bind(this);
+    this.saveQuery = this.saveQuery.bind(this);
+    this.onSqlChanged = this.onSqlChanged.bind(this);
+    this.setQueryEditorAndSaveSql = this.setQueryEditorAndSaveSql.bind(this);
+    this.setQueryEditorAndSaveSqlWithDebounce = debounce(
+      this.setQueryEditorAndSaveSql.bind(this),
+      SET_QUERY_EDITOR_SQL_DEBOUNCE_MS,
+    );
+    this.queryPane = this.queryPane.bind(this);
+    this.getHotkeyConfig = this.getHotkeyConfig.bind(this);
+    this.getAceEditorAndSouthPaneHeights =
+      this.getAceEditorAndSouthPaneHeights.bind(this);
+    this.getSqlEditorHeight = this.getSqlEditorHeight.bind(this);
+    this.requestValidation = debounce(
+      this.requestValidation.bind(this),
+      VALIDATION_DEBOUNCE_MS,
+    );
+    this.getQueryCostEstimate = this.getQueryCostEstimate.bind(this);
+    this.handleWindowResize = throttle(
+      this.handleWindowResize.bind(this),
+      WINDOW_RESIZE_THROTTLE_MS,
+    );
 
-  const [height, setHeight] = useState(0);
-  const [autorun, setAutorun] = useState(queryEditor.autorun);
-  const [ctas, setCtas] = useState('');
-  const [northPercent, setNorthPercent] = useState(
-    queryEditor.northPercent || INITIAL_NORTH_PERCENT,
-  );
-  const [southPercent, setSouthPercent] = useState(
-    queryEditor.southPercent || INITIAL_SOUTH_PERCENT,
-  );
-  const [autocompleteEnabled, setAutocompleteEnabled] = useState(
-    getItem(LocalStorageKeys.sqllab__is_autocomplete_enabled, true),
-  );
-  const [showCreateAsModal, setShowCreateAsModal] = useState(false);
-  const [createAs, setCreateAs] = useState('');
-  const [showEmptyState, setShowEmptyState] = useState(false);
+    this.onBeforeUnload = this.onBeforeUnload.bind(this);
+    this.renderDropdown = this.renderDropdown.bind(this);
+  }
 
-  const sqlEditorRef = useRef(null);
-  const northPaneRef = useRef(null);
+  UNSAFE_componentWillMount() {
+    if (this.state.autorun) {
+      this.setState({ autorun: false });
+      this.props.queryEditorSetAutorun(this.props.queryEditor, false);
+      this.startQuery();
+    }
+  }
 
-  const startQuery = (ctasArg = false, ctas_method = CtasEnum.TABLE) => {
-    if (!database) {
-      return;
+  componentDidMount() {
+    // We need to measure the height of the sql editor post render to figure the height of
+    // the south pane so it gets rendered properly
+    // eslint-disable-next-line react/no-did-mount-set-state
+    const db = this.props.database;
+    this.setState({ height: this.getSqlEditorHeight() });
+    if (!db || isEmpty(db)) {
+      this.setEmptyState(true);
     }
 
-    dispatch(
-      runQueryFromSqlEditor(
-        database,
-        queryEditor,
-        defaultQueryLimit,
-        ctasArg ? ctas : '',
-        ctasArg,
-        ctas_method,
-      ),
-    );
-    dispatch(setActiveSouthPaneTab('Results'));
-  };
+    window.addEventListener('resize', this.handleWindowResize);
+    window.addEventListener('beforeunload', this.onBeforeUnload);
 
-  const stopQuery = () => {
-    if (latestQuery && ['running', 'pending'].indexOf(latestQuery.state) >= 0) {
-      dispatch(postStopQuery(latestQuery));
+    // setup hotkeys
+    const hotkeys = this.getHotkeyConfig();
+    hotkeys.forEach(keyConfig => {
+      Mousetrap.bind([keyConfig.key], keyConfig.func);
+    });
+  }
+
+  componentWillUnmount() {
+    window.removeEventListener('resize', this.handleWindowResize);
+    window.removeEventListener('beforeunload', this.onBeforeUnload);
+  }
+
+  onResizeStart() {
+    // Set the heights on the ace editor and the ace content area after drag starts
+    // to smooth out the visual transition to the new heights when drag ends
+    document.getElementsByClassName('ace_content')[0].style.height = '100%';
+  }
+
+  onResizeEnd([northPercent, southPercent]) {
+    this.setState({ northPercent, southPercent });
+
+    if (this.northPaneRef.current && this.northPaneRef.current.clientHeight) {
+      this.props.persistEditorHeight(
+        this.props.queryEditor,
+        northPercent,
+        southPercent,
+      );
     }
-  };
+  }
+
+  onBeforeUnload(event) {
+    if (
+      this.props.database?.extra_json?.cancel_query_on_windows_unload &&
+      this.props.latestQuery?.state === 'running'
+    ) {
+      event.preventDefault();
+      this.stopQuery();
+    }
+  }
 
-  useState(() => {
-    if (autorun) {
-      setAutorun(false);
-      dispatch(queryEditorSetAutorun(queryEditor, false));
-      startQuery();
+  onSqlChanged(sql) {
+    this.props.queryEditorSetSql(this.props.queryEditor, sql);
+    this.setQueryEditorAndSaveSqlWithDebounce(sql);
+    // Request server-side validation of the query text
+    if (this.canValidateQuery()) {
+      // NB. requestValidation is debounced
+      this.requestValidation(sql);
     }
-  });
+  }
 
   // One layer of abstraction for easy spying in unit tests
-  const getSqlEditorHeight = () =>
-    sqlEditorRef.current
-      ? sqlEditorRef.current.clientHeight - SQL_EDITOR_PADDING * 2
+  getSqlEditorHeight() {
+    return this.sqlEditorRef.current
+      ? this.sqlEditorRef.current.clientHeight - SQL_EDITOR_PADDING * 2
       : 0;
+  }
 
-  const getHotkeyConfig = () => {
+  // Return the heights for the ace editor and the south pane as an object
+  // given the height of the sql editor, north pane percent and south pane percent.
+  getAceEditorAndSouthPaneHeights(height, northPercent, southPercent) {
+    return {
+      aceEditorHeight:
+        (height * northPercent) / 100 -
+        (SQL_EDITOR_GUTTER_HEIGHT / 2 + SQL_EDITOR_GUTTER_MARGIN) -
+        SQL_TOOLBAR_HEIGHT,
+      southPaneHeight:
+        (height * southPercent) / 100 -
+        (SQL_EDITOR_GUTTER_HEIGHT / 2 + SQL_EDITOR_GUTTER_MARGIN),
+    };
+  }
+
+  getHotkeyConfig() {
     // Get the user's OS
     const userOS = detectOS();
 
@@ -248,8 +315,8 @@ const SqlEditor = ({
         key: 'ctrl+r',
         descr: t('Run query'),
         func: () => {
-          if (queryEditor.sql.trim() !== '') {
-            startQuery();
+          if (this.props.queryEditor.sql.trim() !== '') {
+            this.runQuery();
           }
         },
       },
@@ -258,8 +325,8 @@ const SqlEditor = ({
         key: 'ctrl+enter',
         descr: t('Run query'),
         func: () => {
-          if (queryEditor.sql.trim() !== '') {
-            startQuery();
+          if (this.props.queryEditor.sql.trim() !== '') {
+            this.runQuery();
           }
         },
       },
@@ -268,20 +335,18 @@ const SqlEditor = ({
         key: userOS === 'Windows' ? 'ctrl+q' : 'ctrl+t',
         descr: t('New tab'),
         func: () => {
-          const name = newQueryTabName(queryEditors || []);
-          dispatch(
-            addQueryEditor({
-              ...queryEditor,
-              name,
-            }),
-          );
+          const name = newQueryTabName(this.props.queryEditors || []);
+          this.props.addQueryEditor({
+            ...this.props.queryEditor,
+            name,
+          });
         },
       },
       {
         name: 'stopQuery',
         key: userOS === 'MacOS' ? 'ctrl+x' : 'ctrl+e',
         descr: t('Stop query'),
-        func: stopQuery,
+        func: this.stopQuery,
       },
     ];
 
@@ -297,170 +362,176 @@ const SqlEditor = ({
     }
 
     return base;
-  };
-
-  const handleWindowResize = () => {
-    setHeight(getSqlEditorHeight());
-  };
+  }
 
-  const handleWindowResizeWithThrottle = useMemo(
-    () => throttle(handleWindowResize, WINDOW_RESIZE_THROTTLE_MS),
-    [],
-  );
+  setEmptyState(bool) {
+    this.setState({ showEmptyState: bool });
+  }
 
-  const onBeforeUnload = event => {
-    if (
-      database?.extra_json?.cancel_query_on_windows_unload &&
-      latestQuery?.state === 'running'
-    ) {
-      event.preventDefault();
-      stopQuery();
-    }
-  };
+  setQueryEditorAndSaveSql(sql) {
+    this.props.queryEditorSetAndSaveSql(this.props.queryEditor, sql);
+  }
 
-  useEffect(() => {
-    // We need to measure the height of the sql editor post render to figure the height of
-    // the south pane so it gets rendered properly
-    setHeight(getSqlEditorHeight());
-    if (!database || isEmpty(database)) {
-      setShowEmptyState(true);
+  getQueryCostEstimate() {
+    if (this.props.database) {
+      const qe = this.props.queryEditor;
+      this.props.estimateQueryCost(qe);
     }
+  }
 
-    window.addEventListener('resize', handleWindowResizeWithThrottle);
-    window.addEventListener('beforeunload', onBeforeUnload);
-
-    // setup hotkeys
-    const hotkeys = getHotkeyConfig();
-    hotkeys.forEach(keyConfig => {
-      Mousetrap.bind([keyConfig.key], keyConfig.func);
+  handleToggleAutocompleteEnabled = () => {
+    this.setState(prevState => {
+      setItem(
+        LocalStorageKeys.sqllab__is_autocomplete_enabled,
+        !prevState.autocompleteEnabled,
+      );
+      return {
+        autocompleteEnabled: !prevState.autocompleteEnabled,
+      };
     });
-
-    return () => {
-      window.removeEventListener('resize', handleWindowResizeWithThrottle);
-      window.removeEventListener('beforeunload', onBeforeUnload);
-    };
-  }, []);
-
-  const onResizeStart = () => {
-    // Set the heights on the ace editor and the ace content area after drag starts
-    // to smooth out the visual transition to the new heights when drag ends
-    document.getElementsByClassName('ace_content')[0].style.height = '100%';
   };
 
-  const onResizeEnd = ([northPercent, southPercent]) => {
-    setNorthPercent(northPercent);
-    setSouthPercent(southPercent);
-
-    if (northPaneRef.current?.clientHeight) {
-      dispatch(persistEditorHeight(queryEditor, northPercent, southPercent));
-    }
-  };
+  handleWindowResize() {
+    this.setState({ height: this.getSqlEditorHeight() });
+  }
 
-  const setQueryEditorAndSaveSql = sql => {
-    dispatch(queryEditorSetAndSaveSql(queryEditor, sql));
-  };
+  elementStyle(dimension, elementSize, gutterSize) {
+    return {
+      [dimension]: `calc(${elementSize}% - ${
+        gutterSize + SQL_EDITOR_GUTTER_MARGIN
+      }px)`,
+    };
+  }
 
-  const setQueryEditorAndSaveSqlWithDebounce = useMemo(
-    () => debounce(setQueryEditorAndSaveSql, SET_QUERY_EDITOR_SQL_DEBOUNCE_MS),
-    [],
-  );
+  requestValidation(sql) {
+    const { database, queryEditor, validateQuery } = this.props;
+    if (database) {
+      validateQuery(queryEditor, sql);
+    }
+  }
 
-  const canValidateQuery = () => {
+  canValidateQuery() {
     // Check whether or not we can validate the current query based on whether
     // or not the backend has a validator configured for it.
-    if (database) {
-      return validatorMap.hasOwnProperty(database.backend);
+    if (this.props.database) {
+      return validatorMap.hasOwnProperty(this.props.database.backend);
     }
     return false;
-  };
+  }
 
-  const requestValidation = sql => {
-    if (database) {
-      dispatch(validateQuery(queryEditor, sql));
+  runQuery() {
+    if (this.props.database) {
+      this.startQuery();
     }
-  };
+  }
 
-  const requestValidationWithDebounce = useMemo(
-    () => debounce(requestValidation, VALIDATION_DEBOUNCE_MS),
-    [],
-  );
+  convertToNumWithSpaces(num) {
+    return num.toString().replace(/(\d)(?=(\d{3})+(?!\d))/g, '$1 ');
+  }
 
-  const onSqlChanged = sql => {
-    dispatch(queryEditorSetSql(queryEditor, sql));
-    setQueryEditorAndSaveSqlWithDebounce(sql);
-    // Request server-side validation of the query text
-    if (canValidateQuery()) {
-      // NB. requestValidation is debounced
-      requestValidationWithDebounce(sql);
-    }
-  };
+  startQuery(ctas = false, ctas_method = CtasEnum.TABLE) {
+    const {
+      database,
+      runQueryFromSqlEditor,
+      setActiveSouthPaneTab,
+      queryEditor,
+      defaultQueryLimit,
+    } = this.props;
+    runQueryFromSqlEditor(
+      database,
+      queryEditor,
+      defaultQueryLimit,
+      ctas ? this.state.ctas : '',
+      ctas,
+      ctas_method,
+    );
+    setActiveSouthPaneTab('Results');
+  }
 
-  // Return the heights for the ace editor and the south pane as an object
-  // given the height of the sql editor, north pane percent and south pane percent.
-  const getAceEditorAndSouthPaneHeights = (
-    height,
-    northPercent,
-    southPercent,
-  ) => ({
-    aceEditorHeight:
-      (height * northPercent) / (theme.gridUnit * 25) -
-      (SQL_EDITOR_GUTTER_HEIGHT / 2 + SQL_EDITOR_GUTTER_MARGIN) -
-      SQL_TOOLBAR_HEIGHT,
-    southPaneHeight:
-      (height * southPercent) / (theme.gridUnit * 25) -
-      (SQL_EDITOR_GUTTER_HEIGHT / 2 + SQL_EDITOR_GUTTER_MARGIN),
-  });
-
-  const getQueryCostEstimate = () => {
-    if (database) {
-      dispatch(estimateQueryCost(queryEditor));
+  stopQuery() {
+    if (
+      this.props.latestQuery &&
+      ['running', 'pending'].indexOf(this.props.latestQuery.state) >= 0
+    ) {
+      this.props.postStopQuery(this.props.latestQuery);
     }
-  };
-
-  const handleToggleAutocompleteEnabled = () => {
-    setItem(
-      LocalStorageKeys.sqllab__is_autocomplete_enabled,
-      !autocompleteEnabled,
-    );
-    setAutocompleteEnabled(!autocompleteEnabled);
-  };
+  }
 
-  const elementStyle = (dimension, elementSize, gutterSize) => ({
-    [dimension]: `calc(${elementSize}% - ${
-      gutterSize + SQL_EDITOR_GUTTER_MARGIN
-    }px)`,
-  });
+  createTableAs() {
+    this.startQuery(true, CtasEnum.TABLE);
+    this.setState({ showCreateAsModal: false, ctas: '' });
+  }
 
-  const createTableAs = () => {
-    startQuery(true, CtasEnum.TABLE);
-    setShowCreateAsModal(false);
-    setCtas('');
-  };
+  createViewAs() {
+    this.startQuery(true, CtasEnum.VIEW);
+    this.setState({ showCreateAsModal: false, ctas: '' });
+  }
 
-  const createViewAs = () => {
-    startQuery(true, CtasEnum.VIEW);
-    setShowCreateAsModal(false);
-    setCtas('');
-  };
+  ctasChanged(event) {
+    this.setState({ ctas: event.target.value });
+  }
 
-  const ctasChanged = event => {
-    setCtas(event.target.value);
-  };
+  queryPane() {
+    const hotkeys = this.getHotkeyConfig();
+    const { aceEditorHeight, southPaneHeight } =
+      this.getAceEditorAndSouthPaneHeights(
+        this.state.height,
+        this.state.northPercent,
+        this.state.southPercent,
+      );
+    return (
+      <Split
+        expandToMin
+        className="queryPane"
+        sizes={[this.state.northPercent, this.state.southPercent]}
+        elementStyle={this.elementStyle}
+        minSize={200}
+        direction="vertical"
+        gutterSize={SQL_EDITOR_GUTTER_HEIGHT}
+        onDragStart={this.onResizeStart}
+        onDragEnd={this.onResizeEnd}
+      >
+        <div ref={this.northPaneRef} className="north-pane">
+          <AceEditorWrapper
+            actions={this.props.actions}
+            autocomplete={this.state.autocompleteEnabled}
+            onBlur={this.setQueryEditorSql}
+            onChange={this.onSqlChanged}
+            queryEditor={this.props.queryEditor}
+            database={this.props.database}
+            extendedTables={this.props.tables}
+            height={`${aceEditorHeight}px`}
+            hotkeys={hotkeys}
+          />
+          {this.renderEditorBottomBar(hotkeys)}
+        </div>
+        <ConnectedSouthPane
+          editorQueries={this.props.editorQueries}
+          latestQueryId={this.props.latestQuery && this.props.latestQuery.id}
+          dataPreviewQueries={this.props.dataPreviewQueries}
+          actions={this.props.actions}
+          height={southPaneHeight}
+          displayLimit={this.props.displayLimit}
+          defaultQueryLimit={this.props.defaultQueryLimit}
+        />
+      </Split>
+    );
+  }
 
-  const renderDropdown = () => {
-    const qe = queryEditor;
-    const successful = latestQuery?.state === 'success';
+  renderDropdown() {
+    const qe = this.props.queryEditor;
+    const successful = this.props.latestQuery?.state === 'success';
     const scheduleToolTip = successful
       ? t('Schedule the query periodically')
       : t('You must run the query successfully first');
     return (
-      <Menu css={{ width: theme.gridUnit * 44 }}>
-        <Menu.Item css={{ display: 'flex', justifyContent: 'space-between' }}>
+      <Menu onClick={this.handleMenuClick} style={{ width: 176 }}>
+        <Menu.Item style={{ display: 'flex', justifyContent: 'space-between' }}>
           {' '}
           <span>{t('Autocomplete')}</span>{' '}
           <AntdSwitch
-            checked={autocompleteEnabled}
-            onChange={handleToggleAutocompleteEnabled}
+            checked={this.state.autocompleteEnabled}
+            onChange={this.handleToggleAutocompleteEnabled}
             name="autocomplete-switch"
           />{' '}
         </Menu.Item>
@@ -469,7 +540,7 @@ const SqlEditor = ({
             <TemplateParamsEditor
               language="json"
               onChange={params => {
-                dispatch(queryEditorSetTemplateParams(qe, params));
+                this.props.actions.queryEditorSetTemplateParams(qe, params);
               }}
               queryEditor={qe}
             />
@@ -480,10 +551,10 @@ const SqlEditor = ({
             <ScheduleQueryButton
               defaultLabel={qe.name}
               sql={qe.sql}
-              onSchedule={query => dispatch(scheduleQuery(query))}
+              onSchedule={this.props.actions.scheduleQuery}
               schema={qe.schema}
               dbId={qe.dbId}
-              scheduleQueryWarning={scheduleQueryWarning}
+              scheduleQueryWarning={this.props.scheduleQueryWarning}
               tooltip={scheduleToolTip}
               disabled={!successful}
             />
@@ -491,24 +562,31 @@ const SqlEditor = ({
         )}
       </Menu>
     );
-  };
+  }
 
-  const onSaveQuery = async query => {
-    const savedQuery = await dispatch(saveQuery(query));
-    dispatch(addSavedQueryToTabState(queryEditor, savedQuery));
-  };
+  async saveQuery(query) {
+    const { queryEditor: qe, actions } = this.props;
+    const savedQuery = await actions.saveQuery(query);
+    actions.addSavedQueryToTabState(qe, savedQuery);
+  }
+
+  renderEditorBottomBar() {
+    const { queryEditor: qe } = this.props;
 
-  const renderEditorBottomBar = () => {
-    const { allow_ctas: allowCTAS, allow_cvas: allowCVAS } = database || {};
+    const { allow_ctas: allowCTAS, allow_cvas: allowCVAS } =
+      this.props.database || {};
 
     const showMenu = allowCTAS || allowCVAS;
+    const { theme } = this.props;
     const runMenuBtn = (
       <Menu>
         {allowCTAS && (
           <Menu.Item
             onClick={() => {
-              setShowCreateAsModal(true);
-              setCreateAs(CtasEnum.TABLE);
+              this.setState({
+                showCreateAsModal: true,
+                createAs: CtasEnum.TABLE,
+              });
             }}
             key="1"
           >
@@ -518,8 +596,10 @@ const SqlEditor = ({
         {allowCVAS && (
           <Menu.Item
             onClick={() => {
-              setShowCreateAsModal(true);
-              setCreateAs(CtasEnum.VIEW);
+              this.setState({
+                showCreateAsModal: true,
+                createAs: CtasEnum.VIEW,
+              });
             }}
             key="2"
           >
@@ -534,190 +614,214 @@ const SqlEditor = ({
         <div className="leftItems">
           <span>
             <RunQueryActionButton
-              allowAsync={database ? database.allow_run_async : false}
-              queryEditor={queryEditor}
-              queryState={latestQuery?.state}
-              runQuery={startQuery}
-              stopQuery={stopQuery}
+              allowAsync={
+                this.props.database
+                  ? this.props.database.allow_run_async
+                  : false
+              }
+              queryEditor={qe}
+              queryState={this.props.latestQuery?.state}
+              runQuery={this.runQuery}
+              stopQuery={this.stopQuery}
               overlayCreateAsMenu={showMenu ? runMenuBtn : null}
             />
           </span>
           {isFeatureEnabled(FeatureFlag.ESTIMATE_QUERY_COST) &&
-            database?.allows_cost_estimate && (
+            this.props.database &&
+            this.props.database.allows_cost_estimate && (
               <span>
                 <EstimateQueryCostButton
-                  getEstimate={getQueryCostEstimate}
-                  queryEditor={queryEditor}
+                  getEstimate={this.getQueryCostEstimate}
+                  queryEditor={qe}
                   tooltip={t('Estimate the cost before running a query')}
                 />
               </span>
             )}
           <span>
             <QueryLimitSelect
-              queryEditor={queryEditor}
-              maxRow={maxRow}
-              defaultQueryLimit={defaultQueryLimit}
+              queryEditor={this.props.queryEditor}
+              maxRow={this.props.maxRow}
+              defaultQueryLimit={this.props.defaultQueryLimit}
             />
           </span>
-          {latestQuery && (
+          {this.props.latestQuery && (
             <Timer
-              startTime={latestQuery.startDttm}
-              endTime={latestQuery.endDttm}
-              state={STATE_TYPE_MAP[latestQuery.state]}
-              isRunning={latestQuery.state === 'running'}
+              startTime={this.props.latestQuery.startDttm}
+              endTime={this.props.latestQuery.endDttm}
+              state={STATE_TYPE_MAP[this.props.latestQuery.state]}
+              isRunning={this.props.latestQuery.state === 'running'}
             />
           )}
         </div>
         <div className="rightItems">
           <span>
             <SaveQuery
-              queryEditor={queryEditor}
-              columns={latestQuery?.results?.columns || []}
-              onSave={onSaveQuery}
-              onUpdate={query => dispatch(updateSavedQuery(query))}
-              saveQueryWarning={saveQueryWarning}
-              database={database}
+              queryEditor={qe}
+              columns={this.props.latestQuery?.results?.columns || []}
+              onSave={this.saveQuery}
+              onUpdate={this.props.actions.updateSavedQuery}
+              saveQueryWarning={this.props.saveQueryWarning}
+              database={this.props.database}
             />
           </span>
           <span>
-            <ShareSqlLabQuery queryEditor={queryEditor} />
+            <ShareSqlLabQuery queryEditor={qe} />
           </span>
-          <AntdDropdown overlay={renderDropdown()} trigger="click">
+          <AntdDropdown overlay={this.renderDropdown()} trigger="click">
             <Icons.MoreHoriz iconColor={theme.colors.grayscale.base} />
           </AntdDropdown>
         </div>
       </StyledToolbar>
     );
-  };
+  }
 
-  const queryPane = () => {
-    const hotkeys = getHotkeyConfig();
-    const { aceEditorHeight, southPaneHeight } =
-      getAceEditorAndSouthPaneHeights(height, northPercent, southPercent);
+  render() {
+    const createViewModalTitle =
+      this.state.createAs === CtasEnum.VIEW
+        ? 'CREATE VIEW AS'
+        : 'CREATE TABLE AS';
+
+    const createModalPlaceHolder =
+      this.state.createAs === CtasEnum.VIEW
+        ? 'Specify name to CREATE VIEW AS schema in: public'
+        : 'Specify name to CREATE TABLE AS schema in: public';
+    const leftBarStateClass = this.props.hideLeftBar
+      ? 'schemaPane-exit-done'
+      : 'schemaPane-enter-done';
     return (
-      <Split
-        expandToMin
-        className="queryPane"
-        sizes={[northPercent, southPercent]}
-        elementStyle={elementStyle}
-        minSize={200}
-        direction="vertical"
-        gutterSize={SQL_EDITOR_GUTTER_HEIGHT}
-        onDragStart={onResizeStart}
-        onDragEnd={onResizeEnd}
-      >
-        <div ref={northPaneRef} className="north-pane">
-          <AceEditorWrapper
-            actions={actions}
-            autocomplete={autocompleteEnabled}
-            onBlur={setQueryEditorAndSaveSql}
-            onChange={onSqlChanged}
-            queryEditor={queryEditor}
-            database={database}
-            extendedTables={tables}
-            height={`${aceEditorHeight}px`}
-            hotkeys={hotkeys}
-          />
-          {renderEditorBottomBar(hotkeys)}
-        </div>
-        <ConnectedSouthPane
-          editorQueries={editorQueries}
-          latestQueryId={latestQuery?.id}
-          dataPreviewQueries={dataPreviewQueries}
-          actions={actions}
-          height={southPaneHeight}
-          displayLimit={displayLimit}
-          defaultQueryLimit={defaultQueryLimit}
-        />
-      </Split>
-    );
-  };
-
-  const createViewModalTitle =
-    createAs === CtasEnum.VIEW ? 'CREATE VIEW AS' : 'CREATE TABLE AS';
-
-  const createModalPlaceHolder =
-    createAs === CtasEnum.VIEW
-      ? t('Specify name to CREATE VIEW AS schema in: public')
-      : t('Specify name to CREATE TABLE AS schema in: public');
-
-  const leftBarStateClass = hideLeftBar
-    ? 'schemaPane-exit-done'
-    : 'schemaPane-enter-done';
-  return (
-    <div ref={sqlEditorRef} className="SqlEditor">
-      <CSSTransition classNames="schemaPane" in={!hideLeftBar} timeout={300}>
-        <ResizableSidebar
-          id={`sqllab:${queryEditor.id}`}
-          minWidth={SQL_EDITOR_LEFTBAR_WIDTH}
-          initialWidth={SQL_EDITOR_LEFTBAR_WIDTH}
-          enable={!hideLeftBar}
+      <div ref={this.sqlEditorRef} className="SqlEditor">
+        <CSSTransition
+          classNames="schemaPane"
+          in={!this.props.hideLeftBar}
+          timeout={300}
         >
-          {adjustedWidth => (
-            <StyledSidebar
-              className={`schemaPane ${leftBarStateClass}`}
-              width={adjustedWidth}
-              hide={hideLeftBar}
-            >
-              <SqlEditorLeftBar
-                database={database}
-                queryEditor={queryEditor}
-                tables={tables}
-                actions={actions}
-                setEmptyState={bool => setShowEmptyState(bool)}
-              />
-            </StyledSidebar>
-          )}
-        </ResizableSidebar>
-      </CSSTransition>
-      {showEmptyState ? (
-        <EmptyStateBig
-          image="vector.svg"
-          title={t('Select a database to write a query')}
-          description={t(
-            'Choose one of the available databases from the panel on the left.',
-          )}
-        />
-      ) : (
-        queryPane()
-      )}
-      <Modal
-        visible={showCreateAsModal}
-        title={t(createViewModalTitle)}
-        onHide={() => setShowCreateAsModal(false)}
-        footer={
-          <>
-            <Button onClick={() => setShowCreateAsModal(false)}>
-              {t('Cancel')}
-            </Button>
-            {createAs === CtasEnum.TABLE && (
-              <Button
-                buttonStyle="primary"
-                disabled={ctas.length === 0}
-                onClick={createTableAs}
+          <ResizableSidebar
+            id={`sqllab:${this.props.queryEditor.id}`}
+            minWidth={SQL_EDITOR_LEFTBAR_WIDTH}
+            initialWidth={SQL_EDITOR_LEFTBAR_WIDTH}
+            enable={!this.props.hideLeftBar}
+          >
+            {adjustedWidth => (
+              <StyledSidebar
+                className={`schemaPane ${leftBarStateClass}`}
+                width={adjustedWidth}
+                hide={this.props.hideLeftBar}
               >
-                {t('Create')}
-              </Button>
+                <SqlEditorLeftBar
+                  database={this.props.database}
+                  queryEditor={this.props.queryEditor}
+                  tables={this.props.tables}
+                  actions={this.props.actions}
+                  setEmptyState={this.setEmptyState}
+                />
+              </StyledSidebar>
             )}
-            {createAs === CtasEnum.VIEW && (
+          </ResizableSidebar>
+        </CSSTransition>
+        {this.state.showEmptyState ? (
+          <EmptyStateBig
+            image="vector.svg"
+            title={t('Select a database to write a query')}
+            description={t(
+              'Choose one of the available databases from the panel on the left.',
+            )}
+          />
+        ) : (
+          this.queryPane()
+        )}
+        <StyledModal
+          visible={this.state.showCreateAsModal}
+          title={t(createViewModalTitle)}
+          onHide={() => {
+            this.setState({ showCreateAsModal: false });
+          }}
+          footer={
+            <>
               <Button
-                buttonStyle="primary"
-                disabled={ctas.length === 0}
-                onClick={createViewAs}
+                onClick={() => this.setState({ showCreateAsModal: false })}
               >
-                {t('Create')}
+                Cancel
               </Button>
-            )}
-          </>
-        }
-      >
-        <span>{t('Name')}</span>
-        <Input placeholder={createModalPlaceHolder} onChange={ctasChanged} />
-      </Modal>
-    </div>
-  );
-};
-
+              {this.state.createAs === CtasEnum.TABLE && (
+                <Button
+                  buttonStyle="primary"
+                  disabled={this.state.ctas.length === 0}
+                  onClick={this.createTableAs.bind(this)}
+                >
+                  Create
+                </Button>
+              )}
+              {this.state.createAs === CtasEnum.VIEW && (
+                <Button
+                  buttonStyle="primary"
+                  disabled={this.state.ctas.length === 0}
+                  onClick={this.createViewAs.bind(this)}
+                >
+                  Create
+                </Button>
+              )}
+            </>
+          }
+        >
+          <span>Name</span>
+          <Input
+            placeholder={createModalPlaceHolder}
+            onChange={this.ctasChanged.bind(this)}
+          />
+        </StyledModal>
+      </div>
+    );
+  }
+}
+SqlEditor.defaultProps = defaultProps;
 SqlEditor.propTypes = propTypes;
 
-export default SqlEditor;
+function mapStateToProps({ sqlLab }, { queryEditor }) {
+  let { latestQueryId, dbId, hideLeftBar } = queryEditor;
+  if (sqlLab.unsavedQueryEditor.id === queryEditor.id) {
+    const {
+      latestQueryId: unsavedQID,
+      dbId: unsavedDBID,
+      hideLeftBar: unsavedHideLeftBar,
+    } = sqlLab.unsavedQueryEditor;
+    latestQueryId = unsavedQID || latestQueryId;
+    dbId = unsavedDBID || dbId;
+    hideLeftBar = unsavedHideLeftBar || hideLeftBar;
+  }
+  const database = sqlLab.databases[dbId];
+  const latestQuery = sqlLab.queries[latestQueryId];
+
+  return {
+    hideLeftBar,
+    queryEditors: sqlLab.queryEditors,
+    latestQuery,
+    database,
+  };
+}
+
+function mapDispatchToProps(dispatch) {
+  return bindActionCreators(
+    {
+      addQueryEditor,
+      estimateQueryCost,
+      persistEditorHeight,
+      postStopQuery,
+      queryEditorSetAutorun,
+      queryEditorSetSql,
+      queryEditorSetAndSaveSql,
+      queryEditorSetTemplateParams,
+      runQueryFromSqlEditor,
+      runQuery,
+      saveQuery,
+      addSavedQueryToTabState,
+      scheduleQuery,
+      setActiveSouthPaneTab,
+      updateSavedQuery,
+      validateQuery,
+    },
+    dispatch,
+  );
+}
+
+const themedSqlEditor = withTheme(SqlEditor);
+export default connect(mapStateToProps, mapDispatchToProps)(themedSqlEditor);
diff --git a/superset-frontend/src/SqlLab/constants.ts b/superset-frontend/src/SqlLab/constants.ts
index 29b0f6cf6b..11d990032d 100644
--- a/superset-frontend/src/SqlLab/constants.ts
+++ b/superset-frontend/src/SqlLab/constants.ts
@@ -49,12 +49,6 @@ export const SQL_EDITOR_GUTTER_HEIGHT = 5;
 export const SQL_EDITOR_GUTTER_MARGIN = 3;
 export const SQL_TOOLBAR_HEIGHT = 51;
 export const SQL_EDITOR_LEFTBAR_WIDTH = 400;
-export const SQL_EDITOR_PADDING = 10;
-export const INITIAL_NORTH_PERCENT = 30;
-export const INITIAL_SOUTH_PERCENT = 70;
-export const SET_QUERY_EDITOR_SQL_DEBOUNCE_MS = 2000;
-export const VALIDATION_DEBOUNCE_MS = 600;
-export const WINDOW_RESIZE_THROTTLE_MS = 100;
 
 // kilobyte storage
 export const KB_STORAGE = 1024;


[superset] 10/13: fix: Race conditions with setupExtensions (#21647)

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch 2022.39.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 98fcab48df3dce9898f766ff17dd6548f33c47e1
Author: Geido <60...@users.noreply.github.com>
AuthorDate: Wed Oct 5 12:38:51 2022 +0300

    fix: Race conditions with setupExtensions (#21647)
    
    (cherry picked from commit de444d4de6a917af8f8efe2335fb1a26ac86e6d8)
---
 superset-frontend/src/SqlLab/App.jsx  | 2 ++
 superset-frontend/src/preamble.ts     | 3 ---
 superset-frontend/src/profile/App.tsx | 2 ++
 superset-frontend/src/views/App.tsx   | 2 ++
 4 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/superset-frontend/src/SqlLab/App.jsx b/superset-frontend/src/SqlLab/App.jsx
index ce768fee8c..812202eec2 100644
--- a/superset-frontend/src/SqlLab/App.jsx
+++ b/superset-frontend/src/SqlLab/App.jsx
@@ -29,6 +29,7 @@ import {
   isFeatureEnabled,
   FeatureFlag,
 } from 'src/featureFlags';
+import setupExtensions from 'src/setup/setupExtensions';
 import getInitialState from './reducers/getInitialState';
 import rootReducer from './reducers/index';
 import { initEnhancer } from '../reduxUtils';
@@ -45,6 +46,7 @@ import '../assets/stylesheets/reactable-pagination.less';
 import { theme } from '../preamble';
 
 setupApp();
+setupExtensions();
 
 const appContainer = document.getElementById('app');
 const bootstrapData = JSON.parse(appContainer.getAttribute('data-bootstrap'));
diff --git a/superset-frontend/src/preamble.ts b/superset-frontend/src/preamble.ts
index f6ae99b084..73ccb947a7 100644
--- a/superset-frontend/src/preamble.ts
+++ b/superset-frontend/src/preamble.ts
@@ -24,7 +24,6 @@ import { configure, makeApi, supersetTheme } from '@superset-ui/core';
 import { merge } from 'lodash';
 import setupClient from './setup/setupClient';
 import setupColors from './setup/setupColors';
-import setupExtensions from './setup/setupExtensions';
 import setupFormatters from './setup/setupFormatters';
 import setupDashboardComponents from './setup/setupDasboardComponents';
 import { BootstrapUser, User } from './types/bootstrapTypes';
@@ -34,8 +33,6 @@ if (process.env.WEBPACK_MODE === 'development') {
   setHotLoaderConfig({ logLevel: 'debug', trackTailUpdates: false });
 }
 
-setupExtensions();
-
 // eslint-disable-next-line import/no-mutable-exports
 export let bootstrapData: {
   user?: BootstrapUser;
diff --git a/superset-frontend/src/profile/App.tsx b/superset-frontend/src/profile/App.tsx
index 1f2cd144af..3704dcb4b5 100644
--- a/superset-frontend/src/profile/App.tsx
+++ b/superset-frontend/src/profile/App.tsx
@@ -27,10 +27,12 @@ import App from 'src/profile/components/App';
 import messageToastReducer from 'src/components/MessageToasts/reducers';
 import { initEnhancer } from 'src/reduxUtils';
 import setupApp from 'src/setup/setupApp';
+import setupExtensions from 'src/setup/setupExtensions';
 import { theme } from 'src/preamble';
 import ToastContainer from 'src/components/MessageToasts/ToastContainer';
 
 setupApp();
+setupExtensions();
 
 const profileViewContainer = document.getElementById('app');
 const bootstrap = JSON.parse(
diff --git a/superset-frontend/src/views/App.tsx b/superset-frontend/src/views/App.tsx
index f13b9c5501..21a3868491 100644
--- a/superset-frontend/src/views/App.tsx
+++ b/superset-frontend/src/views/App.tsx
@@ -34,12 +34,14 @@ import setupApp from 'src/setup/setupApp';
 import setupPlugins from 'src/setup/setupPlugins';
 import { routes, isFrontendRoute } from 'src/views/routes';
 import { Logger } from 'src/logger/LogUtils';
+import setupExtensions from 'src/setup/setupExtensions';
 import { RootContextProviders } from './RootContextProviders';
 import { ScrollToTop } from './ScrollToTop';
 import QueryProvider from './QueryProvider';
 
 setupApp();
 setupPlugins();
+setupExtensions();
 
 const user = { ...bootstrapData.user };
 const menu = {