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

Search for fact-checks by multiple keywords regardless the order. #2012

Merged
merged 5 commits into from
Aug 28, 2024

Conversation

caiosba
Copy link
Contributor

@caiosba caiosba commented Aug 28, 2024

Description

Search for fact-checks by multiple keywords regardless the order.

Fixes: CV2-5129.

How has this been tested?

Unit test added.

Things to pay attention to during code review

May not work for all languages.

Checklist

  • I have performed a self-review of my own code
  • I have added unit and feature tests, if the PR implements a new feature or otherwise would benefit from additional testing
  • I have added regression tests, if the PR fixes a bug
  • I have added logging, exception reporting, and custom tracing with any additional information required for debugging
  • I considered secure coding practices when writing this code. Any security concerns are noted above.
  • I have commented my code in hard-to-understand areas, if any
  • I have made needed changes to the README
  • My changes generate no new warnings
  • If I added a third party module, I included a rationale for doing so and followed our current guidelines

@@ -535,7 +535,11 @@ def filtered_fact_checks(filters = {})
query = query.where('fact_checks.report_status' => filters[:report_status].to_a.map(&:to_s)) unless filters[:report_status].blank?

# Filter by text
query = query.where('(fact_checks.title ILIKE ? OR fact_checks.url ILIKE ? OR fact_checks.summary ILIKE ?)', *["%#{filters[:text]}%"]*3) if filters[:text].to_s.size > 2
if filters[:text].to_s.size > 2
tsquery = Team.sanitize_sql_array(["to_tsquery(?)", filters[:text].split(/\s+/).map(&:strip).join(' & ')]) # FIXME: May not work for all languages
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if plainto_tsquery is any better for other languages? It does seem simpler, however as we don't have to split the query: https://www.postgresql.org/docs/current/textsearch-controls.html

Copy link
Contributor

Choose a reason for hiding this comment

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

Or maybe even better is websearch_to_tsquery:

websearch_to_tsquery creates a tsquery value from querytext using an alternative syntax in which simple unformatted text is a valid query. Unlike plainto_tsquery and phraseto_tsquery, it also recognizes certain operators. Moreover, this function will never raise syntax errors, which makes it possible to use raw user-supplied input for search. The following syntax is supported:

Copy link
Contributor

Choose a reason for hiding this comment

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

yes @computermacgyver websearch_to_tsquery is better I used it and verified that everything works as expected

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@melsawy , since we're using websearch_to_tsquery, do we still need to split the query? It's better if we don't need to.

@melsawy melsawy marked this pull request as ready for review August 28, 2024 10:13
@@ -495,7 +495,11 @@ def filtered_explainers(filters = {})
query = query.where(updated_at: Range.new(*format_times_search_range_filter(JSON.parse(filters[:updated_at]), nil))) unless filters[:updated_at].blank?

# Filter by text
query = query.where('(title ILIKE ? OR url ILIKE ? OR description ILIKE ?)', *["%#{filters[:text]}%"]*3) if filters[:text].to_s.size > 2
if filters[:text].to_s.size > 2
tsquery = Team.sanitize_sql_array(["websearch_to_tsquery(?)", filters[:text].split(/\s+/).map(&:strip).join(' & ')]) # FIXME: May not work for all languages
Copy link
Contributor

Choose a reason for hiding this comment

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

My reading of the websearch_to_tsquery documentation is that we can simply use:
tsquery = Team.sanitize_sql_array(["websearch_to_tsquery(?)", filters[:text]])
and do not need to split, strip, and join. I've tried the unit test with this alternative and it passes. I'm building the full app to try it end-to-end.

@melsawy melsawy merged commit 0598a50 into develop Aug 28, 2024
10 checks passed
@melsawy melsawy deleted the fix/CV2-5129-choose-fact-check branch August 28, 2024 11:09
melsawy added a commit that referenced this pull request Aug 28, 2024
)

* Search by multiple keywords regardless the order.

Fixes: CV2-5129.

* CV2-5129: apply same search for Explainer

* CV2-5129: apply PR comment

* No need to split, strip, and join with websearch_to_tsquery

* CV2-5129: fix CC

---------

Co-authored-by: Sawy <[email protected]>
Co-authored-by: computermacgyver <computermacgyver>
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.

3 participants