#14552 : fix feed link breaking if no data found (#14553)

* fix feed link breaking if data not found

* minor change

* changes as per comment mentioned

---------

Co-authored-by: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com>

openmetadata-ui/src/main/resources/ui/src/components/common/PopOverCard/EntityPopOverCard.test.tsx

@@ -0,0 +1,236 @@
+/*
+ *  Copyright 2024 Collate.
+ *  Licensed 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 { act, render, screen } from '@testing-library/react';
+import React from 'react';
+import { EntityType } from '../../../enums/entity.enum';
+import { MOCK_TAG_DATA, MOCK_TAG_ENCODED_FQN } from '../../../mocks/Tags.mock';
+import { getTagByFqn } from '../../../rest/tagAPI';
+import { useApplicationConfigContext } from '../../ApplicationConfigProvider/ApplicationConfigProvider';
+import EntityPopOverCard, { PopoverContent } from './EntityPopOverCard';
+
+const updateCachedEntityData = jest.fn();
+
+jest.mock('../../../utils/CommonUtils', () => ({
+  getTableFQNFromColumnFQN: jest.fn(),
+}));
+
+jest.mock('../../../utils/EntityUtils', () => ({
+  getEntityName: jest.fn(),
+}));
+
+jest.mock('../../../utils/StringsUtils', () => ({
+  getDecodedFqn: jest.fn(),
+  getEncodedFqn: jest.fn(),
+}));
+
+jest.mock('../../Loader/Loader', () => {
+  return jest.fn().mockImplementation(() => <p>Loader</p>);
+});
+
+jest.mock('../../ExploreV1/ExploreSearchCard/ExploreSearchCard', () => {
+  return jest.fn().mockImplementation(() => <p>ExploreSearchCard</p>);
+});
+
+jest.mock('../../../rest/dashboardAPI', () => ({
+  getDashboardByFqn: jest.fn().mockImplementation(() => Promise.resolve({})),
+}));
+
+jest.mock('../../../rest/dataModelsAPI', () => ({
+  getDataModelDetailsByFQN: jest
+    .fn()
+    .mockImplementation(() => Promise.resolve({})),
+}));
+
+jest.mock('../../../rest/dataProductAPI', () => ({
+  getDataProductByName: jest.fn().mockImplementation(() => Promise.resolve({})),
+}));
+
+jest.mock('../../../rest/databaseAPI', () => ({
+  getDatabaseDetailsByFQN: jest
+    .fn()
+    .mockImplementation(() => Promise.resolve({})),
+  getDatabaseSchemaDetailsByFQN: jest
+    .fn()
+    .mockImplementation(() => Promise.resolve({})),
+}));
+
+jest.mock('../../../rest/domainAPI', () => ({
+  getDomainByName: jest.fn().mockImplementation(() => Promise.resolve({})),
+}));
+
+jest.mock('../../../rest/glossaryAPI', () => ({
+  getGlossariesByName: jest.fn().mockImplementation(() => Promise.resolve({})),
+  getGlossaryTermByFQN: jest.fn().mockImplementation(() => Promise.resolve({})),
+}));
+
+jest.mock('../../../rest/mlModelAPI', () => ({
+  getMlModelByFQN: jest.fn().mockImplementation(() => Promise.resolve({})),
+}));
+
+jest.mock('../../../rest/pipelineAPI', () => ({
+  getPipelineByFqn: jest.fn().mockImplementation(() => Promise.resolve({})),
+}));
+
+jest.mock('../../../rest/storageAPI', () => ({
+  getContainerByFQN: jest.fn().mockImplementation(() => Promise.resolve({})),
+}));
+
+jest.mock('../../../rest/storedProceduresAPI', () => ({
+  getStoredProceduresDetailsByFQN: jest
+    .fn()
+    .mockImplementation(() => Promise.resolve({})),
+}));
+
+jest.mock('../../../rest/tableAPI', () => ({
+  getTableDetailsByFQN: jest.fn().mockImplementation(() => Promise.resolve({})),
+}));
+
+jest.mock('../../../rest/tagAPI', () => ({
+  getTagByFqn: jest.fn().mockImplementation(() => Promise.resolve({})),
+}));
+
+jest.mock('../../../rest/topicsAPI', () => ({
+  getTopicByFqn: jest.fn().mockImplementation(() => Promise.resolve({})),
+}));
+
+jest.mock('../../ApplicationConfigProvider/ApplicationConfigProvider', () => ({
+  useApplicationConfigContext: jest.fn().mockImplementation(() => ({
+    cachedEntityData: {},
+    updateCachedEntityData,
+  })),
+}));
+
+describe('Test EntityPopoverCard component', () => {
+  it('EntityPopoverCard should render element', () => {
+    render(
+      <EntityPopOverCard
+        entityFQN={MOCK_TAG_ENCODED_FQN}
+        entityType={EntityType.TAG}>
+        <div data-testid="popover-container">Test_Popover</div>
+      </EntityPopOverCard>
+    );
+
+    expect(screen.getByTestId('popover-container')).toBeInTheDocument();
+  });
+
+  it('EntityPopoverCard should render loader on initial render', async () => {
+    render(
+      <PopoverContent
+        entityFQN={MOCK_TAG_ENCODED_FQN}
+        entityType={EntityType.TAG}
+      />
+    );
+
+    const loader = screen.getByText('Loader');
+
+    expect(loader).toBeInTheDocument();
+  });
+
+  it("EntityPopoverCard should show no data placeholder if entity type doesn't match", async () => {
+    await act(async () => {
+      render(
+        <PopoverContent
+          entityFQN={MOCK_TAG_ENCODED_FQN}
+          entityType={EntityType.APPLICATION}
+        />
+      );
+    });
+
+    expect(screen.getByText('label.no-data-found')).toBeInTheDocument();
+  });
+
+  it('EntityPopoverCard should show no data placeholder if api call fail', async () => {
+    (getTagByFqn as jest.Mock).mockImplementationOnce(() =>
+      Promise.reject({
+        response: {
+          data: { message: 'Error!' },
+        },
+      })
+    );
+
+    await act(async () => {
+      render(
+        <PopoverContent
+          entityFQN={MOCK_TAG_ENCODED_FQN}
+          entityType={EntityType.TAG}
+        />
+      );
+    });
+
+    expect(screen.getByText('label.no-data-found')).toBeInTheDocument();
+  });
+
+  it('EntityPopoverCard should call tags api if entity type is tag card', async () => {
+    const mockTagAPI = getTagByFqn as jest.Mock;
+
+    await act(async () => {
+      render(
+        <PopoverContent
+          entityFQN={MOCK_TAG_ENCODED_FQN}
+          entityType={EntityType.TAG}
+        />
+      );
+    });
+
+    expect(mockTagAPI.mock.calls[0][0]).toBe(MOCK_TAG_ENCODED_FQN);
+  });
+
+  it('EntityPopoverCard should call api and trigger updateCachedEntityData in provider', async () => {
+    const mockTagAPI = getTagByFqn as jest.Mock;
+
+    (getTagByFqn as jest.Mock).mockImplementationOnce(() =>
+      Promise.resolve(MOCK_TAG_DATA)
+    );
+
+    await act(async () => {
+      render(
+        <PopoverContent
+          entityFQN={MOCK_TAG_ENCODED_FQN}
+          entityType={EntityType.TAG}
+        />
+      );
+    });
+
+    expect(mockTagAPI.mock.calls[0][0]).toBe(MOCK_TAG_ENCODED_FQN);
+
+    expect(updateCachedEntityData).toHaveBeenCalledWith({
+      id: MOCK_TAG_ENCODED_FQN,
+      entityDetails: MOCK_TAG_DATA,
+    });
+  });
+
+  it('EntityPopoverCard should not call api if cached data is available', async () => {
+    const mockTagAPI = getTagByFqn as jest.Mock;
+
+    (useApplicationConfigContext as jest.Mock).mockImplementation(() => ({
+      cachedEntityData: {
+        [MOCK_TAG_ENCODED_FQN]: {
+          name: 'test',
+        },
+      },
+    }));
+
+    await act(async () => {
+      render(
+        <PopoverContent
+          entityFQN={MOCK_TAG_ENCODED_FQN}
+          entityType={EntityType.TAG}
+        />
+      );
+    });
+
+    expect(mockTagAPI.mock.calls).toEqual([]);
+    expect(screen.getByText('ExploreSearchCard')).toBeInTheDocument();
+  });
+});

openmetadata-ui/src/main/resources/ui/src/components/common/PopOverCard/EntityPopOverCard.tsx

@@ -11,7 +11,8 @@
  *  limitations under the License.
  */
 
