symfony-cmf / block-bundle

Extends the SonataBlockBundle to integrate with PHPCR ODM
https://cmf.symfony.com
20 stars 51 forks source link

Can not set settings attr class on blocks #213

Open phiamo opened 9 years ago

phiamo commented 9 years ago

I am using bleeding edge of the cmf, but had this problem with 1.0 and 1.1 too:

When tring to create Blocks with attr settings, i cant get it working, to not show a error anywhere:

trying to set the settings like this:

        $block->setSettings(array(
            'attr' => ['class' => 'test']
        ));

and try topersist and flush i get a:

[PHPCR\ValueFormatException]                            
  Can not determine type of property with value "array (  
    'class' => 'test',                                    
  )"  

if i am setting it like this:

        $block->setSettings(array(
            'class' => 'test'
        ));

i can use it in the frontend, but the dev toolbar tells me there was an error, and checking the profiler gives me:

ERROR - [cms::blockContext] block.id=0 - error while resolving options - The option "class" does not exist. Known options are: "attr", "extra_cache_keys", "template", "ttl", "use_cache"

do you have any ideas how to solve this, or is there a hidden feature somewhere?

dbu commented 9 years ago

the problem is that phpcr-odm does not handle nested arrays. it can only handle flat arrays, which are a native structure to phpcr (multivalue). we added support for hashmaps by storing keys and values as separate multivalue fields. there was discussion about adding support to phpcr-odm for them but nobody stepped up to do it yet.

one option would be to serialize the array into a string field. you could extend a block and overwrite the getter and setter methods to do this and store in a different property where you serialize. if you use all the default blocks, this will however be quite some pain i am afraid.

phiamo commented 9 years ago

so how was it inteded to be used? for serveral use cases there are extra keys, like the child_class in container block. probably there should be a general class for other blocks?

dbu commented 9 years ago

i think we where not aware of this use case when we initially built the settings. i guess it would make sense to fix this either in phpcr-odm or in the base block class as it seems a legitimate case to me. not sure if @dantleech had any plans to advance https://github.com/doctrine/phpcr-odm/issues/417 or if you want to volunteer for it? problem with the naive serialize is that this can break between php versions, leading to information loss. we could go for json or xml which would be more neutral.

dantleech commented 9 years ago

Looks interesting. Will see if I have any time tomorrow.

phiamo commented 9 years ago

@dbu i think the cleanest way would be having the attr => {} with nested keyvalue pairs available. doctrine/phpcr-odm#417 would be the solution to store it, but i am not sure if there is a component to handle this in sonata admin easily.

while adding the "class" key to default settings is currently nice manageable with Burgov/KeyValueFormBundle.

dbu commented 9 years ago

indeed, i fear this will be quite challenging in the admin. then again, you should probably do a custom admin widget for such things. its not something a web administrator should do free style in the interface, so many things can go wrong. or you could revert to something like the marmelab phpcr browser (kinda the phpmyadmin approach) - which hopefully somebody integrates with a symfony bundle