Merge branch 'master' into villebro/key-value-flush

apache · Jun 19, 2024 · c6ce18a · c6ce18a
2 parents a17c661 + 9f70697
commit c6ce18a
Show file tree

Hide file tree

Showing 18 changed files with 125 additions and 185 deletions.
diff --git a/.rat-excludes b/.rat-excludes
@@ -71,3 +71,4 @@ snowflake.svg
 # docs-related
 erd.puml
 erd.svg
+intro_header.txt
diff --git a/README.md b/README.md
@@ -1,7 +1,3 @@
----
-hide_title: true
-sidebar_position: 1
----
 <!--
 Licensed to the Apache Software Foundation (ASF) under one
 or more contributor license agreements.  See the NOTICE file

diff --git a/docker/pythonpath_dev/superset_config.py b/docker/pythonpath_dev/superset_config.py
@@ -97,7 +97,6 @@ class CeleryConfig:
 WEBDRIVER_BASEURL = "http://superset:8088/"  # When using docker compose baseurl should be http://superset_app:8088/
 # The base URL for the email report hyperlinks.
 WEBDRIVER_BASEURL_USER_FRIENDLY = WEBDRIVER_BASEURL
-
 SQLLAB_CTAS_NO_LIMIT = True
 
 #

diff --git a/docs/package.json b/docs/package.json
@@ -5,13 +5,13 @@
   "license": "Apache-2.0",
   "scripts": {
     "docusaurus": "docusaurus",
-    "_init": "cp ../README.md docs/intro.md",
+    "_init": "cat src/intro_header.txt ../README.md > docs/intro.md",
     "start": "npm run _init && docusaurus start",
     "build": "npm run _init && docusaurus build",
     "swizzle": "docusaurus swizzle",
     "deploy": "docusaurus deploy",
     "clear": "docusaurus clear",
-    "serve": "docusaurus serve",
+    "serve": "npm run _init && docusaurus serve",
     "write-translations": "docusaurus write-translations",
     "write-heading-ids": "docusaurus write-heading-ids",
     "typecheck": "tsc"

diff --git a/docs/src/intro_header.txt b/docs/src/intro_header.txt
@@ -0,0 +1,4 @@
+---
+hide_title: true
+sidebar_position: 1
+---
diff --git a/scripts/tests/run.sh b/scripts/tests/run.sh
@@ -61,7 +61,10 @@ function test_init() {
 DB_NAME="test"
 DB_USER="superset"
 DB_PASSWORD="superset"
+
+# Pointing to use the test database in local docker-compose setup
 export SUPERSET__SQLALCHEMY_DATABASE_URI=${SUPERSET__SQLALCHEMY_DATABASE_URI:-postgresql+psycopg2://"${DB_USER}":"${DB_PASSWORD}"@localhost/"${DB_NAME}"}
+
 export SUPERSET_CONFIG=${SUPERSET_CONFIG:-tests.integration_tests.superset_test_config}
 RUN_INIT=1
 RUN_RESET_DB=1

diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx
@@ -303,7 +303,10 @@ const SqlEditor: FC<Props> = ({
   );
   const [showCreateAsModal, setShowCreateAsModal] = useState(false);
   const [createAs, setCreateAs] = useState('');
-  const [showEmptyState, setShowEmptyState] = useState(false);
+  const showEmptyState = useMemo(
+    () => !database || isEmpty(database),
+    [database],
+  );
 
   const sqlEditorRef = useRef<HTMLDivElement>(null);
   const northPaneRef = useRef<HTMLDivElement>(null);
@@ -562,12 +565,6 @@ const SqlEditor: FC<Props> = ({
     // TODO: Remove useEffectEvent deps once https://github.com/facebook/react/pull/25881 is released
   }, [onBeforeUnload, loadQueryEditor, isActive]);
 
-  useEffect(() => {
-    if (!database || isEmpty(database)) {
-      setShowEmptyState(true);
-    }
-  }, [database]);
-
   useEffect(() => {
     // setup hotkeys
     const hotkeys = getHotkeyConfig();
@@ -911,7 +908,6 @@ const SqlEditor: FC<Props> = ({
               <SqlEditorLeftBar
                 database={database}
                 queryEditorId={queryEditor.id}
-                setEmptyState={bool => setShowEmptyState(bool)}
               />
             </StyledSidebar>
           )}

diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx
@@ -16,14 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import {
-  useEffect,
-  useCallback,
-  useMemo,
-  useState,
-  Dispatch,
-  SetStateAction,
-} from 'react';
+import { useEffect, useCallback, useMemo, useState } from 'react';
 import { shallowEqual, useDispatch, useSelector } from 'react-redux';
 import querystring from 'query-string';
 
@@ -60,7 +53,6 @@ export interface SqlEditorLeftBarProps {
   queryEditorId: string;
   height?: number;
   database?: DatabaseObject;
-  setEmptyState?: Dispatch<SetStateAction<boolean>>;
 }
 
 const StyledScrollbarContainer = styled.div`
@@ -108,7 +100,6 @@ const SqlEditorLeftBar = ({
   database,
   queryEditorId,
   height = 500,
-  setEmptyState,
 }: SqlEditorLeftBarProps) => {
   const tables = useSelector<SqlLabRootState, Table[]>(
     ({ sqlLab }) =>
@@ -148,7 +139,6 @@ const SqlEditorLeftBar = ({
   }, []);
 
   const onDbChange = ({ id: dbId }: { id: number }) => {
-    setEmptyState?.(false);
     dispatch(queryEditorSetDb(queryEditor, dbId));
   };
 

diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.ts b/superset-frontend/src/SqlLab/reducers/getInitialState.ts
@@ -102,6 +102,7 @@ export default function getInitialState({
         id: id.toString(),
         loaded: false,
         name: label,
+        dbId: undefined,
       };
     }
     queryEditors = {

diff --git a/...-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx b/...-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
@@ -175,7 +175,7 @@ const FilterValue: FC<FilterControlProps> = ({
       setIsRefreshing(true);
       getChartDataRequest({
         formData: newFormData,
-        force: false,
+        force: shouldRefresh,
         ownState: filterOwnState,
       })
         .then(({ response, json }) => {

diff --git a/superset-frontend/src/hooks/apiResources/catalogs.ts b/superset-frontend/src/hooks/apiResources/catalogs.ts
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { useCallback, useEffect, useRef } from 'react';
+import { useCallback, useEffect } from 'react';
 import useEffectEvent from 'src/hooks/useEffectEvent';
 import { api, JsonResponse } from './queryApi';
 
@@ -68,7 +68,6 @@ export const {
 export const EMPTY_CATALOGS = [] as CatalogOption[];
 
 export function useCatalogs(options: Params) {
-  const isMountedRef = useRef(false);
   const { dbId, onSuccess, onError } = options || {};
   const [trigger] = useLazyCatalogsQuery();
   const result = useCatalogsQuery(
@@ -78,47 +77,28 @@ export function useCatalogs(options: Params) {
     },
   );
 
-  const handleOnSuccess = useEffectEvent(
-    (data: CatalogOption[], isRefetched: boolean) => {
-      onSuccess?.(data, isRefetched);
-    },
-  );
-
-  const handleOnError = useEffectEvent(() => {
-    onError?.();
-  });
-
-  const refetch = useCallback(() => {
-    if (dbId) {
-      trigger({ dbId, forceRefresh: true }).then(
-        ({ isSuccess, isError, data }) => {
+  const fetchData = useEffectEvent(
+    (dbId: FetchCatalogsQueryParams['dbId'], forceRefresh = false) => {
+      if (dbId && (!result.currentData || forceRefresh)) {
+        trigger({ dbId, forceRefresh }).then(({ isSuccess, isError, data }) => {
           if (isSuccess) {
-            handleOnSuccess(data || EMPTY_CATALOGS, true);
+            onSuccess?.(data || EMPTY_CATALOGS, forceRefresh);
           }
           if (isError) {
-            handleOnError();
+            onError?.();
           }
-        },
-      );
-    }
-  }, [dbId, handleOnError, handleOnSuccess, trigger]);
+        });
+      }
+    },
+  );
+
+  const refetch = useCallback(() => {
+    fetchData(dbId, true);
+  }, [dbId, fetchData]);
 
   useEffect(() => {
-    if (isMountedRef.current) {
-      const { requestId, isSuccess, isError, isFetching, data, originalArgs } =
-        result;
-      if (!originalArgs?.forceRefresh && requestId && !isFetching) {
-        if (isSuccess) {
-          handleOnSuccess(data || EMPTY_CATALOGS, false);
-        }
-        if (isError) {
-          handleOnError();
-        }
-      }
-    } else {
-      isMountedRef.current = true;
-    }
-  }, [result, handleOnSuccess, handleOnError]);
+    fetchData(dbId, false);
+  }, [dbId, fetchData]);
 
   return {
     ...result,

diff --git a/superset-frontend/src/hooks/apiResources/schemas.test.ts b/superset-frontend/src/hooks/apiResources/schemas.test.ts
@@ -88,7 +88,7 @@ describe('useSchemas hook', () => {
         })}`,
       ).length,
     ).toBe(1);
-    expect(onSuccess).toHaveBeenCalledTimes(2);
+    expect(onSuccess).toHaveBeenCalledTimes(1);
     act(() => {
       result.current.refetch();
     });
@@ -100,7 +100,7 @@ describe('useSchemas hook', () => {
         })}`,
       ).length,
     ).toBe(1);
-    expect(onSuccess).toHaveBeenCalledTimes(3);
+    expect(onSuccess).toHaveBeenCalledTimes(2);
     expect(result.current.data).toEqual(expectedResult);
   });
 
@@ -149,28 +149,36 @@ describe('useSchemas hook', () => {
       },
     );
 
-    await waitFor(() => expect(result.current.data).toEqual(expectedResult));
+    await waitFor(() =>
+      expect(result.current.currentData).toEqual(expectedResult),
+    );
     expect(fetchMock.calls(schemaApiRoute).length).toBe(1);
-    expect(onSuccess).toHaveBeenCalledTimes(2);
+    expect(onSuccess).toHaveBeenCalledTimes(1);
 
     rerender({ dbId: 'db2' });
-    await waitFor(() => expect(result.current.data).toEqual(expectedResult2));
+    await waitFor(() =>
+      expect(result.current.currentData).toEqual(expectedResult2),
+    );
     expect(fetchMock.calls(schemaApiRoute).length).toBe(2);
-    expect(onSuccess).toHaveBeenCalledTimes(4);
+    expect(onSuccess).toHaveBeenCalledTimes(2);
 
     rerender({ dbId: expectDbId });
-    await waitFor(() => expect(result.current.data).toEqual(expectedResult));
+    await waitFor(() =>
+      expect(result.current.currentData).toEqual(expectedResult),
+    );
     expect(fetchMock.calls(schemaApiRoute).length).toBe(2);
-    expect(onSuccess).toHaveBeenCalledTimes(5);
+    expect(onSuccess).toHaveBeenCalledTimes(2);
 
     // clean up cache
     act(() => {
       store.dispatch(api.util.invalidateTags(['Schemas']));
     });
 
-    await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(3));
+    await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(4));
     expect(fetchMock.calls(schemaApiRoute)[2][0]).toContain(expectDbId);
-    await waitFor(() => expect(result.current.data).toEqual(expectedResult));
+    await waitFor(() =>
+      expect(result.current.currentData).toEqual(expectedResult),
+    );
   });
 
   test('returns correct schema list by a catalog', async () => {
@@ -201,7 +209,7 @@ describe('useSchemas hook', () => {
 
     await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(1));
     expect(result.current.data).toEqual(expectedResult3);
-    expect(onSuccess).toHaveBeenCalledTimes(2);
+    expect(onSuccess).toHaveBeenCalledTimes(1);
 
     rerender({ dbId, catalog: 'catalog2' });
     await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(2));

diff --git a/superset-frontend/src/hooks/apiResources/schemas.ts b/superset-frontend/src/hooks/apiResources/schemas.ts
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { useCallback, useEffect, useRef } from 'react';
+import { useCallback, useEffect } from 'react';
 import useEffectEvent from 'src/hooks/useEffectEvent';
 import { api, JsonResponse } from './queryApi';
 
@@ -72,7 +72,6 @@ export const {
 export const EMPTY_SCHEMAS = [] as SchemaOption[];
 
 export function useSchemas(options: Params) {
-  const isMountedRef = useRef(false);
   const { dbId, catalog, onSuccess, onError } = options || {};
   const [trigger] = useLazySchemasQuery();
   const result = useSchemasQuery(
@@ -82,62 +81,34 @@ export function useSchemas(options: Params) {
     },
   );
 
-  const handleOnSuccess = useEffectEvent(
-    (data: SchemaOption[], isRefetched: boolean) => {
-      onSuccess?.(data, isRefetched);
+  const fetchData = useEffectEvent(
+    (
+      dbId: FetchSchemasQueryParams['dbId'],
+      catalog: FetchSchemasQueryParams['catalog'],
+      forceRefresh = false,
+    ) => {
+      if (dbId && (!result.currentData || forceRefresh)) {
+        trigger({ dbId, catalog, forceRefresh }).then(
+          ({ isSuccess, isError, data }) => {
+            if (isSuccess) {
+              onSuccess?.(data || EMPTY_SCHEMAS, forceRefresh);
+            }
+            if (isError) {
+              onError?.();
+            }
+          },
+        );
+      }
     },
   );
 
-  const handleOnError = useEffectEvent(() => {
-    onError?.();
-  });
-
   useEffect(() => {
-    if (dbId) {
-      trigger({ dbId, catalog, forceRefresh: false }).then(
-        ({ isSuccess, isError, data }) => {
-          if (isSuccess) {
-            handleOnSuccess(data || EMPTY_SCHEMAS, true);
-          }
-          if (isError) {
-            handleOnError();
-          }
-        },
-      );
-    }
-  }, [dbId, catalog, handleOnError, handleOnSuccess, trigger]);
+    fetchData(dbId, catalog, false);
+  }, [dbId, catalog, fetchData]);
 
   const refetch = useCallback(() => {
-    if (dbId) {
-      trigger({ dbId, catalog, forceRefresh: true }).then(
-        ({ isSuccess, isError, data }) => {
-          if (isSuccess) {
-            handleOnSuccess(data || EMPTY_SCHEMAS, true);
-          }
-          if (isError) {
-            handleOnError();
-          }
-        },
-      );
-    }
-  }, [dbId, catalog, handleOnError, handleOnSuccess, trigger]);
-
-  useEffect(() => {
-    if (isMountedRef.current) {
-      const { requestId, isSuccess, isError, isFetching, data, originalArgs } =
-        result;
-      if (!originalArgs?.forceRefresh && requestId && !isFetching) {
-        if (isSuccess) {
-          handleOnSuccess(data || EMPTY_SCHEMAS, false);
-        }
-        if (isError) {
-          handleOnError();
-        }
-      }
-    } else {
-      isMountedRef.current = true;
-    }
-  }, [catalog, result, handleOnSuccess, handleOnError]);
+    fetchData(dbId, catalog, true);
+  }, [dbId, catalog, fetchData]);
 
   return {
     ...result,