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

[RFC] default admin #3908

Closed
greg0ire opened this issue Jun 2, 2016 · 11 comments
Closed

[RFC] default admin #3908

greg0ire opened this issue Jun 2, 2016 · 11 comments

Comments

@greg0ire
Copy link
Contributor

greg0ire commented Jun 2, 2016

Question Answer
Bundle version Sonata 3 and above
Symfony version irrelevant
php version irrelevant

Given an entity (or document ?) for which several admin are defined, when you try to reference an object of this type, sonata will not be able to choose and will prompt the user for an admin_code options in most cases.

Being forced to give this option can get tedious, and can also cause regressions : for instance, if you define and admin, develop a lot of things around it, and then define another admin for the same class, you have to carefully check your application for this error and fix it.

I think this problem could be resolved by injecting where needed a service that would be able to make the right choice when asked for the admin to use for a given class.

The interface to implement would be the following :

<?php
namespace To\Be\Defined\Please\Help;

interface DefaultAdminResolverInterface
{
    /**
     * Decides which admin best fits the required FQCN
     *
     * @param string $fqcn the fully qualified class of a model that has several admins
     *
     * @return AdminInterface the chosen admin
     */
    public function getDefaultAdminForModel($fqcn);
}

I think a good implementation would gather the possible admin services, and discard services that are children admin, and if this does not reduces the number of possible admin to exactly one, then use a mapping model => defaultAdmin .

Whether to give this job to AdminPool or create a new class just for this is to be discussed, it turns out it is the class that throws the exception about admin_code : https://github.com/sonata-project/SonataAdminBundle/blob/3.x/Admin/Pool.php#L186

@clytemnestra
Copy link

clytemnestra commented Jun 3, 2016

What about check each admin for a boolean $isMainAdmin and then have the default value for admin_code to go with the first admin that it finds and has $isMainAdmin true, when there are multiple admins for an Entity?

@greg0ire
Copy link
Contributor Author

greg0ire commented Jun 3, 2016

I must say I thought about it, but discarded the idea: although it seems more simple, I can see two issues. First, the false value of the boolean will be useless, and second, what if you define several main admin for a model? I think the right data structure for this is a hash.

@greg0ire
Copy link
Contributor Author

greg0ire commented Jun 8, 2016

ping @sonata-project/contributors

@greg0ire
Copy link
Contributor Author

ping @sonata-project/contributors , what do you think about this issue?

@core23
Copy link
Member

core23 commented Jul 25, 2016

Sounds like a good idea 👍

@greg0ire
Copy link
Contributor Author

greg0ire commented Jul 25, 2016

Cool, but I'm still torn on this issue:

Either this :

# config.yml
sonata_admin:
    default_admin:
        My\Entity: my_entity_main_admin_id

Or something like @clytemnestra suggested, but with null instead of a useless boolean :

# admin.yml
services:
    my_entity_main_admin_id:
        tags:
             -
                  is_main: ~ # If I go with this implementation, I must gather tags and check for uniqueness, so more complicated

Can I have your view on this in particular?

@core23
Copy link
Member

core23 commented Jul 25, 2016

Maybe we can use existing key:

# config.yml
sonata_admin:
    admin_services:
        id.of.admin.service:
            entities:
                Foo\Entity\Bar

@greg0ire
Copy link
Contributor Author

Definitely going to do that, I had forgotten about this key!

@greg0ire
Copy link
Contributor Author

greg0ire commented Jul 25, 2016

… not. This key is meant to configure services that will be injected in all admin services.

@DeonKuhn
Copy link

DeonKuhn commented Dec 4, 2017

What's the solve for this?

@stale
Copy link

stale bot commented Jan 30, 2020

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

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

No branches or pull requests

6 participants