Fix stuck WorkBench attachment import

specifyweb/backend/workbench/upload/upload_attachments.py

@@ -20,6 +20,16 @@
 BASE_TABLE_NAME = "baseTable"
 ATTACHMENTS_COLUMN = "_UPLOADED_ATTACHMENTS"
 
+def serialize_attachment_field_value(field: str, value) -> str:
+    if field == "ispublic":
+        if value is None:
+            return "false"
+        return "true" if bool(value) else "false"
+    return "" if value is None else str(value)
+
+def attachment_field_default(field: str) -> str:
+    return "false" if field == "ispublic" else ""
+
 def get_attachments(row: Row):
     if has_attachments(row):
         return json.loads(cast(str, row.get(ATTACHMENTS_COLUMN)))
@@ -91,7 +101,10 @@ def add_attachments_to_plan(
 
         new_row[f"_ATTACHMENT_ORDINAL_{index}"] = str(spdatasetattachment.ordinal)
         for field in attachment_fields_to_copy:
-            new_row[f"_ATTACHMENT_{field.upper()}_{index}"] = str(getattr(spdatasetattachment.attachment, field) or "")
+            new_row[f"_ATTACHMENT_{field.upper()}_{index}"] = serialize_attachment_field_value(
+                field,
+                getattr(spdatasetattachment.attachment, field, None),
+            )
 
         # Inject attachment tables into upload plan
         table_name = attachment["table"]
@@ -121,7 +134,7 @@ def add_attachments_to_plan(
                     column=f"_ATTACHMENT_{field.upper()}_{index}",
                     matchBehavior="ignoreNever",
                     nullAllowed=True,
-                    default="0"
+                    default=attachment_field_default(field)
                 )
             attachment_uploadable = UploadTable(
                 name="Attachment",

specifyweb/frontend/js_src/lib/components/Attachments/__tests__/uploadFile.test.ts

@@ -7,41 +7,57 @@ import { uploadFile } from '../attachments';
 requireContext();
 
 describe('uploadFile', () => {
-  const xhrMock = {
-    open: jest.fn(),
-    send: jest.fn(),
-    // Immediately call the callback
-    addEventListener,
-    readyState: 4,
-    status: Http.OK as number, // Need to loosen up the type.
-    _error: undefined as unknown,
+  type EventName = 'abort' | 'error' | 'readystatechange' | 'timeout';
+  type MockXhr = {
+    readonly open: jest.Mock;
+    readonly send: jest.Mock;
+    readonly addEventListener: jest.Mock;
+    readonly removeEventListener: jest.Mock;
+    readonly upload: {
+      readonly addEventListener: jest.Mock;
+    };
+    readonly readyState: number;
+    readonly status: number;
+    readonly responseText: string;
+    readonly timeout: number;
   };
 
-  // Functions get hoisted
-  function addEventListener(_: string, callback: () => void) {
-    try {
-      callback();
-    } catch (error) {
-      console.log('got here!');
-      xhrMock._error = error;
-    }
-  }
+  let nextEvent: EventName = 'readystatechange';
+  let xhrMock: MockXhr;
+
+  const makeXhrMock = (): MockXhr => {
+    const listeners: Partial<Record<EventName, () => void>> = {};
+
+    return {
+      open: jest.fn(),
+      send: jest.fn((..._args: readonly unknown[]) => listeners[nextEvent]?.()),
+      addEventListener: jest.fn((eventName: EventName, callback: () => void) => {
+        listeners[eventName] = callback;
+      }),
+      removeEventListener: jest.fn(
+        (eventName: EventName, callback: () => void) => {
+          if (listeners[eventName] === callback) listeners[eventName] = undefined;
+        }
+      ),
+      upload: {
+        addEventListener: jest.fn(),
+      },
+      readyState: 4,
+      status: Http.OK,
+      responseText: '',
+      timeout: 0,
+    };
+  };
 
   let previousImplementation: typeof window.XMLHttpRequest;
   let previousTelemetry: typeof globalThis.performance.getEntries;
 
   beforeEach(() => {
     previousImplementation = window.XMLHttpRequest;
     previousTelemetry = globalThis.performance.getEntries;
+    xhrMock = makeXhrMock();
 
-    xhrMock.open.mockClear();
-    xhrMock.send.mockClear();
-    /*
-     * This is done like this so that a test can alter the return code
-     * to test errors.
-     */
-    xhrMock.status = Http.OK;
-    xhrMock._error = undefined;
+    nextEvent = 'readystatechange';
 
     // @ts-expect-error
     jest.spyOn(window, 'XMLHttpRequest').mockImplementation(() => xhrMock);
@@ -79,9 +95,10 @@ describe('uploadFile', () => {
       'POST',
       attachmentSettings.write
     );
+    expect(xhrMock.timeout).toBe(30 * 60 * 1000);
 
     expect(xhrMock.send).toHaveBeenCalledTimes(1);
-    const formData: FormData = xhrMock.send.mock.calls.at(-1)[0];
+    const formData = xhrMock.send.mock.calls.at(-1)?.[0] as FormData;
 
     expect(formData.get('token')).toBe(testToken);
     expect(formData.get('store')).toBe(testAttachmentLocation);
@@ -117,7 +134,7 @@ describe('uploadFile', () => {
     );
 
     expect(xhrMock.send).toHaveBeenCalledTimes(1);
-    const formData: FormData = xhrMock.send.mock.calls.at(-1)[0];
+    const formData = xhrMock.send.mock.calls.at(-1)?.[0] as FormData;
 
     expect(formData.get('token')).toBe(newToken);
     expect(formData.get('store')).toBe(newLocation);
@@ -130,4 +147,17 @@ describe('uploadFile', () => {
     expect(attachment?.get('origFilename')).toBe(testFileName);
     expect(attachment?.get('title')).toBe(testFileName);
   });
+
+  test('fails when upload request times out', async () => {
+    nextEvent = 'timeout';
+    const consoleError = jest
+      .spyOn(console, 'error')
+      .mockImplementation(() => undefined);
+
+    await expect(uploadFile({ file: testFile })).rejects.toThrow(
+      'Invalid response code 0.'
+    );
+    expect(consoleError).toHaveBeenCalled();
+    consoleError.mockRestore();
+  });
 });

specifyweb/frontend/js_src/lib/components/Attachments/attachments.ts

@@ -50,6 +50,7 @@ export const attachmentSettingsPromise = load<AttachmentSettings | IR<never>>(
 });
 
 export const attachmentsAvailable = (): boolean => typeof settings === 'object';
+const uploadTimeoutMilliseconds = 30 * 60 * 1000;
 
 /*
  * This function is useful when testing functions that depend on the settings.
@@ -264,33 +265,90 @@ export async function uploadFile({
       handleProgress(event.lengthComputable ? event.loaded / event.total : true)
     );
   }
-  xhr.open('POST', settings.write);
-  xhr.send(formData);
   const DONE = 4;
-  await new Promise((resolve, reject) =>
-    xhr.addEventListener('readystatechange', () => {
-      if (xhr.readyState === DONE)
+  xhr.open('POST', settings.write);
+  xhr.timeout = uploadTimeoutMilliseconds;
+  await new Promise<void>((resolve, reject) => {
+    let settled = false;
+
+    const handleReadyStateChange = () => {
+      if (xhr.readyState !== DONE) return;
+      settle(() => {
         try {
-          resolve(
-            handleAjaxResponse({
-              expectedErrors: [],
-              accept: undefined,
-              // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
-              response: {
-                ok: xhr.status === Http.OK,
-                status: xhr.status,
-                url: settings!.write,
-              } as Response,
-
-              errorMode: strict ? 'visible' : 'silent',
-              text: xhr.responseText,
-            })
-          );
+          handleAjaxResponse({
+            expectedErrors: [],
+            accept: undefined,
+            // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
+            response: {
+              ok: xhr.status === Http.OK,
+              status: xhr.status,
+              url: settings!.write,
+            } as Response,
+            errorMode: strict ? 'visible' : 'silent',
+            text: xhr.responseText,
+          });
+          resolve();
         } catch (error) {
           reject(error);
         }
-    })
-  );
+      });
+    };
+
+    const handleFailureWithoutResponse = (message: string) => {
+      settle(() => {
+        try {
+          handleAjaxResponse({
+            expectedErrors: [],
+            accept: undefined,
+            // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
+            response: {
+              ok: false,
+              status: 0,
+              url: settings!.write,
+            } as Response,
+            errorMode: strict ? 'visible' : 'silent',
+            text: message,
+          });
+          reject(new Error(message));
+        } catch (error) {
+          reject(error);
+        }
+      });
+    };
+
+    const settle = (callback: () => void) => {
+      if (settled) return;
+      settled = true;
+      cleanup();
+      callback();
+    };
+
+    const handleError = () =>
+      handleFailureWithoutResponse(
+        'Upload failed before receiving a response.'
+      );
+    const handleAbort = () =>
+      handleFailureWithoutResponse(
+        'Upload was aborted before receiving a response.'
+      );
+    const handleTimeout = () =>
+      handleFailureWithoutResponse(
+        `Upload timed out after ${uploadTimeoutMilliseconds} milliseconds.`
+      );
+
+    const cleanup = () => {
+      xhr.removeEventListener('readystatechange', handleReadyStateChange);
+      xhr.removeEventListener('error', handleError);
+      xhr.removeEventListener('abort', handleAbort);
+      xhr.removeEventListener('timeout', handleTimeout);
+    };
+
+    xhr.addEventListener('readystatechange', handleReadyStateChange);
+    xhr.addEventListener('error', handleError);
+    xhr.addEventListener('abort', handleAbort);
+    xhr.addEventListener('timeout', handleTimeout);
+    xhr.send(formData);
+  });
 
   return new tables.Attachment.Resource({
     attachmentlocation: data.attachmentLocation,

specifyweb/frontend/js_src/lib/components/AttachmentsBulkImport/Upload.tsx

@@ -142,6 +142,11 @@ export function AttachmentUpload({
   >('main');
 
   const loading = React.useContext(LoadingContext);
+  const [attachmentIsPublicDefault] = collectionPreferences.use(
+    'general',
+    'attachments',
+    'attachment.is_public_default'
+  );
 
   React.useEffect(() => {
     if (upload !== 'confirmed' || baseTableName === undefined) return;
@@ -174,8 +179,9 @@ export function AttachmentUpload({
         uploadAttachmentSpec: uploadable.uploadTokenSpec,
         dryRun,
         triggerRetry,
+        attachmentIsPublicDefault,
       }),
-    [baseTableName]
+    [baseTableName, attachmentIsPublicDefault]
   );
   const [uploadedCount, setUploadedCount] = React.useState<number | undefined>(
     undefined
@@ -282,8 +288,10 @@ async function uploadFileWrapped<KEY extends keyof Tables>({
   uploadAttachmentSpec,
   dryRun,
   triggerRetry,
+  attachmentIsPublicDefault = false,
 }: WrappedActionProps<KEY> & {
   readonly uploadAttachmentSpec?: UploadAttachmentSpec;
+  readonly attachmentIsPublicDefault?: boolean;
 }): Promise<PartialUploadableFileSpec> {
   const getUploadableCommited = ({
     status,
@@ -329,12 +337,6 @@ async function uploadFileWrapped<KEY extends keyof Tables>({
 
   if (dryRun) return getUploadableCommited({ status: record });
 
-  const [attachmentIsPublicDefault] = collectionPreferences.use(
-    'general',
-    'attachments',
-    'attachment.is_public_default'
-  );
-
   /*
    * TODO: Make this smarter if it causes performance problems.
    * Fetch multiple tokens at once

specifyweb/frontend/js_src/lib/components/WbImportAttachments/index.tsx

@@ -68,14 +68,9 @@ export function WbImportAttachmentsView(): JSX.Element {
 
 function uploadFiles(
   files: RA<File>,
-  handleProgress: (progress: (progress: number | undefined) => number) => void
+  handleProgress: (progress: (progress: number | undefined) => number) => void,
+  attachmentIsPublicDefault: boolean
 ): RA<Promise<SpecifyResource<Attachment>>> {
-  const [attachmentIsPublicDefault] = collectionPreferences.use(
-    'general',
-    'attachments',
-    'attachment.is_public_default'
-  );
-
   return files.map(async (file) =>
     uploadFile({ file, attachmentIsPublicDefault })
       .then(async (attachment) =>
@@ -126,6 +121,11 @@ async function saveDataSetAttachments(
 
 function FilesPicked({ files }: { readonly files: RA<File> }): JSX.Element {
   const navigate = useNavigate();
+  const [attachmentIsPublicDefault] = collectionPreferences.use(
+    'general',
+    'attachments',
+    'attachment.is_public_default'
+  );
   const [fileUploadProgress, setFileUploadProgress] = React.useState<
     number | undefined
   >(undefined);
@@ -137,7 +137,12 @@ function FilesPicked({ files }: { readonly files: RA<File> }): JSX.Element {
   ): Promise<void> => {
     setFileUploadProgress(0);
 
-    return Promise.all(uploadFiles(files, setFileUploadProgress)) // Upload all selected files/attachments
+    return Promise.resolve()
+      .then(() =>
+        Promise.all(
+          uploadFiles(files, setFileUploadProgress, attachmentIsPublicDefault)
+        )
+      ) // Upload all selected files/attachments
       .then(async (attachments) =>
         f
           .all({