-import { Popover } from 'antd';
+import { Popover, Typography } from 'antd';
+import { isUndefined } from 'lodash';
 import React, {
   FC,
   HTMLAttributes,
@@ -20,6 +21,7 @@ import React, {
   useMemo,
   useState,
 } from 'react';
+import { useTranslation } from 'react-i18next';
 import { EntityType } from '../../../enums/entity.enum';
 import { Table } from '../../../generated/entity/data/table';
 import { Include } from '../../../generated/type/include';
@@ -40,6 +42,7 @@ import { getPipelineByFqn } from '../../../rest/pipelineAPI';
 import { getContainerByFQN } from '../../../rest/storageAPI';
 import { getStoredProceduresDetailsByFQN } from '../../../rest/storedProceduresAPI';
 import { getTableDetailsByFQN } from '../../../rest/tableAPI';
+import { getTagByFqn } from '../../../rest/tagAPI';
 import { getTopicByFqn } from '../../../rest/topicsAPI';
 import { getTableFQNFromColumnFQN } from '../../../utils/CommonUtils';
 import { getEntityName } from '../../../utils/EntityUtils';
@@ -48,6 +51,7 @@ import { useApplicationConfigContext } from '../../ApplicationConfigProvider/App
 import { EntityUnion } from '../../Explore/ExplorePage.interface';
 import ExploreSearchCard from '../../ExploreV1/ExploreSearchCard/ExploreSearchCard';
 import Loader from '../../Loader/Loader';
+import { SearchedDataProps } from '../../SearchedData/SearchedData.interface';
 import './popover-card.less';
 
 interface Props extends HTMLAttributes<HTMLDivElement> {
@@ -55,17 +59,33 @@ interface Props extends HTMLAttributes<HTMLDivElement> {
   entityFQN: string;
 }
 
-const PopoverContent: React.FC<{
+export const PopoverContent: React.FC<{
   entityFQN: string;
   entityType: string;
 }> = ({ entityFQN, entityType }) => {
+  const { t } = useTranslation();
   const [loading, setLoading] = useState(true);
   const { cachedEntityData, updateCachedEntityData } =
     useApplicationConfigContext();
-  const entityData = useMemo(
-    () => cachedEntityData[entityFQN],
-    [cachedEntityData, entityFQN]
-  );
+
+  const entityData: SearchedDataProps['data'][number]['_source'] | undefined =
+    useMemo(() => {
+      const data = cachedEntityData[entityFQN];
+
+      return data
+        ? {
+            ...data,
+            name: data.name,
+            displayName: getEntityName(data),
+            id: data.id ?? '',
+            description: data.description ?? '',
+            fullyQualifiedName: getDecodedFqn(entityFQN),
+            tags: (data as Table)?.tags,
+            entityType: entityType,
+            serviceType: (data as Table)?.serviceType,
+          }
+        : data;
+    }, [cachedEntityData, entityFQN]);
 
   const getData = useCallback(async () => {
     const fields = 'tags,owner';
@@ -146,6 +166,11 @@ const PopoverContent: React.FC<{
 
         break;
 
+      case EntityType.TAG:
+        promise = getTagByFqn(entityFQN);
+
+        break;
+
       default:
         break;
     }
@@ -162,11 +187,14 @@ const PopoverContent: React.FC<{
     } else {
       setLoading(false);
     }
-  }, [entityType, entityFQN]);
+  }, [entityType, entityFQN, updateCachedEntityData]);
 
   useEffect(() => {
     const entityData = cachedEntityData[entityFQN];
-    if (!entityData) {
+
+    if (entityData) {
+      setLoading(false);
+    } else {
       getData();
     }
   }, [entityFQN]);
@@ -175,21 +203,15 @@ const PopoverContent: React.FC<{
     return <Loader size="small" />;
   }
 
+  if (isUndefined(entityData)) {
+    return <Typography.Text>{t('label.no-data-found')}</Typography.Text>;
+  }
+
   return (
     <ExploreSearchCard
       id="tabledatacard"
       showTags={false}
-      source={{
-        ...entityData,
-        name: entityData.name,
-        displayName: getEntityName(entityData),
-        id: entityData.id ?? '',
-        description: entityData.description ?? '',
-        fullyQualifiedName: getDecodedFqn(entityFQN),
-        tags: (entityData as Table).tags,
-        entityType: entityType,
-        serviceType: (entityData as Table).serviceType,
-      }}
+      source={entityData}
     />
   );
 };

openmetadata-ui/src/main/resources/ui/src/mocks/Tags.mock.ts

@@ -11,6 +11,8 @@
  *  limitations under the License.
  */
 
+export const MOCK_TAG_ENCODED_FQN = '"%22Mock.Tag%22.Tag_1"';
+
 export const mockTagList = [
   {
     id: 'e649c601-44d3-449d-bc04-fbbaf83baf19',
@@ -26,3 +28,30 @@ export const mockTagList = [
     mutuallyExclusive: true,
   },
 ];
+
+export const MOCK_TAG_DATA = {
+  id: 'e8bc85c8-a87f-471c-872e-46904c5ea888',
+  name: 'search_part_2',
+  displayName: '',
+  fullyQualifiedName: 'advanceSearch.search_part_2',
+  description: 'this is search_part_2',
+  style: {},
+  classification: {
+    id: '16c5865a-8804-4474-a1dd-14ee9da443b2',
+    type: 'classification',
+    name: 'advanceSearch',
+    fullyQualifiedName: 'advanceSearch',
+    description: 'advanceSearch',
+    displayName: '',
+    deleted: false,
+    href: 'http://sandbox-beta.open-metadata.org/api/v1/classifications/16c5865a-8804-4474-a1dd-14ee9da443b2',
+  },
+  version: 0.1,
+  updatedAt: 1704261482857,
+  updatedBy: 'ashish',
+  href: 'http://sandbox-beta.open-metadata.org/api/v1/tags/e8bc85c8-a87f-471c-872e-46904c5ea888',
+  deprecated: false,
+  deleted: false,
+  provider: 'user',
+  mutuallyExclusive: false,
+};

openmetadata-ui/src/main/resources/ui/src/rest/tagAPI.ts

@@ -19,6 +19,7 @@ import { CreateTag } from '../generated/api/classification/createTag';
 import { Classification } from '../generated/entity/classification/classification';
 import { Tag } from '../generated/entity/classification/tag';
 import { EntityHistory } from '../generated/type/entityHistory';
+import { ListParams } from '../interface/API.interface';
 import { getURLWithQueryFields } from '../utils/APIUtils';
 import APIClient from './index';
 
@@ -108,6 +109,14 @@ export const patchClassification = async (id: string, data: Operation[]) => {
   return response.data;
 };
 
+export const getTagByFqn = async (fqn: string, params?: ListParams) => {
+  const response = await APIClient.get<Tag>(`tags/name/${fqn}`, {
+    params,
+  });
+
+  return response.data;
+};
+
 export const createTag = async (data: CreateTag) => {
   const response = await APIClient.post<CreateTag, AxiosResponse<Tag>>(
     `/tags`,

openmetadata-ui/src/main/resources/ui/src/utils/APIUtils.ts

@@ -191,6 +191,9 @@ export const formatSearchTagsResponse = (
   }));
 };
 
+/**
+ * @deprecated getURLWithQueryFields is deprecated, Please use params to pass query parameters wherever it is required
+ */
 export const getURLWithQueryFields = (
   url: string,
   lstQueryFields?: string | string[],