fix(ui): bulk upload silently counts failed files as saved (#16532)

Backports #16528

Author: AlessioGr

packages/ui/src/elements/BulkUpload/FormsManager/index.tsx

@@ -28,6 +28,7 @@ import { useUploadHandlers } from '../../../providers/UploadHandlers/index.js'
 import { hasSavePermission as getHasSavePermission } from '../../../utilities/hasSavePermission.js'
 import { LoadingOverlay } from '../../Loading/index.js'
 import { useLoadingOverlay } from '../../LoadingOverlay/index.js'
+import { FieldErrorsToast } from '../../Toasts/fieldErrors.js'
 import { useBulkUpload } from '../index.js'
 import { createFormData } from './createFormData.js'
 import { formsManagementReducer } from './reducer.js'
@@ -404,7 +405,9 @@ export function FormsManagerProvider({ children }: FormsManagerProps) {
 
           const json = await req.json()
 
-          if (req.status === 201 && json?.doc) {
+          const wasSuccessful = req.status === 201 && json?.doc
+
+          if (wasSuccessful) {
             newDocs.push({
               collectionSlug,
               doc: json.doc,
@@ -490,14 +493,29 @@ export function FormsManagerProvider({ children }: FormsManagerProps) {
 
             toast.error(nonFieldErrors[0]?.message)
           } else {
+            let errorCount = fieldErrors.length
+
+            // Fall back to non-field errors when no field errors are present
+            // (e.g., APIError thrown from a hook).
+            if (!wasSuccessful && errorCount === 0) {
+              errorCount = nonFieldErrors.length || 1
+            }
+
             currentForms[i] = {
-              errorCount: fieldErrors.length,
+              errorCount,
               formID: currentForms[i].formID,
               formState: fieldReducer(currentForms[i].formState, {
                 type: 'ADD_SERVER_ERRORS',
                 errors: fieldErrors,
               }),
             }
+
+            // Mimic forms/Form/index.tsx.
+            if (!wasSuccessful) {
+              nonFieldErrors.forEach((err) => {
+                toast.error(<FieldErrorsToast errorMessage={err.message || t('error:unknown')} />)
+              })
+            }
           }
         } catch (_) {
           // swallow

test/uploads/collections/BulkUploadsHookError/index.ts

@@ -0,0 +1,40 @@
+import type { CollectionConfig } from 'payload'
+
+import path from 'path'
+import { APIError } from 'payload'
+import { fileURLToPath } from 'url'
+
+import { bulkUploadsHookErrorSlug } from '../../shared.js'
+
+const filename = fileURLToPath(import.meta.url)
+const dirname = path.dirname(filename)
+
+export const BulkUploadsHookErrorCollection: CollectionConfig = {
+  slug: bulkUploadsHookErrorSlug,
+  upload: {
+    staticDir: path.resolve(dirname, '../../media'),
+  },
+  admin: {
+    useAsTitle: 'title',
+  },
+  fields: [
+    {
+      name: 'title',
+      type: 'text',
+    },
+    {
+      name: 'shouldFail',
+      type: 'checkbox',
+    },
+  ],
+  hooks: {
+    beforeChange: [
+      ({ data }) => {
+        if (data?.shouldFail) {
+          throw new APIError('Simulated hook error in beforeChange', 422, undefined, true)
+        }
+        return data
+      },
+    ],
+  },
+}

test/uploads/config.ts

@@ -8,6 +8,7 @@ import { AdminThumbnailWithSearchQueries } from './collections/AdminThumbnailWit
 import { AdminUploadControl } from './collections/AdminUploadControl/index.js'
 import { AnyImageTypeCollection } from './collections/AnyImageType/index.js'
 import { BulkUploadsCollection } from './collections/BulkUploads/index.js'
+import { BulkUploadsHookErrorCollection } from './collections/BulkUploadsHookError/index.js'
 import { CustomUploadFieldCollection } from './collections/CustomUploadField/index.js'
 import { FileMimeType } from './collections/FileMimeType/index.js'
 import { NoFilesRequired } from './collections/NoFilesRequired/index.js'
@@ -1004,6 +1005,7 @@ export default buildConfigWithDefaults({
       },
     },
     BulkUploadsCollection,
+    BulkUploadsHookErrorCollection,
     SimpleRelationshipCollection,
     FileMimeType,
     {

test/uploads/e2e.spec.ts

@@ -33,6 +33,7 @@ import {
   adminUploadControlSlug,
   animatedTypeMedia,
   audioSlug,
+  bulkUploadsHookErrorSlug,
   bulkUploadsSlug,
   constructorOptionsSlug,
   customFileNameMediaSlug,
@@ -109,6 +110,7 @@ let consoleErrorsFromPage: string[] = []
 let collectErrorsFromPage: () => boolean
 let stopCollectingErrorsFromPage: () => boolean
 let bulkUploadsURL: AdminUrlUtil
+let bulkUploadsHookErrorURL: AdminUrlUtil
 let fileMimeTypeURL: AdminUrlUtil
 let svgOnlyURL: AdminUrlUtil
 let mediaWithoutDeleteAccessURL: AdminUrlUtil
@@ -153,6 +155,7 @@ describe('Uploads', () => {
     threeDimensionalURL = new AdminUrlUtil(serverURL, threeDimensionalSlug)
     constructorOptionsURL = new AdminUrlUtil(serverURL, constructorOptionsSlug)
     bulkUploadsURL = new AdminUrlUtil(serverURL, bulkUploadsSlug)
+    bulkUploadsHookErrorURL = new AdminUrlUtil(serverURL, bulkUploadsHookErrorSlug)
     fileMimeTypeURL = new AdminUrlUtil(serverURL, fileMimeTypeSlug)
     svgOnlyURL = new AdminUrlUtil(serverURL, svgOnlySlug)
     mediaWithoutDeleteAccessURL = new AdminUrlUtil(serverURL, mediaWithoutDeleteAccessSlug)
@@ -1671,6 +1674,51 @@ describe('Uploads', () => {
       const errorCount = bulkUploadModal.locator('.file-selections .error-pill__count').first()
       await expect(errorCount).toHaveText('1')
     })
+
+    test('should report failure when beforeChange hook throws non-field error', async () => {
+      await page.goto(bulkUploadsHookErrorURL.list)
+
+      await expect(page.locator('.list-header__title')).toBeVisible()
+
+      const bulkUploadButton = page.locator('.list-header__title-actions button', {
+        hasText: 'Bulk Upload',
+      })
+      await expect(bulkUploadButton).toBeEnabled()
+
+      const dropzoneInput = page.locator('.dropzone input[type="file"]')
+      await expect(async () => {
+        await bulkUploadButton.click()
+        await expect(dropzoneInput).toBeAttached({ timeout: 1500 })
+      }).toPass({ timeout: 5000, intervals: [500] })
+
+      await page
+        .locator('.dropzone input[type="file"]')
+        .setInputFiles([path.resolve(dirname, './image.png'), path.resolve(dirname, './small.png')])
+
+      const nextButton = page.locator('.bulk-upload--actions-bar__controls button:nth-of-type(2)')
+      await nextButton.click()
+
+      await page.locator('#field-shouldFail').check()
+
+      const saveButton = page.locator('.bulk-upload--actions-bar__saveButtons button')
+      await saveButton.click()
+
+      await expect(page.locator('.payload-toast-container .toast-success')).toContainText(
+        'Successfully saved 1 files',
+      )
+      await expect(
+        page.locator('.payload-toast-container .toast-error:has-text("Failed to save 1 files")'),
+      ).toBeVisible()
+      await expect(
+        page.locator(
+          '.payload-toast-container .toast-error:has-text("Simulated hook error in beforeChange")',
+        ),
+      ).toBeVisible()
+
+      await expect(page.locator('.file-selections .file-selections__fileRowContainer')).toHaveCount(
+        1,
+      )
+    })
   })
 
   describe('remote url fetching', () => {

test/uploads/payload-types.ts

@@ -121,6 +121,7 @@ export interface Config {
     'three-dimensional': ThreeDimensional;
     'constructor-options': ConstructorOption;
     'bulk-uploads': BulkUpload;
+    'bulk-uploads-hook-error': BulkUploadsHookError;
     'simple-relationship': SimpleRelationship;
     'file-mime-type': FileMimeType;
     'svg-only': SvgOnly;
@@ -189,6 +190,7 @@ export interface Config {
     'three-dimensional': ThreeDimensionalSelect<false> | ThreeDimensionalSelect<true>;
     'constructor-options': ConstructorOptionsSelect<false> | ConstructorOptionsSelect<true>;
     'bulk-uploads': BulkUploadsSelect<false> | BulkUploadsSelect<true>;
+    'bulk-uploads-hook-error': BulkUploadsHookErrorSelect<false> | BulkUploadsHookErrorSelect<true>;
     'simple-relationship': SimpleRelationshipSelect<false> | SimpleRelationshipSelect<true>;
     'file-mime-type': FileMimeTypeSelect<false> | FileMimeTypeSelect<true>;
     'svg-only': SvgOnlySelect<false> | SvgOnlySelect<true>;
@@ -1702,6 +1704,26 @@ export interface SimpleRelationship {
   updatedAt: string;
   createdAt: string;
 }
+/**
+ * This interface was referenced by `Config`'s JSON-Schema
+ * via the `definition` "bulk-uploads-hook-error".
+ */
+export interface BulkUploadsHookError {
+  id: string;
+  title?: string | null;
+  shouldFail?: boolean | null;
+  updatedAt: string;
+  createdAt: string;
+  url?: string | null;
+  thumbnailURL?: string | null;
+  filename?: string | null;
+  mimeType?: string | null;
+  filesize?: number | null;
+  width?: number | null;
+  height?: number | null;
+  focalX?: number | null;
+  focalY?: number | null;
+}
 /**
  * This interface was referenced by `Config`'s JSON-Schema
  * via the `definition` "file-mime-type".
@@ -2093,6 +2115,10 @@ export interface PayloadLockedDocument {
         relationTo: 'bulk-uploads';
         value: string | BulkUpload;
       } | null)
+    | ({
+        relationTo: 'bulk-uploads-hook-error';
+        value: string | BulkUploadsHookError;
+      } | null)
     | ({
         relationTo: 'simple-relationship';
         value: string | SimpleRelationship;
@@ -3691,6 +3717,25 @@ export interface BulkUploadsSelect<T extends boolean = true> {
   focalX?: T;
   focalY?: T;
 }
+/**
+ * This interface was referenced by `Config`'s JSON-Schema
+ * via the `definition` "bulk-uploads-hook-error_select".
+ */
+export interface BulkUploadsHookErrorSelect<T extends boolean = true> {
+  title?: T;
+  shouldFail?: T;
+  updatedAt?: T;
+  createdAt?: T;
+  url?: T;
+  thumbnailURL?: T;
+  filename?: T;
+  mimeType?: T;
+  filesize?: T;
+  width?: T;
+  height?: T;
+  focalX?: T;
+  focalY?: T;
+}
 /**
  * This interface was referenced by `Config`'s JSON-Schema
  * via the `definition` "simple-relationship_select".

test/uploads/shared.ts

@@ -36,6 +36,7 @@ export const listViewPreviewSlug = 'list-view-preview'
 export const threeDimensionalSlug = 'three-dimensional'
 export const constructorOptionsSlug = 'constructor-options'
 export const bulkUploadsSlug = 'bulk-uploads'
+export const bulkUploadsHookErrorSlug = 'bulk-uploads-hook-error'
 export const restrictedMimeTypesSlug = 'restricted-mime-types'
 export const pdfOnlySlug = 'pdf-only'