symfony-cmf / media-bundle

UNMAINTAINED - Minimalistic interfaces to handle media in the context of the CMF
http://cmf.symfony.com/
30 stars 40 forks source link

File upload - Form type for 'content' property instead of entire document #136

Closed eiannone closed 9 years ago

eiannone commented 9 years ago

I noticed that MediaBundle defines only a form type for image documents (called 'cmf_media_image'), and doesn't have any basic type for generic binary files (like for example a 'cmf_media_file', or a 'cmf_media_binary'). This was discussed also in https://github.com/symfony-cmf/MediaBundle/issues/99.

Regarding the definition of this new form type for generic file documents, I was wondering if it would make sense to link it to the 'content' property of the ODM Document, rather than to the entire document itself. I.e. instead of using '...MediaBundle\Doctrine\Phpcr\File' as data_class, use 'Doctrine\ODM\PHPCR\Document\Resource'. This because a document class extending Phpcr\File could have other properties mapped to different form fields, such as author, description and dates, some of which can be required, and other no, and we want to control them individually, while the file upload part should only be relevant to the binary data. What do you think about?

dbu commented 9 years ago

for creation, we will need a form type that reflects File. that said, in terms of the forms architecture, i think your idea makes a lot of sense. we should reflect that Resource is a child of File and also separate the form types and use the ResourceFormType in the FileFormType.

eiannone commented 9 years ago

OK, so I'll try to implement this ResourceFormType, calling it 'cmf_media_resource', as a child of 'file' form type. We need also a ModelToResourceTransformer, similar to ModelToFileTransformer, and a FileUploadHandler, similar to UploadFileHelperDoctrine. But as in this case we are dealing only with the uploaded binary data, simply converting it to a Resource object, we don't need neither Doctrine and MediaManager dependency nor EditorHelper stuff (I suppose), so maybe I could put the upload/transformation logic in the data transformer, without the need of a separate FileUploadHandler. I'm new to MediaBundle, so I am not sure if this is correct. I will test it to see if it works.

dbu commented 9 years ago

i am afraid the people who originally contributed this bundle are not very active anymore. so i can't tell you exactly. try to see what makes sense. don't hesitate to do a WIP pull request that just shows the idea and has failing tests or incomplete code in it (put WIP into the title so its obvious) and we can discuss how it can be done. also, i am no super-duper form expert, just not to get your expectations too high...

eiannone commented 9 years ago

I think I was wrong about mapping File document properties to a form. After looking at the code more carefully, I realized that all those properties of the File object should be considered as file metadata and not supposed to be editable in a web form. From what I understand, their values may be extracted from file binary data, for example by the UploadFileHelper during upload, by the MediaManager after upload, or by an EventSubscriber (see DoctrineImageDimensionsSubscriber for example) which hooks prePersist event for the file document and set properties just before it is persisted. If we need additional editable fields/properties related to the file, we should create a separate "content" document, of which the file document will become a child, like it was done in the MediaBundle WebTest examples for the image document.

So I reverted back to the idea of creating a cmf_media_file form type linked to the entire File document and submitted a PR: https://github.com/symfony-cmf/MediaBundle/pull/137 So as far as I'm concerned this issue can be closed.

dbu commented 9 years ago

see #137