New API - changes in generator.yml structure

[ioleo]

Useing Symfony2 Config Component

I plan to implement useing symfony2 config compoment to validate generator YAML files. This is also a great opportunity to improve our API (and for obvious reasons, we can only do that now, before v2.0-stable).

New API - what needs improvement?

I'd like to start a discussion about the pros and cons of current API. Each of us used Admingenerator for diffrent projects and faced diffrent problems. I'd like to hear your thoughts. To ignite the discussion, I'll throw in a bunch of thoughts.

In my projects I had multiple custom object actions. To not repeat myself and share the configuration between builders, I had them defined in the global params section.

However, there was no easy way to overwrite/customize the list per builder. Eg. if I had:

params:
  object_actions:
    custom1: ~
    custom2: ~
    edit: ~
    show: ~
    delete: ~

But on Show builder, I'd like to display all of them... except show (for obvious reasons). By the same argument, on edit builder, I want to remove the edit action. On Action builder I wanted only custom1 and custom2 (without the default actions edit/show/delete).

Also, in some cases I wanted to simply change the order. Eg. in Show builder: custom2, custom1, while in Edit builder custom1, custom2.

Note: later on, we've implemented a very hacky trick, that if you set your field to null, it will read the global config.. and merge it... why hacky? Well, have a look at that code and tell me it's easy to read :)

Another example - to allow translateing messages from generator files, we've introduced the parameter bag feature, which provides a structured syntax that (if detected) will output a call to translator service with the correct parameters.

However, over time I feel like generator files are "pandora boxes" where you do everything:

• configure basic parameters (like generator used, translation domain, namespace or model)
• configure fields
• configure forms
• add (and translate) labels, help blocks, and other messages
• configure actions
• configure filters

This is OK for a VERY SIMPLE project. But as soon as your project grows in size or complexity, you end up with huge generator files, which are almost unreadable.

I'd like to change that. I'd like to move some things out of the generator file. I think the "generator" file should be only for basic configuration (to generate a "stub") and all those details (like form configuration) should live in seperate PHP classes.

So... the "ideal" generator for me, would only contain some basic parameters, and "switches" that turn on/off certain features. Eg. say you want to generate ONLY the show builder.

This could also improve the cache generation time - if a feature was disabled, certaint blocks of code would be skipped.

[ioleo]

/cc @sescandell @ksn135

I'm still thinking about how it should look like. As soon as I have some solid idea in my head, I'll post it here.

[sescandell]

Hi @loostro

Don't have time to make something more precise for now, so very quickly:

• I agree with you, we need to try to move some things and make YAML files easier to use (if possible)
• Moving to the Config/Processor Component is something we should really do (IMO)
• But, I really thing the strength of this project is to mainly configure everything from YAML files. I don't want this project to be "a clone or heavy thing of Sonata Configuration". There is some thing that should not be in YAML (like form specific configurations, and that's why we introduced by the past the formOptions PHP method). But for some other things, it's working pretty well I think. I agree we should try to make YAML easier to read. I think the main tricky thing for users is the actions part configuration (you need to set it in the global space, then in the builder, then per builder...). So I agree with you, this should be improved. About Filters, like you, I think filters belongs to list builder. For now, if I'm not wrong, there is just one little bug to resolve about that (getting filters not from the list display parameter) and it should be fine... oh well... no... because if you want to customize filters form / options... OK, I'll try to think about that use case too on my side.

I'll be more precise, contribute more on that subject as soon as possible

[ioleo]

I've been thinking about this, and I've come up with an idea. We could split the configuration into several YAML files. Eg.:

• Model-generator.yml general config, switching certain features on/off (eg. "do not generate excel builder")
• Model-actions.yml actions configuration (the "avaliable" actions and their config)
• Model-fields.yml fields/columns configuration

An example of what it could look like:

