Problem/Motivation

The module fails uploading files when there are no custom fields set in the module config.

Steps to reproduce

Leave the "custom fields" at /admin/config/media/file-upload-options empty. Try to upload a file (through a media upload popup for example)

Proposed resolution

Add empty checks, adding a patch in a minute.

CommentFileSizeAuthor
#10 interdiff_3171579_6-10.txt3.07 KBankithashetty
#10 3171579-10.patch1.1 KBankithashetty
#6 3171579-6.patch3.95 KBankithashetty
#2 custom_fields_empty_check-3171579-2.patch3.11 KBmatthiasm11

Comments

matthiasm11 created an issue. See original summary.

matthiasm11’s picture

matthiasm11 commented
Assigned: matthiasm11 » Unassigned
Status: Active » Needs review
StatusFileSize
new custom_fields_empty_check-3171579-2.patch3.11 KB

The attached patch adds some empty checks to fix this issue.

imclean’s picture

imclean commented
Status: Needs review » Needs work

Thanks. I'll try to reproduce the issue.

There are some unrelated changes in the patch. Please leave the variables as camel case.

matthiasm11’s picture

matthiasm11 commented
Status: Needs work » Needs review

Hi, thank you for your response!
I'm following the coding standards, I've only altered the variables that are being used in the empty checks needed to fix this issue. There are (a lot of) other violations to the coding standards which I did not fix, because they are indeed unrelated to the patch. A new ticket should be created to fix those. Again, thank you for your quick response!

imclean’s picture

imclean commented
Status: Needs review » Needs work

Coding standards fixes are unrelated to what this patch is trying to achieve. Please start a separate issue for those.

This makes it easier to review the functional changes of the patch.

ankithashetty’s picture

ankithashetty
Location Karnataka, India
commented
Status: Needs work » Needs review
StatusFileSize
new 3171579-6.patch3.95 KB

Updated the patch in #2 with the changes specified by @imclean in #3...

Certain lines in the modified file (i.e., file_upload_options.module ) had additional spaces at the end, which got auto-corrected on saving. It's included in the patch as well... To help with the patch review, the functional changes made by @matthiasm11 is in the lines between 180 to 190...

Hope this helps in moving the issue forward, Thanks!

imclean’s picture

imclean commented
Status: Needs review » Needs work

Thanks @ankithashetty.

Certain lines in the modified file (i.e., file_upload_options.module ) had additional spaces at the end, which got auto-corrected on saving. It's included in the patch as well..

I know it's annoying, but please roll these into the coding standards patch. I'd really like to keep the commits clean.

ankithashetty’s picture

ankithashetty
Location Karnataka, India
commented
Assigned: Unassigned » ankithashetty
imclean’s picture

imclean commented 
+++ b/file_upload_options.module
@@ -177,11 +177,17 @@ function file_upload_options_file_managed_file_save_upload(array $element, FormS
+    $replaceOption = NULL;

This should probably be FileSystemInterface::EXISTS_RENAME to keep the current behaviour.

ankithashetty’s picture

ankithashetty
Location Karnataka, India
commented
Assigned: ankithashetty » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3171579-10.patch1.1 KB
new interdiff_3171579_6-10.txt3.07 KB

Updated the above patch and made the coding standard changes in a different issue #3172121: Fix Drupal Coding Standard Errors as requested, kindly review...

Thank you!

imclean’s picture

imclean commented

Check out Dreditor to help you see unrelated changes such as whitespace.

imclean’s picture

imclean commented

Sorry I haven't had a chance to reproduce the issue yet.

  • imclean committed b4e101a on 8.x-1.x authored by ankithashetty 
    Issue #3171579 by ankithashetty, matthiasm11: Custom fields empty check
imclean’s picture

imclean commented
Status: Needs review » Fixed

Thank you. This is a fairly straight forward fix. Sorry it's taken so long.

Status: Fixed » Closed (fixed)

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