Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Doc][Bug] It's throwing an exception when we are decorating PageAdmin #1639

Closed
eerison opened this issue Nov 2, 2022 · 13 comments · Fixed by #1641
Closed

[Doc][Bug] It's throwing an exception when we are decorating PageAdmin #1639

eerison opened this issue Nov 2, 2022 · 13 comments · Fixed by #1641
Labels

Comments

@eerison
Copy link
Contributor

eerison commented Nov 2, 2022

in version 4.x was removed those parameters

https://github.com/sonata-project/SonataPageBundle/blob/3.x/src/Resources/config/admin.xml#L7-L25

I didn't test it yet, But I suspect the only way to overwrite the classes like PageAdmin is decorating right?

I don't know! I'll do some tests and add some doc/UPGRADE notes for this!

@eerison
Copy link
Contributor Author

eerison commented Nov 2, 2022

In this case if I want to overwrite the PageAdmin I need to do like this right?

<?php

namespace App\Admin;

use Knp\Menu\ItemInterface;
use Sonata\AdminBundle\Admin\AbstractAdmin;
use Sonata\AdminBundle\Admin\AdminInterface;
use Sonata\AdminBundle\Datagrid\DatagridMapper;
use Sonata\AdminBundle\Datagrid\ListMapper;
use Sonata\AdminBundle\Form\FormMapper;
use Sonata\AdminBundle\Route\RouteCollectionInterface;
use Sonata\AdminBundle\Show\ShowMapper;

class PageAdmin extends AbstractAdmin
{
    private AdminInterface $admin;

    public function __construct(AbstractAdmin $admin)
    {
        parent::__construct();

        $this->admin = $admin;
    }

    protected function configureRoutes(RouteCollectionInterface $collection): void
    {
        $this->admin->configureRoutes($collection);
    }

    protected function preUpdate(object $object): void
    {
        $this->admin->preUpdate($object);
    }

    protected function prePersist(object $object): void
    {
        $this->admin->prePersist($object);
    }

    protected function getAccessMapping(): array
    {
        return $this->admin->getAccessMapping();
    }

    protected function configureBatchActions(array $actions): array
    {
        return $this->admin->configureBatchActions($actions);
    }

    protected function alterNewInstance(object $object): void
    {
        $this->admin->alterNewInstance($object);
    }

    protected function configurePersistentParameters(): array
    {
        return $this->admin->configurePersistentParameters();
    }

    protected function configureShowFields(ShowMapper $show): void
    {
        $this->admin->configureShowFields($show);
    }

    protected function configureListFields(ListMapper $list): void
    {
        $this->admin->configureListFields($list);
    }

    protected function configureDatagridFilters(DatagridMapper $filter): void
    {
        $this->admin->configureDatagridFilters($filter);
    }

    protected function configureFormFields(FormMapper $form): void
    {
        $this->admin->configureFormFields($form);
    }

    protected function configureTabMenu(ItemInterface $menu, string $action, ?AdminInterface $childAdmin = null): void
    {
        $this->admin->configureTabMenu($menu, $action, $childAdmin);
    }
}
App\Admin\PageAdmin:
        decorates: sonata.page.admin.page
        arguments: [ '@App\Admin\PageAdmin.inner' ]
        tags:
            - {name: 'sonata.admin', model_class: 'App\Entity\SonataPagePage', controller: 'sonata.page.controller.admin.page', manager_type: 'orm', group: 'sonata_page', translation_domain: 'SonataPageBundle', label: 'page', label_translator_strategy: 'sonata.admin.label.strategy.underscore', icon: 'fa fa-sitemap' }

@eerison
Copy link
Contributor Author

eerison commented Nov 2, 2022

When I try to decorate the admin as I did above, I got this exception :'(

InvalidArgumentException:
Template named "tree" doesn't exist.

  at vendor/sonata-project/admin-bundle/src/Templating/AbstractTemplateRegistry.php:47
  at Sonata\AdminBundle\Templating\AbstractTemplateRegistry->getTemplate('tree')
     (vendor/sonata-project/page-bundle/src/Controller/PageAdminController.php:122)
  at Sonata\PageBundle\Controller\PageAdminController->treeAction(object(Request))
     (vendor/symfony/http-kernel/HttpKernel.php:153)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor/symfony/http-kernel/HttpKernel.php:75)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor/symfony/http-kernel/Kernel.php:202)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (public/index.php:25) 

