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

refactor(collections): refactor collection search to match monorepo #7124

Merged

Conversation

eatyourgreens
Copy link
Contributor

@eatyourgreens eatyourgreens commented Jun 8, 2024

Panoptes full-text search requires at least 3 characters in order to return results. If your search string is longer than 3 characters, collectionsSearch passes it to the Panoptes full-text search API. Otherwise, it fetches as many collections as it can then filters for collections that include the search string in the collection name.

The Panoptes API returns odd results for search terms that are 4 or 5 characters (not whole words), but the API’s not something that can be fixed here.

Screen.Recording.2024-06-08.at.19.08.54.mov

Required Manual Testing

  • Does the non-logged in home page render correctly?
  • Does the logged in home page render correctly?
  • Does the projects page render correctly?
  • Can you load project home pages?
  • Can you load the classification page?
  • Can you submit a classification?
  • Does talk load correctly?
  • Can you post a talk comment?

Review Checklist

  • Does it work in all major browsers: Firefox, Chrome, Edge, Safari?
  • Does it work on mobile?
  • Can you npm ci and app works as expected?
  • If the component is in coffeescript, is it converted to ES6? Is it free of eslint errors? Is the conversion its own commit?
  • Are the tests passing locally and on GitHub Actions?

Optional

@coveralls
Copy link

Coverage Status

coverage: 56.915% (-0.08%) from 56.991%
when pulling f1017cd on eatyourgreens:refactor-collections-search
into b19edc1 on zooniverse:master.

@eatyourgreens eatyourgreens force-pushed the refactor-collections-search branch from f1017cd to b8f7630 Compare June 8, 2024 18:15
@coveralls
Copy link

Coverage Status

coverage: 56.917% (-0.07%) from 56.991%
when pulling b8f7630 on eatyourgreens:refactor-collections-search
into b19edc1 on zooniverse:master.

@coveralls
Copy link

Coverage Status

coverage: 56.912% (-0.08%) from 56.991%
when pulling 4e12d78 on eatyourgreens:refactor-collections-search
into b19edc1 on zooniverse:master.

@coveralls
Copy link

Coverage Status

coverage: 56.907% (-0.08%) from 56.991%
when pulling ba10dbf on eatyourgreens:refactor-collections-search
into b19edc1 on zooniverse:master.

import apiClient from 'panoptes-client/lib/api-client';

// Tune this value to determine when a search term is long enough to use Panoptes full-text search.
const MIN_SEARCH_LENGTH = 3;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Increasing this value might be a workaround for weird results when the search is 4 or 5 characters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updating to add that this breaks collections search for Gravity Spy, where there are 2-character collection names.

@eatyourgreens eatyourgreens force-pushed the refactor-collections-search branch 2 times, most recently from 83c865a to 49b461e Compare June 10, 2024 09:07
@coveralls
Copy link

Coverage Status

coverage: 56.911% (-0.08%) from 56.991%
when pulling 83c865a on eatyourgreens:refactor-collections-search
into b19edc1 on zooniverse:master.

@coveralls
Copy link

Coverage Status

coverage: 56.911% (-0.08%) from 56.991%
when pulling 83c865a on eatyourgreens:refactor-collections-search
into b19edc1 on zooniverse:master.

@coveralls
Copy link

Coverage Status

coverage: 56.911% (-0.08%) from 56.991%
when pulling 49b461e on eatyourgreens:refactor-collections-search
into b19edc1 on zooniverse:master.

@coveralls
Copy link

Coverage Status

coverage: 56.907% (-0.09%) from 56.994%
when pulling 0f20654 on eatyourgreens:refactor-collections-search
into 408ffca on zooniverse:master.

@eatyourgreens eatyourgreens force-pushed the refactor-collections-search branch from 0f20654 to 5a3bedc Compare June 22, 2024 07:57
@coveralls
Copy link

Coverage Status

coverage: 56.907% (-0.08%) from 56.991%
when pulling 5a3bedc on eatyourgreens:refactor-collections-search
into 78699da on zooniverse:master.

Panoptes full-text search requires at least 4 characters in order to return results. If your search string is longer than 4 characters, `collectionSearch` passes it to the Panoptes full-text search API. Otherwise, it fetches as many collections as it can that include the search string in the collection name.
@eatyourgreens eatyourgreens force-pushed the refactor-collections-search branch from 5a3bedc to 4d38d04 Compare July 11, 2024 17:16
@mcbouslog mcbouslog self-requested a review August 7, 2024 14:47
@mcbouslog mcbouslog self-assigned this Aug 7, 2024
@coveralls
Copy link

Coverage Status

coverage: 56.84% (-0.08%) from 56.923%
when pulling 6665c9d on eatyourgreens:refactor-collections-search
into a685eeb on zooniverse:master.

Copy link
Contributor

@mcbouslog mcbouslog left a comment

Choose a reason for hiding this comment

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

Changes LGTM! Confirmed issue then confirmed these changes locally do indeed improve collections search similar to zooniverse/front-end-monorepo/pull/4867 .

@mcbouslog mcbouslog merged commit c0e17f9 into zooniverse:master Aug 8, 2024
4 of 5 checks passed
@eatyourgreens eatyourgreens deleted the refactor-collections-search branch August 8, 2024 16:58
eatyourgreens added a commit to eatyourgreens/Panoptes-Front-End that referenced this pull request Aug 11, 2024
Since zooniverse#7124, collections search no longer finds short collection names for Gravity Spy.
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.

Bad search results while adding a subject to an existing collection
3 participants