feat: client uploads#30
Conversation
- Extract shared publicID module for consistent ID generation - Rewrite server handler with proper signature generation - Rewrite client handler to use server-provided params (no NEXT_PUBLIC env var) - Add afterChange hook for client upload metadata persistence - Guard initClientUploads with clientUploads check - Fix example config missing collections - Update CHANGELOG.md and README.md with client upload docs - Fix publish script with non-interactive mode and npm token support
📝 WalkthroughWalkthroughThis PR implements client-side uploads to Cloudinary for Payload CMS, allowing browsers to upload directly to Cloudinary using server-signed credentials. It includes a shared public ID generation module, server route for signature generation, client-side upload handler, refactored server-side upload processing, plugin integration with metadata persistence, and supporting version/dependency updates. ChangesClient-side Uploads Feature
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser/Client
participant PayloadServer as Payload Server
participant ServerRoute as getClientUploadRoute
participant Cloudinary as Cloudinary API
participant Plugin as Cloudinary Plugin
Browser->>ServerRoute: POST upload metadata (filename, docPrefix, collectionSlug)
ServerRoute->>Cloudinary: Reconfigure SDK + sign parameters (timestamp, public_id, asset_folder)
Cloudinary-->>ServerRoute: Signature
ServerRoute-->>Browser: JSON (signature, apiKey, uploadUrl, uploadParams, publicId)
Browser->>Cloudinary: POST file + signed FormData
Cloudinary-->>Browser: uploadResult
Browser->>PayloadServer: File upload + clientUploadContext
PayloadServer->>Plugin: afterChange event
Plugin->>Plugin: processUploadResult() for metadata
Plugin->>PayloadServer: Persist doc.cloudinary metadata
PayloadServer-->>Browser: Updated document
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds client-side Cloudinary uploads to bypass server request-size limits (e.g. Vercel's 4.5 MB cap). It introduces a server-signed credential endpoint, a browser-side upload handler, an
Confidence Score: 3/5The client-upload feature has two issues that could prevent it from working correctly in production: the signature endpoint has no auth check, and the config object passed to initClientUploads may not be the same instance that reaches cloudStoragePlugin. The unauthenticated signature endpoint lets any browser caller upload arbitrary files to the Cloudinary account. The config-object mismatch between initClientUploads and cloudStoragePlugin risks the server-handler route never being registered, silently breaking the entire client-upload flow. These two issues together make the beta unsafe to ship without fixes. src/getClientUploadRoute.ts (auth guard) and src/index.ts (config object passed to initClientUploads) Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser as Browser (Admin UI)
participant PayloadAPI as Payload API
participant SignatureRoute as /cloudinary-client-upload-route
participant Cloudinary as Cloudinary API
participant AfterChange as afterChange Hook
Browser->>PayloadAPI: POST (create document)
PayloadAPI->>SignatureRoute: "POST {filename, collectionSlug, docPrefix}"
SignatureRoute-->>PayloadAPI: "{signature, apiKey, uploadUrl, uploadParams}"
PayloadAPI-->>Browser: signed credentials
Browser->>Cloudinary: POST FormData (file + signed params)
Cloudinary-->>Browser: "uploadResult {public_id, secure_url, ...}"
Browser->>PayloadAPI: "complete create with clientUploadContext={uploadResult}"
PayloadAPI->>AfterChange: fires with doc + clientUploadContext
AfterChange->>PayloadAPI: "payload.update({cloudinary: metadata})"
PayloadAPI-->>Browser: final document with cloudinary metadata
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export const getClientUploadRoute = (options: GetClientUploadRouteOptions) => { | ||
| return async (req: PayloadRequest) => { | ||
| // Reconfigure cloudinary here to ensure it uses the correct config for this plugin instance | ||
| cloudinary.config({ | ||
| cloud_name: options.cloudName, | ||
| api_key: options.apiKey, | ||
| api_secret: options.apiSecret, | ||
| }) | ||
|
|
||
| const body = (typeof req.json === 'function' ? await req.json() : req.body) as Record<string, any> | ||
| const { collectionSlug, docPrefix, filename, filesize, mimeType } = body || {} | ||
|
|
||
| try { | ||
| const timestamp = Math.floor(Date.now() / 1000) |
There was a problem hiding this comment.
The route handler never verifies req.user, so any unauthenticated browser request to /api/cloudinary-client-upload-route can obtain a valid Cloudinary upload signature. An anonymous caller could generate signatures to upload arbitrary content to the configured Cloudinary account, bypassing access controls entirely. Add a check such as if (!req.user) return Response.json({ errors: [{ message: 'Unauthorized' }] }, { status: 401 }) before the try-block.
| if (cloudinaryOptions.clientUploads && Object.keys(pluginCollections).length > 0) { | ||
| initClientUploads< | ||
| CloudinaryClientUploadHandlerExtra, | ||
| CloudinaryStorageOptions["collections"][string] | ||
| >({ | ||
| clientHandler: | ||
| "payload-cloudinary/client#CloudinaryClientUploadHandler", | ||
| collections: pluginCollections, | ||
| config: incomingConfig, | ||
| enabled: Boolean(cloudinaryOptions.clientUploads), | ||
| extraClientHandlerProps: () => ({ | ||
| useCompositePrefixes: !!cloudinaryOptions.useCompositePrefixes, | ||
| }), | ||
| serverHandler: getClientUploadRoute({ | ||
| apiKey: cloudinaryOptions.config.api_key, | ||
| apiSecret: cloudinaryOptions.config.api_secret, | ||
| cloudName: cloudinaryOptions.config.cloud_name, | ||
| folder: cloudinaryOptions.folder || 'payload-media', | ||
| publicID: cloudinaryOptions.publicID, | ||
| versioning: cloudinaryOptions.versioning, | ||
| }), | ||
| serverHandlerPath: "/cloudinary-client-upload-route", | ||
| }); |
There was a problem hiding this comment.
initClientUploads receives the original config, not the modified one
config is built as a shallow copy of incomingConfig with an entirely new collections array (containing the afterChange hooks and extra fields). initClientUploads is then passed incomingConfig — if the utility assigns new top-level properties (e.g. replaces endpoints with a new array rather than pushing to the existing one), those assignments land on incomingConfig and are not visible in config. The server-handler endpoint would then never be included in the config that cloudStoragePlugin finally returns. Passing config (the already-modified object) here ensures any mutations or reassignments made by initClientUploads are on the same object that gets passed to cloudStoragePlugin.
| const paramsToSign: Record<string, any> = { | ||
| timestamp, | ||
| public_id: publicIdValue, | ||
| asset_folder: folderPath, | ||
| use_filename: true, | ||
| unique_filename: true, | ||
| } |
There was a problem hiding this comment.
use_filename/unique_filename ignore publicID options
generatePublicID correctly consults options.publicID.useFilename and options.publicID.uniqueFilename, but paramsToSign always sets both to true regardless. When those options are false the generated public_id contains no timestamp/filename component, yet the signed params tell Cloudinary the opposite. While Cloudinary gives precedence to the explicit public_id, having signed parameters that contradict the plugin's own config leads to confusing divergence from the server-side upload path.
| const paramsToSign: Record<string, any> = { | |
| timestamp, | |
| public_id: publicIdValue, | |
| asset_folder: folderPath, | |
| use_filename: true, | |
| unique_filename: true, | |
| } | |
| const paramsToSign: Record<string, any> = { | |
| timestamp, | |
| public_id: publicIdValue, | |
| asset_folder: folderPath, | |
| use_filename: options.publicID?.useFilename !== false, | |
| unique_filename: options.publicID?.uniqueFilename !== false, | |
| } |
| docPrefix, | ||
| extra: { useCompositePrefixes = false }, |
There was a problem hiding this comment.
useCompositePrefixes is received but never used
useCompositePrefixes is destructured from extra and declared in CloudinaryClientUploadHandlerExtra, but it has no effect on the handler's logic. docPrefix is forwarded to the server unconditionally whether or not useCompositePrefixes is true. The README and type definition both advertise it as controlling document-path prefixes, so users who set this option will get no different behavior. Either apply it (e.g. conditionally include docPrefix in the request body) or remove the option and its documentation.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const signatureResponse = await fetch(endpointRoute, { | ||
| method: 'POST', | ||
| credentials: 'include', | ||
| body: JSON.stringify({ | ||
| collectionSlug, | ||
| docPrefix, | ||
| filename: file.name, | ||
| filesize: file.size, | ||
| mimeType: file.type, | ||
| }), | ||
| }) |
There was a problem hiding this comment.
Content-Type: application/json header on signature request
The body is JSON.stringify(...) but no Content-Type header is set. Some middleware in the Payload/Next.js request pipeline uses the header to decide whether to call the JSON parser. Without it the body may arrive unparsed on the server side, causing filename, collectionSlug, etc. to be undefined and the signature generation to fail silently with a generic error.
| const signatureResponse = await fetch(endpointRoute, { | |
| method: 'POST', | |
| credentials: 'include', | |
| body: JSON.stringify({ | |
| collectionSlug, | |
| docPrefix, | |
| filename: file.name, | |
| filesize: file.size, | |
| mimeType: file.type, | |
| }), | |
| }) | |
| const signatureResponse = await fetch(endpointRoute, { | |
| method: 'POST', | |
| credentials: 'include', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: JSON.stringify({ | |
| collectionSlug, | |
| docPrefix, | |
| filename: file.name, | |
| filesize: file.size, | |
| mimeType: file.type, | |
| }), | |
| }) |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces direct client-side uploads to Cloudinary to bypass server request limits, featuring secure server-side signature generation, a client-side upload handler, and an afterChange hook to persist metadata. Key feedback includes improving robustness by moving request body parsing inside a try-catch block and validating the filename parameter in the server route, adding defensive guards for potentially undefined parameters in the client handler, and wrapping signature response parsing in a try-catch block. Additionally, the reviewer recommends implementing an isClientUploadsEnabled helper to correctly respect configurations where clientUploads is an object with enabled: false.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const body = (typeof req.json === 'function' ? await req.json() : req.body) as Record<string, any> | ||
| const { collectionSlug, docPrefix, filename, filesize, mimeType } = body || {} | ||
|
|
||
| try { | ||
| const timestamp = Math.floor(Date.now() / 1000) |
There was a problem hiding this comment.
Move the request body parsing inside the try-catch block to prevent unhandled exceptions/500 crashes if the request body is empty or contains invalid JSON. Additionally, add explicit validation for the required filename parameter to prevent runtime errors when calling path.extname(filename).
| const body = (typeof req.json === 'function' ? await req.json() : req.body) as Record<string, any> | |
| const { collectionSlug, docPrefix, filename, filesize, mimeType } = body || {} | |
| try { | |
| const timestamp = Math.floor(Date.now() / 1000) | |
| try { | |
| const body = (typeof req.json === 'function' ? await req.json() : req.body) as Record<string, any> | |
| const { collectionSlug, docPrefix, filename, filesize, mimeType } = body || {} | |
| if (!filename || typeof filename !== 'string') { | |
| return Response.json( | |
| { errors: [{ message: 'Missing or invalid required parameter: filename' }] }, | |
| { status: 400 } | |
| ) | |
| } | |
| const timestamp = Math.floor(Date.now() / 1000) |
| const safeServerURL = serverURL.replace(/\/$/, '') | ||
| const safeApiRoute = apiRoute.replace(/^\/|\/$/g, '') | ||
| const safeHandlerPath = serverHandlerPath.replace(/^\//, '') |
There was a problem hiding this comment.
Add defensive guards for serverURL, apiRoute, and serverHandlerPath to prevent runtime TypeError crashes if any of these parameters are undefined or null.
| const safeServerURL = serverURL.replace(/\/$/, '') | |
| const safeApiRoute = apiRoute.replace(/^\/|\/$/g, '') | |
| const safeHandlerPath = serverHandlerPath.replace(/^\//, '') | |
| const safeServerURL = (serverURL || '').replace(/\/$/, '') | |
| const safeApiRoute = (apiRoute || '').replace(/^\/|\/$/g, '') | |
| const safeHandlerPath = (serverHandlerPath || '').replace(/^\//, '') |
| if (!signatureResponse.ok) { | ||
| const { errors } = (await signatureResponse.json()) as { | ||
| errors: { message: string }[] | ||
| } | ||
| throw new Error(errors.reduce((acc, err) => `${acc ? `${acc}, ` : ''}${err.message}`, '')) | ||
| } |
There was a problem hiding this comment.
Improve error handling when fetching the signed upload parameters. If the server returns a non-JSON error or a JSON response without an errors array, the current code will throw a TypeError or SyntaxError. Wrapping the parsing in a try-catch block ensures a robust fallback error message is always thrown.
if (!signatureResponse.ok) {
let message = 'Failed to get upload signature'
try {
const resJson = await signatureResponse.json()
if (resJson && Array.isArray(resJson.errors)) {
message = resJson.errors.map((err: any) => err.message).join(', ')
}
} catch {
try {
message = await signatureResponse.text()
} catch {}
}
throw new Error(message)
}| // Ensure collections is always an object (defensive handling) | ||
| const pluginCollections = cloudinaryOptions.collections || {}; |
There was a problem hiding this comment.
Define a helper boolean isClientUploadsEnabled to correctly resolve the enabled state when clientUploads is configured as an object (e.g., { enabled: false }). Using Boolean(cloudinaryOptions.clientUploads) directly would incorrectly evaluate to true for any object, even if enabled is set to false.
// Ensure collections is always an object (defensive handling)
const pluginCollections = cloudinaryOptions.collections || {};
const isClientUploadsEnabled = typeof cloudinaryOptions.clientUploads === 'object'
? cloudinaryOptions.clientUploads.enabled !== false
: !!cloudinaryOptions.clientUploads;| // for client-uploaded files (since the file is already on Cloudinary). | ||
| // This hook catches those cases and persists the Cloudinary metadata that | ||
| // would normally be saved by handleUpload. | ||
| if (cloudinaryOptions.clientUploads) { |
There was a problem hiding this comment.
Use the isClientUploadsEnabled helper to conditionally add the afterChange hook, ensuring it respects { enabled: false } configurations.
| if (cloudinaryOptions.clientUploads) { | |
| if (isClientUploadsEnabled) { |
| if (cloudinaryOptions.clientUploads && Object.keys(pluginCollections).length > 0) { | ||
| initClientUploads< | ||
| CloudinaryClientUploadHandlerExtra, | ||
| CloudinaryStorageOptions["collections"][string] | ||
| >({ | ||
| clientHandler: | ||
| "payload-cloudinary/client#CloudinaryClientUploadHandler", | ||
| collections: pluginCollections, | ||
| config: incomingConfig, | ||
| enabled: Boolean(cloudinaryOptions.clientUploads), |
There was a problem hiding this comment.
Use the isClientUploadsEnabled helper here to prevent initializing client uploads when clientUploads is configured as an object with enabled: false.
// Initialize client uploads only if explicitly enabled and collections are configured
if (isClientUploadsEnabled && Object.keys(pluginCollections).length > 0) {
initClientUploads<
CloudinaryClientUploadHandlerExtra,
CloudinaryStorageOptions["collections"][string]
>({
clientHandler:
"payload-cloudinary/client#CloudinaryClientUploadHandler",
collections: pluginCollections,
config: incomingConfig,
enabled: isClientUploadsEnabled,![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
scripts/publish.sh (1)
48-69: 💤 Low valueConsider quoting variables to prevent word splitting.
The non-interactive mode logic looks good, but consider quoting the
$versionand$tagvariables in function calls to prevent potential issues if they contain spaces or special characters.🛡️ Proposed fix
- publish_version $version $tag + publish_version "$version" "$tag"As per coding guidelines, Shellcheck SC2086 recommends quoting variables to prevent globbing and word splitting.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/publish.sh` around lines 48 - 69, The script leaves $version and $tag unquoted which can cause word-splitting/globbing; change the assignments to version="$1" and tag="$2" and quote uses of these vars when calling functions or commands (e.g. call publish_version "$version" "$tag" and ensure any other usages that are currently unquoted are wrapped in quotes) so values with spaces or special chars are preserved; locate the variables and the publish_version invocation to apply the fixes.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/getClientUploadRoute.ts`:
- Around line 51-52: The signed client upload params are hardcoded with
use_filename: true and unique_filename: true; change them to honor the same
publicID options used by server uploads by setting use_filename to
publicID?.useFilename !== false and unique_filename to publicID?.uniqueFilename
!== false in the function that builds the signed params (look for
getClientUploadRoute / the object where use_filename and unique_filename are
set) so client-initiated uploads respect publicID configuration consistently
with handleUpload.ts.
In `@src/index.ts`:
- Around line 195-198: The early return that checks doc.cloudinary?.public_id
causes updates to skip applying new uploadResult; change the logic so you only
skip when there is no new upload result instead of any existing cloudinary
metadata—i.e., remove or tighten the guard around doc.cloudinary?.public_id and
ensure the code that reads uploadResult and assigns/overwrites doc.cloudinary
with the new Cloudinary metadata (the uploadResult handling code) runs when
uploadResult is present or when the asset has changed; update the branch that
currently returns early to instead detect absence of uploadResult (or compare
incoming file identifiers) and only return in that case so replacement uploads
update doc.cloudinary.
- Around line 244-257: The update call that writes cloudinaryMetadata via
req.payload.update currently only sets data: { cloudinary: cloudinaryMetadata }
and thus drops version history for client-uploaded collections; modify the
update payload to include the version history when versioning.storeHistory is
enabled (same logic as in handleUpload.ts) by adding data.versions (e.g.,
merging or appending the new cloudinary entry into the existing versions array
or creating a versions array if missing) so that req.payload.update,
cloudinaryMetadata, versioning.storeHistory and data.versions are handled
consistently across both upload paths.
- Around line 178-183: The current afterChange hook (on
modifiedCollection.hooks.afterChange) calls req.payload.update which re-triggers
the collection's afterChange chain and causes duplicate side effects; to fix
this, set a one-time marker on the request context (e.g.,
req.context.skipCloudinaryClientUpload = true) immediately before calling
req.payload.update and clear it after the update so other hooks can detect and
short-circuit (they should check req.context.skipCloudinaryClientUpload), or
alternatively pass a request object that contains that flag into
req.payload.update; reference modifiedCollection.hooks.afterChange, the async
hook function, skipCloudinaryClientUpload, and req.payload.update when making
the change.
In `@src/publicID.ts`:
- Around line 31-36: The generatePublicID call is passing folder and prefix
reversed; update the call site so PublicIDOptions.generatePublicID(filename,
prefix, folder) is called with prefix = path.dirname(folderPath) and folder =
path.basename(folderPath) (i.e., swap the 2nd and 3rd arguments), or
alternatively change the PublicIDOptions signature/README to match the current
order—but the preferred fix is to swap the arguments where generatePublicID is
invoked so its parameters align with the documented (filename, prefix, folder)
semantics.
---
Nitpick comments:
In `@scripts/publish.sh`:
- Around line 48-69: The script leaves $version and $tag unquoted which can
cause word-splitting/globbing; change the assignments to version="$1" and
tag="$2" and quote uses of these vars when calling functions or commands (e.g.
call publish_version "$version" "$tag" and ensure any other usages that are
currently unquoted are wrapped in quotes) so values with spaces or special chars
are preserved; locate the variables and the publish_version invocation to apply
the fixes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9aebe839-5e5d-4904-82df-64869488a14b
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (10)
CHANGELOG.mdREADME.mdpackage.jsonscripts/publish.shsrc/client/CloudinaryClientUploadHandler.tssrc/getClientUploadRoute.tssrc/handleUpload.tssrc/index.tssrc/publicID.tssrc/types.ts
| use_filename: true, | ||
| unique_filename: true, |
There was a problem hiding this comment.
Respect publicID configuration for filename handling.
The server-side upload handler in handleUpload.ts respects the publicID.useFilename and publicID.uniqueFilename options using publicID?.useFilename !== false and publicID?.uniqueFilename !== false, but these lines hardcode both values to true.
This creates inconsistent behavior: if a user configures publicID: { useFilename: false }, server-initiated uploads will honor it, but client-initiated uploads will not. The signed parameters should match the intended configuration.
🔧 Proposed fix to align with server upload logic
const paramsToSign: Record<string, any> = {
timestamp,
public_id: publicIdValue,
asset_folder: folderPath,
- use_filename: true,
- unique_filename: true,
+ use_filename: options.publicID?.useFilename !== false,
+ unique_filename: options.publicID?.uniqueFilename !== false,
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use_filename: true, | |
| unique_filename: true, | |
| const paramsToSign: Record<string, any> = { | |
| timestamp, | |
| public_id: publicIdValue, | |
| asset_folder: folderPath, | |
| use_filename: options.publicID?.useFilename !== false, | |
| unique_filename: options.publicID?.uniqueFilename !== false, | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/getClientUploadRoute.ts` around lines 51 - 52, The signed client upload
params are hardcoded with use_filename: true and unique_filename: true; change
them to honor the same publicID options used by server uploads by setting
use_filename to publicID?.useFilename !== false and unique_filename to
publicID?.uniqueFilename !== false in the function that builds the signed params
(look for getClientUploadRoute / the object where use_filename and
unique_filename are set) so client-initiated uploads respect publicID
configuration consistently with handleUpload.ts.
| const existingAfterChange = modifiedCollection.hooks?.afterChange || []; | ||
| modifiedCollection.hooks = { | ||
| ...modifiedCollection.hooks, | ||
| afterChange: [ | ||
| ...existingAfterChange, | ||
| async ({ doc, req, operation }) => { |
There was a problem hiding this comment.
Avoid re-entering the collection’s full afterChange chain here.
This hook appends itself after existingAfterChange, then performs req.payload.update(...) inside afterChange. That second write will invoke the collection’s other afterChange hooks again; only this plugin hook checks skipCloudinaryClientUpload, so user hooks/webhooks/side effects will run twice for one client upload.
Also applies to: 250-257
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/index.ts` around lines 178 - 183, The current afterChange hook (on
modifiedCollection.hooks.afterChange) calls req.payload.update which re-triggers
the collection's afterChange chain and causes duplicate side effects; to fix
this, set a one-time marker on the request context (e.g.,
req.context.skipCloudinaryClientUpload = true) immediately before calling
req.payload.update and clear it after the update so other hooks can detect and
short-circuit (they should check req.context.skipCloudinaryClientUpload), or
alternatively pass a request object that contains that flag into
req.payload.update; reference modifiedCollection.hooks.afterChange, the async
hook function, skipCloudinaryClientUpload, and req.payload.update when making
the change.
| // Already has cloudinary metadata (e.g., from a prior hook run) | ||
| if (doc.cloudinary?.public_id) { | ||
| return doc; | ||
| } |
There was a problem hiding this comment.
Don’t skip metadata refresh when replacing an existing asset.
This early return makes client-upload replacements keep the old cloudinary block. Any update against a document that already has cloudinary.public_id bails out before applying the new uploadResult, so the stored Cloudinary metadata can become stale.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/index.ts` around lines 195 - 198, The early return that checks
doc.cloudinary?.public_id causes updates to skip applying new uploadResult;
change the logic so you only skip when there is no new upload result instead of
any existing cloudinary metadata—i.e., remove or tighten the guard around
doc.cloudinary?.public_id and ensure the code that reads uploadResult and
assigns/overwrites doc.cloudinary with the new Cloudinary metadata (the
uploadResult handling code) runs when uploadResult is present or when the asset
has changed; update the branch that currently returns early to instead detect
absence of uploadResult (or compare incoming file identifiers) and only return
in that case so replacement uploads update doc.cloudinary.
| // Persist the metadata via an update call | ||
| if (!req.context) { | ||
| req.context = {}; | ||
| } | ||
| req.context.skipCloudinaryClientUpload = true; | ||
|
|
||
| try { | ||
| await req.payload.update({ | ||
| id: doc.id, | ||
| collection: collection.slug, | ||
| data: { cloudinary: cloudinaryMetadata }, | ||
| depth: 0, | ||
| req, | ||
| }); |
There was a problem hiding this comment.
Persist version history on the client-upload path too.
src/handleUpload.ts still appends data.versions when versioning.storeHistory is enabled, but this fallback only writes cloudinary. Collections using client uploads will silently stop recording Cloudinary version history, so history/rollback data becomes incomplete.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/index.ts` around lines 244 - 257, The update call that writes
cloudinaryMetadata via req.payload.update currently only sets data: {
cloudinary: cloudinaryMetadata } and thus drops version history for
client-uploaded collections; modify the update payload to include the version
history when versioning.storeHistory is enabled (same logic as in
handleUpload.ts) by adding data.versions (e.g., merging or appending the new
cloudinary entry into the existing versions array or creating a versions array
if missing) so that req.payload.update, cloudinaryMetadata,
versioning.storeHistory and data.versions are handled consistently across both
upload paths.
| if (publicIDOptions?.generatePublicID) { | ||
| return publicIDOptions.generatePublicID( | ||
| filename, | ||
| path.dirname(folderPath), | ||
| path.basename(folderPath), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if there are any existing custom generatePublicID implementations that might break
rg -n -C3 'generatePublicID.*:.*\(' --type=tsRepository: SyedMuzamilM/payload-cloudinary
Length of output: 367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the callback type + its documented parameter meaning
sed -n '70,140p' src/types.ts
# Show the call site in src/publicID.ts around the reported lines
sed -n '1,90p' src/publicID.ts
# Find all call sites / exports of generatePublicID usage
rg -n 'generatePublicID' srcRepository: SyedMuzamilM/payload-cloudinary
Length of output: 5061
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' src/getClientUploadRoute.ts
sed -n '1,220p' src/handleUpload.ts
# Also locate how folderPath is formed in the storage options/config
rg -n "folderPath" src -S
rg -n "publicIDOptions|publicID" src/handleUpload.ts src/getClientUploadRoute.ts src -SRepository: SyedMuzamilM/payload-cloudinary
Length of output: 13635
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any documentation/examples referencing generatePublicID
rg -n "generatePublicID" -S . \
|| true
# If repo has README/docs, search there too
ls -la
fd -a -t f 'README*|docs*|*.md' . | head
# Show any block where PublicIDOptions/custom generator is described
rg -n "PublicIDOptions|publicID|prefix\\?:|folder\\?:|`@param` prefix|`@param` folder" src -SRepository: SyedMuzamilM/payload-cloudinary
Length of output: 2637
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show README example around the generatePublicID signature
sed -n '120,190p' README.md
# Show README table row for publicID.generatePublicID
sed -n '520,580p' README.mdRepository: SyedMuzamilM/payload-cloudinary
Length of output: 4731
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the exact section describing generatePublicID callback params
rg -n "generatePublicID:|publicID\\.generatePublicID|prefix, folder|\\(filename, prefix, folder\\)" README.md
# Grab more context around the snippet we saw
sed -n '130,180p' README.md
# Also check if README defines "folder" vs "prefix" elsewhere
rg -n "docPrefix|useCompositePrefixes|prefix.*folder" README.mdRepository: SyedMuzamilM/payload-cloudinary
Length of output: 1950
Fix generatePublicID callback argument semantics (prefix vs folder).
PublicIDOptions.generatePublicID is documented/used as (filename, prefix, folder) where folder is the base folder and prefix is the (doc/file) prefix, but src/publicID.ts calls it as generatePublicID(filename, path.dirname(folderPath), path.basename(folderPath)) (so for folderPath = <baseFolder>/<prefixSegment>, the callback receives prefix=<baseFolder> and folder=<prefixSegment>).
Swap the 2nd/3rd arguments (or adjust PublicIDOptions + README to match the current behavior).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/publicID.ts` around lines 31 - 36, The generatePublicID call is passing
folder and prefix reversed; update the call site so
PublicIDOptions.generatePublicID(filename, prefix, folder) is called with prefix
= path.dirname(folderPath) and folder = path.basename(folderPath) (i.e., swap
the 2nd and 3rd arguments), or alternatively change the PublicIDOptions
signature/README to match the current order—but the preferred fix is to swap the
arguments where generatePublicID is invoked so its parameters align with the
documented (filename, prefix, folder) semantics.
1 participant
[2.4.0-beta.1] - 2025-06-09
Added
clientUploads: truein the plugin options.NEXT_PUBLIC_CLOUDINARY_API_KEYenvironment variable is no longer required — the API key is securely passed from the server handler.useCompositePrefixesfor flexible document paths.afterChangehook that persists Cloudinary metadata (public_id, secure_url, format, dimensions, etc.) for client-uploaded files. This fixes the core issue whereplugin-cloud-storageskipshandleUploadfor client uploads.generatePublicIDandsanitizeForPublicIDinto a sharedpublicID.tsmodule so both server-side and client-side uploads produce identical public IDs.Changed
getClientUploadRoute.ts): Now generates properpublic_idusing the same logic ashandleUpload, signs all upload parameters (not just timestamp), and returns the API key and resource-type-specific upload URL.CloudinaryClientUploadHandler.ts): Uses server-provided signed params and API key instead of manually constructing FormData with environment variables.initClientUploads: Only initializes client upload providers whenclientUploadsis truthy and collections are configured, with the correctenabledflag.collectionshandling: Plugin no longer crashes ifcollectionsis omitted from config — defaults to an empty object../clientmodule inpackage.jsonso theCloudinaryClientUploadHandleris correctly resolved by the Payload Admin UI.handleUploadnow imports from sharedpublicID.tsmodule.Fixed
public_idinconsistency between client and server upload paths.collectionsproperty.createServerHandler,generateClientUploadSignature) from a previous approach.Removed
NEXT_PUBLIC_CLOUDINARY_API_KEYdependency: The client handler no longer reads fromprocess.env. API key is provided by the server handler response.Summary by CodeRabbit
New Features
clientUploadsanduseCompositePrefixesconfiguration options.Changed
NEXT_PUBLIC_CLOUDINARY_API_KEYenvironment variable.Documentation