search

dustin10 / VichUploaderBundle

A simple Symfony bundle to ease file uploads with ORM entities and ODM documents.
MIT License
1.85k stars 519 forks source link

Use an array instead of an object #1361

Closed YummYume closed 1 year ago

YummYume commented 1 year ago
Q A
Bundle version 2.0.1
Symfony version 6.2.6
PHP version 8.2.2

Support Question

Hello. I am currently running into an issue that could be solved easily by just reading the documentation, but the documentation is outdated.

I want to generate a path (preferably in a twig template) to my image using an array instead of an object. This is because I am using Meilisearch and for performance reasons, I only get results as an array.

In previous versions, it seemed possible to pass in an array and the class name as a third argument, as stated in the doc. However, it was marked as deprecated in earlier versions and later removed. Is there still any support for this? Or do I absolutely need to pass in an entity?

garak commented 1 year ago

Yes, the feature has been removed and the documentation is outdated (a PR to fix it would be appreciated). However, array is still supported by the resolveUri method in Storage classes, so you can still create your twig extension and use arrays instead of objects.

YummYume commented 1 year ago

Thanks for the quick answer @garak.

So I did try with resolveUri, and although this method does accept an array or an object, the getFilename method only accepts an object... So it ends up throwing an error.

Vich\UploaderBundle\Storage\AbstractStorage::getFilename(): Argument #1 ($obj) must be of type object, array given, called in /app/vendor/vich/uploader-bundle/src/Storage/FileSystemStorage.php on line 60
garak commented 1 year ago

It seems we're wrong about that since I see that afterwards resolveUri allows arrays again. Can you propose a PR to fix it? At this point, we can restore the possibility in the Twig extension as well

YummYume commented 1 year ago

Hi @garak

I am working on it :smile: Can you tell me what tests I should include with this change?

garak commented 1 year ago

Hi @garak

I am working on it smile Can you tell me what tests I should include with this change?

Having current tests passing could be enough for now. I can add more tests later

YummYume commented 1 year ago

Hi @garak

Alright, I made the PR, please tell me if I need to change anything