Brijesh619 commented on code in PR #688:
URL: https://github.com/apache/atlas/pull/688#discussion_r3923346527
##########
dashboard/src/components/EntityDisplayImage.tsx:
##########
@@ -15,89 +15,60 @@
* limitations under the License.
*/
-import { useEffect, useState } from "react";
-import { Avatar, Skeleton } from "@mui/material";
+import type { SyntheticEvent } from "react";
+import { Avatar } from "@mui/material";
import { getEntityIconPath } from "../utils/Utils";
-import axios from "axios";
+
+interface DisplayImageProps {
+ entity: Record<string, unknown>;
+ width?: string | number;
+ height?: string | number;
+ avatarDisplay?: boolean;
+ isProcess?: boolean;
+}
const DisplayImage = ({
entity,
width,
height,
avatarDisplay,
isProcess
-}: any) => {
- const [imageUrl, setImageUrl] = useState<any>(null);
- const [checkEntityImage, setCheckEntityImage] = useState<any>({
- [entity.guid]: false
- });
-
- useEffect(() => {
- const fetchImagePath = async () => {
- let entityData = { ...entity, ...{ isProcess: isProcess } };
- let imagePath: any = getEntityIconPath({ entityData: entityData });
- try {
- const response = await axios.get(imagePath, {
- responseType: "blob"
- });
- const contentType: any = response.headers["content-type"];
-
- if (contentType && contentType.startsWith("image/")) {
- let cache = { [entityData.guid]: imagePath };
- setCheckEntityImage(cache);
- setImageUrl(getEntityIconPath({ entityData: entityData }));
- } else {
- setImageUrl(
- getEntityIconPath({ entityData: entityData, errorUrl: imagePath })
- );
- }
- } catch (_error) {
- setImageUrl(
- getEntityIconPath({ entityData: entityData, errorUrl: imagePath })
- );
- }
- };
+}: DisplayImageProps) => {
+ const entityData = { ...entity, isProcess };
+
+ const primaryUrl = getEntityIconPath({ entityData }) || "";
+ const fallbackUrl = getEntityIconPath({ entityData, errorUrl: primaryUrl })
|| "";
- fetchImagePath();
- }, []);
+ const handleError = (e: SyntheticEvent<HTMLImageElement, Event>) => {
+ const target = e.currentTarget;
+ if (target.dataset.fallbackApplied !== "true") {
+ target.dataset.fallbackApplied = "true";
+ target.onerror = null;
+ target.src = fallbackUrl;
+ }
+ };
- return imageUrl != undefined ? (
+ return (
<div className="search-result-table-name-col" data-cy="entityIcon">
- {checkEntityImage[entity.guid] !== false ? (
- avatarDisplay == undefined ? (
- <img
- className="search-result-table-img"
- id={entity.guid}
- data-cy={entity.guid}
- src={checkEntityImage[entity.guid]}
- alt="Entity Icon"
- />
- ) : (
- <Avatar
- alt="entityImg"
- src={checkEntityImage[entity.guid]}
- sx={{ width: width, height: height }}
- variant="square"
- ></Avatar>
- )
- ) : avatarDisplay == undefined ? (
+ {avatarDisplay === undefined ? (
<img
className="search-result-table-img"
- id={entity.guid}
- data-cy={entity.guid}
- src={imageUrl}
+ id={entity.guid ? String(entity.guid) : undefined}
Review Comment:
This has been addressed. I've updated the id and data-cy attributes to
conditionally use entity.guid ? String(entity.guid) : undefined so they don't
produce "undefined" strings, and added a negative test case in
EntityDisplayImage.test.tsx to ensure these attributes are omitted when guid is
missing.
##########
dashboard/src/views/SideBar/__tests__/SideBarBody.test.tsx:
##########
@@ -454,6 +465,12 @@ describe('SideBarBody', () => {
expect(metricsSlice.fetchMetricEntity).toHaveBeenCalled();
});
+ it('should dispatch fetchVersionData on mount', () => {
+ renderWithProviders();
+
+ expect(sessionSlice.fetchVersionData).toHaveBeenCalled();
+ });
+
it('should pass loading state to tree components', () => {
Review Comment:
Addressed. I've removed the obsolete should pass loading state to tree
components test, as well as the unused loading prop from the defaultProps in
SideBarBody.test.tsx, since the component no longer accepts or passes this prop.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
