Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve training + uploads images for training, trainer and place #29

Merged
merged 10 commits into from
Dec 27, 2018

Conversation

TomasVotruba
Copy link

@TomasVotruba TomasVotruba commented Dec 26, 2018

@TomasVotruba TomasVotruba changed the title improve training improve training + uploads images fro training, trainer and place Dec 26, 2018
@TomasVotruba TomasVotruba changed the title improve training + uploads images fro training, trainer and place improve training + uploads images for training, trainer and place Dec 26, 2018
*/
class Trainer
{
use UploadableImageTrait;

/**
* @ORM\Id()
* @ORM\GeneratedValue()
Copy link
Author

@TomasVotruba TomasVotruba Dec 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alterphp Hi, I tried to integrate VichUploaderBundle to EAB. It was pain to use and configure manually for each grid and form...

So I've added trait + ConfigPass drop-in support for uploadable images.

All we need to do is active the config:

services:

    OpenTraining\EasyAdmin\ConfigPass\ImagePropertyAutoconfigureConfigPass:
        tags:
            - { name: 'easyadmin.config_pass', priority: 2 }

And decorate entity with trait and annotation like this line comment.


How useful this could be in EasyAdminExtension?

Copy link

@alterphp alterphp Dec 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @TomasVotruba !

To me it's too specific : how is it useful when you need many files on the same entity (ex: logo, banner, backgroundImage, etc) ?

As an alternative, here is how I use Vich : an array field to store and persist metadata and a File property to hold the file itself when loaded. It takes 2 properties for each file but I keep more metadata :

    // Entity

    /**
     * @ORM\Column(type="json_array", nullable=true)
     *
     * @var array
     */
    private $banner;

    /**
     * NOTE: This is not a mapped field of entity metadata, just a simple property.
     *
     * @Vich\UploadableField(mapping="anim_banner", fileNameProperty="banner[name]", size="banner[size]", mimeType="banner[mimeType]", originalName="banner[originalName]", dimensions="banner[dimensions]")
     *
     * @var File
     */
    private $bannerFile;

In easy_admin config, reference propertyFile field.

Copy link
Author

@TomasVotruba TomasVotruba Dec 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's related to single-image use only. For more complex needs, custom code is needed. The EasyAdmin way :)

But for single-image, current setup is overkill:

  • define properties in entites
  • define setters/getters in entities
  • activate random property override so the image is loaded - this is big WTF that took me 30 minutes to make work
  • configure image for grids - all of them! → figure out what are their names
  • configure imageFile for form - all of them! → -||-
  • use explicit fields, because exclude_fields cannot be used - so the whole exclude_fields idea is killed everywhere with image

Copy link
Author

@TomasVotruba TomasVotruba Dec 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This setup is needed without this ConfigPass. So much unneccesary complexity it hurts my 👀

+parameters:
+    training_grid_fields:
+        - 'name'
+        - { property: 'image', type: 'image', base_path: '%training_image_path%' }
+        - 'perex'
+        - 'description'
+        - 'capacity'
+        - 'price'
+        - 'place'
+        - 'trainer'

easy_admin:
    entities:
        Training:
            label: 'Školení'
            class: 'OpenTraining\Training\Entity\Training'
            show: 
-               exclude_fields:
-                   - terms
+               fields: '%training_grid_fields%'
            list: 
-               exclude_fields:
-                   - terms
+               fields: '%training_grid_fields%'

Copy link

@alterphp alterphp Dec 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but ... doing this way is like moving complexity to an extension bundle, by pushing it a new feature that increases its dependency map (EasyAdminExtensionBundle would now depend on VichUploaderBundle too).

To me, uploadedAt should be a metadata directly handled (as a \DateTimeImmutable) by VichUploaderBundle itself, as well as size, mimeType and dimensions. So that you don't need need a 3rd property, 2nd persisted one, to deal with an image...

You put every metadata in a Doctrine json_array column, and then you take advantage of a ConfigPass in the extension bundle. It usable by anybody, even those who needs metadata, not only the raw basic implementation.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No troubles, I'll keep it here

TomasVotruba added a commit that referenced this pull request Dec 28, 2018
TomasVotruba added a commit that referenced this pull request Dec 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants