From 02f9be2c4a15eed6bba07f504319a2213ea411df Mon Sep 17 00:00:00 2001 From: Jacob Fletcher Date: Mon, 30 Jan 2023 11:52:09 -0500 Subject: [PATCH 1/3] feat: deletes old media upon re-upload #1897 --- src/collections/operations/update.ts | 39 ++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/src/collections/operations/update.ts b/src/collections/operations/update.ts index 20154580c8..be7037b200 100644 --- a/src/collections/operations/update.ts +++ b/src/collections/operations/update.ts @@ -1,6 +1,6 @@ import fs from 'fs'; import { promisify } from 'util'; - +import path from 'path'; import httpStatus from 'http-status'; import { Config as GeneratedTypes } from 'payload/generated-types'; import { MarkOptional } from 'ts-essentials'; @@ -8,7 +8,7 @@ import { Where, Document } from '../../types'; import { Collection } from '../config/types'; import sanitizeInternalFields from '../../utilities/sanitizeInternalFields'; import executeAccess from '../../auth/executeAccess'; -import { NotFound, Forbidden, APIError, ValidationError } from '../../errors'; +import { NotFound, Forbidden, APIError, ValidationError, ErrorDeletingFile } from '../../errors'; import { PayloadRequest } from '../../express/types'; import { hasWhereAccessResult } from '../../auth/types'; import { saveVersion } from '../../versions/saveVersion'; @@ -20,6 +20,8 @@ import { afterRead } from '../../fields/hooks/afterRead'; import { generateFileData } from '../../uploads/generateFileData'; import { getLatestEntityVersion } from '../../versions/getLatestCollectionVersion'; import { mapAsync } from '../../utilities/mapAsync'; +import fileExists from '../../uploads/fileExists'; +import { FileData } from '../../uploads/types'; const unlinkFile = promisify(fs.unlink); @@ -157,6 +159,39 @@ async function update( data = newFileData; + // ///////////////////////////////////// + // Delete any associated files + // ///////////////////////////////////// + + if (collectionConfig.upload) { + const { staticDir } = collectionConfig.upload; + + const staticPath = path.resolve(config.paths.configDir, staticDir); + + const fileToDelete = `${staticPath}/${doc.filename}`; + + if (await fileExists(fileToDelete)) { + fs.unlink(fileToDelete, (err) => { + if (err) { + throw new ErrorDeletingFile(t); + } + }); + } + + if (doc.sizes) { + Object.values(doc.sizes).forEach(async (size: FileData) => { + const sizeToDelete = `${staticPath}/${size.filename}`; + if (await fileExists(sizeToDelete)) { + fs.unlink(sizeToDelete, (err) => { + if (err) { + throw new ErrorDeletingFile(t); + } + }); + } + }); + } + } + // ///////////////////////////////////// // beforeValidate - Fields // ///////////////////////////////////// From 827428d6b57a7abe858ca5ddaad5ba4ec9d3a270 Mon Sep 17 00:00:00 2001 From: Jacob Fletcher Date: Mon, 30 Jan 2023 15:41:14 -0500 Subject: [PATCH 2/3] fix: replaced media not rendering after document save --- .../views/collections/Edit/Default.tsx | 5 ++-- .../views/collections/Edit/Upload/index.tsx | 30 +++++++++++-------- .../views/collections/Edit/Upload/types.ts | 10 ++----- .../views/collections/Edit/index.tsx | 24 +++++++-------- .../views/collections/Edit/types.ts | 2 +- 5 files changed, 36 insertions(+), 35 deletions(-) diff --git a/src/admin/components/views/collections/Edit/Default.tsx b/src/admin/components/views/collections/Edit/Default.tsx index f6a661f268..e7e0629583 100644 --- a/src/admin/components/views/collections/Edit/Default.tsx +++ b/src/admin/components/views/collections/Edit/Default.tsx @@ -46,7 +46,7 @@ const DefaultEditView: React.FC = (props) => { onSave, permissions, isLoading, - initialState, + internalState, apiURL, action, hasSavePermission, @@ -90,7 +90,7 @@ const DefaultEditView: React.FC = (props) => { action={action} onSuccess={onSave} disabled={!hasSavePermission} - initialState={initialState} + initialState={internalState} > = (props) => { )} { }; const Upload: React.FC = (props) => { + const { + collection, + internalState, + internalState: { + filename, + }, + } = props; + const inputRef = useRef(null); const dropRef = useRef(null); const [selectingFile, setSelectingFile] = useState(false); @@ -33,13 +42,7 @@ const Upload: React.FC = (props) => { const [dragCounter, setDragCounter] = useState(0); const [replacingFile, setReplacingFile] = useState(false); const { t } = useTranslation('upload'); - - const { - data = {} as Data, - collection, - } = props; - - const { filename } = data; + const [doc, setDoc] = useState(reduceFieldsToValues(internalState)); const { value, @@ -99,6 +102,11 @@ const Upload: React.FC = (props) => { } }, [selectingFile, inputRef, setSelectingFile]); + useEffect(() => { + setDoc(reduceFieldsToValues(internalState)); + setReplacingFile(false); + }, [internalState]); + useEffect(() => { const div = dropRef.current; if (div) { @@ -118,10 +126,6 @@ const Upload: React.FC = (props) => { return () => null; }, [handleDragIn, handleDragOut, handleDrop, value]); - useEffect(() => { - setReplacingFile(false); - }, [data]); - const classes = [ baseClass, dragging && `${baseClass}--dragging`, @@ -136,7 +140,7 @@ const Upload: React.FC = (props) => { /> {(filename && !replacingFile) && ( { setReplacingFile(true); diff --git a/src/admin/components/views/collections/Edit/Upload/types.ts b/src/admin/components/views/collections/Edit/Upload/types.ts index c90cb15fa0..d1296496ee 100644 --- a/src/admin/components/views/collections/Edit/Upload/types.ts +++ b/src/admin/components/views/collections/Edit/Upload/types.ts @@ -1,13 +1,9 @@ import { SanitizedCollectionConfig } from '../../../../../../collections/config/types'; - -export type Data = { - filename: string - mimeType: string - filesize: number -} +import { Fields } from '../../../../forms/Form/types'; export type Props = { - data?: Data + internalState?: Fields + data?: Fields collection: SanitizedCollectionConfig adminThumbnail?: string mimeTypes?: string[]; diff --git a/src/admin/components/views/collections/Edit/index.tsx b/src/admin/components/views/collections/Edit/index.tsx index 5db899b38a..570c1e1642 100644 --- a/src/admin/components/views/collections/Edit/index.tsx +++ b/src/admin/components/views/collections/Edit/index.tsx @@ -40,13 +40,18 @@ const EditView: React.FC = (props) => { const { params: { id } = {} } = useRouteMatch>(); const { state: locationState } = useLocation(); const history = useHistory(); - const [initialState, setInitialState] = useState(); + const [internalState, setInternalState] = useState(); const [updatedAt, setUpdatedAt] = useState(); const { user } = useAuth(); const { getVersions, preferencesKey, getDocPermissions, docPermissions } = useDocumentInfo(); const { getPreference } = usePreferences(); const { t } = useTranslation('general'); + const [{ data, isLoading: isLoadingData, isError }] = usePayloadAPI( + (isEditing ? `${serverURL}${api}/${slug}/${id}` : null), + { initialParams: { 'fallback-locale': 'null', depth: 0, draft: 'true' } }, + ); + const onSave = useCallback(async (json: any) => { getVersions(); getDocPermissions(); @@ -55,26 +60,21 @@ const EditView: React.FC = (props) => { setRedirect(`${admin}/collections/${collection.slug}/${json?.doc?.id}`); } else { const state = await buildStateFromSchema({ fieldSchema: collection.fields, data: json.doc, user, id, operation: 'update', locale, t }); - setInitialState(state); + setInternalState(state); } }, [admin, collection, isEditing, getVersions, user, id, t, locale, getDocPermissions]); - const [{ data, isLoading: isLoadingData, isError }] = usePayloadAPI( - (isEditing ? `${serverURL}${api}/${slug}/${id}` : null), - { initialParams: { 'fallback-locale': 'null', depth: 0, draft: 'true' } }, - ); - const dataToRender = (locationState as Record)?.data || data; useEffect(() => { - const awaitInitialState = async () => { + const awaitInternalState = async () => { setUpdatedAt(dataToRender?.updatedAt); const state = await buildStateFromSchema({ fieldSchema: fields, data: dataToRender, user, operation: isEditing ? 'update' : 'create', id, locale, t }); await getPreference(preferencesKey); - setInitialState(state); + setInternalState(state); }; - awaitInitialState(); + awaitInternalState(); }, [dataToRender, fields, isEditing, id, user, locale, isLoadingData, preferencesKey, getPreference, t]); useEffect(() => { @@ -92,7 +92,7 @@ const EditView: React.FC = (props) => { const apiURL = `${serverURL}${api}/${slug}/${id}${collection.versions.drafts ? '?draft=true' : ''}`; const action = `${serverURL}${api}/${slug}${isEditing ? `/${id}` : ''}?locale=${locale}&depth=0&fallback-locale=null`; const hasSavePermission = (isEditing && docPermissions?.update?.permission) || (!isEditing && (docPermissions as CollectionPermission)?.create?.permission); - const isLoading = !initialState || !docPermissions || isLoadingData; + const isLoading = !internalState || !docPermissions || isLoadingData; return ( @@ -107,7 +107,7 @@ const EditView: React.FC = (props) => { permissions: docPermissions, isEditing, onSave, - initialState, + internalState, hasSavePermission, apiURL, action, diff --git a/src/admin/components/views/collections/Edit/types.ts b/src/admin/components/views/collections/Edit/types.ts index ed65c4b3b9..0560a0671b 100644 --- a/src/admin/components/views/collections/Edit/types.ts +++ b/src/admin/components/views/collections/Edit/types.ts @@ -15,7 +15,7 @@ export type Props = IndexProps & { id?: string permissions: CollectionPermission isLoading: boolean - initialState?: Fields + internalState?: Fields apiURL: string action: string hasSavePermission: boolean From e6ed6761172a4e541f171aa4a433a818d14589d3 Mon Sep 17 00:00:00 2001 From: Jacob Fletcher Date: Tue, 31 Jan 2023 16:04:42 -0500 Subject: [PATCH 3/3] chore: passing uploads-related e2e tests --- .../components/elements/FileDetails/index.tsx | 27 +++++++++++++++++-- .../components/elements/FileDetails/types.ts | 6 ++++- .../views/collections/Edit/Upload/index.tsx | 13 ++++----- test/uploads/config.ts | 4 +-- test/uploads/e2e.spec.ts | 4 +++ test/uploads/int.spec.ts | 11 ++++---- 6 files changed, 46 insertions(+), 19 deletions(-) diff --git a/src/admin/components/elements/FileDetails/index.tsx b/src/admin/components/elements/FileDetails/index.tsx index e07dec215c..67ad8dae03 100644 --- a/src/admin/components/elements/FileDetails/index.tsx +++ b/src/admin/components/elements/FileDetails/index.tsx @@ -1,4 +1,4 @@ -import React, { useState } from 'react'; +import React, { useEffect, useState } from 'react'; import AnimateHeight from 'react-animate-height'; import { useTranslation } from 'react-i18next'; import Thumbnail from '../Thumbnail'; @@ -6,11 +6,27 @@ import Button from '../Button'; import Meta from './Meta'; import Chevron from '../../icons/Chevron'; import { Props } from './types'; +import { FileSizes, Upload } from '../../../../uploads/types'; import './index.scss'; const baseClass = 'file-details'; +// sort to the same as imageSizes +const sortSizes = (sizes: FileSizes, imageSizes: Upload['imageSizes']) => { + if (!imageSizes || imageSizes.length === 0) return sizes; + + const orderedSizes: FileSizes = {}; + + imageSizes.forEach(({ name }) => { + if (sizes[name]) { + orderedSizes[name] = sizes[name]; + } + }); + + return orderedSizes; +}; + const FileDetails: React.FC = (props) => { const { doc, @@ -21,6 +37,7 @@ const FileDetails: React.FC = (props) => { const { upload: { staticURL, + imageSizes, }, } = collection; @@ -34,6 +51,12 @@ const FileDetails: React.FC = (props) => { url, } = doc; + const [orderedSizes, setOrderedSizes] = useState(() => sortSizes(sizes, imageSizes)); + + useEffect(() => { + setOrderedSizes(sortSizes(sizes, imageSizes)); + }, [sizes, imageSizes]); + const [moreInfoOpen, setMoreInfoOpen] = useState(false); const { t } = useTranslation('upload'); @@ -94,7 +117,7 @@ const FileDetails: React.FC = (props) => { height={moreInfoOpen ? 'auto' : 0} >
    - {Object.entries(sizes).map(([key, val]) => { + {Object.entries(orderedSizes).map(([key, val]) => { if (val?.filename) { return (
  • diff --git a/src/admin/components/elements/FileDetails/types.ts b/src/admin/components/elements/FileDetails/types.ts index 119b26edcb..578ad3c5a8 100644 --- a/src/admin/components/elements/FileDetails/types.ts +++ b/src/admin/components/elements/FileDetails/types.ts @@ -1,7 +1,11 @@ import { SanitizedCollectionConfig } from '../../../../collections/config/types'; +import { FileSizes } from '../../../../uploads/types'; +import { Data } from '../../forms/Form/types'; export type Props = { collection: SanitizedCollectionConfig - doc: Record + doc: Data & { + sizes?: FileSizes + } handleRemove?: () => void, } diff --git a/src/admin/components/views/collections/Edit/Upload/index.tsx b/src/admin/components/views/collections/Edit/Upload/index.tsx index 2842ff1a5e..80e7163b82 100644 --- a/src/admin/components/views/collections/Edit/Upload/index.tsx +++ b/src/admin/components/views/collections/Edit/Upload/index.tsx @@ -7,9 +7,9 @@ import Button from '../../../../elements/Button'; import FileDetails from '../../../../elements/FileDetails'; import Error from '../../../../forms/Error'; import { Props } from './types'; +import reduceFieldsToValues from '../../../../forms/Form/reduceFieldsToValues'; import './index.scss'; -import reduceFieldsToValues from '../../../../forms/Form/reduceFieldsToValues'; const baseClass = 'file-field'; @@ -30,9 +30,6 @@ const Upload: React.FC = (props) => { const { collection, internalState, - internalState: { - filename, - }, } = props; const inputRef = useRef(null); @@ -42,7 +39,7 @@ const Upload: React.FC = (props) => { const [dragCounter, setDragCounter] = useState(0); const [replacingFile, setReplacingFile] = useState(false); const { t } = useTranslation('upload'); - const [doc, setDoc] = useState(reduceFieldsToValues(internalState)); + const [doc, setDoc] = useState(reduceFieldsToValues(internalState || {}, true)); const { value, @@ -103,7 +100,7 @@ const Upload: React.FC = (props) => { }, [selectingFile, inputRef, setSelectingFile]); useEffect(() => { - setDoc(reduceFieldsToValues(internalState)); + setDoc(reduceFieldsToValues(internalState || {}, true)); setReplacingFile(false); }, [internalState]); @@ -138,7 +135,7 @@ const Upload: React.FC = (props) => { showError={showError} message={errorMessage} /> - {(filename && !replacingFile) && ( + {(doc.filename && !replacingFile) && ( = (props) => { }} /> )} - {(!filename || replacingFile) && ( + {(!doc.filename || replacingFile) && (
    {value && (
    diff --git a/test/uploads/config.ts b/test/uploads/config.ts index c4a9f1c0b7..c01d9148c0 100644 --- a/test/uploads/config.ts +++ b/test/uploads/config.ts @@ -53,7 +53,7 @@ export default buildConfig({ { name: 'maintainedAspectRatio', width: 1024, - height: null, + height: undefined, crop: 'center', position: 'center', formatOptions: { format: 'png', options: { quality: 90 } }, @@ -61,7 +61,7 @@ export default buildConfig({ { name: 'differentFormatFromMainImage', width: 200, - height: null, + height: undefined, formatOptions: { format: 'jpg', options: { quality: 90 } }, }, { diff --git a/test/uploads/e2e.spec.ts b/test/uploads/e2e.spec.ts index 523046da59..226d63539b 100644 --- a/test/uploads/e2e.spec.ts +++ b/test/uploads/e2e.spec.ts @@ -67,6 +67,10 @@ describe('uploads', () => { await saveDocAndAssert(page); }); + test('should update file upload', async () => { + await page.goto(mediaURL.edit(mediaDoc.id)); + }); + test('should show resized images', async () => { await page.goto(mediaURL.edit(mediaDoc.id)); diff --git a/test/uploads/int.spec.ts b/test/uploads/int.spec.ts index be07c3bcfa..12487ce4e2 100644 --- a/test/uploads/int.spec.ts +++ b/test/uploads/int.spec.ts @@ -7,7 +7,6 @@ import { RESTClient } from '../helpers/rest'; import config, { mediaSlug, relationSlug } from './config'; import payload from '../../src'; import getFileByPath from '../../src/uploads/getFileByPath'; -import type { Media } from './payload-types'; const stat = promisify(fs.stat); @@ -176,9 +175,9 @@ describe('Collections - Uploads', () => { expect(status).toBe(200); - // Check that previously existing files weren't affected - expect(await fileExists(path.join(__dirname, './media', mediaDoc.filename))).toBe(true); - expect(await fileExists(path.join(__dirname, './media', mediaDoc.sizes.icon.filename))).toBe(true); + // Check that previously existing files were removed + expect(await fileExists(path.join(__dirname, './media', mediaDoc.filename))).toBe(false); + expect(await fileExists(path.join(__dirname, './media', mediaDoc.sizes.icon.filename))).toBe(false); }); it('should remove extra sizes on update', async () => { @@ -186,13 +185,13 @@ describe('Collections - Uploads', () => { const file = await getFileByPath(filePath); const small = await getFileByPath(path.resolve(__dirname, './small.png')); - const { id } = await payload.create({ + const { id } = await payload.create({ collection: mediaSlug, data: {}, file, }); - const doc = await payload.update({ + const doc = await payload.update({ collection: mediaSlug, id, data: {},