Skip to content
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

Fix empty query term conditions #6227

Conversation

guillaume-sainthillier
Copy link
Contributor

@guillaume-sainthillier guillaume-sainthillier commented Mar 25, 2024

When only numeric fields are searchable, EntityRepository::addSearchClause generates a bugged DQL when search contains only non numeric terms.

@Seb33300
Copy link
Contributor

Seb33300 commented Apr 17, 2024

@javiereguiluz This PR is the correct fix for #6223

#6242 must be reverted (it breaks postgres) and replaced by this PR. See details in #6262

@Seb33300
Copy link
Contributor

@guillaume-sainthillier could you commit revert of #6242 in this PR?

@guillaume-sainthillier
Copy link
Contributor Author

@guillaume-sainthillier could you commit revert of #6242 in this PR?

done

hellomedia referenced this pull request Apr 20, 2024
This PR was merged into the 4.x branch.

Discussion
----------

Fixed invalid DQL

Fixes #6223

When searching text having only integer fields, an invalid DQL like `SELECT entity FROM App\Entity\Product entity WHERE ` is generated because there are no any `$queryTermConditions`.  This PR should fix the issue.

Commits
-------

6511215 Fixed invalid DQL
@guillaume-sainthillier
Copy link
Contributor Author

Hello @Seb33300, any updates about this PR? Does I need to add something?

@Seb33300
Copy link
Contributor

Hi @guillaume-sainthillier, I just tried to help by reviewing it but I am not a maintainer of the project.
We need to wait for @javiereguiluz to review and merge the PR.

@guillaume-sainthillier
Copy link
Contributor Author

guillaume-sainthillier commented Jun 26, 2024

Hello @javiereguiluz, any news about this PR? Does I need to add something to get this PR merged?

phpstan error is not related to PR changes

@javiereguiluz javiereguiluz added this to the 4.x milestone Oct 19, 2024
@javiereguiluz
Copy link
Collaborator

Thanks a lot, Guillaume!

I'm sorry this PR fell off my radar and wasn't merged earlier.

@javiereguiluz javiereguiluz merged commit beeb804 into EasyCorp:4.x Oct 19, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants