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

Adds support for AND, OR, custom keyword matching #811

Merged
merged 2 commits into from
Apr 19, 2024

Conversation

JPrevost
Copy link
Member

@JPrevost JPrevost commented Apr 19, 2024

NOTE: while you can use the GraphQL playground to review these changes, there is also an experimental branch of TIMDEX-UI deployed and configured to use this branch that may also be helpful to more easily see the effects of these proposed changes.

Why are these changes being introduced:

  • TIMDEX currently ORs all keyword searches together which leads to a lot of search results, many of which are not relevant (to the extreme of sometimes having no records returned that have all the terms which is sometimes the expectation)

Relevant ticket(s):

How does this address that need:

  • Leverages OpenSearch minimum_should_match feature along with a GraphQL feature to choose a preconfigured value or supply your own

Document any side effects to this change:

  • once we get feedback from stakeholders, it is likely we'll want to adjust the options available, document them more thoroughly in the API docs, and provide them with meaningful names, as well as probably remove some of the options

Developer

  • All new ENV is documented in README
  • All new ENV has been added to Heroku Pipeline, Staging and Prod
  • ANDI or Wave has been run in accordance to
    our guide and
    all issues introduced by these changes have been resolved or opened as new
    issues (link to those issues in the Pull Request details above)
  • Stakeholder approval has been confirmed (or is not needed)

Code Reviewer

  • The commit message is clear and follows our guidelines
    (not just this pull request message)
  • There are appropriate tests covering any new functionality
  • The documentation has been updated or is unnecessary
  • The changes have been verified
  • New dependencies are appropriate or there were no changes

Requires database migrations?

NO

Includes new or updated dependencies?

NO

Why are these changes being introduced:

* TIMDEX currently ORs all keyword searches together which leads to a
  lot of search results, many of which are not relevant (to the
  extreme of sometimes having no records returned that have all the
  terms which is sometimes the expectation)

Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/GDT-281

How does this address that need:

* Leverages OpenSearch `minimum_should_match` [feature](https://opensearch.org/docs/latest/query-dsl/minimum-should-match/#valid-values)
  along with a GraphQL feature to choose a preconfigured value or supply your own

Document any side effects to this change:

* once we get feedback from stakeholders, it is likely we'll want to adjust the options available, document
  them more thoroughly in the API docs, and provide them with meaningful names, as well as probably remove
  some of the options
@jazairi jazairi self-assigned this Apr 19, 2024
jazairi

This comment was marked as duplicate.

app/graphql/types/query_type.rb Outdated Show resolved Hide resolved
app/models/opensearch.rb Show resolved Hide resolved
Document any side effects to this change:

- some options will be available but not documented. If we choose to
  keep them beyond our initial experiments, we should name them
  appropriately and document them. More than likely they'll be removed
  so not publicly advertising them makes sense unless we want open
  feedback (which we're not looking for yet)
@JPrevost JPrevost force-pushed the gdt-281-more-than-or branch from 80dfb0c to 26fc86b Compare April 19, 2024 19:57
@JPrevost JPrevost temporarily deployed to timdex-pr-811 April 19, 2024 19:58 Inactive
@JPrevost JPrevost merged commit 121e430 into main Apr 19, 2024
3 of 4 checks passed
@JPrevost JPrevost deleted the gdt-281-more-than-or branch April 19, 2024 20:05
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