Skip to content

Implement Uppy uploader metadata editing and event handling#2288

Open
corovcam wants to merge 4 commits intoinveniosoftware:masterfrom
oarepo:oarepo-contribution-uppy-uploader-file-metadata-edit
Open

Implement Uppy uploader metadata editing and event handling#2288
corovcam wants to merge 4 commits intoinveniosoftware:masterfrom
oarepo:oarepo-contribution-uppy-uploader-file-metadata-edit

Conversation

@corovcam
Copy link
Copy Markdown

Replaced: #2278


❤️ Thank you for your contribution!

Description

Implemented modular metadata editing support in the Uppy uploader and refactored event handling for better extensibility and maintainability.

Main changes:

  • Extracted metadata-related logic into reusable utilities.
  • Extracted event-related logic into a separate module.
  • Introduced a centralized Uppy events enum and replaced hardcoded event strings.
  • Made Dashboard metadata fields optional and extensible so integrators can provide custom renderer behavior via allowed metadata field configuration.
  • Improved metadata default value processing before upload.
  • Updated uploader prop type definitions to correctly document metadata field configuration and event-related props.
  • Added JSDoc documentation for metadata and event utilities.

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

Frontend

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

  1. You license your contribution under the same terms as the current repository’s license.
  2. You agree that you have the right to license your contribution under the current repository’s license.


Object.keys(files).forEach((fileID) => {
const file = files[fileID];
file.meta["metadata"] = file.meta?.metadata || {};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

direct mutation of file.meta - should be immutable

Copy link
Copy Markdown
Author

@corovcam corovcam Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, originally file.meta is used by Uppy to update values from UI fields in metadata editor, but we also use file.meta to add additional info during multipart handling logic (e.g. parts). To distinguish between the Uppy and Multipart logic, I rather copied the Uppy fields into file.meta.metadata, and changed a bit Multipart uppy state updates to not override it.

mapDispatchToProps
)(UppyUploaderComponent);

export { UPPY_EVENTS } from "./events";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only used internally, is there a reason to export for other parts of UI?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make available events a public enum. Not just a string literal. But it's optional ofc.

onBeforeUpload: (files) => {
return activeAllowedMetaFields && activeAllowedMetaFields.length > 0
? onBeforeUploadProcessMetaFields(files, activeAllowedMetaFields)
: files;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shadows variable from outer scope

@mirekys
Copy link
Copy Markdown
Contributor

mirekys commented Apr 1, 2026

This would require also a PR to InvenioRDM docs. Explaining, how instance admins could add certain metadata fields to certain file type: https://github.com/inveniosoftware/docs-invenio-rdm/blob/2a8d853626d756717e692c035b80ea24483143f2/docs/operate/customize/file-uploads/uploader.md?plain=1

@mirekys
Copy link
Copy Markdown
Contributor

mirekys commented Apr 2, 2026

This would require also a PR to InvenioRDM docs. Explaining, how instance admins could add certain metadata fields to certain file type: https://github.com/inveniosoftware/docs-invenio-rdm/blob/2a8d853626d756717e692c035b80ea24483143f2/docs/operate/customize/file-uploads/uploader.md?plain=1

Currently it is unclear how metadata fields could be passed to deposit form config, If, say, I want to annotate all .txt files in my RDM instance to have a metadata.description field of type string, how do i do it?

* @property {function(Object, Function): Object} [metaFields[].render] - Optional custom render function for advanced UI rendering of the field using Preact `h` function. If not provided, a standard text input will be rendered when `name` is set. See {@link https://uppy.io/docs/dashboard/#metafields|Uppy Dashboard metaFields documentation}.
* @property {function(Object): Boolean} [metaFields[].condition] - Optional function to conditionally attach or render the field based on the respective file properties (e.g. file.type).
*/
export const defaultAllowedMetaFields = [
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented out example doesn't work, remove or fix (input is not writeable):

Screen.Recording.2026-04-02.at.10.35.29.mov

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not work, because of inveniosoftware/invenio-app-rdm#3386. Both PRs need to be merged to make Uppy components display correctly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, thanks! :)

* @param {Object} uppyFiles - The Uppy files object dictionary.
* @param {Object} invenioFiles - The files object from Invenio.
* @param {Array<Object>} allowedMetaFields - Configuration for metadata fields.
* @returns {Object} Updated files dictionary.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type inconsistency, the function returns updatedFiles but the docstring doesn’t fully explain when/why it’s needed. Could potentially return an empty object if there are no uppyFiles.

* - The default implementation calls out to Companion’s S3 signing endpoints.
*/
async completeMultipartUpload(file) {
if (file.meta?.metadata && Object.keys(file.meta.metadata).length > 0 && this.opts.updateFileMetadata) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If metadata update fails, the whole upload will fail without calling finalize, will it be properly cleaned up? I think finalize will handle cleanup properly if it fails

uppy.setOptions({
onBeforeUpload: (uppyFiles) => {
return activeAllowedMetaFields && activeAllowedMetaFields.length > 0
? onBeforeUploadProcessMetaFields(uppyFiles, files, activeAllowedMetaFields)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Files from closure not in deps, could get stale

@corovcam
Copy link
Copy Markdown
Author

Added docs for this: inveniosoftware/docs-invenio-rdm#926

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants