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.

Sign up for GitHub

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

Jump to bottom

Fixed: Low search accuracy for people #5786

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fixed: Low search accuracy for people #5786

wants to merge 1 commit into from
+17 −1

Conversation

shashankvish0010
Copy link
Contributor

Now the search is accurate and displays the best match on top.

  • Added a logic to put the searched person based on the name to the top within SingleEntitySelectMenuItemsWithSearch
  • Used a useMemo with necessary dependencies.

fixes: #5722

Screenshot (346)
Screenshot (347)

greptile-apps[bot]
greptile-apps bot reviewed Jun 9, 2024
View reviewed changes
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

  • Added logic to prioritize best match for person in search results
  • Introduced useMemo hook with necessary dependencies to optimize search
  • Enhanced search accuracy by placing best match at the top

...c/modules/object-record/relation-picker/components/SingleEntitySelectMenuItemsWithSearch.tsx
Comment on lines 71 to +85
objectNameSingular: relationObjectNameSingular,
});

const entitiesToSelect = useMemo(() => {
const index = entities.entitiesToSelect.findIndex(
(person) =>
person.name.toLocaleLowerCase() ===
relationPickerSearchFilter.toLocaleLowerCase(),
);

return index < 0
? entities.entitiesToSelect
: [
...entities.entitiesToSelect.splice(index, 1),
...entities.entitiesToSelect,
Copy link

Choose a reason for hiding this comment

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

Consider handling cases where person.name or relationPickerSearchFilter might be undefined to avoid potential runtime errors.

@charlesBochet
Copy link
Member

charlesBochet commented Jun 25, 2024 •
edited
Loading

@shashankvish0010 thanks a lot for your PR
I feel this whole logic should be done on the backend :( We should have a long term plan on search. @Weiko has done very promising investigations about the search (using postgres ts-vector last week)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greptile-apps greptile-apps[bot] greptile-apps left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@charlesBochet charlesBochet

Labels
PR: awaiting review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Low search accuracy for people
3 participants
@shashankvish0010 @charlesBochet @FelixMalfait