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

Allow default sort typesense configuration (#304) #305

Merged
merged 2 commits into from
Dec 19, 2024
Merged

Conversation

blms
Copy link
Contributor

@blms blms commented Dec 6, 2024

In this PR

Per #304 (see also performant-software/core-data-cloud#323):

  • Allow users to set a default sort in the typesense configuration
    • _text_match:desc is the default if no sort_by is passed; include that as the primary sort, then configured option as the secondary (i.e. the default sort if no text query is entered, and the tiebreaker for equivalent text matches)

Questions

  • Are these the right places to set this? I basically just looked for wherever a typesense configuration was set.
  • Is it ok to assume we just want an ascending sort on whatever field is passed to default_sort, or should the user have to specify direction?
  • Should I publish a beta version in order to test this?

@blms blms requested a review from dleadbetter December 6, 2024 18:03
@blms blms added the v2.2.19 Issues in v2.2.19 label Dec 6, 2024
@@ -100,7 +101,8 @@ const createTypesenseAdapter = (config: TypesenseConfig, options = {}) => (
geoLocationField: 'coordinates',
additionalSearchParameters: {
query_by: config.query_by,
limit: config.limit || 250
limit: config.limit || 250,
sort_by: `_text_match:desc${config.default_sort ? `,${config.default_sort}:asc` : ''}`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this string interpolation need to be done in both places (here and Peripleo utils)? I believe the TypesenseConfig that gets passed into here will contain the transformed sort_by value from Peripleo utils, not the default_sort config option.

@blms blms requested a review from dleadbetter December 6, 2024 19:29
@blms blms merged commit c6e1157 into master Dec 19, 2024
@blms blms deleted the feature/sort-by branch December 19, 2024 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2.2.19 Issues in v2.2.19
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants