Add unit-test coverage and loading state improvement (#20423)

openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/Extensions/File/AttachmentComponents/AttachmentPlaceholder.test.tsx

@@ -0,0 +1,124 @@
+/*
+ *  Copyright 2025 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 { cleanup, render, screen } from '@testing-library/react';
+import React from 'react';
+import { useTranslation } from 'react-i18next';
+import { getFileIcon } from '../../../../../utils/BlockEditorUtils';
+import { FileType } from '../../../BlockEditor.interface';
+import AttachmentPlaceholder from './AttachmentPlaceholder';
+
+// Mock the translation hook
+jest.mock('react-i18next', () => ({
+  useTranslation: jest.fn(),
+}));
+
+// Mock the getFileIcon utility to return a valid React component
+jest.mock('../../../../../utils/BlockEditorUtils', () => ({
+  getFileIcon: jest
+    .fn()
+    .mockReturnValue(() => <div data-testid="mock-file-icon" />),
+}));
+
+describe('AttachmentPlaceholder', () => {
+  const mockTranslate = jest.fn();
+
+  beforeEach(() => {
+    // Reset all mocks before each test
+    jest.clearAllMocks();
+
+    // Setup translation mock
+    (useTranslation as jest.Mock).mockReturnValue({
+      t: mockTranslate,
+    });
+
+    // Setup getFileIcon mock
+    (getFileIcon as jest.Mock).mockReturnValue(() => (
+      <div data-testid="mock-file-icon" />
+    ));
+  });
+
+  afterEach(() => {
+    cleanup();
+  });
+
+  it('should render the placeholder with correct file type', () => {
+    const fileType = FileType.FILE;
+    mockTranslate.mockImplementation((key, options) => {
+      if (key === 'label.add-an-file-type') {
+        return `Add a ${options.fileType} file`;
+      }
+      if (key === `label.${fileType}`) {
+        return fileType;
+      }
+
+      return key;
+    });
+
+    render(<AttachmentPlaceholder fileType={fileType} />);
+
+    // Verify the placeholder is rendered
+    expect(screen.getByTestId('image-placeholder')).toBeInTheDocument();
+
+    // Verify the icon is rendered
+    expect(screen.getByTestId('mock-file-icon')).toBeInTheDocument();
+
+    // Verify the translation was called with correct parameters
+    expect(mockTranslate).toHaveBeenCalledWith('label.add-an-file-type', {
+      fileType: expect.any(String),
+    });
+  });
+
+  it.each([
+    [FileType.FILE, 'file'],
+    [FileType.IMAGE, 'image'],
+    [FileType.VIDEO, 'video'],
+  ])('should render with %s file type', (fileType, expectedText) => {
+    mockTranslate.mockImplementation((key, options) => {
+      if (key === 'label.add-an-file-type') {
+        return `Add a ${options.fileType} file`;
+      }
+      if (key === `label.${fileType}`) {
+        return fileType;
+      }
+
+      return key;
+    });
+
+    render(<AttachmentPlaceholder fileType={fileType} />);
+
+    expect(screen.getByTestId('image-placeholder')).toBeInTheDocument();
+    expect(screen.getByTestId('mock-file-icon')).toBeInTheDocument();
+    expect(screen.getByText(`Add a ${expectedText} file`)).toBeInTheDocument();
+  });
+
+  it('should have contentEditable set to false', () => {
+    const fileType = FileType.FILE;
+    mockTranslate.mockImplementation((key, options) => {
+      if (key === 'label.add-an-file-type') {
+        return `Add a ${options.fileType} file`;
+      }
+      if (key === `label.${fileType}`) {
+        return fileType;
+      }
+
+      return key;
+    });
+
+    render(<AttachmentPlaceholder fileType={fileType} />);
+
+    const placeholder = screen.getByTestId('image-placeholder');
+
+    expect(placeholder).toHaveAttribute('contentEditable', 'false');
+  });
+});

openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/Extensions/File/AttachmentComponents/FileAttachment.test.tsx

@@ -0,0 +1,131 @@
+/*
+ *  Copyright 2025 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 { fireEvent, render, screen } from '@testing-library/react';
+import { NodeViewProps } from '@tiptap/react';
+import React from 'react';
+import { bytesToSize } from '../../../../../utils/StringsUtils';
+import FileAttachment from './FileAttachment';
+
+describe('FileAttachment', () => {
+  // Create a minimal mock that only includes what the component needs
+  const createMockNode = (attrs: unknown) =>
+    ({
+      attrs: {
+        url: '',
+        fileName: '',
+        fileSize: 0,
+        mimeType: '',
+        isUploading: false,
+        uploadProgress: 0,
+        tempFile: null,
+        ...(attrs as NodeViewProps['node']['attrs']),
+      },
+    } as unknown as NodeViewProps['node']); // Type assertion to avoid TipTap Node type complexity
+
+  const mockNode = createMockNode({
+    url: 'https://example.com/file.pdf',
+    fileName: 'test.pdf',
+    fileSize: 1024 * 1024, // 1MB
+    mimeType: 'application/pdf',
+  });
+
+  const mockProps = {
+    node: mockNode,
+    isFileLoading: false,
+    deleteNode: jest.fn(),
+    onFileClick: jest.fn(),
+  };
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('renders file attachment with correct details', () => {
+    render(<FileAttachment {...mockProps} />);
+
+    // Check if file name is displayed
+    expect(screen.getByText('test.pdf')).toBeInTheDocument();
+
+    // Check if file size is displayed correctly
+    expect(screen.getByText(bytesToSize(1024 * 1024))).toBeInTheDocument();
+
+    // Check if download button is present
+    expect(screen.getByRole('button')).toBeInTheDocument();
+  });
+
+  it('handles file click correctly', () => {
+    render(<FileAttachment {...mockProps} />);
+
+    const fileLink = screen.getByText('test.pdf');
+    fireEvent.click(fileLink);
+
+    expect(mockProps.onFileClick).toHaveBeenCalled();
+  });
+
+  it('handles delete button click correctly', () => {
+    render(<FileAttachment {...mockProps} />);
+
+    const deleteButton = screen.getByLabelText('delete');
+    fireEvent.click(deleteButton);
+
+    expect(mockProps.deleteNode).toHaveBeenCalled();
+  });
+
+  it('shows upload progress when file is uploading', () => {
+    const uploadingNode = createMockNode({
+      ...mockNode.attrs,
+      isUploading: true,
+      uploadProgress: 50,
+    });
+
+    render(<FileAttachment {...mockProps} node={uploadingNode} />);
+
+    // Check if upload progress is displayed
+    const progressBar = screen.getByTestId('upload-progress');
+
+    expect(progressBar).toHaveStyle({ width: '50%' });
+
+    // Check if delete button is not present during upload
+    expect(screen.queryByLabelText('delete')).not.toBeInTheDocument();
+  });
+
+  it('shows loading state on download button when isFileLoading is true', () => {
+    render(<FileAttachment {...mockProps} isFileLoading />);
+
+    const downloadButton = screen.getByRole('button');
+
+    expect(downloadButton).toHaveClass('ant-btn-loading');
+  });
+
+  it('uses tempFile details when available', () => {
+    const tempFileNode = createMockNode({
+      ...mockNode.attrs,
+      tempFile: {
+        name: 'temp.pdf',
+        size: 2048 * 1024, // 2MB
+        type: 'application/pdf',
+      },
+      fileName: null,
+      fileSize: null,
+      mimeType: null,
+    });
+
+    render(<FileAttachment {...mockProps} node={tempFileNode} />);
+
+    // Check if temp file name is displayed
+    expect(screen.getByText('temp.pdf')).toBeInTheDocument();
+
+    // Check if temp file size is displayed correctly
+    expect(screen.getByText(bytesToSize(2048 * 1024))).toBeInTheDocument();
+  });
+});

openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/Extensions/File/AttachmentComponents/FileAttachment.tsx

@@ -16,11 +16,9 @@ import {
   FileOutlined,
 } from '@ant-design/icons';
 import { NodeViewProps } from '@tiptap/react';
-import { Spin } from 'antd';
+import { Button } from 'antd';
 import React from 'react';
-import { useTranslation } from 'react-i18next';
 import { bytesToSize } from '../../../../../utils/StringsUtils';
-import Loader from '../../../../common/Loader/Loader';
 
 const FileAttachment = ({
   node,
@@ -33,8 +31,6 @@ const FileAttachment = ({
   deleteNode: () => void;
   onFileClick: (e: React.MouseEvent) => void;
 }) => {
-  const { t } = useTranslation();
-
   const {
     url,
     fileName,
@@ -46,57 +42,58 @@ const FileAttachment = ({
   } = node.attrs;
 
   return (
-    <Spin
+    <div className="file-link-container" onClick={(e) => e.preventDefault()}>
-      indicator={<Loader size="small" />}
+      <div className="file-content-wrapper">
-      spinning={isFileLoading || isUploading}
+        <FileOutlined className="file-icon" />
-      tip={isUploading ? t('label.uploading') : t('label.loading')}>
+        <div className="file-details">
-      <div className="file-link-container" onClick={(e) => e.preventDefault()}>
+          <a
-        <div className="file-content-wrapper">
+            className="file-link"
-          <FileOutlined className="file-icon" />
+            data-filename={fileName || tempFile?.name}
-          <div className="file-details">
+            data-filesize={(fileSize || tempFile?.size)?.toString()}
-            <a
+            data-mimetype={mimeType || tempFile?.type}
-              className="file-link"
+            data-type="file-attachment"
-              data-filename={fileName || tempFile?.name}
+            data-url={url}
-              data-filesize={(fileSize || tempFile?.size)?.toString()}
+            href="#"
-              data-mimetype={mimeType || tempFile?.type}
+            onClick={onFileClick}>
-              data-type="file-attachment"
+            <span className="file-name">{fileName || tempFile?.name}</span>
-              data-url={url}
+          </a>
-              href="#"
+          <div className="file-meta">
-              onClick={onFileClick}>
+            <span className="file-size">
-              <span className="file-name">{fileName || tempFile?.name}</span>
+              {bytesToSize(fileSize || tempFile?.size)}
-            </a>
+            </span>
-            <div className="file-meta">
+            {isUploading ? (
-              <span className="file-size">
+              <div
-                {bytesToSize(fileSize || tempFile?.size)}
+                className="upload-progress"
-              </span>
+                data-testid="upload-progress"
-              {isUploading ? (
+                style={{ width: `${uploadProgress || 0}%` }}
-                <div
+              />
-                  className="upload-progress"
+            ) : (
-                  style={{ width: `${uploadProgress || 0}%` }}
+              <>
+                <span className="separator">|</span>
+                <Button
+                  className="file-percentage"
+                  icon={<DownloadOutlined />}
+                  loading={isFileLoading}
+                  size="small"
+                  type="text"
+                  onClick={onFileClick}
                 />
-              ) : (
+              </>
-                <>
+            )}
-                  <span className="separator">|</span>
-                  <span className="file-percentage">
-                    <DownloadOutlined onClick={onFileClick} />
-                  </span>
-                </>
-              )}
-            </div>
           </div>
         </div>
-        {!isUploading && (
-          <DeleteOutlined
-            className="delete-icon"
-            onClick={(e) => {
-              e.preventDefault();
-              e.stopPropagation();
-              deleteNode();
-            }}
-          />
-        )}
       </div>
-    </Spin>
+      {!isUploading && (
+        <DeleteOutlined
+          className="delete-icon"
+          onClick={(e) => {
+            e.preventDefault();
+            e.stopPropagation();
+            deleteNode();
+          }}
+        />
+      )}
+    </div>
   );
 };
 

openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/Extensions/File/AttachmentComponents/ImageAttachment.test.tsx

@@ -0,0 +1,151 @@
+/*
+ *  Copyright 2025 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 { fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { NodeViewProps } from '@tiptap/react';
+import React from 'react';
+import ImageAttachment from './ImageAttachment';
+
+describe('ImageAttachment', () => {
+  const mockNode = {
+    attrs: {
+      url: 'https://example.com/image.jpg',
+      alt: 'Test Image',
+      isUploading: false,
+    },
+  } as unknown as NodeViewProps['node'];
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should render loading state when isMediaLoading is true and needs authentication', () => {
+    const authenticatedNode = {
+      ...mockNode,
+      attrs: {
+        ...mockNode.attrs,
+        url: '/api/v1/attachments/123',
+      },
+    } as unknown as NodeViewProps['node'];
+
+    render(
+      <ImageAttachment isMediaLoading mediaSrc="" node={authenticatedNode} />
+    );
+
+    expect(screen.getByTestId('loader')).toBeInTheDocument();
+    expect(screen.getByText('label.loading')).toBeInTheDocument();
+  });
+
+  it('should render uploading state when isUploading is true', () => {
+    const uploadingNode = {
+      ...mockNode,
+      attrs: {
+        ...mockNode.attrs,
+        isUploading: true,
+      },
+    } as unknown as NodeViewProps['node'];
+
+    render(
+      <ImageAttachment
+        isMediaLoading={false}
+        mediaSrc=""
+        node={uploadingNode}
+      />
+    );
+
+    expect(screen.getByTestId('loader')).toBeInTheDocument();
+    expect(screen.getByText('label.uploading')).toBeInTheDocument();
+  });
+
+  it('should render image when mediaSrc is provided', async () => {
+    const mediaSrc = 'https://example.com/image.jpg';
+    render(
+      <ImageAttachment
+        isMediaLoading={false}
+        mediaSrc={mediaSrc}
+        node={mockNode}
+      />
+    );
+
+    const image = screen.getByTestId('uploaded-image-node');
+
+    expect(image).toBeInTheDocument();
+    expect(image).toHaveAttribute('src', mediaSrc);
+    expect(image).toHaveAttribute('alt', 'Test Image');
+  });
+
+  it('should show error state when image fails to load', async () => {
+    render(
+      <ImageAttachment
+        isMediaLoading={false}
+        mediaSrc="invalid-url"
+        node={mockNode}
+      />
+    );
+
+    const image = screen.getByTestId('uploaded-image-node');
+    fireEvent.error(image);
+
+    await waitFor(() => {
+      expect(screen.getByTestId('uploaded-image-node')).toHaveStyle({
+        visibility: 'hidden',
+      });
+    });
+  });
+
+  it('should handle authenticated image URLs correctly', async () => {
+    const authenticatedNode = {
+      ...mockNode,
+      attrs: {
+        ...mockNode.attrs,
+        url: '/api/v1/attachments/123',
+      },
+    } as unknown as NodeViewProps['node'];
+
+    render(
+      <ImageAttachment
+        isMediaLoading={false}
+        mediaSrc=""
+        node={authenticatedNode}
+      />
+    );
+
+    expect(screen.getByTestId('loader')).toBeInTheDocument();
+    expect(screen.getByText('label.loading')).toBeInTheDocument();
+  });
+
+  it('should reset states when url or mediaSrc changes', async () => {
+    const { rerender } = render(
+      <ImageAttachment
+        isMediaLoading={false}
+        mediaSrc="https://example.com/image1.jpg"
+        node={mockNode}
+      />
+    );
+
+    // Simulate image load
+    const image = screen.getByTestId('uploaded-image-node');
+    fireEvent.load(image);
+
+    // Rerender with new mediaSrc
+    rerender(
+      <ImageAttachment
+        isMediaLoading={false}
+        mediaSrc="https://example.com/image2.jpg"
+        node={mockNode}
+      />
+    );
+
+    // Image should be hidden again until it loads
+    expect(image).toHaveStyle({ visibility: 'hidden' });
+  });
+});

openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/Extensions/File/FileNodeView.tsx

@@ -168,7 +168,7 @@ const FileNodeView: FC<NodeViewProps> = ({
           onOpenChange={handlePopoverVisibleChange}>
           <Spin
             indicator={<Loader size="small" />}
-            spinning={isMediaLoading || isFileLoading || isUploading}
+            spinning={isMediaLoading || isUploading}
             tip={isUploading ? t('label.uploading') : t('label.loading')}>
             {renderContent()}
           </Spin>

openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/Extensions/File/file-node.less

@@ -392,11 +392,8 @@
   }
 
   .ant-spin {
-    min-height: 200px;
+    min-height: max-content;
     background: @grey-1;
-    display: flex;
-    align-items: center;
-    justify-content: center;
 
     .ant-spin-dot {
       transform: scale(1.2);