Open ioleo opened 9 years ago
/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.
Hi @loostro
Don't have time to make something more precise for now, so very quickly:
I'll be more precise, contribute more on that subject as soon as possible
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 configurationAn 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?
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+'),
),
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 ?
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.
I agree with @ksn135 that one file per entity is better than three... frankly speaking I like yaml files like they are :smiley:
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...
Another idea:
Model-generator.php
files (which must have getConfig()
method returning an array)Model-generator.yml
configThe cacheWarmer finds:
-generator.yml
files-generator.php
filesparams.model
(eg. Acme\DemoBundle\Entity\Post
)getConfig
Yaml::parse
to get the configThis would have various benefits:
What do you think?
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;
}
/cc @sescandell @ksn135 @calvera
@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 ?
I agree with @ksn135 ...
@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:
edit, show, delete, custom1, custom2
custom1, show, delete, custom2
(note: removed edit action and changed order)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.
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'),
)
)
);
}
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
)),
);
}
@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.
@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:
@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:
edit/show
object action in Edit/Show builderbuilder.list: ~
or not present in yaml file at all)inject_object_default
trigger with a seperate param inject_default: object
inject_batch_default
trigger with a seperate param inject_default: batch
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:
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:
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 theedit
action. On Action builder I wanted onlycustom1
andcustom2
(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 buildercustom1, custom2
.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:
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.