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

added public_name search function #2164

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

biskwikman
Copy link
Contributor

closes #2082

I basically mimicked the structure of the code for usernames, but pointed the new query to the public_name column of the participants table, it seems to work fine.

Copy link
Member

@Changaco Changaco left a comment

Choose a reason for hiding this comment

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

I think it's probably better to have a single query and unified results section for both usernames and public names.

Also, a new trigram index is needed on the public_name column to speed up the search:

CREATE INDEX public_name_trgm_idx ON participants
    USING GIN (lower(public_name) gin_trgm_ops)
    WHERE status = 'active'
      AND public_name IS NOT null;

@biskwikman
Copy link
Contributor Author

Should we include the username and public name under the avatar on all search results from now on? I think it would help with any confusion if an account has a similar username and public name.

@Changaco
Copy link
Member

Yes, both names should be displayed for all results.

@biskwikman
Copy link
Contributor Author

I've made it so the username and public name are returned in a single query, and both are displayed. I also added the index.

I'm not sure what word to use instead of "username" now. For example in the phrasing "Found a matching username". I was thinking of just using "user", but maybe "individual" would be more consistent.

www/search.spt Outdated Show resolved Hide resolved
www/search.spt Outdated Show resolved Hide resolved
@biskwikman
Copy link
Contributor Author

thanks for the suggestions. I just wanted to check, the tests will have to be updated for this, right? It looks like two are failing now.

@Changaco
Copy link
Member

The tests aren't failing with the latest commit. I think GitHub is still showing earlier test results because there's currently a merge conflict (because #2177 hasn't been deployed yet, because I've been waiting for new machine and human translations (liberapay/salon#520)).

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

Successfully merging this pull request may close these issues.

Searching for the last name does not find an account
2 participants