From 911b41f7f3e441935a4a4e4f67e0b6582c6e90f4 Mon Sep 17 00:00:00 2001
From: Aniket Katkar <aniketkatkar97@gmail.com>
Date: Wed, 16 Aug 2023 15:31:57 +0530
Subject: [PATCH] fix(ui): schema page loader bug (#12786)

* Fixed issue for database schema page where no data placeholder was showing while loading permissions

* removed unnecessary condition check
---
 .../DatabaseSchemaPage.component.tsx          | 23 ++++---
 .../DatabaseSchemaPage.test.tsx               | 62 ++++++++++---------
 2 files changed, 44 insertions(+), 41 deletions(-)

diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/DatabaseSchemaPage/DatabaseSchemaPage.component.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/DatabaseSchemaPage/DatabaseSchemaPage.component.tsx
index c3079c1d530..bba821b4f2d 100644
--- a/openmetadata-ui/src/main/resources/ui/src/pages/DatabaseSchemaPage/DatabaseSchemaPage.component.tsx
+++ b/openmetadata-ui/src/main/resources/ui/src/pages/DatabaseSchemaPage/DatabaseSchemaPage.component.tsx
@@ -89,7 +89,7 @@ const DatabaseSchemaPage: FunctionComponent = () => {
   const [threadType, setThreadType] = useState<ThreadType>(
     ThreadType.Conversation
   );
-  const [isLoading, setIsLoading] = useState(true);
+  const [isPermissionsLoading, setIsPermissionsLoading] = useState(true);
   const [databaseSchema, setDatabaseSchema] = useState<DatabaseSchema>(
     {} as DatabaseSchema
   );
@@ -132,7 +132,7 @@ const DatabaseSchemaPage: FunctionComponent = () => {
   );
 
   const fetchDatabaseSchemaPermission = useCallback(async () => {
-    setIsLoading(true);
+    setIsPermissionsLoading(true);
     try {
       const response = await getEntityPermissionByFqn(
         ResourceEntity.DATABASE_SCHEMA,
@@ -142,7 +142,7 @@ const DatabaseSchemaPage: FunctionComponent = () => {
     } catch (error) {
       showErrorToast(error as AxiosError);
     } finally {
-      setIsLoading(false);
+      setIsPermissionsLoading(false);
     }
   }, [databaseSchemaFQN]);
 
@@ -193,7 +193,6 @@ const DatabaseSchemaPage: FunctionComponent = () => {
     } catch (err) {
       // Error
     } finally {
-      setIsLoading(false);
       setIsSchemaDetailsLoading(false);
     }
   }, [databaseSchemaFQN]);
@@ -435,6 +434,10 @@ const DatabaseSchemaPage: FunctionComponent = () => {
     [tableData, getSchemaTables]
   );
 
+  useEffect(() => {
+    fetchDatabaseSchemaPermission();
+  }, [databaseSchemaFQN]);
+
   useEffect(() => {
     if (viewDatabaseSchemaPermission) {
       fetchDatabaseSchemaDetails();
@@ -443,14 +446,10 @@ const DatabaseSchemaPage: FunctionComponent = () => {
   }, [viewDatabaseSchemaPermission, databaseSchemaFQN]);
 
   useEffect(() => {
-    if (databaseSchemaFQN) {
+    if (viewDatabaseSchemaPermission && databaseSchemaFQN) {
       getSchemaTables();
     }
-  }, [showDeletedTables, databaseSchemaFQN]);
-
-  useEffect(() => {
-    fetchDatabaseSchemaPermission();
-  }, [databaseSchemaFQN]);
+  }, [showDeletedTables, databaseSchemaFQN, viewDatabaseSchemaPermission]);
 
   // always Keep this useEffect at the end...
   useEffect(() => {
@@ -558,7 +557,7 @@ const DatabaseSchemaPage: FunctionComponent = () => {
     },
   ];
 
-  if (isLoading) {
+  if (isPermissionsLoading) {
     return <Loader />;
   }
 
@@ -572,7 +571,7 @@ const DatabaseSchemaPage: FunctionComponent = () => {
       pageTitle={t('label.entity-detail-plural', {
         entity: getEntityName(databaseSchema),
       })}>
-      {isEmpty(databaseSchema) ? (
+      {isEmpty(databaseSchema) && !isSchemaDetailsLoading ? (
         <ErrorPlaceHolder className="m-0">
           {getEntityMissingError(EntityType.DATABASE_SCHEMA, databaseSchemaFQN)}
         </ErrorPlaceHolder>
diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/DatabaseSchemaPage/DatabaseSchemaPage.test.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/DatabaseSchemaPage/DatabaseSchemaPage.test.tsx
index 3ee62a7402d..e0c9ea270c7 100644
--- a/openmetadata-ui/src/main/resources/ui/src/pages/DatabaseSchemaPage/DatabaseSchemaPage.test.tsx
+++ b/openmetadata-ui/src/main/resources/ui/src/pages/DatabaseSchemaPage/DatabaseSchemaPage.test.tsx
@@ -35,23 +35,15 @@ jest.mock('../../utils/ToastUtils', () => ({
 }));
 
 jest.mock('components/Loader/Loader', () =>
-  jest.fn().mockImplementation(() => <div data-testid="Loader">Loader</div>)
+  jest.fn().mockImplementation(() => <div>Loader</div>)
 );
 
 jest.mock('components/common/rich-text-editor/RichTextEditorPreviewer', () =>
-  jest
-    .fn()
-    .mockImplementation(() => (
-      <div data-testid="RichTextEditorPreviewer">RichTextEditorPreviewer</div>
-    ))
+  jest.fn().mockImplementation(() => <div>RichTextEditorPreviewer</div>)
 );
 
 jest.mock('components/common/next-previous/NextPrevious', () =>
-  jest
-    .fn()
-    .mockImplementation(() => (
-      <div data-testid="NextPrevious">NextPrevious</div>
-    ))
+  jest.fn().mockImplementation(() => <div>NextPrevious</div>)
 );
 
 jest.mock('components/FeedEditor/FeedEditor', () => {
@@ -62,7 +54,7 @@ jest.mock('components/common/error-with-placeholder/ErrorPlaceHolder', () =>
   jest
     .fn()
     .mockImplementation(({ children }) => (
-      <div data-testid="ErrorPlaceHolder">{children}</div>
+      <div data-testid="error-placeHolder">{children}</div>
     ))
 );
 
@@ -72,7 +64,6 @@ jest.mock('components/common/description/Description', () =>
     .mockImplementation(
       ({ onThreadLinkSelect, onDescriptionEdit, onDescriptionUpdate }) => (
         <div
-          data-testid="Description"
           onClick={() => {
             onThreadLinkSelect('threadLink');
             onDescriptionEdit();
@@ -86,12 +77,7 @@ jest.mock('components/common/description/Description', () =>
 
 jest.mock(
   'components/ActivityFeed/ActivityThreadPanel/ActivityThreadPanel',
-  () =>
-    jest
-      .fn()
-      .mockImplementation(() => (
-        <div data-testid="ActivityThreadPanel">ActivityThreadPanel</div>
-      ))
+  () => jest.fn().mockImplementation(() => <div>ActivityThreadPanel</div>)
 );
 
 jest.mock('components/PermissionProvider/PermissionProvider', () => ({
@@ -177,11 +163,11 @@ describe.skip('Tests for DatabaseSchemaPage', () => {
 
     const entityPageInfo = await screen.findByTestId('entityPageInfo');
     const tabsPane = await screen.findByTestId('tabs');
-    const richTextEditorPreviewer = await screen.findAllByTestId(
+    const richTextEditorPreviewer = await screen.findAllByText(
       'RichTextEditorPreviewer'
     );
-    const description = await screen.findByTestId('Description');
-    const nextPrevious = await screen.findByTestId('NextPrevious');
+    const description = await screen.findByText('Description');
+    const nextPrevious = await screen.findByText('NextPrevious');
     const databaseSchemaTable = await screen.findByTestId(
       'databaseSchema-tables'
     );
@@ -194,6 +180,26 @@ describe.skip('Tests for DatabaseSchemaPage', () => {
     expect(databaseSchemaTable).toBeInTheDocument();
   });
 
+  it('Loader should be visible if the permissions are being fetched', async () => {
+    await act(async () => {
+      render(<DatabaseSchemaPageComponent />, {
+        wrapper: MemoryRouter,
+      });
+
+      const loader = screen.getByText('Loader');
+      const errorPlaceHolder = screen.queryByText('error-placeHolder');
+
+      expect(loader).toBeInTheDocument();
+      expect(errorPlaceHolder).toBeNull();
+    });
+
+    const entityPageInfo = await screen.findByTestId('entityPageInfo');
+    const tabsPane = await screen.findByTestId('tabs');
+
+    expect(entityPageInfo).toBeInTheDocument();
+    expect(tabsPane).toBeInTheDocument();
+  });
+
   it('Activity Feed List should render properly for "Activity Feeds" tab', async () => {
     mockParams.tab = 'activity_feed';
 
@@ -208,7 +214,7 @@ describe.skip('Tests for DatabaseSchemaPage', () => {
     expect(activityFeedList).toBeInTheDocument();
   });
 
-  it('AcivityThreadPanel should render properly after clicked on thread panel button', async () => {
+  it('ActivityThreadPanel should render properly after clicked on thread panel button', async () => {
     mockParams.tab = 'table';
     await act(async () => {
       render(<DatabaseSchemaPageComponent />, {
@@ -216,7 +222,7 @@ describe.skip('Tests for DatabaseSchemaPage', () => {
       });
     });
 
-    const description = await screen.findByTestId('Description');
+    const description = await screen.findByText('Description');
 
     expect(description).toBeInTheDocument();
 
@@ -224,9 +230,7 @@ describe.skip('Tests for DatabaseSchemaPage', () => {
       fireEvent.click(description);
     });
 
-    const activityThreadPanel = await screen.findByTestId(
-      'ActivityThreadPanel'
-    );
+    const activityThreadPanel = await screen.findByText('ActivityThreadPanel');
 
     expect(activityThreadPanel).toBeInTheDocument();
   });
@@ -242,7 +246,7 @@ describe.skip('Tests for DatabaseSchemaPage', () => {
       });
     });
 
-    const errorPlaceHolder = await screen.findByTestId('ErrorPlaceHolder');
+    const errorPlaceHolder = await screen.findByTestId('error-placeHolder');
     const errorMessage = await screen.findByTestId('error-message');
 
     expect(errorPlaceHolder).toBeInTheDocument();
@@ -266,7 +270,7 @@ describe.skip('Tests for DatabaseSchemaPage', () => {
       });
     });
 
-    const errorPlaceHolder = await screen.findByTestId('ErrorPlaceHolder');
+    const errorPlaceHolder = await screen.findByTestId('error-placeHolder');
 
     expect(errorPlaceHolder).toBeInTheDocument();
   });