symfony-cmf / block-bundle

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

[proposal] Refactor BlockAdmin to support inheritance #50

Closed dantleech closed 10 years ago

dantleech commented 11 years ago

I think it should be possible to support inheritance in SonataAdmin, so effectively we can have just a single list of block contents and one item on the dashboard "Block Content"

I think this would make the interface less confusing and remove duplicate code.

We could also tidy up the code a bit whilst we're at it, maybe moving the various XML admin configs into a single file, or at least prefix them with "admin" instead of suffixing them. (difficult to see whats going in the config folder)

dbu commented 11 years ago

i think this is a duplicate of https://github.com/sonata-project/SonataDoctrinePhpcrAdminBundle/issues/80 - we want a shared list but still need the specific details forms. sonata seems to call that subcollections. @sjopet has some code about that that nobody ported to sonata yet: https://github.com/sjopet/sf2bundle-admin-extensions (but see also https://github.com/sonata-project/SonataDoctrinePhpcrAdminBundle/issues/80 as there is some more discussion there)

dantleech commented 11 years ago

Yeah, exactly 0 shared list / creation UI and separate admins for the actual objects. I noticed this merged PR from many months ago in Sonata: https://github.com/sonata-project/SonataAdminBundle/pull/815

rmsint commented 11 years ago

The media bundle is also doing something similar, but then with custom code. It is using the persistent parameters to switch between providers. If multiple media providers are configured a provider selection page is first displayed to the user and depending on the choice a form is displayed. See for the creation part fe. https://github.com/sonata-project/SonataMediaBundle/blob/master/Controller/MediaAdminController.php#L64

dbu commented 11 years ago

ok, can we rename this issue to point out that we want to leverage https://github.com/sonata-project/SonataDoctrinePhpcrAdminBundle/issues/80 once that is done?

sjopet commented 11 years ago

Hi guys,

After a stressfull period the project i was working on went live on monday i hope to read trough the huge list of cmf stuff this week and get involved again i planned some cmf time at work in the comming weeks and will have a look at this soon.

dbu commented 11 years ago

great, looking forward to it!

sjopet commented 11 years ago

It seems to me the optimal solution for this would be to configure some extra service in the sonata admin dashboard groups section:

# app/config/config.yml
sonata_admin:
    dashboard:
        groups:
            blog:
                label: blog
                list_handler: symfony_cmf_blog.custom.group_list.handler
                items:
                    - symfony_cmf_blog.admin
                    - symfony_cmf_post.admin

the list_handler would be something like a regular admin, providing the list view for all admins in that group. This service maybe a crud which you can overwrite to customize the query, filters columns etc. Some other actions need a custom controller as well like batch actions and the create menu on the dashboard.

Extensions can be used to fix the routes for all admins in the group, so the list action points to the list_handler. I'm waiting on a pr https://github.com/sonata-project/SonataAdminBundle/pull/1352 to get merged which should make that a lot easier, though there might be a better way to fix the routes.

I found there's a couple of ways to do this stuff but I think I need to have some expert advice from @rande on how to best do some things.

dantleech commented 11 years ago

Could this also be done by creating the Admin class to work on the abstract class, for example, AbstractBlock, and then use the new extension feature to customize the admin for each sub class?

That should already work infact, I've done it before, but had to do some hacking to get the correct form to display correctly.

dantleech commented 11 years ago

https://github.com/dantleech/DCMS/blob/sonata_admin/src/DCMS/Bundle/CoreBundle/Admin/EndpointAdmin.php

So in this case I use the Admin API to set the valid sub classes and use some adhoc logic to get the correct form for editing.

rmsint commented 11 years ago

What is to be fixed about the routes for all admins in the group?

For the blocks form and validation config can be automated and retrieved from the block services in the handler. Also for the action block it would be usefull to allow further customization per block class somehow. The sonata admin already has logic to determine the admin service for a class if it is defined, it is used for mapping field associations. I think it could be reused in the handler to automate further customizations per block class.

Maybe a better name for the ''list_handler'' will be something like ''mixed_collection_handler'', "list" implies more that it is only about handling the list config for admins.

sjopet commented 11 years ago

To have a combined list view for all admins in a group we need to define a new admin and add all the admins in the group as child admins.The new GroupAdmin has a custom query to fetch a list made up out of the combined results for all it's child admins.

We want all the child admins to handle the edit create and other actions as usual but the list view for all these child admins should be handled by the GroupAdmin so we need to 'fix' the LIST route for all the child admins and have it point to the listAction of the GroupAdmin. I hope that explains it a bit.

I have not worked on this lately because of dependency issues, waiting on some commits to get tagged. Since a new round of tags has just been pushed I should be able to continue with this.

Btw the solution I'm working towards should be a generic SonataAdminBundle feature that has nothing to do really with the cmf-block-bundle however the sandbox configuration of the block-bundle is an ideal testing situation.

dbu commented 11 years ago

is it realistic to keep this in the 1.0 milestone? will the sonata side changes be finished in the next 2 weeks at most? otherwise i propose we postpone this. 1.1 also needs some interesting goodies so people want to update :-)

sjopet commented 11 years ago

doesn't seem realistic no +1 for moving it to 1.1

dbu commented 11 years ago

i think this relates to https://github.com/sonata-project/SonataDoctrinePhpcrAdminBundle/pull/141

dbu commented 10 years ago

see https://github.com/sonata-project/SonataDoctrinePhpcrAdminBundle/issues/80