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

Improve entity_field Twig_Function for entities with a custom primary key #89

Closed
ogizanagi opened this issue Jan 29, 2015 · 5 comments
Closed
Labels

Comments

@ogizanagi
Copy link
Contributor

Found a minor issue with the new feature (#80) I didn't pick up before: https://github.com/javiereguiluz/EasyAdminBundle/blob/master/Twig/EasyAdminTwigExtension.php#L97
If an entity defines a custom primary key not named id, no link will be generated as the the method getIdprobably doesn't exist.

As we currently have no way to access the entity primary_key_field_name in there, the easiest workaround is to always define a getId method on entities with a custom pk name.

Maybe this should be a non-fix ?

@Pierstoval
Copy link
Contributor

Can't we inject the configurator inside this twig extension and check the entity config in order to retrieve the primary key value ?

@ogizanagi
Copy link
Contributor Author

Yes, that's the main solution.
I opened this issue because I wonder if this worth the dependency, as the workaround is pretty easy, and a getId method on an entity isn't a non-sense anyway.

@Pierstoval
Copy link
Contributor

I think that injecting the configurator in the twig extension might be right, as it's also probably the best way to get the whole entities list and make it a "view-accessible" property, instead of passing it as parameter from the controller (which is kind boring when extending the AdminController).

Sounds good to me to add this dependency to the twig extension :)

tag @javiereguiluz

@javiereguiluz
Copy link
Collaborator

I'm going to think about this issue because it looks wrong to provide support for non-id primary keys and then, don't make this little extra effort to provide cool links for associations.

ogizanagi added a commit to ogizanagi/EasyAdminBundle that referenced this issue Jan 29, 2015
…n if not named `id`

Inject the Configurator into the twig extension in order to call proper getter for association entities primary key.
We might have injected the Doctrine ObjectManager instead, in order to get the primary key name, but then we'd have been unable to know if the target entity is configured in easy_admin.entities.
Indeed, in addition, we only provide a link to the target entity if the EasyAdminBundle can handle it.
@ogizanagi
Copy link
Contributor Author

Did not make a PR for now, but here is what was ready to be done considering this issue.
Maybe it'll be helpful.

I'm not fan of the try/catch, but simple way to keep something to display if the entity isn't configured to be used in the bundle, and avoiding generating broken link as currently.

I think it would be helpful to handle the fact the entity isn't present in the bundle configuration in the getEntityConfiguration method. (returning null and avoid triggering exceptions)

@javiereguiluz javiereguiluz changed the title Feature #80: entity_field Twig_Function and association with custom id Improve entity_field Twig_Function for entities with a custom primary key Jan 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants