verbb / vizy

A flexible visual editor for Craft CMS
Other
44 stars 8 forks source link

Assets in Vizy block with dynamic path are uploaded to temp folder #80

Closed ThinkGraphical closed 2 years ago

ThinkGraphical commented 2 years ago

Description

Configured an asset field with dynamic path {owner.slug}, added it to a Vizy block. When I attach assets, the assets are being saved in temp folder.

Noticed when i manually create the subfolder (using the slug of the entry) and then upload the files, the files are placed in the correct folder.

So the configuration seem to be correct, but looks to me like creating the subfolder is failing.

Steps to reproduce

  1. Create asset field with dynamic path {owner.slug}
  2. Incorporate into Vizy block
  3. Attach assets

Additional info

Additional context

anita-chouhan commented 2 years ago

We’re also facing the same problem when we have an Asset field in a Vizy block that is inside a Matrix block. If we try to upload an Asset file while adding/editing an entry after adding the Matrix block to a section’s entry type, it remains in the Temporary Uploads folder instead of transferring to the correct location.

I tested it on a regular field without Matrix as well by adding the Asset field to a Vizy block and adding it directly to a section’s entry type. The problem still persists.

I tried the dynamic path without owner as well by setting Default Asset Location as {now|date('Y')}. That didn't work either.

Steps to reproduce

  1. Create an Asset field with the Default Asset Location set to  {owner.owner.slug}.
  2. Create a Matrix field, and add a block.
  3. Add the Vizy field to the Matrix block.
  4. Add the Asset field from step 1 to the Vizy field’s block configuration.
  5. Add the Matrix field created in step 2, to a section’s entry type field layout.
  6. Try uploading a file to this asset field.
  7. Save the Entry.
  8. Check the uploaded asset file’s location.

Expected Result

Uploaded asset file should be inside a folder created using the Entry’s slug as name.

Actual Result

Uploaded asset file remains inside Temporary Uploads.

Additional info

anita-chouhan commented 2 years ago

Update

I performed some tests and found that the issue does not exist when using Craft CMS version 3.7.13. I think that changes made in later versions of Craft CMS (or may be Vizy) had an impact on the dynamic path construction for Assets.

brandonkelly commented 2 years ago

This is fixed via https://github.com/craftcms/cms/commit/e7ac41bc5e7a96020d0946e5cf69bb2c38587970.

The real issue is that Vizy blocks don’t have IDs though, and it looks like they are going through some hoops to work with field types despite not being “real” elements (such as calling afterElementSave() at inappropriate times, when the block element doesn’t have an ID so it hasn’t actually been saved).

engram-design commented 2 years ago

Thanks @brandonkelly I was diving into this yesterday and was about to chat to you about it further. Thanks for the fix!

I'll appreciate the Vizy is doing some odd things to get around elements, so maybe I'm having things a bit more difficult for everyone 😅

anita-chouhan commented 2 years ago

Update

After upgrading the Craft CMS to version 3.7.45 as suggested by @brandonkelly the issue was resolved. But I found that when we upload an asset for the first time into the Asset field (inside Vizy block in Matrix field) and the Default Asset Location has a dynamic path value, an error message stating "Asset can’t be blank." appears.

After another try, everything is fine. See the image below:

error
anita-chouhan commented 2 years ago

@engram-design Hi Josh, Any updates on the issue?

engram-design commented 2 years ago

@anita-chouhan Have you tried with the latest 1.0.16 release? There were a few things in that release that could have potentially fixed this one.

Otherwise, if you wouldn't mind sending through your field setup (maybe using Field Manager to export the Matrix field) I can try to replicate it on my end. I can't seem to replicate the issue with my example fields and on the latest version

anita-chouhan commented 2 years ago

@engram-design Hi Josh, after upgrading the plugin to the latest 1.0.16 the issue was resolved. Thank you for the update.

kennyheard commented 2 years ago

@engram-design Has this issue also been resolved in the Craft 4 version (2.x.x) of the plugin? I'm experiencing the exact same problem on version 2.0.5.

engram-design commented 2 years ago

@kennyheard It should be incorporated into 2.0.5, yes, but the main fix was in Craft itself via https://github.com/craftcms/cms/commit/e7ac41bc5e7a96020d0946e5cf69bb2c38587970

JshGrn commented 2 years ago

@engram-design Has this issue also been resolved in the Craft 4 version (2.x.x) of the plugin? I'm experiencing the exact same problem on version 2.0.5.

I am also on latest versions of Craft 4 and this plugin.

If I upload an Image to an asset field they are uploaded to storage/runtime/assets/tempuploads/user_1/car.svg, the expected upload location ( that works outside of Vizy ) is storage/public/popup-dialogs/{owner.slug}/icons/car.svg.

I am not sure why user_1 is the {owner.slug} field as this is unexpected unless I am doing something wrong here.

Looking at the network tab nothing is coming back as an error, it returns an asset ID which would indicate everything went smoothly. Clicking the file returns 'Missing filesystem handle' which I presume because it was never moved from temp?

@engram-design Would you be able to assist on this and I will continue building out my functionality without this for now in hope that it will be resolved?

engram-design commented 2 years ago

@JshGrn Fixed for the next release. To get this fix early, change your verbb/vizy requirement in composer.json to:

"require": {
  "verbb/vizy": "dev-craft-4 as 2.0.6",
  "...": "..."
}

Then run composer update.

engram-design commented 2 years ago

Fixed in 2.0.7

JshGrn commented 1 year ago

I am getting this issue again within Craft 4.4.1 and Vizy 2.0.12

brandonkelly commented 1 year ago

(Worked this out with @JshGrn and fixed via craftcms/cms@ac5c4bbed4ccbb6d599d298bb1d139286abd2f35.)

engram-design commented 1 year ago

Nice one @brandonkelly !