# Model-generator.yml
generator:          admingenerator.generator.doctrine
model:              Acme\DemoBundle\Entity\Model
namespace_prefix:   Acme
concurrency_lock:   ~
bundle_name:        DemoBundle
i18n_catalog:       Model
pk_requirement:     \d+
builders:
    list: # enables builder with custom features on
        filters:    false
        sorting:    false
        pagination: false
        # this only decides which actions are used and in what order
        # the configuration is defined in "Model-actions.yml"
        generic_actions: [ 'custom1', 'new' ]
        object_actions: [ 'custom1', 'custom2', 'edit', 'show', 'delete' ]
        batch_actions: [ 'custom2', 'delete' ]
    new:      true # enables builder with default features on
    edit:     true
    show:     true
    actions:  true
    excel:    false # disables builder

# Model-actions.yml
generic:
    new: ~ # use the default configuration
    custom1:
        route: custom1genericRoute
        # etc...
object:
    custom1:
        inject_custom_defaults: true # use default custom action configuration for actions builder...
        label: "Custom 1 object action" # ... except the "label", which is explicitly set
    custom2: 
        inject_custom_defaults: true
    edit: ~ # use the default configuration
    show: ~ # use the default configuration
    delete: ~ # use the default configuration
batch_actions:
    custom2:
        inject_custom_defaults: true # use default custom action configuration for actions builder
    delete: ~ # use the default configuration

# Model-fields.yml
fields:
    createdAt:
        label: "Created at"
        dbType: datetime
        formType: s2a_datetime_picker
        filterType: s2a_datetime_picker
    name:
        label: "Name"
        dbType: text
        formType: text
        filterType: text
   # etc

What do you think?

[ksn135]

Just my $0,02...
IMHO, we should re-think _routing configuration_, may be make it more comfortable to developer. Let me explain myself. In legacy symfony 1.4 to to create new custom action someone needs to specify action name, label and may be optional icon. You don't need to specify route and route parameters (of course you CAN if you want to), new custom action in sf1.4 automatically generate route with parameter. May be we need something similar to this legacy behaviour? For example, automatically add routes for all custom actions via RoutingLoader class ?
When you add new custom action, then s2a automatically create route:

        'custom' => array(
                    'pattern'      => '/action/{pk}/custom',
                    'defaults'     => array(), 
                    'requirements' => array('pk' => '\d+'),
                ),

[ksn135]

And for me separation of ONE *-generator.yml per module to SEVERAL files is pure evil... )
Just imagine, I have now about 20 entities and 20 *-generator.yml files in one bundle.
When this quantity will become about 60 files... It's really too much...
We should think about move some configuration items (fields? action?) in ONE place, may be one(three?) global file(s?) with reasonable defaults ?

[ksn135]

Ought! What's about ability to include one YAML file from another ?
If someone (like @loostro) wants to have separated YAML config files then he should use some kind of include directive. And opposite if someone (like me) wants to use one big flat YAML config file per module – no problem, too.

[calvera]

I agree with @ksn135 that one file per entity is better than three... frankly speaking I like yaml files like they are 😃

dumb idea: did you think about using annotations? The cons are that admin annotations are not separated from non-admin and too much work for implementation...

[ioleo]

Another idea:

• add support for Model-generator.php files (which must have getConfig() method returning an array)
• deprecate (but not remove!) Model-generator.yml config
• add "convert YAML config to PHP config" command

The cacheWarmer finds:

• all -generator.yml files
• all -generator.php files
• it reads the params.model (eg. Acme\DemoBundle\Entity\Post)
• if it finds both PHP and YAML config for the same model, it will use PHP config
• for PHP config -> it will inject container service and run getConfig
• for YAML config -> it will use Yaml::parse to get the config
• then the config is validated useing Symfony2 config component
• the API (config structure) does not change

This would have various benefits:

• the API does not change
• there is only one config file (PHP or YAML) per Model
• it's fully backwards compatible (the YAML config is still supported)
• for advanced configurations (where you want to merge/unset elements of global config or you need some service like Translator, or Routeing) you can convert your YAML to PHP config, then "customize it"
• if the PHP config proves to be better, we can deprecate YAML config (and remove it in future versions)
• smoother transition from legacy bundle to new-unstable bundle

