[ui-sb] fixes file preview for non-readable file

apps/filebrowser/src/filebrowser/api.py

@@ -394,6 +394,7 @@ def display(request):
 
   # Get contents as string for text mode, or at least try
   file_contents = None
+  is_content_readable = True
   if isinstance(contents, str):
     file_contents = contents
     mode = 'text'
@@ -402,14 +403,15 @@ def display(request):
       file_contents = contents.decode(encoding)
       mode = 'text'
     except UnicodeDecodeError:
-      file_contents = contents
+      is_content_readable = False
 
   data = {
     'contents': file_contents,
     'offset': offset,
     'length': length,
     'end': offset + len(contents),
     'mode': mode,
+    "is_content_readable": is_content_readable
   }
 
   return JsonResponse(data)

desktop/core/src/desktop/js/apps/storageBrowser/StorageBrowserTab/StorageBrowserTab.tsx

@@ -30,7 +30,10 @@ import './StorageBrowserTab.scss';
 import StorageFilePage from '../StorageFilePage/StorageFilePage';
 import changeURL from '../../../utils/url/changeURL';
 import LoadingErrorWrapper from '../../../reactComponents/LoadingErrorWrapper/LoadingErrorWrapper';
-import { getFileSystemAndPath } from '../../../reactComponents/PathBrowser/PathBrowser.util';
+import {
+  getFileNameFromPath,
+  getFileSystemAndPath
+} from '../../../reactComponents/PathBrowser/PathBrowser.util';
 import RefreshIcon from '@cloudera/cuix-core/icons/react/RefreshIcon';
 import HomeIcon from '@cloudera/cuix-core/icons/react/HomeIcon';
 import DeleteIcon from '@cloudera/cuix-core/icons/react/DeleteIcon';
@@ -55,8 +58,7 @@ const StorageBrowserTab = ({ fileSystem, testId }: StorageBrowserTabProps): JSX.
     urlFileSystem === fileSystem.name ? urlFilePath : fileSystem.userHomeDirectory;
 
   const [filePath, setFilePath] = useState<string>(initialFilePath);
-  const fileName =
-    filePath.split('/').pop() !== '' ? (filePath.split('/').pop() ?? '') : filePath.split('://')[0];
+  const fileName = getFileNameFromPath(filePath);
 
   const { t } = i18nReact.useTranslation();
 
@@ -195,7 +197,7 @@ const StorageBrowserTab = ({ fileSystem, testId }: StorageBrowserTabProps): JSX.
           />
         )}
         {fileStats?.type === BrowserViewType.file && !isLoading && (
-          <StorageFilePage fileName={fileName} fileStats={fileStats} onReload={reloadData} />
+          <StorageFilePage fileStats={fileStats} onReload={reloadData} />
         )}
       </div>
     </LoadingErrorWrapper>

desktop/core/src/desktop/js/apps/storageBrowser/StorageDirectoryPage/StorageDirectoryPage.tsx

@@ -55,6 +55,7 @@ import FileUploadQueue from '../../../reactComponents/FileUploadQueue/FileUpload
 import { useWindowSize } from '../../../utils/hooks/useWindowSize/useWindowSize';
 import LoadingErrorWrapper from '../../../reactComponents/LoadingErrorWrapper/LoadingErrorWrapper';
 import StorageDirectoryActions from './StorageDirectoryActions/StorageDirectoryActions';
