Optimize dataset hooks and LeftColumn visibility logic

apache · Feb 2, 2023 · c6a9b07 · c6a9b07
1 parent a80c6b5
commit c6a9b07
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 54 deletions.
diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx
@@ -17,8 +17,8 @@
  * under the License.
  */
 import { styled, t } from '@superset-ui/core';
-import React, { useEffect } from 'react';
-import { useGetDatasetRelatedObjects } from 'src/views/CRUD/data/hooks';
+import React from 'react';
+import { useGetDatasetRelatedCounts } from 'src/views/CRUD/data/hooks';
 import Badge from 'src/components/Badge';
 import Tabs from 'src/components/Tabs';
 
@@ -53,16 +53,7 @@ const TRANSLATIONS = {
 };
 
 const EditPage = ({ id }: EditPageProps) => {
-  const { getDatasetRelatedObjects, usageCount } =
-    useGetDatasetRelatedObjects(id);
-  useEffect(() => {
-    // Todo: this useEffect should be used to call all count methods conncurently
-    // when we populate data for the new tabs. For right separating out this
-    // api call for building the usage page.
-    if (id) {
-      getDatasetRelatedObjects();
-    }
-  }, [id, getDatasetRelatedObjects]);
+  const { usageCount } = useGetDatasetRelatedCounts(id);
 
   const usageTab = (
     <TabStyles>

diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx
@@ -18,7 +18,7 @@
  */
 import React, { useReducer, Reducer, useEffect, useState } from 'react';
 import { useParams } from 'react-router-dom';
-import { useGetDatasetsList } from 'src/views/CRUD/data/hooks';
+import { useDatasetsList } from 'src/views/CRUD/data/hooks';
 import Header from './Header';
 import EditPage from './EditDataset';
 import DatasetPanel from './DatasetPanel';
@@ -77,23 +77,11 @@ export default function AddDataset() {
   >(datasetReducer, null);
   const [hasColumns, setHasColumns] = useState(false);
   const [editPageIsVisible, setEditPageIsVisible] = useState(false);
-  const encodedSchema = dataset?.schema
-    ? encodeURIComponent(dataset?.schema)
-    : undefined;
 
-  const { getDatasetsList, datasets, datasetNames } = useGetDatasetsList();
-
-  useEffect(() => {
-    const filters = [
-      { col: 'database', opr: 'rel_o_m', value: dataset?.db?.id },
-      { col: 'schema', opr: 'eq', value: encodedSchema },
-      { col: 'sql', opr: 'dataset_is_null_or_empty', value: true },
-    ];
-
-    if (dataset?.schema) {
-      getDatasetsList(filters);
-    }
-  }, [dataset?.db?.id, dataset?.schema, encodedSchema, getDatasetsList]);
+  const { datasets, datasetNames } = useDatasetsList(
+    dataset?.db,
+    dataset?.schema,
+  );
 
   const { datasetId: id } = useParams<{ datasetId: string }>();
   useEffect(() => {
@@ -143,7 +131,6 @@ export default function AddDataset() {
         editPageIsVisible ? EditPageComponent() : DatasetPanelComponent()
       }
       footer={FooterComponent()}
-      editPageIsVisible={editPageIsVisible}
     />
   );
 }
diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx
@@ -37,7 +37,6 @@ interface DatasetLayoutProps {
   datasetPanel?: ReactElement<any, string | JSXElementConstructor<any>> | null;
   rightPanel?: ReactElement<any, string | JSXElementConstructor<any>> | null;
   footer?: ReactElement<any, string | JSXElementConstructor<any>> | null;
-  editPageIsVisible?: boolean;
 }
 
 export default function DatasetLayout({
@@ -46,17 +45,14 @@ export default function DatasetLayout({
   datasetPanel,
   rightPanel,
   footer,
-  editPageIsVisible,
 }: DatasetLayoutProps) {
   return (
     <StyledLayoutWrapper data-test="dataset-layout-wrapper">
       {header && <StyledLayoutHeader>{header}</StyledLayoutHeader>}
       <OuterRow>
-        {!editPageIsVisible && (
+        {leftPanel && (
           <LeftColumn>
-            {leftPanel && (
-              <StyledLayoutLeftPanel>{leftPanel}</StyledLayoutLeftPanel>
-            )}
+            <StyledLayoutLeftPanel>{leftPanel}</StyledLayoutLeftPanel>
           </LeftColumn>
         )}
         <RightColumn>

diff --git a/superset-frontend/src/views/CRUD/data/hooks.ts b/superset-frontend/src/views/CRUD/data/hooks.ts
@@ -18,13 +18,15 @@
  */
 import { useState, useEffect, useCallback } from 'react';
 import { SupersetClient, logging, t } from '@superset-ui/core';
+import rison from 'rison';
 import { addDangerToast } from 'src/components/MessageToasts/actions';
 import { DatasetObject } from 'src/views/CRUD/data/dataset/AddDataset/types';
-import rison from 'rison';
+import { DatabaseObject } from 'src/components/DatabaseSelector';
 
 type BaseQueryObject = {
   id: number;
 };
+
 export function useQueryPreviewState<D extends BaseQueryObject = any>({
   queries,
   fetchData,
@@ -81,8 +83,16 @@ export function useQueryPreviewState<D extends BaseQueryObject = any>({
 /**
  * Retrieves all pages of dataset results
  */
-export const useGetDatasetsList = () => {
+export const useDatasetsList = (
+  db:
+    | (DatabaseObject & {
+        owners: [number];
+      })
+    | undefined,
+  schema: string | null | undefined,
+) => {
   const [datasets, setDatasets] = useState<DatasetObject[]>([]);
+  const encodedSchema = schema ? encodeURIComponent(schema) : undefined;
 
   const getDatasetsList = useCallback(async (filters: object[]) => {
     let results: DatasetObject[] = [];
@@ -119,27 +129,51 @@ export const useGetDatasetsList = () => {
     return results;
   }, []);
 
+  useEffect(() => {
+    const filters = [
+      { col: 'database', opr: 'rel_o_m', value: db?.id },
+      { col: 'schema', opr: 'eq', value: encodedSchema },
+      { col: 'sql', opr: 'dataset_is_null_or_empty', value: true },
+    ];
+
+    if (schema) {
+      getDatasetsList(filters);
+    }
+  }, [db?.id, schema, encodedSchema, getDatasetsList]);
+
   const datasetNames = datasets?.map(dataset => dataset.table_name);
 
-  return { getDatasetsList, datasets, datasetNames };
+  return { datasets, datasetNames };
 };
 
-export const useGetDatasetRelatedObjects = (id: string) => {
+export const useGetDatasetRelatedCounts = (id: string) => {
   const [usageCount, setUsageCount] = useState(0);
 
-  const getDatasetRelatedObjects = () =>
-    SupersetClient.get({
-      endpoint: `/api/v1/dataset/${id}/related_objects`,
-    })
-      .then(({ json }) => {
-        setUsageCount(json?.charts.count);
+  const getDatasetRelatedObjects = useCallback(
+    () =>
+      SupersetClient.get({
+        endpoint: `/api/v1/dataset/${id}/related_objects`,
       })
-      .catch(error => {
-        addDangerToast(
-          t(`There was an error fetching dataset's related objects`),
-        );
-        logging.error(error);
-      });
-
-  return { getDatasetRelatedObjects, usageCount };
+        .then(({ json }) => {
+          setUsageCount(json?.charts.count);
+        })
+        .catch(error => {
+          addDangerToast(
+            t(`There was an error fetching dataset's related objects`),
+          );
+          logging.error(error);
+        }),
+    [id],
+  );
+
+  useEffect(() => {
+    // Todo: this useEffect should be used to call all count methods conncurently
+    // when we populate data for the new tabs. For right separating out this
+    // api call for building the usage page.
+    if (id) {
+      getDatasetRelatedObjects();
+    }
+  }, [id, getDatasetRelatedObjects]);
+
+  return { usageCount };
 };