What do you think?

[ioleo]

Example ModelGenerator.php:

<?php

namespace Acme\DemoBundle\Resources\config;
// actually we should probably put it in a diffrent directory...

use Symfony\Component\DependencyInjection\ContainerAware;

class ModelGenerator extends ContainerAware
{
    const MODEL = 'Acme\DemoBundle\Entity\Model';

    public function getConfig()
    {
        return array(
            'generator'        => 'admingenerator.generator.doctrine',
            'model'            => self::MODEL,
            'namespace_prefix' => 'Acme',
            'bundle_name'      => 'DemoBundle',
            'concurrency_lock' => null,
            'i18n_catalog'     => 'Model',
            'pk_requirement'   => '\d+',
            'builders'         => array(
                'list' => array( /* list config */ )
                // other builders...
            )
        );
    }
}

The power of useing PHP is obvious.. you can define getObjectActions method.. and then, for list use "object_actions" => $this->getObjectActions(), while for edit you can use:

<?php

public function getObjectActionsForEdit()
{
    $objectActions = $this->getObjectActions();
    unset($objectActions['delete']);
    return $objectActions;
}

[ioleo]

/cc @sescandell @ksn135 @calvera

[ksn135]

@loostro I'm personally prefer plain YAML. But your idea is seems good to me until it will continue to support old yaml config. Why you wanna use php config to call function like getObjectActionsForEdit?
I think that it's unnecessary. In all my cases my custom object actions are always generated, but in UI some of them are removed in twig template if needed. Your idea can be implemented right now with the same logic. And even more we can call function to decide show or not action button on per object basis and put some business logic in it. For example: function canDisplayAction( $actionName, $object ). This function reside in controller class and called from twig template before render action button.
What do you think about it ?

[calvera]

I agree with @ksn135 ...

[ioleo]

@ksn135 You may want to display some object actions only on edit or list view.

Your idea can be implemented right now with the same logic. And even more we can call function to decide show or not action button on per object basis and put some business logic in it. For example: function canDisplayAction( $actionName, $object ). This function reside in controller class and called from twig template before render action button.

Useing the credentials for object_action already allows decideing if (currently logged) user can see/perform action (for given object).

But this does not take into account scenarios where you want:

• on list, the order of action to be edit, show, delete, custom1, custom2
• on edit, the order of action to be custom1, show, delete, custom2 (note: removed edit action and changed order)
• on show the order of action to be delete, custom1, custom2, edit

Ofcourse, we could introduce some code that automatically removes edit object action in edit builder and show object action in show builder, and edit, show, delete object actions in actions builder... but this would still not allow you to customize the order.

Another scenario

What if you have 20 custom object actions custom1, custom2, custom3, .... , custom20 and all of them share the same logic? Eg. all of them call external routes in Google Analytics with some custom params?

Then you need to repeat the same config 20 times in your YAML file. A PHP class however, would allow you to create a method getCustomConfig() and you could:

<?php
public function getConfig()
{
    return array(
        `params` => array(
            'object_actions' => array(
                'custom1' => array_merge($this->getCustomConfig(), array('param1' => 'Model.id'),
                'custom2' => array_merge($this->getCustomConfig(), array('param2' => 'Model.id'),
                'custom3' => array_merge($this->getCustomConfig(), array('param3' => 'Model.id'),
                // ...
                'custom20' => array_merge($this->getCustomConfig(), array('param20' => 'Model.id'),
            )
        )
    );
}

yet another scenario

What if you have the same action/logic in 50 entities in your project? You have to repeat the same config in 50 YAML files. The PHP class however, allows you to create a special service/class for that and call it:

<?php

use Acme/DemoBundle/SomeCommonConfigClass;

public function getConfig()
{
    return array(
        `params` => array_merge(SomeCommonConfigClass::getComminConfig(), array(
            // only define non-common stuff
        )),
    );
}

[ioleo]

@ksn135 @calvera You may not need it now.. (or ever), becouse your projects are simple and do not have much repetitive configuration, but introduceing PHP would enhance current features (for some projects) and also it would allow us to remove all the dirty hacks introduced... becouse of YAML shortcomings.

The 'yaml to php' config converter and a deprecation period (with both PHP and YAML supported) would allow you to painlessly upgrade. I strongly believe this could benefit the project.

[ksn135]

@loostro My projects aren't simple. I'm using a different approach. For example, from my current project, I have an approval business process in plain YAML-file with more then 15 custom action. No one of custom actions resides in *-generator.yml. I can understand that if you need to describe in YAML more then 20+ custom actions then it is very uncomfortable and annoying. I'm really trying to understand why you need so much custom actions and why you describe all of them in *-generator.yml ? Why ?
My configuration for Contract entity business process with three branching:

[ioleo]

@ksn135 simple anwser is: I'm not useing FSM. I'll have a look at that, as it might be useful for my future projects.

I'm not going to force my solutions, and your comments make me rethink these problems :)

I'll list them seperately:

• automatically remove edit/show object action in Edit/Show builder
• remove this hack and find a diffrent solution to the problem
• disable / do not generate builder (if it's empty, eg: builder.list: ~ or not present in yaml file at all)
• generate routes for custom actions (see @ksn135 comment above)
• replace the inject_object_default trigger with a seperate param inject_default: object
• replace the inject_batch_default trigger with a seperate param inject_default: batch

[sescandell]

Hi there,

Sorry for the delay.

If I have to make it short, one more time I fully understand all advantages to move on PHP configuration (especially all comments about DRY). But, on the other hand, I really think that the strength of this bundle is to be based on simple YAML files. Ok, this part is not easy: but it's transparent to the user. It's up to us to make things clearer and easier to maintain (thanks to the Config component?).

I'm surprised about some things said here.
For example:

But this does not take into account scenarios where you want:

• on list, the order of action to be edit, show, delete, custom1, custom2
• on edit, the order of action to be custom1, show, delete, custom2 (note: removed edit action and changed order)
• on show the order of action to be delete, custom1, custom2, edit

I don't understand... this is already possible. edit/show actions are already not added on edit/show actions (at least, it was the case before)

disable / do not generate builder

I tought it is already the case... but ok, this is more like an issue than anything to change regardng YAML or not

generate routes for custom actions (see @ksn135 comment above)

I don't get it here, what's the issue?

• replace the inject_object_default trigger with a seperate param inject_default: object
• replace the inject_batch_default trigger with a seperate param inject_default: batch

This was not present in legacy bundle. If I well understand, the purpose of these is to automatically inject object_actions, right? Is it really necessary?

If I well understand the initial message, one main issue is managing actions. And especially the fact that we usually copy/paste some things. What if actually the main issue is more about documentation and best practices than configuration itself. I've made several projects with this bundle (well, the legacy one) and with very different logic about actions. Some of them depending on the objet, some on the user, some on both of them and so on. And, finally, it's more a question of organization then copy/past/repeat things. I think the "credential" functionnality is more powerfull than people can imagine (and that's why the demo bundle and documentation are very important). Actually, for now, I don't really have any real case that tell me "ok, we absolutely need to change this".

For example you said:

What if you have 20 custom object actions custom1, custom2, custom3, .... , custom20 and all of them share the same logic? Eg. all of them call external routes in Google Analytics with some custom params?

But for me, you don't have to use custom actions for that but more to overwrite Twig object_actions block (for this example, I agree for some it could be real object actions).

For me, the main thing we should change is the actions builder part. I think it is very borring to have to define the entry actions into builders whereas we should be able to find them from the YAML file (borring and source of issues sometimes).

But this doesn't make things moving. So, keeping the maximum of things into the YAML config file, what does really need to be changed for 2.0 version?

IMO:

• filters: customizing form filters on the list builder
• documentation