fix: avoid re-uploading the file unless changed (#13500)

Fixes https://github.com/payloadcms/payload/issues/13182

Before, the condition in
b714e6b151/packages/payload/src/uploads/generateFileData.ts#
was always passing. Now, with `shouldReupload` we properly check the
difference (whether the image was cropped or the focal point was
changed)

packages/payload/src/uploads/generateFileData.ts

@@ -37,6 +37,33 @@ type Result<T> = Promise<{
   files: FileToSave[]
 }>
 
+const shouldReupload = (
+  uploadEdits: UploadEdits,
+  fileData: Record<string, unknown> | undefined,
+) => {
+  if (!fileData) {
+    return false
+  }
+
+  if (uploadEdits.crop || uploadEdits.heightInPixels || uploadEdits.widthInPixels) {
+    return true
+  }
+
+  // Since uploadEdits always has focalPoint, compare to the value in the data if it was changed
+  if (uploadEdits.focalPoint) {
+    const incomingFocalX = uploadEdits.focalPoint.x
+    const incomingFocalY = uploadEdits.focalPoint.y
+
+    const currentFocalX = 'focalX' in fileData && fileData.focalX
+    const currentFocalY = 'focalY' in fileData && fileData.focalY
+
+    const isEqual = incomingFocalX === currentFocalX && incomingFocalY === currentFocalY
+    return !isEqual
+  }
+
+  return false
+}
+
 export const generateFileData = async <T>({
   collection: { config: collectionConfig },
   data,
@@ -82,7 +109,10 @@ export const generateFileData = async <T>({
 
   const incomingFileData = isDuplicating ? originalDoc : data
 
-  if (!file && uploadEdits && incomingFileData) {
+  if (
+    !file &&
+    (isDuplicating || shouldReupload(uploadEdits, incomingFileData as Record<string, unknown>))
+  ) {
     const { filename, url } = incomingFileData as unknown as FileData
 
     if (filename && (filename.includes('../') || filename.includes('..\\'))) {

test/uploads/config.ts

@@ -339,7 +339,12 @@ export default buildConfigWithDefaults({
     },
     {
       slug: mediaSlug,
-      fields: [],
+      fields: [
+        {
+          type: 'text',
+          name: 'alt',
+        },
+      ],
       upload: {
         staticDir: path.resolve(dirname, './media'),
         // crop: false,

test/uploads/e2e.spec.ts

@@ -1,6 +1,7 @@
 import type { Page } from '@playwright/test'
 
 import { expect, test } from '@playwright/test'
+import { statSync } from 'fs'
 import { toggleColumn } from 'helpers/e2e/toggleColumn.js'
 import { openDocDrawer } from 'helpers/e2e/toggleDocDrawer.js'
 import path from 'path'
@@ -1603,6 +1604,24 @@ describe('Uploads', () => {
     await saveDocAndAssert(page, '#action-save', 'error')
   })
 
+  test('should not rewrite file when updating collection fields', async () => {
+    await page.goto(mediaURL.create)
+    await page.setInputFiles('input[type="file"]', path.resolve(dirname, './test-image.png'))
+    await saveDocAndAssert(page)
+    const imageID = page.url().split('/').pop()!
+    const { doc } = await client.findByID({ slug: mediaSlug, id: imageID, auth: true })
+    const filename = doc.filename as string
+    const filePath = path.resolve(dirname, 'media', filename)
+    const before = statSync(filePath)
+
+    const altField = page.locator('#field-alt')
+    await altField.fill('test alt')
+
+    await saveDocAndAssert(page)
+    const after = statSync(filePath)
+    expect(after.mtime.getTime()).toEqual(before.mtime.getTime())
+  })
+
   test('should be able to replace the file even if the user doesnt have delete access', async () => {
     const docID = (await payload.find({ collection: mediaWithoutDeleteAccessSlug, limit: 1 }))
       .docs[0]?.id as string

test/uploads/payload-types.ts

@@ -237,6 +237,7 @@ export interface Relation {
  */
 export interface Media {
   id: string;
+  alt?: string | null;
   updatedAt: string;
   createdAt: string;
   url?: string | null;
@@ -2395,6 +2396,7 @@ export interface FocalNoSizesSelect<T extends boolean = true> {
  * via the `definition` "media_select".
  */
 export interface MediaSelect<T extends boolean = true> {
+  alt?: T;
   updatedAt?: T;
   createdAt?: T;
   url?: T;