+import { getFileNameFromPath } from '../../../reactComponents/PathBrowser/PathBrowser.util';
 
 interface StorageDirectoryPageProps {
   fileStats: FileStats;
@@ -125,7 +126,7 @@ const StorageDirectoryPage = ({
     }
 
     return filesData?.files?.map(file => ({
-      name: file.path.split('/').pop() ?? '',
+      name: getFileNameFromPath(file.path),
       size: file.type === BrowserViewType.file ? formatBytes(file.size) : '',
       user: file.user,
       group: file.group,

desktop/core/src/desktop/js/apps/storageBrowser/StorageFilePage/StorageFilePage.scss

@@ -130,7 +130,7 @@
         height: 90%;
       }
 
-      .preview__compressed {
+      .preview__unsupported {
         font-size: vars.$font-size-lg;
       }
     }

desktop/core/src/desktop/js/apps/storageBrowser/StorageFilePage/StorageFilePage.test.tsx

@@ -37,14 +37,14 @@ jest.mock('../../../api/utils', () => ({
   post: () => mockSave()
 }));
 
-const mockData = jest.fn().mockReturnValue({
-  contents: 'Initial file content',
-  compression: 'none'
-});
+let mockData = {
+  isContentReadable: true,
+  contents: 'Initial file content'
+};
 
 jest.mock('../../../utils/hooks/useLoadData/useLoadData', () => {
   return jest.fn(() => ({
-    data: mockData(),
+    data: mockData,
     loading: false
   }));
 });
@@ -108,19 +108,6 @@ describe('StorageFilePage', () => {
     expect(screen.queryByRole('button', { name: 'Cancel' })).toBeNull();
   });
 
-  // TODO: fix this test when mocking of useLoadData onSuccess callback is mproperly mocked
-  it.skip('should hide edit button when compression is available', async () => {
-    mockData.mockImplementation(() => ({
-      contents: 'Initial file content',
-      compression: 'zip'
-    }));
-    render(
-      <StorageFilePage fileName={mockFileName} fileStats={mockFileStats} onReload={mockReload} />
-    );
-
-    expect(screen.queryByRole('button', { name: 'Edit' })).toBeNull();
-  });
-
   it('should show save and cancel buttons when editing', async () => {
     const user = userEvent.setup();
     render(
@@ -305,7 +292,12 @@ describe('StorageFilePage', () => {
     expect(video.children[0]).toHaveAttribute('src', expect.stringContaining('videofile.mp4'));
   });
 
-  it('should display a message for compressed file types', () => {
+  it('should display a message for unsupported file types', () => {
+    mockData = {
+      isContentReadable: false,
+      contents: ''
+    };
+
     render(
       <StorageFilePage
         fileStats={{
@@ -317,6 +309,6 @@ describe('StorageFilePage', () => {
       />
     );
 
-    expect(screen.getByText(/preview not available for compressed file/i)).toBeInTheDocument();
+    expect(screen.getByText(/Preview is not available for this file/i)).toBeInTheDocument();
   });
 });

desktop/core/src/desktop/js/apps/storageBrowser/StorageFilePage/StorageFilePage.tsx

@@ -33,15 +33,16 @@ import useLoadData from '../../../utils/hooks/useLoadData/useLoadData';
 import { getLastKnownConfig } from '../../../config/hueConfig';
 import LoadingErrorWrapper from '../../../reactComponents/LoadingErrorWrapper/LoadingErrorWrapper';
 import { inTrash } from '../../../utils/storageBrowserUtils';
+import { getFileNameFromPath } from '../../../reactComponents/PathBrowser/PathBrowser.util';
 
 interface StorageFilePageProps {
   onReload: () => void;
-  fileName: string;
   fileStats: FileStats;
 }
 
-const StorageFilePage = ({ fileName, fileStats, onReload }: StorageFilePageProps): JSX.Element => {
+const StorageFilePage = ({ fileStats, onReload }: StorageFilePageProps): JSX.Element => {
   const config = getLastKnownConfig();
+  const fileName = getFileNameFromPath(fileStats.path);
   const fileType = getFileType(fileName);
 
   const { t } = i18nReact.useTranslation();
@@ -54,11 +55,7 @@ const StorageFilePage = ({ fileName, fileStats, onReload }: StorageFilePageProps
 
   const { loading: isSaving, save } = useSaveData(SAVE_FILE_API_URL);
 
-  const {
-    data: fileData,
-    loading: loadingPreview,
-    error: errorPreview
-  } = useLoadData<FilePreview>(FILE_PREVIEW_API_URL, {
+  const { data, loading, error } = useLoadData<FilePreview>(FILE_PREVIEW_API_URL, {
     params: {
       path: fileStats.path,
       offset: pageOffset,
@@ -80,7 +77,7 @@ const StorageFilePage = ({ fileName, fileStats, onReload }: StorageFilePageProps
 
   const handleCancel = () => {
     setIsEditing(false);
-    setFileContent(fileData?.contents);
+    setFileContent(data?.contents);
   };
 
   const handleSave = () => {
@@ -115,6 +112,7 @@ const StorageFilePage = ({ fileName, fileStats, onReload }: StorageFilePageProps
     config?.storage_browser.max_file_editor_size &&
     config?.storage_browser.max_file_editor_size > fileStats.size &&
     EDITABLE_FILE_FORMATS.has(fileType) &&
+    data?.isContentReadable &&
     !inTrash(fileStats.path);
 
   const pageStats = {
@@ -126,7 +124,7 @@ const StorageFilePage = ({ fileName, fileStats, onReload }: StorageFilePageProps
 
   const errorConfig = [
     {
-      enabled: !!errorPreview,
+      enabled: !!error,
       message: t('An error occurred while fetching file content for path "{{path}}".', {
         path: fileStats.path
       }),
@@ -151,7 +149,7 @@ const StorageFilePage = ({ fileName, fileStats, onReload }: StorageFilePageProps
           ))}
         </div>
 
-        <LoadingErrorWrapper loading={loadingPreview || isSaving} errors={errorConfig}>
+        <LoadingErrorWrapper loading={loading || isSaving} errors={errorConfig} hideChildren>
           <div className="preview">
             <div className="preview__title-bar">
               {t('Content')}
@@ -171,7 +169,7 @@ const StorageFilePage = ({ fileName, fileStats, onReload }: StorageFilePageProps
                       data-testid="preview--save--button"
                       data-event=""
                       onClick={handleSave}
-                      disabled={fileContent === fileData?.contents}
+                      disabled={fileContent === data?.contents}
                     >
                       {t('Save')}
                     </PrimaryButton>
@@ -200,7 +198,7 @@ const StorageFilePage = ({ fileName, fileStats, onReload }: StorageFilePageProps
             </div>
 
             <div className="preview__content">
-              {[SupportedFileTypes.TEXT, SupportedFileTypes.OTHER].includes(fileType) && (
+              {data?.isContentReadable === true && (
                 <div className="preview__editable-file">
                   <textarea
                     value={fileContent}
@@ -214,6 +212,13 @@ const StorageFilePage = ({ fileName, fileStats, onReload }: StorageFilePageProps
                 </div>
               )}
 
+              {(data?.isContentReadable === false ||
+                fileType === SupportedFileTypes.COMPRESSED) && (
+                <div className="preview__unsupported">
+                  {t('Preview is not available for this file.')}
+                </div>
+              )}
+
               {fileType === SupportedFileTypes.IMAGE && <img src={filePreviewUrl} alt={fileName} />}
 
               {fileType === SupportedFileTypes.DOCUMENT && (
@@ -244,14 +249,6 @@ const StorageFilePage = ({ fileName, fileStats, onReload }: StorageFilePageProps
                   {t('Your browser does not support the video element.')}
                 </video>
               )}
-
-              {fileType === SupportedFileTypes.COMPRESSED && (
-                <div className="preview__compresed">
-                  {t(
-                    'Preview not available for compressed file. Please download the file to view.'
-                  )}
-                </div>
-              )}
             </div>
           </div>
         </LoadingErrorWrapper>

desktop/core/src/desktop/js/apps/storageBrowser/types.ts

@@ -64,6 +64,7 @@ export interface FilePreview {
   length: number;
   mode: string;
   offset: number;
+  isContentReadable: boolean;
 }
 
 export interface ListDirectory {

desktop/core/src/desktop/js/reactComponents/LoadingErrorWrapper/LoadingErrorWrapper.scss

@@ -22,3 +22,10 @@
   align-items: center;
   gap: 16px;
 }
+
+.antd.cuix {
+  .loading-error-wrapper__sppiner {
+    // this overrides the max-height: 400px in antd library
+    max-height: 100% !important;
+  }
+}

desktop/core/src/desktop/js/reactComponents/LoadingErrorWrapper/LoadingErrorWrapper.tsx

@@ -30,21 +30,23 @@ interface LoadingErrorWrapperProps {
   loading: boolean;
   errors: WrapperError[];
   children: React.ReactNode;
+  hideChildren?: boolean;
 }
 
 const LoadingErrorWrapper = ({
   loading,
   errors,
+  hideChildren = false,
   children
 }: LoadingErrorWrapperProps): JSX.Element => {
   if (loading) {
-    // TODO: discuss in UI meeting, if we need to render children while loading
-    // Initial thoughts:
-    // -- On first render -> hide children
-    // -- On re-render -> render children
     return (
-      <Spin spinning={loading} data-testid="loading-error-wrapper__sppiner">
-        {children}
+      <Spin
+        spinning={loading}
+        data-testid="loading-error-wrapper__sppiner"
+        className="loading-error-wrapper__sppiner"
+      >
+        {hideChildren === false && children}
       </Spin>
     );
   }

desktop/core/src/desktop/js/reactComponents/PathBrowser/PathBrowser.util.test.ts

@@ -14,7 +14,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import { getBreadcrumbs, getFileSystemAndPath } from './PathBrowser.util';
+import { getBreadcrumbs, getFileSystemAndPath, getFileNameFromPath } from './PathBrowser.util';
 
 describe('PathBrowser utils', () => {
   describe('getFileSystemAndPath', () => {
@@ -65,7 +65,7 @@ describe('PathBrowser utils', () => {
       const result = getBreadcrumbs('hdfs', hdfsPath);
 
       expect(result).toEqual([
-        { url: '/', label: '/' },
+        { url: '/', label: 'hdfs' },
         { url: '/test', label: 'test' },
         { url: '/test/folder', label: 'folder' }
       ]);
@@ -87,7 +87,7 @@ describe('PathBrowser utils', () => {
       const result = getBreadcrumbs('hdfs', pathWithTrailingSlash);
 
       expect(result).toEqual([
-        { url: '/', label: '/' },
+        { url: '/', label: 'hdfs' },
         { url: '/folder', label: 'folder' },
         { url: '/folder/with', label: 'with' },
         { url: '/folder/with/trailing', label: 'trailing' },
@@ -100,7 +100,7 @@ describe('PathBrowser utils', () => {
       const result = getBreadcrumbs('hdfs', pathWithLeadingSlash);
 
       expect(result).toEqual([
-        { url: '/', label: '/' },
+        { url: '/', label: 'hdfs' },
         { url: '/path', label: 'path' },
         { url: '/path/to', label: 'to' },
         { url: '/path/to/file', label: 'file' }
@@ -130,3 +130,35 @@ describe('PathBrowser utils', () => {
     });
   });
 });
+
+describe('getFileNameFromPath', () => {
+  it('should return the file name from a valid path', () => {
+    const filePath = '/user/documents/file.txt';
+    const result = getFileNameFromPath(filePath);
+    expect(result).toBe('file.txt');
+  });
+
+  it('should return an empty string for a path ending with a slash', () => {
+    const filePath = '/user/documents';
+    const result = getFileNameFromPath(filePath);
+    expect(result).toBe('documents');
+  });
+
+  it('should return correct filename for S3A path', () => {
+    const filePath = 's3a://example/hue/';
+    const result = getFileNameFromPath(filePath);
+    expect(result).toBe('hue');
+  });
+
+  it('should return correct filename for root S3A directory', () => {
+    const filePath = 's3a://';
+    const result = getFileNameFromPath(filePath);
+    expect(result).toBe('s3a');
+  });
+
+  it('should return correct filename for root hdfs directory', () => {
+    const filePath = '/';
+    const result = getFileNameFromPath(filePath);
+    expect(result).toBe('hdfs');
+  });
+});

desktop/core/src/desktop/js/reactComponents/PathBrowser/PathBrowser.util.ts

@@ -47,7 +47,7 @@ export const getFileSystemAndPath = (
 export const getBreadcrumbs = (fileSystem: string, path: string): BreadcrumbData[] => {
   const pathParts = path.split('/').filter(Boolean);
   const rootUrl = fileSystem === 'hdfs' ? '/' : `${fileSystem}://`;
-  const rootlabel = fileSystem === 'hdfs' ? '/' : fileSystem;
+  const rootlabel = fileSystem === 'hdfs' ? 'hdfs' : fileSystem;
   const rootNode = {
     url: rootUrl,
     label: rootlabel
@@ -62,3 +62,9 @@ export const getBreadcrumbs = (fileSystem: string, path: string): BreadcrumbData
     [rootNode]
   );
 };
+
+export const getFileNameFromPath = (filePath: string): string => {
+  const { fileSystem, path } = getFileSystemAndPath(filePath);
+  const sanitizedPath = path.endsWith('/') ? path.slice(0, -1) : path;
+  return sanitizedPath.split('/').pop() || fileSystem;
+};