Sonataadminbundle: Add a way to define the prefered Admin for a specific Entity

Created on 9 Aug 2020  路  20Comments  路  Source: sonata-project/SonataAdminBundle

Feature Request

I know i can use admin_code to specify the admin to use when I create multiple Admin for an entity.

But it would be easier to add some property to the Admin class to define the precedence of admins.

final class DefaultAdmin extends AbstractAdmin
{
    protected $priority = 0; //default
    ...
}

final class OtherAdmin extends AbstractAdmin
{
    protected $priority = 999; // some technical admin because sometimes sub admin routes are picky
    ...
}

so in the Pool

    public function getAdminByClass($class)
    {
        ...
        // the first one would/could be correct if no admin_code specified
        return $this->getInstance($this->adminClasses[$class][0]);
    }


feature

All 20 comments

Do you want to work on a Pr ?

When I think about it, maybe the priority is not the best way.

If admin A has a priority of 0, B a priority of 1, and C a priority of 2, C will always be taken, so it feel like it's useless to set a priority for B.

A boolean seems enough (primary ? Main ? ...)

But maybe the priority is a good idea, if we can find others context to use this property !

Do you want to work on a Pr ?

why not... have you some pointers ? I've looked around and some stuff are in the injection pass...

in addition this property must be tweakable in the service definition because sometimes we want to reuse admin class

admin.cool:
        class: App\Admin\CoolAdmin
        arguments: [~, App\Entity\Cool, ~]

admin.cool_copy:
        class: App\Admin\CoolAdmin
        arguments: [~, App\Entity\Cool, ~]
        calls:
              - [setPriority, [999]]

EDIT:

If admin A has a priority of 0, B a priority of 1, and C a priority of 2, C will always be taken, so it feel like it's useless to set a priority for B.

boolean can be OK OC, the code should throw an exception if there is more than 1 admin instance for a specific entity and all priorites are equals (all 0 or or true or false).

Do you want to work on a Pr ?

Est-ce qu'il y a un dev startup kit ? / Is there a dev startup kit ?

Indeed, using the admin configuration seems a better idea than a class property.

If i remember correctly, there is a "display_in_dashboard" property in the config. (Default true). Maybe we could have something similar ?

Do you want to work on a Pr ?

Est-ce qu'il y a un dev startup kit ? / Is there a dev startup kit ?

Not sure what you're looking for. So i would say no. Just clone the projet, modify the code, add tests, and its ok.

Here, there is a show_in_dashboard property https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/DependencyInjection/Compiler/AddDependencyCallsCompilerPass.php

But maybe @sonata-project/contributors have a better idea than something like a main property in the admin config.

IMHO new array config where user can set default admin for the entity. By default first register admin.

IMHO new array config where user can set default admin for the entity. By default first register admin.

Make sens using the order of declaration is way better than adding some arbitrary field

IMHO new array config where user can set default admin for the entity. By default first register admin.

Make sens using the order of declaration is way better than adding some arbitrary field

The order of declaration has already an impact about where the admin is displayed in the side-menu.

If I have three adminds for the same entity: foo, bar, baz.

  • I want the order foo, bar, baz in the menu.
  • I want the default one to be bar.

How will I be able to do, if the order has an impact about the default one ?

How will I be able to do, if the order has an impact about the default one ?

Good point too. But IMHO non default admins are often technical admins I don't want to show in side menu.

But we can have both, default value for priority set by order of declaration and updatable with some conf param you mentioned earlier.

How will I be able to do, if the order has an impact about the default one ?

Good point too. But IMHO non default admins are often technical admins I don't want to show in side menu.

But we can have both, default value for priority set order of declaration and updatable with some conf param you mentioned earlier.

You should configure by dashbord group config. We taking about admina where admin_code is not set.

You should configure by dashbord group config. We taking about admina where admin_code is not set.

I don't understand, can you provide a use case ?

How will I be able to do, if the order has an impact about the default one ?

Good point too. But IMHO non default admins are often technical admins I don't want to show in side menu.

My project use lot of non-default admins which are display on the side menu. So this can't be considered as a general rule.

But we can have both, default value for priority set by order of declaration and updatable with some conf param you mentioned earlier

I'm keeping this one. Is it ok ?

for the name

entity_precedence ?

entity_affinity ?

entity_priority ?

Lesser numbers will have priority

But we can have both, default value for priority set by order of declaration and updatable with some conf param you mentioned earlier

I'm keeping this one. Is it ok ?

for the name

entity_precedence ?

entity_affinity ?

entity_priority ?

Lesser numbers will have priority

See https://github.com/sonata-project/SonataAdminBundle/issues/6266#issuecomment-671061587
What would be the advantage of priority over a simple boolean ?

We could add something like a main conf param.

App\Admin\FooAdmin:
        arguments:
            - ~
            - App\Entity\Foo
            - ~
        tags:
            - name: sonata.admin
              manager_type: orm
              group: foo
              label: foo
              show_in_dashboard: true
              main: true

App\Admin\Foo2Admin:
        arguments:
            - ~
            - App\Entity\Foo
            - ~
        tags:
            - name: sonata.admin
              manager_type: orm
              group: foo
              label: foo
              show_in_dashboard: true

And for the implementation, you'll need to modify this case

if (\count($this->adminClasses[$class]) > 1) {
            throw new \RuntimeException(sprintf(
                'Unable to find a valid admin for the class: %s, there are too many registered: %s',
                $class,
                implode(', ', $this->adminClasses[$class])
            ));
        }

With the logic:

  • If there is no main, change the exception for "You should define a main admin"
  • If there is multiple main, change the exception for "Too many main"
  • else return the only main

But maybe the priority is a good idea, if we can find others context to use this property !

I stayed with this one but go for a Boolean.

Maybe a less scope wide name, (main could mean everything) ? default_for_entity ?

Maybe a less scope wide name, (main could mean everything) ? default_for_entity ?

Since the method is getAdminByClass, the name could use the same words, like default_for_class, or default_class_admin.

I'll recommend you to start a PR

  • With the config
  • Some doc about it
  • Some test about it

Reviewers will certainly have idea about the name, and it will be easy to update the PR.

What when we set it in two admins? IMHO better will be add optional new
sonata_admin:
default_for_class:
Class_fqnc: admin_code
Othet_class: other_admin_code

What when we set it in two admins?

If there is multiple main, an exception for "Too many main".
That's not different of the current behaviour

if (\count($this->adminClasses[$class]) > 1) {
            throw new \RuntimeException(sprintf(
                'Unable to find a valid admin for the class: %s, there are too many registered: %s',
                $class,
                implode(', ', $this->adminClasses[$class])
            ));
        }

I agree with it. We should suggest it to project and avoid in bundles(where admin_code should be set)

Was this page helpful?
0 / 5 - 0 ratings