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

Training search #2151

Merged
merged 3 commits into from
May 24, 2022
Merged

Training search #2151

merged 3 commits into from
May 24, 2022

Conversation

maneesha
Copy link
Contributor

@maneesha maneesha commented Apr 1, 2022

Adds functionality to search trainees by personal and family name together. This does add some slightly redundant code, but only twice. We can consider refactoring if needed.

Addresses #2149

@maneesha
Copy link
Contributor Author

maneesha commented Apr 5, 2022

As I think about this more, I'm not entirely pleased with this approach (not just this PR but the original approach) as if someone has a space in their personal or family names, this will not work. For example, searching for Mary Ann will not return the person named Mary Ann Smith.

@pbanaszkiewicz
Copy link
Contributor

@maneesha I'm okay with your changes.

@maneesha @fmichonneau I also conducted some testing of PostgreSQL's Full Text Search.
Here's how the code would look like:

            persons = list(
                Person.objects.annotate(
                    name_search=SearchVector('personal', 'middle', 'family')
                ).filter(
                    Q(name_search=term)
                    | Q(email__icontains=term)
                    | Q(secondary_email__icontains=term)
                    | Q(github__icontains=term)
                ).order_by("family")
            )

Diff:
obraz
In my opinion, it's much much cleaner.

This code also solves for the case you mentioned (Mary Ann Smith). I created two accounts for that name (1: personal="Mary Ann", family="Smith", 2: personal="Mary", middle="Ann", family="Smith") and they both appear in the results:
obraz

There's a caveat though. In some cases the results would not contain the same set of people as previously. Here's search results for term peter:
obraz
Here's database persons with highlighted term peter:
obraz
Clearly Shawn Peterson wasn't returned by the Full Text Search.

@maneesha @fmichonneau Please let me know if you would like to switch to Full Text Search.

@pbanaszkiewicz pbanaszkiewicz self-requested a review May 15, 2022 14:34
@maneesha
Copy link
Contributor Author

What is the logic that causes Shawn Peterson to not be returned but Joshua Peters and epeterson@gmail.com both are?

I don't see deep explanation in the SearchVector documentation.

@pbanaszkiewicz
Copy link
Contributor

@maneesha I don't know why Shawn Peterson is not returned. My guess is that PostgreSQL engine doesn't "think" Peter is close enough to Peterson.

@maneesha
Copy link
Contributor Author

It seems illogical to me that Shawn Peterson is not returned and epeterson@gmail.com is returned. Looking at the code, the search is including the github field. Was peter in the github name for Shawn Peterson?

If that's not the case, then I don't like this approach as the logic doesn't make sense.

@pbanaszkiewicz
Copy link
Contributor

On a call today we agreed with @maneesha to not use full-text search for this kind of searching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants