Merge branch 'develop' into fb_exceptionsCAN256

cnathe · cnathe · commit 66dbf5f97a06 · 2025-05-28T09:52:04.000-05:00
# Conflicts:
#	packages/components/package-lock.json
#	packages/components/package.json
#	packages/components/releaseNotes/components.md
diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json
diff --git a/packages/components/package.json b/packages/components/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@labkey/components",
-  "version": "6.44.0-exceptionsCAN256.0",
+  "version": "6.44.1",
   "description": "Components, models, actions, and utility functions for LabKey applications and pages",
   "sideEffects": false,
   "files": [
@@ -50,7 +50,7 @@
   "homepage": "https://github.com/LabKey/labkey-ui-components#readme",
   "dependencies": {
     "@hello-pangea/dnd": "18.0.1",
-    "@labkey/api": "1.41.1",
+    "@labkey/api": "1.41.2",
     "@testing-library/dom": "~10.4.0",
     "@testing-library/jest-dom": "~6.6.3",
     "@testing-library/react": "~16.3.0",
diff --git a/packages/components/releaseNotes/components.md b/packages/components/releaseNotes/components.md
@@ -5,6 +5,11 @@ Components, models, actions, and utility functions for LabKey applications and p
 *Released*: TBD
 - Issue 53164: AssayPicker.tsx gives JS error if no specialty assay providers available for server
 
+### version 6.44.1
+*Released*: 28 May 2025
+- Introduce `getTransferItemDirectoryEntry` to centralize handling/wrapping of calling `webkitGetAsEntry()` on a `DataTransferItemList` item.
+- Address Issue 53149 by updating endpoint wrapper to be tolerant of invalid payload upon success.
+
 ### version 6.44.0
 *Released*: 27 May 2025
 - QueryModel/QueryConfig change useSavedSettings from boolean to enum SavedSettings
diff --git a/packages/components/src/internal/components/files/FileAttachmentContainer.test.tsx b/packages/components/src/internal/components/files/FileAttachmentContainer.test.tsx
@@ -16,7 +16,12 @@
 import React from 'react';
 import { render } from '@testing-library/react';
 
-import { FileAttachmentContainer } from './FileAttachmentContainer';
+import {
+    FileAttachmentContainer,
+    getTransferItemDirectoryEntry,
+    isDirectoryEntry,
+    isFileEntry,
+} from './FileAttachmentContainer';
 
 describe('FileAttachmentContainer', () => {
     test('with single file', () => {
@@ -29,7 +34,7 @@ describe('FileAttachmentContainer', () => {
         );
 
         expect(document.querySelector('.file-upload__container').className).toContain('hidden');
-        expect(document.querySelector('.attached-file__container').textContent).toBe('file1.txt');
+        expect(document.querySelector('.attached-file__container')).toHaveTextContent('file1.txt');
     });
 
     test('with multiple files', () => {
@@ -46,8 +51,8 @@ describe('FileAttachmentContainer', () => {
 
         expect(document.querySelector('.file-upload__container').className).toContain('block');
         expect(document.querySelectorAll('.attached-file__container')).toHaveLength(2);
-        expect(document.querySelectorAll('.attached-file__container')[0].textContent).toBe('file1.txt');
-        expect(document.querySelectorAll('.attached-file__container')[1].textContent).toBe('file2.txt');
+        expect(document.querySelectorAll('.attached-file__container')[0]).toHaveTextContent('file1.txt');
+        expect(document.querySelectorAll('.attached-file__container')[1]).toHaveTextContent('file2.txt');
 
         expect(document.querySelectorAll('.file-upload__file-entry-listing')).toHaveLength(1);
         expect(document.querySelectorAll('.file-upload__scroll-footer')).toHaveLength(0);
@@ -91,7 +96,7 @@ describe('FileAttachmentContainer', () => {
         expect(document.querySelector('.file-upload__container').className).toContain('block');
         expect(document.querySelectorAll('.attached-file__container')).toHaveLength(2);
         expect(document.querySelectorAll('.file-upload__file-entry-listing')).toHaveLength(1);
-        expect(document.querySelector('.file-upload__scroll-footer').textContent).toBe('2 files will be uploaded.');
+        expect(document.querySelector('.file-upload__scroll-footer')).toHaveTextContent('2 files will be uploaded.');
     });
 
     test('fileCountSuffix with single', () => {
@@ -109,6 +114,92 @@ describe('FileAttachmentContainer', () => {
         expect(document.querySelector('.file-upload__container').className).toContain('block');
         expect(document.querySelectorAll('.attached-file__container')).toHaveLength(1);
         expect(document.querySelectorAll('.file-upload__file-entry-listing')).toHaveLength(1);
-        expect(document.querySelector('.file-upload__scroll-footer').textContent).toBe('1 file will be uploaded.');
+        expect(document.querySelector('.file-upload__scroll-footer')).toHaveTextContent('1 file will be uploaded.');
+    });
+});
+
+describe('File System Helper Functions', () => {
+    function mockFileSystemEntry(isDirectory: boolean, isFile: boolean): FileSystemEntry {
+        return {
+            filesystem: undefined,
+            fullPath: undefined,
+            isDirectory,
+            isFile,
+            name: undefined,
+            getParent: jest.fn(),
+        };
+    }
+
+    function mockDataTransferItemList(entry: FileSystemEntry): DataTransferItemList {
+        return {
+            0: {
+                webkitGetAsEntry: jest.fn().mockReturnValue(entry),
+            },
+        } as unknown as DataTransferItemList;
+    }
+
+    describe('isDirectoryEntry', () => {
+        it('should return true when entry is a directory', () => {
+            const mockDirEntry = mockFileSystemEntry(true, false);
+            expect(isDirectoryEntry(mockDirEntry)).toBe(true);
+        });
+
+        it('should return false when entry is a file', () => {
+            const mockFileEntry = mockFileSystemEntry(false, true);
+            expect(isDirectoryEntry(mockFileEntry)).toBe(false);
+        });
+
+        it('should return false when entry is undefined', () => {
+            expect(isDirectoryEntry(undefined)).toBe(false);
+            expect(isDirectoryEntry(null)).toBe(false);
+            expect(isDirectoryEntry({} as unknown as FileSystemEntry)).toBe(false);
+        });
+    });
+
+    describe('isFileEntry', () => {
+        it('should return true when entry is a file', () => {
+            const mockFileEntry = mockFileSystemEntry(false, true);
+            expect(isFileEntry(mockFileEntry)).toBe(true);
+        });
+
+        it('should return false when entry is a directory', () => {
+            const mockDirEntry = mockFileSystemEntry(true, false);
+            expect(isFileEntry(mockDirEntry)).toBe(false);
+        });
+
+        it('should return false when entry is undefined', () => {
+            expect(isFileEntry(undefined)).toBe(false);
+            expect(isFileEntry(null)).toBe(false);
+            expect(isFileEntry({} as unknown as FileSystemEntry)).toBe(false);
+        });
+    });
+
+    describe('getTransferItemDirectoryEntry', () => {
+        it('should return directory entry when item at index is a directory', () => {
+            const mockDirEntry = mockFileSystemEntry(true, false);
+            const mockTransferItems = mockDataTransferItemList(mockDirEntry);
+
+            const result = getTransferItemDirectoryEntry(mockTransferItems, 0);
+            expect(result).toBe(mockDirEntry);
+            expect(mockTransferItems[0].webkitGetAsEntry).toHaveBeenCalledTimes(1);
+        });
+
+        it('should return undefined when item at index is a file', () => {
+            const mockFileEntry = mockFileSystemEntry(false, true);
+            const mockTransferItems = mockDataTransferItemList(mockFileEntry);
+
+            const result = getTransferItemDirectoryEntry(mockTransferItems, 0);
+            expect(result).toBeUndefined();
+        });
+
+        it('should return undefined when index is out of bounds', () => {
+            expect(getTransferItemDirectoryEntry(undefined as DataTransferItemList, 0)).toBeUndefined();
+            expect(getTransferItemDirectoryEntry({} as DataTransferItemList, 999)).toBeUndefined();
+        });
+
+        it('should return undefined when webkitGetAsEntry returns null', () => {
+            const mockTransferItems = mockDataTransferItemList(null);
+            expect(getTransferItemDirectoryEntry(mockTransferItems, 0)).toBeUndefined();
+        });
     });
 });
diff --git a/packages/components/src/internal/components/files/FileAttachmentContainer.tsx b/packages/components/src/internal/components/files/FileAttachmentContainer.tsx
@@ -29,8 +29,16 @@ import { fileMatchesAcceptedFormat, fileSizeLimitCompare } from './actions';
 import { FileAttachmentEntry } from './FileAttachmentEntry';
 import { ALL_FILES_LIMIT_KEY } from './models';
 
-const isDirectoryEntry = (entry: FileSystemEntry): entry is FileSystemDirectoryEntry => entry.isDirectory;
-const isFileEntry = (entry: FileSystemEntry): entry is FileSystemFileEntry => entry.isFile;
+export const isDirectoryEntry = (entry: FileSystemEntry): entry is FileSystemDirectoryEntry => !!entry?.isDirectory;
+export const isFileEntry = (entry: FileSystemEntry): entry is FileSystemFileEntry => !!entry?.isFile;
+export const getTransferItemDirectoryEntry = (
+    transferItems: DataTransferItemList,
+    index: number
+): FileSystemDirectoryEntry | undefined => {
+    const entry = transferItems?.[index]?.webkitGetAsEntry?.();
+    if (entry && isDirectoryEntry(entry)) return entry;
+    return undefined;
+};
 
 interface Props {
     acceptedFormats?: string; // comma separated list of allowed extensions i.e. '.png, .jpg, .jpeg'
@@ -173,7 +181,7 @@ export class FileAttachmentContainer extends React.PureComponent<Props, State> {
         const invalidNames = new Set<string>();
 
         Array.from(fileList).forEach((file, index) => {
-            if (transferItems && transferItems[index].webkitGetAsEntry().isDirectory) {
+            if (getTransferItemDirectoryEntry(transferItems, index)) {
                 if (!allowDirectories) {
                     invalidDirectories.push(file.name);
                     invalidNames.add(file.name);
@@ -312,7 +320,7 @@ export class FileAttachmentContainer extends React.PureComponent<Props, State> {
                 newFiles[file.name] = file;
                 haveValidFiles = true;
 
-                if (transferItems && transferItems[index].webkitGetAsEntry().isDirectory) {
+                if (getTransferItemDirectoryEntry(transferItems, index)) {
                     hasDirectory = true;
                 }
             }
@@ -325,8 +333,8 @@ export class FileAttachmentContainer extends React.PureComponent<Props, State> {
                 this.dirCbCount = 0;
                 this.fileCbCount = 0;
                 Array.from(fileList).forEach((file, index) => {
-                    const entry = transferItems[index].webkitGetAsEntry();
-                    if (isDirectoryEntry(entry)) {
+                    const entry = getTransferItemDirectoryEntry(transferItems, index);
+                    if (entry) {
                         delete files[file.name];
                         this.getFilesFromDirectory(files, entry, this, () => {
                             this._handleFiles(files);
diff --git a/packages/components/src/internal/components/forms/input/FileInput.tsx b/packages/components/src/internal/components/forms/input/FileInput.tsx
@@ -29,6 +29,8 @@ import { FILELINK_RANGE_URI } from '../../domainproperties/constants';
 
 import { fileMatchesAcceptedFormat } from '../../files/actions';
 
+import { getTransferItemDirectoryEntry } from '../../files/FileAttachmentContainer';
+
 import { DisableableInput, DisableableInputProps, DisableableInputState } from './DisableableInput';
 
 export interface FileInputProps extends DisableableInputProps {
@@ -42,11 +44,11 @@ export interface FileInputProps extends DisableableInputProps {
     labelClassName?: string;
     maxFileSize?: number;
     name?: string;
+    onChange?: (fileMap: Record<string, File>) => void;
     queryColumn?: QueryColumn;
     renderFieldLabel?: (queryColumn: QueryColumn, label?: string, description?: string) => ReactNode;
     showLabel?: boolean;
     toggleDisabledTooltip?: string;
-    onChange?: (fileMap: Record<string, File>) => void;
 }
 
 type FileInputImplProps = FileInputProps & FormsyInjectedProps<any>;
@@ -107,7 +109,7 @@ class FileInputImpl extends DisableableInput<FileInputImplProps, State> {
             return;
         }
 
-        if (transferItems && transferItems[0].webkitGetAsEntry().isDirectory) {
+        if (getTransferItemDirectoryEntry(transferItems, 0)) {
             this.setState({ error: 'Folders are not supported, only one file allowed' });
             return;
         }
@@ -122,7 +124,9 @@ class FileInputImpl extends DisableableInput<FileInputImplProps, State> {
         }
 
         if (maxFileSize && file.size > maxFileSize) {
-            this.setState({ error: `File size must not exceed ${Math.round(maxFileSize / 1024).toLocaleString()} KB.` });
+            this.setState({
+                error: `File size must not exceed ${Math.round(maxFileSize / 1024).toLocaleString()} KB.`,
+            });
             return;
         }
         if (emptyFileNotAllowed && file.size === 0) {
diff --git a/packages/components/src/internal/components/lineage/actions.ts b/packages/components/src/internal/components/lineage/actions.ts
@@ -279,30 +279,32 @@ export class ServerLineageAPIWrapper implements LineageAPIWrapper {
         return new Promise((resolve, reject) => {
             const seed = options.lsid;
 
+            function failure(error?: { exception: string; exceptionClass?: string }): void {
+                let message = `Failed to fetch lineage for seed "${seed}".`;
+
+                if (error?.exception) {
+                    message = error.exception;
+
+                    // When a server exception occurs
+                    if (error.exceptionClass) {
+                        message = `${error.exceptionClass}: ` + error.exception;
+                    }
+                }
+
+                reject({ message, seed });
+            }
+
             Experiment.lineage({
                 ...options,
                 success: lineage => {
-                    resolve(LineageResult.create(lineage));
-                },
-                failure: error => {
-                    let message = `Failed to fetch lineage for seed "${seed}".`;
-
-                    if (error) {
-                        if (error.exception) {
-                            message = error.exception;
-
-                            // When a server exception occurs
-                            if (error.exceptionClass) {
-                                message = `${error.exceptionClass}: ` + error.exception;
-                            }
-                        }
+                    // Issue 53149: Lineage loads undefined object
+                    if (lineage) {
+                        resolve(LineageResult.create(lineage));
+                    } else {
+                        failure();
                     }
-
-                    reject({
-                        seed,
-                        message,
-                    });
                 },
+                failure,
             });
         });
     };
