[WIP] sonata media example #159

Closed
wants to merge 3 commits into
from

6 participants

@rmsint

This PR adds an example implementation for the SonataMediaBundle.

Related to:

Questions:

  • No frontend example is added, what do we add?
    • a banner with sponsor logo's could be added or a slide show with a short intro etc.
    • will there also be an example created for the SlideshowBlock? Then the examples should be in sync, maybe devided or each example has several implementations.
    • for media fixtures see the object sharing issue mentioned underneath
  • "web/media/cache" dir for LiipImagineBundle is not automatically created + permissions have to be set for "web/uploads", document that this has to be done? see also "Setting up Permissions"

The following subjects / issues I could not solve:

  • fix bug with annotations - PrePersist/PreUpdate not being called, see: here and here
  • jackrabbit (works with dbal) ReferenceOne to media: "ConstraintViolationException: HTTP 409: Unable to perform operation. Node is protected. ", see: here and here - running jackrabbit 2.6.0 seems to solve this
  • doctrine configuration referenceable="true" is not detected on mapped-superclass, has to be defined on the document extending the mapper superclass
  • Doctrine PHPCR DataFixtures cannot share objects between fixtures, as described here
  • Implement doctrine integration for PHPCR in JMSDIExtra to be able to inject base paths in Repository classes, see: here , here and here
@dbu dbu commented on the diff Feb 18, 2013
.gitignore
@@ -1,5 +1,7 @@
composer.phar
web/bundles/
+web/uploads/
@dbu
Symfony CMF member
dbu added a line comment Feb 18, 2013

what is this directory? file uploads should not go into the web/ folder, that is very dangerous for injection attacks. (if i manage to upload a php file, i can call it in a browser and pretty much own the server...) or am i misinterpreting the name?

@rmsint
rmsint added a line comment Feb 18, 2013

