fix: ORM filters can use doctrine expressions in join conditions

[mrossard]

Q	A
Branch?	3.1
Tickets	#5648

A simple fix to force the joinCondition to a string before using it in preg_replace.

The related issue actually happens when the query also has a where part (hence the "andWhere('1=1')" in the test), not when there is another SearchFilter. Investigating why this code involving the join condition is not triggered when there is no where may be a good idea too, I didn't dig that far into it.

[dunglas]

Thanks @mrossard!

[mrossard]

i mixed the commit messages with my other open PR in the version you merged, sorry!

[dunglas]

It looks like this patch breaks a test: https://github.com/api-platform/core/actions/runs/5612545684/job/15206541377

[mrossard]

It looks like this patch breaks a test: https://github.com/api-platform/core/actions/runs/5612545684/job/15206541377

Looks like i was really doing too many things at the same time yesterday, sorry. That's actually the test i added to test the fix...i'll get back to it. Is it possible to reopen this PR / revert the bad commit?

[mrossard]

I don't get it, on my machine the test passes by itself, either on php 8.1 or 8.2 :

$ git status
On branch 3.1
Your branch is up to date with 'origin/3.1'.
$ ./vendor/bin/behat --profile=default --stop-on-failure -vvv --format=progress --name="Custom search filters can use Doctrine Expressions as join conditions"
.....

1 scenario (1 passed)
5 steps (5 passed)
0m2.95s (53.77Mb)

Am I missing something obvious?

[soyuka]

Why are we mixing entities and resources when it's not needed? I'd have liked to check this use case as I don't think it's valid, or it'd need a provider and custom queries. Anyways this will probably change when we refactor filters so I think it's okay (if we fix the broken test)

[mrossard]

Why are we mixing entities and resources when it's not needed? I'd have liked to check this use case as I don't think it's valid, or it'd need a provider and custom queries

I don't get what you mean here, is it that I used a separate resource that actually mirrors the entity? That was really just to keep the test simple. My real world use for this was obviously more complicated.

. Anyways this will probably change when we refactor filters so I think it's okay (if we fix the broken test)

The test isn't broken anymore, it only failed because of a regression introduced by @dunglas in a merge - he fixed that today here: 24133df .