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
RFC: Filters do not respect the configured name converter. #2090
Comments
I would be in favor of fixing this. It looks totally unexpected on my side. If someone complains... we'll provide a fix like a flag or something like that to restore the buggy behavior. WDYT? |
Cool, while obviously a bug, I didn't expect a fix to be prioritized over the BC break. happy with that! |
Most of us are using the default name converter, therefore nothing will change. |
Was it ever claimed that the filters would use the name converter? 😛 I don't know if it could be considered a bug if it's a missing feature... |
Sorry to have opened a duplicate of this issue. |
@dunglas To avoid BC break, we could provide a |
I really need this fix and would like to help making it real, but I don't even know where to start. Could you please give us some clues? Thanks! |
To me it’s a bug fix, so don’t bother with BC because the current behavior is not correct (this case has just been forgotten). A PR would be very appreciated, just target master instead of 2.3 to limit the consequences in case someone was relying on the buggy behavior. |
@dunglas Do you have any clues to give about how to fix it? |
@soullivaneuh Quick and dirty but I guess a start would be master...antograssiot:nameconverter-filter But it would be a BC break I guess to add an argument in the middle of the constructor, and adding it in the last position, would break definitions like app.my_dummy_resource.date_filter:
parent: 'api_platform.doctrine.orm.date_filter'
arguments: [ { 'dummyDate': ~, 'relatedDummy.dummyDate': ~, 'embeddedDummy.dummyDate': ~ } ]
tags: [ { name: 'api_platform.filter', id: 'my_dummy.date' } ] as we rely on properties being the last arg It needs to be tested more deeply and I havent put effort in the SearchFilter yet, if it is considered as BC break. |
@antograssiot I think that your changes will do the trick because we use
To me it's just a missing feature, filter properties are not documented as converted but the same as the property names. I definitely agree that this is inconsistent though. |
@antograssiot on your POC, you use the name converter on each filter Why not leave this like that and use the name converter only on the documentation normalizer and the filter request extraction? This will be also better for eventually other supports than Doctrine ORM. |
I didn't think a lot about the issue @soullivaneuh , I just quickly update with the first (not best) idea I had to see if it would work.. The discalmer was |
@antograssiot Ha ha right ! 😉 I tried an another approach: #2377 |
think this is done! |
Hey all,
Here's an issue I discovered today.
If your API has a configured name converter, (let's say
CamelCaseToSnakeCaseNameConverter
), filtering on fields is unintuitive for the consumer.Given an ApiResource with a field called
createdAt
,createdAt
will be serialized/deserialized to/from the consumer ascreated_at
.However, if the consumer wants to then filter on
createdAt
, they must use?createdAt...
, which is very unintuitive as that is not how the field is represented.The fix seems simple - make all filters respect the configured name converter.
However, this would be a big BC break, so will need to go in 3.0?
Cheers
The text was updated successfully, but these errors were encountered: