whiteoctober / WhiteOctoberAdminBundle

A different take on an AdminBundle for Symfony2
MIT License
79 stars 14 forks source link

Bad closure var names in UpdateAction #17

Closed weaverryan closed 13 years ago

weaverryan commented 13 years ago

Hey guys-

This fixes an exception, though I don't pretend to understand the architecture yet :).

I am a little bit confused overall (granted, this is feedback based on very initial work with the bundle) about how different actions extend each other and how these closures work. Specifically, where does this findDataByIdClosure come from? Why do I see a method in the ListAction only called getFindDataByIdClosure()? Are those related? Should I have access to that somehow from UpdateAction so I'm calling a method and not a var name?

Thanks guys!

pablodip commented 13 years ago

Hi Ryan,

Very glad seeing you trying the bundle and giving us feedback, that's really appreciated :)

The architecture is not completely defined, so don't worry saying things are really bad and proposing things to change, really I would love it ;)

Well, in the bundle there are two parts, the part that provides an architecture to build any admin bundle or code using actions on the top of it and the part that is the admin. Both are related of course, because what we are trying to get here is an admin, although you can build another reusable things apart from an admin.

I was looking for a completely reusable architecture to be able to be able to use different data managers (Doctrine, Mandango, Propel), to extend the admins easily with actions and to customize them as much as possible.

So we have admin classes to configure, and being able to extend them we are able to reuse configuration. We have actions in different classes using them in the way than a normal controller and been able to customize them.

Currently there are two parts of the architecture that I would really like to improve, the admins identification and dependences and the actions dependences and configuration. The first one is to be able to relate admins, thus we can work in a really good way with associations. The second one is to improve the parts you have mentioned.

I added base actions for the admin actions to have the same configuration and not repeat code between the different data managers.

We need a way to relate actions and share configuration, and what I added at the moment is a shared parameter bag called actionVars. I added there different closures there to customize them in the data managers and be able to reuse in different actions added to the base admin, like $findDataById, which is quite common if you add new actions.

We need also a better way for the options in the actions, options that you can just change the value and option "collections" to add values to process all of them later.

So like you can see we have to improve things, that's why help and ideas are more than welcome. I think the base of the architecture is really good, and improving it solving this issues we can build a really good things on the top of it.

I also thought about adding "extensions" to reuse different things in the admins, for instance when you want parent and child.

Pablo

johnwards commented 13 years ago

Nice long reply Pablo, I'll try and digest it on my computer in the morning, too long for reading on my phone!

However, what about Ryans pull? Good to go? You know how much I like merging pulls via my phone!

willdurand commented 13 years ago

It should heavily be merged, it doesn't work without...

pablodip commented 13 years ago

Oops, merged. But let's see if we continue the discussion here or anywhere :)