it is more or less the default dir for uploading images that is configured as cdn path for the media bundle. See also fe. here and here. Using configuration the allowed file types can be limited, see here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dbu dbu and 1 other commented on an outdated diff Feb 18, 2013
composer.json
},
+ "repositories": [
@dbu
Symfony CMF member
dbu added a line comment Feb 18, 2013

this is temporary until all your PR are merged, right?

@rmsint
rmsint added a line comment Feb 18, 2013

yes, so it can be quickly installed to see it in action. When it is no more "WIP" it can be removed indeed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dbu dbu commented on the diff Feb 18, 2013
src/Sandbox/BannerBundle/Admin/BannerImageAdmin.php
@@ -0,0 +1,53 @@
+<?php
+
+namespace Sandbox\BannerBundle\Admin;
+
+use Sonata\DoctrinePHPCRAdminBundle\Admin\Admin;
+use Sonata\AdminBundle\Route\RouteCollection;
+use Sonata\AdminBundle\Form\FormMapper;
+use Sonata\AdminBundle\Datagrid\ListMapper;
+
+class BannerImageAdmin extends Admin
+{
+// protected $baseRouteName = 'sandbox_banner_image_admin';
@dbu
Symfony CMF member
dbu added a line comment Feb 18, 2013

before merging we have to clean up commented out code. either its irrelevant and can be deleted, or it should be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dbu dbu commented on the diff Feb 18, 2013
src/Sandbox/BannerBundle/Document/BannerImage.php
@@ -0,0 +1,267 @@
+<?php
+
+namespace Sandbox\BannerBundle\Document;
+
+use Doctrine\ODM\PHPCR\Mapping\Annotations as PHPCRODM;
+use Sandbox\BannerBundle\Document\BannerBlockInterface;
+use Sandbox\MediaBundle\Document\Media;
+
+/**
+ * Document that contains a Media and additional banner information
+ *
+ * @PHPCRODM\Document(referenceable=true)
+ */
+class BannerImage
@dbu
Symfony CMF member
dbu added a line comment Feb 18, 2013

we should integrate this with the Slideshow blocks symfony-cmf/block-bundle#32

btw, you do not implement the interface you declare.

@rmsint
rmsint added a line comment Feb 18, 2013

The BannerBlockInterface is implemented by the ImageBlock, it is used here to only allow blocks of that type to be added as parent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dbu dbu commented on the diff Feb 18, 2013
src/Sandbox/BannerBundle/Document/BannerImage.php
+ public function getPosition()
+ {
+ return $this->position;
+
+// $siblings = $this->getParentDocument()->getItems();
+//
+// return array_search($siblings->indexOf($this), $siblings->getKeys());
+ }
+
+ /**
+ * Set createdAt
+ *
+ * @PHPCRODM\PrePersist()
+ * @param \Datetime $createdAt
+ */
+ public function setCreatedAt(\DateTime $createdAt = null)
@dbu
Symfony CMF member
dbu added a line comment Feb 18, 2013

we should try to use mix:created and mix:lastModified for this instead of custom code

@rmsint
rmsint added a line comment Feb 18, 2013

ok, did not know. Can you point me in the right direction to find more information?

@dbu
Symfony CMF member
dbu added a line comment Feb 24, 2013

hm, this seems to not be supported yet by phpcr-odm. i created http://www.doctrine-project.org/jira/browse/PHPCR-99

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dbu
Symfony CMF member

cool. yep i think slideshow would be a good example to demo a couple of things on blocks and media. /cc @elHornair
is what you call Banner about the same as a slideshow? there is symfony-cmf/block-bundle#32 about slideshows

regarding LiipImagineBundle and the cache directory, if the doc does not say i am sure @lsmith77 knows.

bugs:

  • annotations: that surprises me, i thought it would work. are you sure its not called, or could it be that its too late to do the things you want at that point? because the changesets are already determined...
  • referenceable should be inherited, see http://www.doctrine-project.org/jira/browse/PHPCR-77 - are you using the latest version of phpcr-odm?
  • sharing fixtures: i guess nobody tried that before so there might be bugs. can you track this down and open an issue at the right place?
  • injecting base paths: can't we do that in the DependencyInjection Extension class as well? having it in annotations is less flexible if people want to use multiple instances of a repository for example...
@rande

for the record, the SonataMediaBundle provides some similar blocks: https://github.com/sonata-project/SonataMediaBundle/blob/master/Block/

@dbu
Symfony CMF member

oh ups. @elHornair @rmsint how can we best integrate the different approaches here?

the slideshow block (container block) is not limited to specific children type, right? so that one is usable regardless of how we do slideshow items. should the item base on the sonata media block? guess if somebody needs a non sonata based media block he needs to do custom things anyways, right?

@rmsint, do you think we can reduce the amount of code needed in the sandbox and move things to the BlockBundle?

@rmsint
  • for the different approaches of the slideshow block and this example I think the end result is more or less the same only the implementation is different. Depending on the situation one of the implementations can be more appropriate. I think it is nice to showcase them both to see this. Also I think this example showcases the possibilities of the sonata admin.
  • the implementation of the ImageBlock is an example to do only slides with images, but it is also possible to combine all media types in a gallery.
  • @rande would be nice indeed to use the already available blocks as example! Is there an example implementation of them? And we probably need to make specific documents for the blocks first, isn't it?
  • To reduce code, we can give it a try. Much of the code is dedicated to sonata at the moment. Let's see what the 3rd party integration page says ... :-)

bugs:

  • annotations: I checked it and noticed it was not called. The gallery of the sonata media bundle is using the same code and the metadata configuration is loaded using XML, then the PrePersist/PreUpdate method is called.
  • referenceable should be inherited: the code is pointing here, maybe it is not fixed for XML?
  • sharing fixtures: if I remember correctly it throws an exception in the DocumentManager or UnitOfWork, will open a ticket then on phpcr-odm
  • injecting base paths: I don't know how to do this with Doctrine Repositories, can you maybe point me in the right direction?
@dbu
Symfony CMF member

i am a bit unhappy about the size of code we introduce here with the BannerBundle. I agree it would be cool to have a block for gallery items of sonata media. can you do a MediaBlock for the BlockBundle that would do this? how does Sonata Media think, is everything a generic "Media" item or are there different classes for different things? btw, we created a basic "image" document in phpcr-odm doctrine/phpcr-odm#250 that might be reused if possible. and we are working at creating an image form element doctrine/DoctrinePHPCRBundle#37

bugs:

@rmsint

Thanks a lot for all hints! I can move forward on all topics. When subjects are fixed I will leave a note or something to easily see the status. And yes, there is a generic "Media" document that is used to manage all different types of media.

@lsmith77
Symfony CMF member

ping?

@rmsint

no update, sorry, I hope that I am able to work on the PR soon

@rmsint

Did try to update the code to the latest versions and then found and remembered the following related todo's:

  • sonata-project/SonataMediaBundle#273, I prefer using LiipImagine for thumbnail generation and hope a solution is found for the integration
  • doctrine orm is installed when updating sonata-project/media-bundle, the following bundle's should be suggested in composer and not required for proper PHPCR integration:
    • sonata-project/notification-bundle: 2.2.*@dev
    • sonata-project/doctrine-extensions: 1.*
    • sonata-project/easy-extends-bundle: 2.1.*
@dbu
Symfony CMF member

ok, i guess you need to sort the dependencies out with @rande seems to me that easy-extends does not depend on anything bad, but notification does, and doctrine-extensions make no sense for phpcr-odm of course. looking at https://github.com/sonata-project/SonataMediaBundle/blob/master/composer.json it seems it is supposed to work with other things than orm, so some probably need to move to require-dev + suggests

rmsint added some commits Jan 30, 2013
@rmsint rmsint sonata media example f0172aa
@rmsint rmsint add CmfMediaBundle to validate interfaces 5c1d17d
@rmsint rmsint add config for slideshowblock
1be5759
@dbu
Symfony CMF member

@rmsint is this still relevant? shall we keep it around as reminder once all the cmf media stuff is merged?

@rmsint

Yes, after the cmf_media and 1.0 we can look at the SonataMedia integration and perhaps then update this PR

@dbu
Symfony CMF member

@rmsint ok if we remove the 1.0 milestone from this issue? i guess its something we can look at later.

@rmsint

@dbu yes sure, removed it.

@lsmith77
Symfony CMF member

ping

@nicolas-bastien
Symfony CMF member

ping +1

@lsmith77 lsmith77 added ready and removed in progress labels May 1, 2015
@lsmith77 lsmith77 added the wip/poc label May 11, 2015
@wouterj
Symfony CMF member

Closing as the CmfMediaBundle is abandoned.

@wouterj wouterj closed this Aug 4, 2016
@lsmith77 lsmith77 removed the wip/poc label Aug 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment