Problem/Motivation

After validateForm() was implemented in Drupal\media_bulk_upload\Form\MediaBulkUploadForm for
#3291228: Add form validation to inform other module to valid the new uploaded files, bulk uploading files using the core file upload (not DropzoneJS) fails validation. This occurs because validateForm() assumes that the form state values for the file uploads are structured how the dropzoneJS element provides them. In addition, the validation implemented is unnecessary for the core element, because it already calls the file_validate hooks.

Steps to reproduce

  1. Install Drupal with standard profile
  2. Install media_bulk_upload
  3. Add a media bulk upload configuration (be sure to set Upload location)
  4. Go to /media/bulk-upload
  5. Drag files to the file upload field (or use the widget to select files)
  6. Observe that there is an No media files have been provided. error, even though uploaded images are now listed
  7. Observe that the error prevents form submit

    8. Validation error for bulk upload

Proposed resolution

The functionality in validateForm applies only to the dropzoneJS widget, so it should be moved to the media_bulk_upload_dropzonejs submodule.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#26 3355468-can_not_upload_using_core_file_upload_element-2.patch2.39 KBdeltarazero
#24 3355468-can_not_upload_using_core_file_upload_element.patch2.39 KBcharlliequadros
#19 Screenshot 2025-05-07 at 04.02.27.png139.39 KBthe_g_bomb
#11 Image 02-02-24 at 4.17 PM.jpeg334.1 KBSambit15
#4 media_bulk_upload-can-not-upload-using-core-element-3355468-06ea2f58.patch7.39 KBericgsmith
Screenshot 2023-04-20 at 12.06.08 PM.png55.85 KBgodotislate

Issue fork media_bulk_upload-3355468

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

godotislate created an issue. See original summary.

godotislate opened merge request !8

godotislate’s picture

godotislate
he/him
commented
Status: Active » Needs review

Put in an MR for a new subclass of Drupal\media_bulk_upload\Form\MediaBulkUploadForm in the media_bulk_upload_dropzonejs submodule. Moved validateForm() as well as the contents of media_bulk_upload_dropzonejs_form_media_bulk_upload_form_alter() to the new subclass, and altered the bulk upload route definition.

ericgsmith’s picture

ericgsmith commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new media_bulk_upload-can-not-upload-using-core-element-3355468-06ea2f58.patch7.39 KB

Had the same issue after upgrading to 3.0.2.

Reviewed MR as at commit 06ea2f58ab7cb441440ff8303bfa2d6b551eae81 - looks good and resolves the issue when media_bulk_upload_dropzonejs is not enabled.

When media_bulk_upload_dropzonejs is enabled the MediaBulkUploadDropzoneJsForm form is used and validates as expected.

Attaching a patch file of godotislate's MR at 06ea2f58ab7cb441440ff8303bfa2d6b551eae81 for anybody that relies on a fixed point in time patch workflow. Thanks @godotislate!

eric_a’s picture

eric_a commented

Wow, that's a lot of code to review... Is all that code strictly necessary to fix the critical regression or is it more of an improvement?
I'm wondering if we can just put back the DropzoneJS check that was removed in #3324771: Validation missing when empty bulk media upload form submitted.

eric_a’s picture

eric_a commented
Priority: Normal » Critical
Related issues: +#3324771: Validation missing when empty bulk media upload form submitted
eric_a’s picture

eric_a commented

I guess I'm too busy or too lazy to dive in right now.
But why does the early empty check in HEAD even trigger?
I'll dive in later...

godotislate’s picture

godotislate
he/him
commented

The only real new code is the route subscriber service in the media_bulk_upload_dropzonejs submodule. Otherwise, it's just moving code that was existing in the main module to the dropzonejs submodule, which really is where all things dropzonejs should go.

kushal bansal’s picture

kushal bansal commented

I faced the same issue in 3.0.2 and was able to resolve the same by enabling media_bulk_upload_dropzonejs and using https://unpkg.com/dropzone@5/dist/min/dropzone.min.js and https://unpkg.com/dropzone@5/dist/min/dropzone.min.css and placing them in libraries folder at libraries/dropzone/dist/min as new files dropzone.min.js and dropzone.min.css

Sambit15’s picture

Sambit15 commented

Patch file media_bulk_upload-can-not-upload-using-core-element-3355468-06ea2f58.patch
Attaching the Screenshot for reference image

Tested , and Verified Its working as excepted .

Sambit15’s picture

Sambit15 commented
StatusFileSize
new Image 02-02-24 at 4.17 PM.jpeg334.1 KB
tkiehne’s picture

tkiehne commented

Verified that the patch in #4 works and agree with the methodology: no dependent code should be in base module if the dependency isn't required.

the_g_bomb’s picture

the_g_bomb commented

Confirmed without dropzone this isn't working without the patch in from the MR or #4.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Assigned: Unassigned » kristen pol

Looking at creating a new release soon, so assigning to me.

godotislate’s picture

godotislate
he/him
commented

Rebased to resolve the merge conflict.

Unrelated, but came up in testing against Drupal 11.1.3. The file validators need to be updated per https://www.drupal.org/node/3363700. So this section in Drupal\media_bulk_upload\Form\MediaBulkUploadForm::buildForm():

    $validators = array(
      'file_validate_extensions' => [implode(' ', $this->allowed_extensions)],
      'file_validate_size' => [Bytes::toNumber($this->maxFileSizeForm)],
    );

needs to be

    $validators = array(
      'FileExtension' => ['extensions' => implode(' ', $this->allowed_extensions)],
      'FileSizeLimit' => ['fileLimit' => Bytes::toNumber($this->maxFileSizeForm)],
    );

Also in media_bulk_upload_dropzonejs.module on latest 3.0.x (3ab898b1), this section in media_bulk_upload_dropzonejs_form_media_bulk_upload_form_alter() needs to be changed as well:

if (isset($form['file_upload']['#upload_validators']['file_validate_extensions'][0])) {
  $form['file_upload']['#extensions'] = $form['file_upload']['#upload_validators']['file_validate_extensions'][0];
}

to

if (isset($form['file_upload']['#upload_validators']['FileExtension']['extensions'])) {
  $form['file_upload']['#extensions'] = $form['file_upload']['#upload_validators']['FileExtension']['extensions'];
}

However, the contents of media_bulk_upload_dropzonejs_form_media_bulk_upload_form_alter() are moved to the new MediaBulkUploadDropzoneJsForm class in this MR, so the change would need be done in that class for this MR. The change to the validators for D11 compatibility seems to be out of scope for this issue, so if it's done in another issue, either the work there or work here will need to be rebased, depending on whichever is merged first.

ben.hamelin’s picture

ben.hamelin
he/him
Primary language English
Location Adirondack Mountains, NY
commented

Noting that the MR diff does not apply cleanly to 3.0.2.
I had to use the patch from #4.
I could not get either approach to work (MBU form or DropzoneJS) without patch.
I also used "public://tmp" as my upload location (and created "tmp" folder in sites/default/files), and set Form Mode to "none"

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Assigned: kristen pol » Unassigned

I've been slammed with many other things, so unassigning from me.

the_g_bomb’s picture

the_g_bomb commented

Tested with Drupal 10.4 and media_bulk_upload 3.0.x. using "Form Mode" set to "none" and "Upload location" set to "public://tmp".

After applying the MR, media_bulk_upload worked both with and without the dropzonejs and media_bulk_upload_dropzonejs modules enabled.

I also tried testing with Drupal 11.1, but had to use the fix offered in #3522637: Fixed the file validate constraint plugins. I had to apply it manually due to the changed file locations. With the ['file_validate_extensions'] changed to ['FileExtension']['extensions'], the MR works on D11 as well.

I used this to apply the MR:

composer config --json --merge extra.patches.drupal/media_bulk_upload '{"Issue #3355468: Can not upload using core file upload element": "https://git.drupalcode.org/project/media_bulk_upload/-/merge_requests/8.diff"}'
composer update drupal/media_bulk_upload

Confirming previous state of RTBC after the recent code updates in MR 8

Bulk file uploader successfully showing an uploaded image

the_g_bomb’s picture

the_g_bomb commented
StatusFileSize
new Screenshot 2025-05-07 at 04.02.27.png139.39 KB
kopeboy’s picture

kopeboy
Primary language Italian
Location Milan
commented

#18 solves the issue.

For newbies: there are some requirements though before being able to use those commands correctly (specifically requiring cweagans/composer-patches, otherwise the composer update won't do anything), read more here.

eric_a’s picture

eric_a commented

Just noticed that #3522637: Fixed the file validate constraint plugins is in. (And released as 3.0.4)

The change to the validators for D11 compatibility seems to be out of scope for this issue, so if it's done in another issue, either the work there or work here will need to be rebased, depending on whichever is merged first.

godotislate’s picture

godotislate
he/him
commented
Status: Reviewed & tested by the community » Needs review

Rebased, moving back to NR just to make sure the conflict resolution is OK.

ericgsmith’s picture

ericgsmith commented
Status: Needs review » Reviewed & tested by the community

Looks good, still working.

Tested with dropzone and media_bulk_upload_dropzonejs disabled and the upload form works as expected, critical bug is resolved.

Tested with dropzone and media_bulk_upload_dropzonejs enabled and existing dropzone behaviour is still working as expected.

Maintainers - please lets get this committed. It is a critical bug that has existed for 2 years that several people have confirmed @godotislate's work fixes.

Please trust the community here and lets get this in. Thank you 🙏

charlliequadros’s picture

charlliequadros commented
StatusFileSize
new 3355468-can_not_upload_using_core_file_upload_element.patch2.39 KB

I tried to apply all the changes from the merge request, but I wasn’t able to because some had already been merged while others hadn’t. Since I’m using version 3.0.3, I created a patch that includes only the changes that actually need to be applied.

godotislate’s picture

godotislate
he/him
commented

If it's helpful for consideration in merging, just wanted to mention again (as in #8) that the diff may make the changes look more significant than they are. There's very little actual new code in the MR. Mostly things are just moved around to consolidate in one place in the submodule.

I tried to apply all the changes from the merge request, but I wasn’t able to because some had already been merged while others hadn’t.

I believe the MR diff should apply clean to 3.0.4., if you're willing to update.

deltarazero’s picture

deltarazero commented
StatusFileSize
new 3355468-can_not_upload_using_core_file_upload_element-2.patch2.39 KB

I believe the MR diff should apply clean to 3.0.4., if you're willing to update.

Unfortunately it does not, since in media_bulk_upload_dropzonejs.module the following statements are removed:

use Drupal\Core\StringTranslation\TranslatableMarkup;
use Drupal\Core\Form\FormStateInterface;

while they are still referenced by media_bulk_upload_dropzonejs_form_media_bulk_upload_form_alter().

I've attached an updated patch with these statements intact.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

@kristen pol sorry for the ping, but indeed the priority of the issue shows that this issue is highly relevant and the RTBC is from june - so maybe it would be possible to merge this and tag a new commit?

That might also be a good starting point for #3310813: Make Dropzone fully optional then?

jannakha’s picture

jannakha commented

+1 for release

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

@jannakha thanks, I already tried to ping the maintainers, see my comment above for example. Maybe you could try to send them a contact message?

jannakha’s picture

jannakha commented
Status: Reviewed & tested by the community » Fixed

Thank you for your contributions!
Merged.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.