solspace / craft-freeform

Freeform for Craft: The most reliable form builder that's ready for wherever your project takes you.
https://docs.solspace.com/craft/freeform/v5/
Other
47 stars 59 forks source link

Freeform incorrectly calls craft\services\Assets::getFolderById() when dynamically creating a directory for file uploads #699

Closed tomdavies closed 1 year ago

tomdavies commented 1 year ago

Describe the bug or issue you're experiencing

When uploading a file as part of a form submission to a path/folder that must be dynamically created at submission time, a Craft API is incorrectly called, resulting in an error.

Steps to reproduce

  1. Create a form with an upload control, using a local folder asset volume and /userUploads/{{ form.handle }}/{{ now|date("Y-m-d H:i:s") }}/ as the Upload Location Subfolder value
  2. Submit to the form from the front end (I am using a custom template but pretty sure that doesn't make any difference - see below)
3. I get this error (click to expand): ```
Exception 'TypeError' with message 'craft\services\Assets::getFolderById(): Argument #1 ($folderId) must be of type int, craft\models\VolumeFolder given, called in /var/www/html/vendor/solspace/craft-freeform/packages/plugin/src/Services/FilesService.php on line 448' 

in /var/www/html/vendor/craftcms/cms/src/services/Assets.php:401

Stack trace:
#0 /var/www/html/vendor/solspace/craft-freeform/packages/plugin/src/Services/FilesService.php(448): craft\services\Assets->getFolderById(Object(craft\models\VolumeFolder))
#1 /var/www/html/vendor/solspace/craft-freeform/packages/plugin/src/Services/FilesService.php(83): Solspace\Freeform\Services\FilesService->getFolder(1, 'userUploads/7aa...', Object(Solspace\Freeform\Form\Types\Regular))
#2 /var/www/html/vendor/solspace/craft-freeform/packages/plugin/src/Fields/FileUploadField.php(132): Solspace\Freeform\Services\FilesService->uploadFile(Object(Solspace\Freeform\Fields\FileUploadField), Object(Solspace\Freeform\Form\Types\Regular))
#3 /var/www/html/vendor/solspace/craft-freeform/packages/plugin/src/Library/Composer/Components/Form.php(1298): Solspace\Freeform\Fields\FileUploadField->uploadFile()
#4 /var/www/html/vendor/solspace/craft-freeform/packages/plugin/src/Library/Composer/Components/Form.php(767): Solspace\Freeform\Library\Composer\Components\Form->validate()
#5 /var/www/html/vendor/solspace/craft-freeform/packages/plugin/src/Controllers/SubmitController.php(52): Solspace\Freeform\Library\Composer\Components\Form->handleRequest(Object(craft\web\Request))
#6 [internal function]: Solspace\Freeform\Controllers\SubmitController->actionIndex()
#7 /var/www/html/vendor/yiisoft/yii2/base/InlineAction.php(57): call_user_func_array(Array, Array)
#8 /var/www/html/vendor/yiisoft/yii2/base/Controller.php(178): yii\base\InlineAction->runWithParams(Array)
#9 /var/www/html/vendor/yiisoft/yii2/base/Module.php(552): yii\base\Controller->runAction('', Array)
#10 /var/www/html/vendor/craftcms/cms/src/web/Application.php(302): yii\base\Module->runAction('freeform/submit', Array)
#11 /var/www/html/vendor/craftcms/cms/src/web/Application.php(627): craft\web\Application->runAction('freeform/submit', Array)
#12 /var/www/html/vendor/craftcms/cms/src/web/Application.php(281): craft\web\Application->_processActionRequest(Object(craft\web\Request))
#13 /var/www/html/vendor/yiisoft/yii2/base/Application.php(384): craft\web\Application->handleRequest(Object(craft\web\Request))
#14 /var/www/html/web/index.php(12): yii\base\Application->run()
#15 {main}
```

Inspecting FilesService.php#L445-L449 the fix looks super easy

Freeform has:

            if (!$folder) {
                $volume = \Craft::$app->getVolumes()->getVolumeById($volumeId);
                $folderId = $assetsService->ensureFolderByFullPathAndVolume($subpath, $volume);
                $folder = $assetsService->getFolderById($folderId);
            }

As the error indicates craft\services\Assets::ensureFolderByFullPathAndVolume() returns a VolumeFolder already, so there is no need to call craft\services\Assets::getFolderById()

Looks like you just need:

            if (!$folder) {
                $volume = \Craft::$app->getVolumes()->getVolumeById($volumeId);
                $folder = $assetsService->ensureFolderByFullPathAndVolume($subpath, $volume);
            }

Craft & Plugin Info (please complete the following information):

kjmartens commented 1 year ago

Sorry for the trouble @tomdavies, and thanks for this. I will note this for our developers to review. 🙂

kjmartens commented 1 year ago

This should be resolved now in Freeform 4.1.1. 🙂