-
-
Notifications
You must be signed in to change notification settings - Fork 949
Fix placeholder on exact and partial filters #7547
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
base: 4.2
Are you sure you want to change the base?
Fix placeholder on exact and partial filters #7547
Conversation
| use PHPUnit\Framework\Attributes\DataProvider; | ||
|
|
||
| /** | ||
| * @author Votre Nom <votre@email.com> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to replace it with your name and email
| } | ||
|
|
||
| if (':property' === $key) { | ||
| if (str_contains($key, ':property')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But then the handling of properties aware filters is skipped (see at line 150), by the continue (line 145).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed we should just merge the below branch in this code
if (str_contains($key, ':property') || ((($f = $parameter->getFilter()) && is_a($f, PropertiesAwareInterface::class, true)) || $parameter instanceof PropertiesAwareInterface)) {
$p = [];
foreach ($propertyNames as $prop) {
$p[$this->nameConverter?->denormalize($prop) ?? $prop] = $prop;
}
$parameter = $parameter->withExtraProperties($parameter->getExtraProperties() + ['_properties' => $p]);
}
| properties: ['description', 'name'] | ||
| ), | ||
| ] | ||
| )] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add one to the https://github.com/api-platform/core/blob/main/tests/Fixtures/TestBundle/Entity/SearchFilterParameter.php class?
| $this->loadFixtures(); | ||
| } | ||
|
|
||
| #[DataProvider('partialFilterParameterProvider')] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add these tests to the DoctrineTest file
| } | ||
|
|
||
| if (':property' === $key) { | ||
| if (str_contains($key, ':property')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed we should just merge the below branch in this code
if (str_contains($key, ':property') || ((($f = $parameter->getFilter()) && is_a($f, PropertiesAwareInterface::class, true)) || $parameter instanceof PropertiesAwareInterface)) {
$p = [];
foreach ($propertyNames as $prop) {
$p[$this->nameConverter?->denormalize($prop) ?? $prop] = $prop;
}
$parameter = $parameter->withExtraProperties($parameter->getExtraProperties() + ['_properties' => $p]);
}
|
@soyuka if @NathanPesneau isn't moving this along, could you do that instead? |
|
I'm indeed already working on this (haven't published anything yet). This week I'll have to be afk for a few days as I need to help dismantling a yurt where I live and it's a high priority. Anyways this is my top priority (had to push symfony 8 first). |
Completes #7478