Right now, the DropzoneJS upload handler is using absolute paths to deal with the uploaded files, which is a bit off-the-reservation. It'd be a lot easier and more in line with the Drupal way to use stream wrappers.

Pull request filed at https://github.com/drupal-media/dropzonejs/pull/22

Comments

phenaproxima created an issue. See original summary.

primsi’s picture

Status: Needs review » Needs work
Issue tags: +D8Media

This would need a re-roll, than we can continue.

chr.fritsch’s picture

Issue tags: +dcmuc16
joehoppe’s picture

worked together with michaellenahan and Thomas Schuh at dcmuc16.

We did a reroll and the update hook. Here is the pull request: https://github.com/drupal-media/dropzonejs/pull/35
Unfortunately the travis test failed so we gonna fix that test Drupal\Tests\dropzonejs\Kernel\DropzoneJsUploadControllerTest::testDropzoneJsUploadController

joehoppe’s picture

StatusFileSize
new4.41 KB

this is the failing patch

joehoppe’s picture

We found the reason for the test failing. We need to do the installConfig see some examples here: https://api.drupal.org/api/drupal/core%21tests%21Drupal%21KernelTests%21...
It seems that the KernelTest needs to be told about the configuration.

The test pass in php but ont in hhvm. See https://travis-ci.org/drupal-media/dropzonejs/builds/180464022

michaellenahan’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: 2724149-5.patch, failed testing.

The last submitted patch, 5: 2724149-5.patch, failed testing.

joehoppe’s picture

Status: Needs work » Needs review
StatusFileSize
new5.2 KB

updated patch with working test file

slasher13’s picture

Status: Needs review » Reviewed & tested by the community
chr.fritsch’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new5.34 KB

This patch needed a re-roll

primsi’s picture

Status: Needs review » Needs work

Just two things:

I found one other instance of tmp_dir:
src/Element/DropzoneJs.php:139: $tmp_override = \Drupal::config('dropzonejs.settings')->get('tmp_dir');

+++ b/config/install/dropzonejs.settings.yml
@@ -1 +1 @@
+upload_scheme: temporary

I am not sure about naming this upload_scheme, from that we do not actually know that we are configuring the temporary upload location. Maybe tmp_upload_scheme?

chr.fritsch’s picture

Status: Needs work » Needs review
StatusFileSize
new6.85 KB
new3.35 KB

Addressed the comments from #13

  • chr.fritsch committed 449af84 on 8.x-1.x
    Issue #2724149 by chr.fritsch, joehoppe, phenaproxima: The upload...
chr.fritsch’s picture

Status: Needs review » Fixed

Committed and pushed.

Thanks everyone.

chr.fritsch’s picture

Status: Fixed » Closed (fixed)

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