Skip to content

Commit

Permalink
fix(ui): prevent many upload fields overwriting bulkUpload onSuccess (#…
Browse files Browse the repository at this point in the history
…10189)

<!--

Thank you for the PR! Please go through the checklist below and make
sure you've completed all the steps.

Please review the
[CONTRIBUTING.md](https://github.com/payloadcms/payload/blob/main/CONTRIBUTING.md)
document in this repository if you haven't already.

The following items will ensure that your PR is handled as smoothly as
possible:

- PR Title must follow conventional commits format. For example, `feat:
my new feature`, `fix(plugin-seo): my fix`.
- Minimal description explained as if explained to someone not
immediately familiar with the code.
- Provide before/after screenshots or code diffs if applicable.
- Link any related issues/discussions from GitHub or Discord.
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Fixes #

-->
### What?
This PR fixes an issue where multiple `upload` fields would sequentially
overwrite the `BulkUpload` internal `onSuccess` function causing new
uploads to populate the incorrect field from which the interaction
started.

### Why?
Sequential `upload` fields use a `useEffect` to set the success function
of the `BulkUpload` provider component, however this did not take into
account many `upload` fields in a single document. This PR prevents many
`upload` fields from overriding their sibling's `onSuccess` function in
order to populate those fields correctly.

### How?
By changing the way the bulk upload component handles success functions
from a singular function to a map of functions based on a string path of
the field, or if necessary, using a collection slug in the case of a
bulk upload on an `upload` collection list view.

Fixes #10177

Before (One hasMany, one single):

[Editing-hasmany-single--Post-before--Payload.webm](https://github.com/user-attachments/assets/01aeaa64-a065-4e66-8ab4-6bb9d4fa8556)

Before (Many hasMany):

[Editing-hasmany-two--Post-before--Payload.webm](https://github.com/user-attachments/assets/a65c58aa-9a15-4cca-b2c4-17484c020ddc)

After (One hasMany, one single):

[Editing-hasmany-single--Post-after--Payload.webm](https://github.com/user-attachments/assets/7206f94e-4ce2-41b3-8b45-625f4974d28d)

After (Many hasMany):

[Editing-hasmany-two--Post-after--Payload.webm](https://github.com/user-attachments/assets/72dbbdee-d4a5-4488-8ef0-3dd3918115a9)
  • Loading branch information
akhrarovsaid authored Dec 27, 2024
1 parent d8a62b7 commit ebf3cee
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 13 deletions.
24 changes: 18 additions & 6 deletions packages/ui/src/elements/BulkUpload/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,51 +83,61 @@ export function BulkUploadDrawer() {

type BulkUploadContext = {
collectionSlug: string
currentActivePath: string
drawerSlug: string
initialFiles: FileList
maxFiles: number
onCancel: () => void
onSuccess: (newDocs: JsonObject[], errorCount: number) => void
setCollectionSlug: (slug: string) => void
setCurrentActivePath: (path: string) => void
setInitialFiles: (files: FileList) => void
setMaxFiles: (maxFiles: number) => void
setOnCancel: (onCancel: BulkUploadContext['onCancel']) => void
setOnSuccess: (onSuccess: BulkUploadContext['onSuccess']) => void
setOnSuccess: (path: string, onSuccess: BulkUploadContext['onSuccess']) => void
}

const Context = React.createContext<BulkUploadContext>({
collectionSlug: '',
currentActivePath: undefined,
drawerSlug: '',
initialFiles: undefined,
maxFiles: undefined,
onCancel: () => null,
onSuccess: () => null,
setCollectionSlug: () => null,
setCurrentActivePath: () => null,
setInitialFiles: () => null,
setMaxFiles: () => null,
setOnCancel: () => null,
setOnSuccess: () => null,
})
export function BulkUploadProvider({ children }: { readonly children: React.ReactNode }) {
const [collection, setCollection] = React.useState<string>()
const [onSuccessFunction, setOnSuccessFunction] = React.useState<BulkUploadContext['onSuccess']>()
const [onSuccessFunctionMap, setOnSuccessFunctionMap] =
React.useState<Record<string, BulkUploadContext['onSuccess']>>()
const [onCancelFunction, setOnCancelFunction] = React.useState<BulkUploadContext['onCancel']>()
const [initialFiles, setInitialFiles] = React.useState<FileList>(undefined)
const [maxFiles, setMaxFiles] = React.useState<number>(undefined)
const [currentActivePath, setCurrentActivePath] = React.useState<string>(undefined)
const drawerSlug = useBulkUploadDrawerSlug()

const setCollectionSlug: BulkUploadContext['setCollectionSlug'] = (slug) => {
setCollection(slug)
}

const setOnSuccess: BulkUploadContext['setOnSuccess'] = (onSuccess) => {
setOnSuccessFunction(() => onSuccess)
}
const setOnSuccess: BulkUploadContext['setOnSuccess'] = React.useCallback((path, onSuccess) => {
setOnSuccessFunctionMap((prev) => ({
...prev,
[path]: onSuccess,
}))
}, [])

return (
<Context.Provider
value={{
collectionSlug: collection,
currentActivePath,
drawerSlug,
initialFiles,
maxFiles,
Expand All @@ -137,11 +147,13 @@ export function BulkUploadProvider({ children }: { readonly children: React.Reac
}
},
onSuccess: (docIDs, errorCount) => {
if (typeof onSuccessFunction === 'function') {
if (onSuccessFunctionMap && Object.hasOwn(onSuccessFunctionMap, currentActivePath)) {
const onSuccessFunction = onSuccessFunctionMap[currentActivePath]
onSuccessFunction(docIDs, errorCount)
}
},
setCollectionSlug,
setCurrentActivePath,
setInitialFiles,
setMaxFiles,
setOnCancel: setOnCancelFunction,
Expand Down
17 changes: 13 additions & 4 deletions packages/ui/src/fields/Upload/Input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,14 @@ export function UploadInput(props: UploadInputProps) {
)

const { openModal } = useModal()
const { drawerSlug, setCollectionSlug, setInitialFiles, setMaxFiles, setOnSuccess } =
useBulkUpload()
const {
drawerSlug,
setCollectionSlug,
setCurrentActivePath,
setInitialFiles,
setMaxFiles,
setOnSuccess,
} = useBulkUpload()
const { permissions } = useAuth()
const { code } = useLocale()
const { i18n, t } = useTranslation()
Expand Down Expand Up @@ -269,6 +275,7 @@ export function UploadInput(props: UploadInputProps) {
if (typeof maxRows === 'number') {
setMaxFiles(maxRows)
}
setCurrentActivePath(path)
openModal(drawerSlug)
},
[
Expand All @@ -280,6 +287,8 @@ export function UploadInput(props: UploadInputProps) {
setInitialFiles,
maxRows,
setMaxFiles,
path,
setCurrentActivePath,
],
)

Expand Down Expand Up @@ -426,8 +435,8 @@ export function UploadInput(props: UploadInputProps) {
}, [populateDocs, activeRelationTo, value])

useEffect(() => {
setOnSuccess(onUploadSuccess)
}, [value, onUploadSuccess, setOnSuccess])
setOnSuccess(path, onUploadSuccess)
}, [value, path, onUploadSuccess, setOnSuccess])

const showDropzone =
!value ||
Expand Down
15 changes: 12 additions & 3 deletions packages/ui/src/views/List/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ export const DefaultListView: React.FC<ListViewClientProps> = (props) => {
query,
} = useListQuery()
const { openModal } = useModal()
const { setCollectionSlug, setOnSuccess } = useBulkUpload()
const { setCollectionSlug, setCurrentActivePath, setOnSuccess } = useBulkUpload()
const { drawerSlug: bulkUploadDrawerSlug } = useBulkUpload()

const collectionConfig = getEntityConfig({ collectionSlug }) as ClientCollectionConfig
Expand Down Expand Up @@ -148,9 +148,18 @@ export const DefaultListView: React.FC<ListViewClientProps> = (props) => {

const openBulkUpload = React.useCallback(() => {
setCollectionSlug(collectionSlug)
setCurrentActivePath(collectionSlug)
openModal(bulkUploadDrawerSlug)
setOnSuccess(() => router.refresh())
}, [router, collectionSlug, bulkUploadDrawerSlug, openModal, setCollectionSlug, setOnSuccess])
setOnSuccess(collectionSlug, () => router.refresh())
}, [
router,
collectionSlug,
bulkUploadDrawerSlug,
openModal,
setCollectionSlug,
setCurrentActivePath,
setOnSuccess,
])

useEffect(() => {
if (drawerDepth <= 1) {
Expand Down

0 comments on commit ebf3cee

Please sign in to comment.