symfony-cmf/block-bundle

Settings ignored on ContainerBlock

bobemoe opened this issue · 9 comments

Settings set with the $containerBlock->setSetting() function and persisted to DB are ignored when rendering a ContainerBlock, the settings array just contains the default settings, even though I have confirmed the settings are stored on the object in the DB. I have fixed the issue by making the below change, but now an error is thrown if the block does not have custom settings set, seems the defaults don't exist and the twig template errors because the settings aren't available.

Symfony\Cmf\Bundle\BlockBundle\Doctrine\Phpcr\ContainerBlock
line 79-82

            return $this->renderResponse($blockContext->getTemplate(), array(
                'block'       => $blockContext->getBlock(),
                'settings'    => $blockContext->getSettings(),
            ), $response);

Changed To:

            return $this->renderResponse($blockContext->getTemplate(), array(
                'block'       => $blockContext->getBlock(),
                'settings'    => $blockContext->getBlock()->getSettings(),
            ), $response);

I'm guessing the problem is somewhere to do with merging the defaults with the custom settings, but I cant seem to work out where this should happen. I will keep digging, but any ideas from someone more up on Blocks than me would be appreciated :)

UPDATE: fixed typos.

Forgot to mention I am using version 2.2.11 of Sonata BlockBundle and 1.1.1 of CMF BlockBundle.

can you submit your change as a PR .. makes it easier to review. thanks!

OK, created a PR. I've added a conditional statment to select the block settings if they exist, otherwise fall back to default, but this is far from ideal. Also I notice settings set in config.yml (sonata_block: blocks_by_class: Symfony\Cmf\Bundle\BlockBundle\Doctrine\Phpcr\ContainerBlock) are totally ignored!

I'm not sure if this might be an issue with the SonataBlockBundle? I've not made a custom block yet so am not sure where/how the settings arrays get passed down and merged.

thank you .. will try to have a look this weekend.

@dbu unless you have an idea what could be the cause ..?

dbu commented

i commented on the PR. would have to look into the code to figure out what is going on, but have no time now. commented on the PR though.

I've been having a look at the "Create your own block" docs:
http://symfony.com/doc/current/cmf/bundles/block/create_your_own_blocks.html#create-a-block-service
http://sonata-project.org/bundles/block/master/doc/reference/your_first_block.html
Both these pages suggest that the execute function should merge the settings.
I have implemented this using the OptionsResolver as show in the cmf page above, and will submit another pull request shortly. This allows settings set directly on the block using setSetting() to override the default settings, but settings set in config.yml (as below) still don't seem to get merged in.
Documented here: http://symfony.com/doc/current/cmf/bundles/block/introduction.html#usage
My config.yml:

sonata_block:
    blocks_by_class:
        Symfony\Cmf\Bundle\BlockBundle\Doctrine\Phpcr\ContainerBlock:
            settings:
                divisible_by: 3
                divisible_class: row
                child_class: col-md-4

I'm currently looking into the DependencyInjection to see where the blocks_by_class setting is getting missed.

OK, I've updated my branch - it looks like the PR has updated to the latest commit automatically so I don't think I need to submit another?

I think this modification now satisfactory solves the issue, allowing on-block settings to override the default settings. However settings from config.yml are sill not merging in.

From digging around in the BlockContextManager it look like the get() function loads the default base block settings and merges in the settings by class/type from the config.yml and then passes the OptionsResolver to the actual Blocks setDefaultSettings() function which then sets a fresh set of default settings, overriding those set in config.yml. I'm thinking this could be a bug in the SonataBlockBundle order of loading setting. What do you guys think? Should we get someone from SonataBlockBundle to take a look now?

@bobemoe indeed PRs are automatically updated when you push to the same branch. thanks for your work.

ping @rande

dbu commented

fixed in #205