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

Circulars: Implement query_string method with fallback #2930

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tylerbarna
Copy link
Member

@tylerbarna tylerbarna commented Mar 5, 2025

Description

This changes the search method to query_string rather than simple_query_string. The former addresses all of the use cases outlined in #2288, but there is the drawback of it not being as tolerant to incorrect syntax as the latter. As a result, I implemented a fallback where opensearch switches to multi_match if the query is invalid for query_string.

Note: this is still behind the CIRCULARS_LUCENE feature flag

Related Issue(s)

Resolves #2288

Testing

  1. Navigate to the circulars archive
  2. Submit a standard search for something like LIGO. This should return the results as expected.
  3. Submit a lucene search for something like subject:LIGO AND subject:Swift . The only results should include circulars with both "LIGO" and "Swift" in the subject.
  4. Submit a lucene search for something like submitter:LIGO. This should only return a couple of circulars where the submitter has their LIGO association included.
  5. Submit a lucene search for something like submitter:LIGO AND. Rather than failing due to the incorrect lucene syntax, it should fall back onto the multi_match method and show results different from the previous test.

switch to query_string addresses needs for searching. That being said, this also exposes users to more oppurtunities for raising errors, so it will need to be tuned so that users are either made aware of this and have the oppurtunity to toggle it off, or we have something that automatically falls back to multi-match in the case of failure
this ensures that, if a user submits a query that would raise an error with the query_string search method, the existing multi_match method is used as a fallback. The current queryObj constant is somewhat redundant with the addition of this fallback, but this will likely be removed once the feature flag is removed.
Copy link

codecov bot commented Mar 5, 2025

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Project coverage is 6.09%. Comparing base (6a9c6a9) to head (bc79c55).
Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
app/routes/circulars/circulars.server.ts 0.00% 7 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##            main   #2930      +/-   ##
========================================
+ Coverage   5.97%   6.09%   +0.11%     
========================================
  Files        171     175       +4     
  Lines       4334    4351      +17     
  Branches     476     474       -2     
========================================
+ Hits         259     265       +6     
- Misses      4073    4084      +11     
  Partials       2       2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

Circulars: add support for Lucene-style search queries in archive
1 participant