phoenix
phoenix copied to clipboard
Add typed_fields option to ORM configuration to configure TypedFieldMapper
This fixes #1696
Configuration syntax is based on the suggestion by @nicolas-grekas here
Love to hear your thoughts, although I'm not an expert in this area.
This is a good starting point, but I want to highlight that this doesn't support custom type field mappers. I'm afraid if we did this, in few months someone will complain they can't define custom mapper through configuration. Any suggestions to avoid that? Other than that, this PR is completely missing tests of course. And DefaultTypeFieldMapper service should be defined in orm.xml file. DoctrineExtension will then only reference to it.
I see your point. This would be the easiest for users to configure, but not the most flexible.
Another solution I see is allowing one to simply set a TypedFieldMapper service.
dbal:
orm:
typed_field_mapper: '@Doctrine\ORM\Mapping\DefaultTypedFieldMapper'
Which then needs to be configured in services.yaml
If that's the preferred way, I can work on that (including tests)
Let's go with this: For now we don't need support for something else than DefaultTypedFieldMapper. In future if there is a need for custom field mapper, we will add another key (like the one you proposed) and disallow combining it with typed_fields keys in this PR. So this PR is quite feature complete (at least for now), but please add tests.
Hi, this issue is open for over a year and is blocking the rest of the 2.14.0 release.
Is there a way we could move forwards with it, or maybe move it to 2.15.0 (or later) milestone?
As can be seen by my comment and PR label, tests are needed to be added
As can be seen by my comment and PR label, tests are needed to be added
@tijnema any chance you could work on that soon?
@ostrolucky could we move this issue to 2.15.0 for now and release 2.14.0 sooner?
For my current project we don't really need this anymore. Not sure if this PR is still relevant actually. Isn't everything needed implemented with #1802 ?
Indeed, so I guess this is a duplicate!