[Autocomplete] Don't add WHERE IN criteria without params #561
Conversation
norkunas
commented
Nov 25, 2022
norkunas
commented
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| Tickets | Fix #549 |
| License | MIT |
| if ($params) { | ||
| $queryBuilder | ||
| ->where(sprintf("o.$idField IN (%s)", implode(', ', array_keys($params)))) | ||
| ->setParameters(new ArrayCollection($params)); |
There was a problem hiding this comment.
My two cents, using ->setParameters() will override the previous bound parameters (if any) to the QueryBuilder.
IMO it would be preferable to use ->setParameter() for each $params, to be future-proof.
There was a problem hiding this comment.
There can't be any previous parameters, because the query is constructed 4 lines before :)
There was a problem hiding this comment.
For the moment yes, but we can't predict the future.
There was a problem hiding this comment.
Imho they could be added to $params,no? :)
|
@norkunas Great fix. Would you mind adding a testcase? |
| ->getResult(); | ||
| $queryBuilder = $repository->createQueryBuilder('o'); | ||
|
|
||
| if ($params) { |
There was a problem hiding this comment.
Why don't just check if $data['autocomplete'] is [] and skip this else part?
Btw this can be simplified a lot like $repository->createQueryBuilder('o')->->where(sprintf('o.%s IN (:ids)', $idField)))->setParameter('ids', $data['autocomplete'])->getQuery()->getResult(), doctrine will handle the rest, doesn't it?
There was a problem hiding this comment.
No it won't handle the types properly,that's what I've fixed before.
norkunas
force-pushed
the
fix-autocomplete
branch
from
4dec440 to
15a3bf6
Compare
testcase added. |
norkunas
force-pushed
the
fix-autocomplete
branch
from
15a3bf6 to
d581ea7
Compare
|
Awesome, thank you Tomas! |