This was referenced Nov 2, 2022
@VincentLanglet
Copy link
Member

When I try to decorate the admin as I did above, I got this exception :'(

InvalidArgumentException:
Template named "tree" doesn't exist.

  at vendor/sonata-project/admin-bundle/src/Templating/AbstractTemplateRegistry.php:47
  at Sonata\AdminBundle\Templating\AbstractTemplateRegistry->getTemplate('tree')
     (vendor/sonata-project/page-bundle/src/Controller/PageAdminController.php:122)
  at Sonata\PageBundle\Controller\PageAdminController->treeAction(object(Request))
     (vendor/symfony/http-kernel/HttpKernel.php:153)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor/symfony/http-kernel/HttpKernel.php:75)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor/symfony/http-kernel/Kernel.php:202)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (public/index.php:25) 

It's because the template is automatically injected for the admin
https://github.com/sonata-project/SonataPageBundle/blob/4.x/src/DependencyInjection/SonataPageExtension.php#L225

would be interesting to see if it's possible to do the same for decorated services.

@eerison
Copy link
Contributor Author

eerison commented Nov 2, 2022

hmmmm ok, I'll check :D

@eerison
Copy link
Contributor Author

eerison commented Nov 2, 2022

Hey @VincentLanglet is there a other solution But I don't know if it's ok

App\Admin\PageAdmin:
        decorates: sonata.page.admin.page
        arguments:
            - '@App\Admin\PageAdmin.inner'
+        calls:
+            - setTemplate: ['tree', '@@SonataPage/PageAdmin/tree.html.twig']
        tags:
            - {name: 'sonata.admin', model_class: 'App\Entity\SonataPagePage', controller: 'sonata.page.controller.admin.page', manager_type: 'orm', group: 'sonata_page', translation_domain: 'SonataPageBundle', label: 'page', label_translator_strategy: 'sonata.admin.label.strategy.underscore', icon: 'fa fa-sitemap' }

I found others PR related with it, using a new tag

sonata-project/SonataAdminBundle#6566
sonata-project/SonataAdminBundle#6766

But it's not really clean how I can use it :/

I tested something like this

tags:
    - {name: sonata.template_registry, type: show, template:  '@@FooAdmin/CRUD/show.html.twig'}

But it doesn't work, do you have any idea how could I use it?

@VincentLanglet
Copy link
Member

Hey @VincentLanglet is there a other solution But I don't know if it's ok

Sure it can be done with the configuration, but you shouldn't have to know every templates to add in the configuration.
Especially one day if you want to change the tree template, you'll have to update the sonata page config and all of your admins.

I would expect something from the config here
https://github.com/sonata-project/SonataPageBundle/blob/4.x/src/DependencyInjection/SonataPageExtension.php#L223-L230
to do the job.

$container->getDefinition('sonata.page.admin.page'); is done,
I dunno if we can also get the definition of all the service which decorate this one in order to add the setTemplate call too.

@eerison
Copy link
Contributor Author

eerison commented Nov 2, 2022

but you shouldn't have to know every templates to add in the configuration.

That's true

@eerison
Copy link
Contributor Author

eerison commented Nov 4, 2022

I know it's a huge change to do in 4.x, But what do you think we split the admin in small piece of code?

for example, we extract the methods from the PageAdmin to small classes/interfaces and inject it in constructor like

https://github.com/sonata-project/SonataPageBundle/blob/4.x/src/Admin/PageAdmin.php

final class PageAdmin extends AbstractAdmin
{
    public function __construct(private AdminListFields $adminListFields) {}
    
    ....
    
    protected function configureListFields(ListMapper $list): void
    {
        $this->adminListFields-> configureListFields($list);
    }
}
interface AdminListFields
{
    public function configureListFields(ListMapper $list): void;
}

final class PageAdminListFields implements AdminListFields
{
    public function configureListFields(ListMapper $list): void
    {
        $list
            ->add('hybrid', null, ['template' => '@SonataPage/PageAdmin/field_hybrid.html.twig'])
            ->addIdentifier('name')
            ->add('type')
            ->add('pageAlias')
            ->add('site', null, [
                'sortable' => 'site.name',
            ])
            ->add('decorate', null, ['editable' => true])
            ->add('enabled', null, ['editable' => true])
            ->add('edited', null, ['editable' => true])
            ->add(ListMapper::NAME_ACTIONS, ListMapper::TYPE_ACTIONS, [
                'translation_domain' => 'SonataAdminBundle',
                'actions' => [
                    'edit' => [],
                ],
            ]);
    }
}

This way you just need to decorate/customize what is really necessary, and you don't need to implements too many methods that you don't want to touch like in the current solution: #1639 (comment)

in this case I will decorate like this

#[AsDecorator(decorates: PageAdminListFields::class)]
class YourCustomPageAdminListFields implements AdminListFields
{
    public function __construct(#[MapDecorated] private AdminListFields $currentPageAdminListFields) {}

    public function configureListFields(ListMapper $list): void
    {
        $this->currentPageAdminListFields-> configureListFields($list);
        $list->add('YourCustomField');
    }
}

https://symfony.com/doc/current/service_container/service_decoration.html

@VincentLanglet @jordisala1991

@eerison eerison changed the title [Doc] Parameters removed [Doc][Bug] It's throwing an exception when we are decorating PageAdmin Nov 4, 2022
@eerison
Copy link
Contributor Author

eerison commented Nov 4, 2022

I know it's a huge change to do in 4.x, But what do you think we split the admin in small piece of code?

for example, we extract the methods from the PageAdmin to small classes/interfaces and inject it in constructor like

https://github.com/sonata-project/SonataPageBundle/blob/4.x/src/Admin/PageAdmin.php

final class PageAdmin extends AbstractAdmin
{
    public function __construct(private AdminListFields $adminListFields) {}
    
    ....
    
    protected function configureListFields(ListMapper $list): void
    {
        $this->adminListFields-> configureListFields($list);
    }
}
interface AdminListFields
{
    public function configureListFields(ListMapper $list): void;
}

final class PageAdminListFields implements AdminListFields
{
    public function configureListFields(ListMapper $list): void
    {
        $list
            ->add('hybrid', null, ['template' => '@SonataPage/PageAdmin/field_hybrid.html.twig'])
            ->addIdentifier('name')
            ->add('type')
            ->add('pageAlias')
            ->add('site', null, [
                'sortable' => 'site.name',
            ])
            ->add('decorate', null, ['editable' => true])
            ->add('enabled', null, ['editable' => true])
            ->add('edited', null, ['editable' => true])
            ->add(ListMapper::NAME_ACTIONS, ListMapper::TYPE_ACTIONS, [
                'translation_domain' => 'SonataAdminBundle',
                'actions' => [
                    'edit' => [],
                ],
            ]);
    }
}

This way you just need to decorate/customize what is really necessary, and you don't need to implements too many methods that you don't want to touch like in the current solution: #1639 (comment)

in this case I will decorate like this

#[AsDecorator(decorates: PageAdminListFields::class)]
class YourCustomPageAdminListFields implements AdminListFields
{
    public function __construct(#[MapDecorated] private AdminListFields $currentPageAdminListFields) {}

    public function configureListFields(ListMapper $list): void
    {
        $this->currentPageAdminListFields-> configureListFields($list);
        $list->add('YourCustomField');
    }
}

https://symfony.com/doc/current/service_container/service_decoration.html

@VincentLanglet @jordisala1991

Extension works better then my suggestion :D

I'll test that :)

@eerison
Copy link
Contributor Author

eerison commented Nov 7, 2022

Using extension works!

<?php

namespace App\Admin\Extension;

use Sonata\AdminBundle\Admin\AbstractAdminExtension;
use Sonata\AdminBundle\Datagrid\ListMapper;

class PageAdminExtension extends AbstractAdminExtension
{
    public function configureListFields(ListMapper $list): void
    {
        $list->add('id');
    }
}

But it added the fields in the end of the list like this

Screenshot 2022-11-07 at 09 10 29

are there anyway to put the field in the begin of table

@VincentLanglet
Copy link
Member

are there anyway to put the field in the begin of table

You can play with the reorder method

@eerison
Copy link
Contributor Author

eerison commented Nov 7, 2022

I didn't find anything in sonata docs about reorder method 👀

@VincentLanglet
Copy link
Member

VincentLanglet commented Nov 7, 2022

https://github.com/sonata-project/SonataAdminBundle/blob/4.x/src/Datagrid/ListMapper.php#L193

Might be interesting to add docs explaining how to use it then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants