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

Proposal: Validate config keys #1667

Closed
weaverryan opened this issue Jun 7, 2017 · 3 comments
Closed

Proposal: Validate config keys #1667

weaverryan opened this issue Jun 7, 2017 · 3 comments

Comments

@weaverryan
Copy link
Contributor

Hi there!

Basically, some of the admin config uses the prototype('variable') type in the Configuration class, so the bundle doesn't catch types. For example, beneath an entity, I typed contorller. Instead of an error, it just didn't see my controller :).

To keep with the entities section, the valid config keys below this are a known list, correct? Could we update the Configuration class for these?

Thanks!

@javiereguiluz
Copy link
Collaborator

I like this ... but I don't know if it's possible to do it with the Config component.

Basically, the problem to solve is: I want to define a config node (easy_admin.entities) which allows any number of subnodes (User, Product, Category, etc.) which in turn allow any config option of any type ... but, if some specific options are used (controller, class, label, etc.) I want to validate those.

I remember trying to solve this in the past and I couldn't. Symfony didn't like this idea: "this config allows total freedom ... except when using certain options, which will be strictly validated".

@weaverryan
Copy link
Contributor Author

I would honestly do it in the EasyAdminExtension. Basically, do as much validation as possible on Configuration, but ultimately, you can manually add if checks in EasyAdminExtension and throw exceptions there. The Configuration class was originally created to help remove the many if checks in the extension classes. If it can't do something, I think it makes sense to manually add it.

@javiereguiluz
Copy link
Collaborator

I'm closing this issue because we're starting a new phase in the history of this bundle (see #2059). We've moved it into a new GitHub organization and we need to start from scratch: no past issues, no pending pull requests, etc.

I understand if you are angry or disappointed by this, but we really need to "reset" everything in order to reignite the development of this bundle.

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

No branches or pull requests

2 participants