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

IBX-7236: Added support of % on both sides of a 'like' statement in fulltext criterion #298

Closed
wants to merge 7 commits into from

Conversation

kisztof
Copy link
Contributor

@kisztof kisztof commented Nov 29, 2023

Question Answer
JIRA issue IBX-7236
Type improvement
Target Ibexa version v4.6
BC breaks no

This PR adds support of % on both sides of like statement in fulltext criterion

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Checked that target branch is set correctly (main for features, the oldest supported for bugs).
  • Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
  • Asked for a review (ping @ibexa/engineering).

@kisztof kisztof changed the title IBX-7236: Added support of % on both sides of like statement in fulltext criterion IBX-7236: Added support of % on both sides of a 'like' statement in fulltext criterion Nov 29, 2023
Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see some integration coverage for this use case. Looks rather complicated, even though it's just ~8 LOC.

@alongosz alongosz requested a review from a team November 29, 2023 13:28
@webhdx webhdx requested a review from a team December 1, 2023 11:21
Comment on lines +168 to +171
[
'*row*',
[[2, 5, 8, 9, 11, 12, 14, 15]],
],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a closer look this test doesn't test anything related to the changes. See that this entire test case is meant for advanced search capabilities. LSE does not offer them so the test gets skipped.

The test can stay to ensure feature parity (we want this to work on Solr and it seems it's doing that OOTB), but we need dedicated test testing that FullText criterion used on LSE is working too, since you're changing LSE behavior.

Prepare a content with some TextLine (ezstring) containing phrases and add several cases (negative and positive ones). Think it through - what you expect from this feature, what is not desired?

@kisztof kisztof force-pushed the ibx-7236-support-like-expression-for-fulltext branch from 0dcb48a to 9aca6a1 Compare December 19, 2023 13:44
Copy link

sonarcloud bot commented Dec 19, 2023

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@adamwojs
Copy link
Member

Closing as obsolete

@adamwojs adamwojs closed this Jun 11, 2024
@adamwojs adamwojs deleted the ibx-7236-support-like-expression-for-fulltext branch June 11, 2024 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants