Allow to override any action easily

[javiereguiluz]

This PR introduces a simple overriding mechanism for actions and controller methods. Examples:

Customize the instantiation of a single entity

Create a new AdminController extending from the default one and add this method:

public function createNew<EntityName>Entity()
{
    ...
}

Customize the instantiation of all entities

Create a new AdminController extending from the default one and add this method:

public function createNewEntity()
{
    ...
}

Tweak a specific entity before persisting/updating/removing it

Create a new AdminController extending from the default one and add this method:

public function prePersist<EntityName>Entity()
public function preUpdate<EntityName>Entity()
public function preRemove<EntityName>Entity()
{
    ...
}

Tweak all entities before persisting/updating/removing them

Create a new AdminController extending from the default one and add this method:

public function prePersistEntity()
public function preUpdateEntity()
public function preRemoveEntity()
{
    ...
}

Tweak the list/new/edit/show/delete method for a specific entity

Create a new AdminController extending from the default one and add this method:

public function list<EntityName>Action()
public function new<EntityName>Action()
public function edit<EntityName>Action()
public function show<EntityName>Action()
public function delete<EntityName>Action()
{
    ...
}

Tweak the list/new/edit/show/delete method for all entities

Create a new AdminController extending from the default one and add this method:

public function listAction()
public function newAction()
public function editAction()
public function showAction()
public function deleteAction()
{
    ...
}

Modify the backend using events

Hook into any of the new events defined by the bundle:

// Initialization related events
const PRE_INITIALIZE = 'easy_admin.pre_initialize';
const POST_INITIALIZE = 'easy_admin.post_initialize';

// Backend views related events
const PRE_DELETE = 'easy_admin.pre_delete';
const POST_DELETE = 'easy_admin.post_delete';
const PRE_EDIT = 'easy_admin.pre_edit';
const POST_EDIT = 'easy_admin.post_edit';
const PRE_LIST = 'easy_admin.pre_list';
const POST_LIST = 'easy_admin.post_list';
const PRE_NEW = 'easy_admin.pre_new';
const POST_NEW = 'easy_admin.post_new';
const PRE_SEARCH = 'easy_admin.pre_search';
const POST_SEARCH = 'easy_admin.post_search';
const PRE_SHOW = 'easy_admin.pre_show';
const POST_SHOW = 'easy_admin.post_show';

// Doctrine related events
const PRE_PERSIST = 'easy_admin.pre_persist';
const POST_PERSIST = 'easy_admin.post_persist';
const PRE_UPDATE = 'easy_admin.pre_update';
const POST_UPDATE = 'easy_admin.post_update';
const PRE_REMOVE = 'easy_admin.pre_remove';
const POST_REMOVE = 'easy_admin.post_remove';

Single entity actions (edit, new, delete, etc.) receive the $entity as the event subject. The actions related to multiple entities (list, search) receive the $paginator object as the subject of the event. In addition, the event receives all the controller variables as arguments. You can access them via getArgument() or via the array access provided by Symfony for the GenericEvent:

<?php

namespace AppBundle\EventListener;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\EventDispatcher\GenericEvent;

class EasyAdminSubscriber implements EventSubscriberInterface
{
    public static function getSubscribedEvents()
    {
        return array(
            'easy_admin.pre_update' => array('preUpdate'),
        );
    }

    public function preUpdate(GenericEvent $event)
    {
        $entity = $event->getSubject();
        $view = $event['view'];
        $entity = $event['entity'];

        // ...
    }
}

[Pierstoval]

For the method_exists checks, do you think it might improve readability to have some private method like $this->resolveAction('new') that will automatically check the function existence ? And for the functions that accept one or more argument, use it as a variadic function and send the parameters automatically, except the first one that is the action name.

[Pierstoval]

Might I also suggest using a system like the one you have in the twig extensions, like this:

// Entity currently used: Post
$this->resolveAction('new'); // Returns 'newPostAction'
$this->resolveAction('prePersist*Entity'); // Returns 'prePersistPostEntity'

When there is no wildcard, the entity name is appended. Unless, the wildcard is replaced by the entity name.

[Pierstoval]

I think we might need to check entity-specific method for the createNewForm too

[ogizanagi]

createEditForm too ?

[Pierstoval]

Excluding what I said, seems all good 👍

[ogizanagi]

I'm definitively not fan of "magic" methods like:

public function createNew<EntityName>Entity()
{
    ...
}

I don't see any value to these, as the user is already able to do that, with it's own logic and methods.

---

The event system looks great ! 👍

[Pierstoval]

The goal is mainly to prevent this:

public function newAction() {

    if ($this->entity['name'] === 'Post') {
        return $this->newPostAction();
    } if ($this->entity['name'] === 'Product') {
        return $this->newProductAction();
    } if ($this->entity['name'] === 'Category') {
        return $this->newCategoryAction();
    } if ($this->entity['name'] === 'Customer') {
        return $this->newCustomerAction();
    } if ($this->entity['name'] === 'Page') {
        return $this->newPageAction();
    } if ($this->entity['name'] === 'Video') {
        return $this->newVideoAction();
    } if ($this->entity['name'] === 'BlogArticle') {
        return $this->newBlogArticleAction();
    }

    return parent::newAction();
}

[ogizanagi]

@Pierstoval : I understood the purpose, but I don't think this is required, and why the bundle should take care of such things. This isn't even about DX IMO. 😕

If they really need it, developers may easily create their own solutions in order to avoid such if cascading. I don't see why the bundle should handle this itself, and impose hardcoded conventions which we don't even know will change or not in the future (Remember the issue/PR about entities' names and suffixes ?), and then fail any backend relying on it. There is a chance the developper doesn't even understand why it failed, as he didn't "fully control the code".

I do not know if I am making myself clear on this. But I think it's only source of trouble for easyadmin.

[javiereguiluz]

@ogizanagi in my opinion this system doesn't bring magic but conventions. And I think those are two very different things. In Symfony 1 for example the filter system changed your response without you being aware of it (that's magic). In Symfony 2 if you create a file with a very specic name (e.g. messages.fr_FR.xlf) and in a very specific location (e.g. app/Resources/translations/) that file is loaded and used in the application (that's a convention).

Regarding the createNewCustomerEntity(), editProductAction(), etc. methods, not only I think they are necessary, but in practice they will be only methods used by developers. Otherwise you have to override a very complex method (newAction()) or do some ugly stuff with if or switch as pointed by @Pierstoval.

In short, I think this new feature is consisent with the rest of the bundle behavior:

1. Do you want some basic customization? Use the tens of configuration options available.
2. Do you want to do some cool stuff? Use two coventions: app/Resources/views/easy_admin/<EntityName>/<templateName>.html.twig templates and <actionName><EntityName>Action() methods.
3. Do you want extreme customization? Use your own templates (define them in design.templates option) and the event system of the AdminController.

[ogizanagi]

@javiereguiluz : But the entity's name is (magic). Because the developper doesn't really have full control over it, and the name might change if there is another entity with the same name (edge case, right, but still :/ ).
BTW, didn't we say the entity's name should be kept as an internal thing only ?

What do you think about what we discussed later in https://github.com/javiereguiluz/EasyAdminBundle/pull/276#issuecomment-99604657 ?
Or / and should the entity's name be configurable ?

I didn't realized it before with templates, but it's indeed the same problem. 😕

[javiereguiluz]

@ogizanagi yes, the entity name is "magic" for one use case: when you have totally different entities that share the same name. How often does this happen? In my experience very infrequently ... but I'd love to hear other experiences.

And yes, I plan to make the entity name configurable in the future to avoid problems in this edge case.

[javiereguiluz]

I think I have a solution for this problem:

1. If you use the simple config format, the name is autogenerated:

easy_admin:
    entities:
        - 'AppBundle\Entity\Product'
        - '...'

If there are more than one entity with the same name, EasyAdmin autogenerates incremental names (Product, Product2, Product3).

This is not a problem because if you are using this config format, you are obviously doing some quick-and-dirty test of this bundle and you are not going to extend the backend with a custom controller.

2. If you use the other config formats, the YAML key will be used as the entity name. This solves our problem because you obviously can't have different YAML keys with the same value:

# config format 2
easy_admin:
    entities:
        Product: 'AppBundle\Entity\Product'
        ProductArchive:  'AppBundle\Entity\Archive\Product'

# config format 3
easy_admin:
    entities:
        Product:
            class: 'AppBundle\Entity\Product'
        ProductArchive:
            class: 'AppBundle\Entity\Archive\Product'

There is only one catch: right now we use the YAML key as the label (unless you define the label option for the entity). This means that someone could do the following:

# config format 3
easy_admin:
    entities:
        'Current Products':
            class: 'AppBundle\Entity\Product'
        'Product Archive (2015)':
            class: 'AppBundle\Entity\Archive\Product'

And that would be a problem because you cannot name a PHP method as newCurrent ProductsAction().

So this is my proposal:

• Always use the YAML key as the entity name, so the developer has full control over it (unless the basic config format is used, where names are autogenerated).
• Throw an error if you use white spaces or unsafe characters in the YAML key. This is a BC break, but it's not that important and we'll throw good error messages.
• If someone wants to customize the label of the entity, use the label option in that entity (this is the same as of now, but this proposal will turn it into the only way to do this).

What do you think?

[ogizanagi]

I think it's the most straightforward and ideal solution.
Anyway, for more consistency, I think getting rid of the "strange" behavior you mentioned with the entities keys in the config will be for the best (IIRC, I was the one who made this, for minor BC reasons when introducing the label option for translations).

👍

[javiereguiluz]

@Pierstoval I like your comments about reducing code repetition and extracting the logic about checking if a custom method exists. But I'm not sure about the proposed alternatives. That's why I'm going to merge this PR as is and I'll think about how to improve that part of the code in the future.

After merging this PR, this is the roadmap (to be compelted in different PRs):

• Fix the handling of the entity names as discussed above. Basically we must prvent developers from using unsafe chars in the YAML key associated with each entity.
• Reorganize the documentation as discussed in Documentation refactoring #279
• Add more functional tests, specially for the new features introduced recently
• Release the new stable version