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

Bugfix: Search on multiple fields #42

Merged
merged 3 commits into from
Feb 26, 2021
Merged

Bugfix: Search on multiple fields #42

merged 3 commits into from
Feb 26, 2021

Conversation

AetherUnbound
Copy link
Collaborator

We had two separate users report that last name searching was effectively broken. We found that the frontend was querying the API with empty form fields as None (e.g. a search on first name would add last_name=None to the query params). While this worked for first name searches, for some reason it failed on last name searches. This PR addresses the issue by skipping any None fields when composing the query params.

It also adds an additional warning for the user on badge fuzzy searching, which was already present in the UI. We should try to add some mechanism for badge to be searched through the fuzzy endpoint, even if it's a strict search under the hood - we get that complaint a lot.

This was causing an issue where a value would be sent as None and fail the search
Copy link
Collaborator

@mpuckett159 mpuckett159 left a comment

Choose a reason for hiding this comment

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

Looks good like the fix.

src/api.py Outdated
Comment on lines 46 to 47
if value is None:
continue
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh thanks for commenting on this specific line because I just realized it was pasted weird and the indentation is off

@AetherUnbound AetherUnbound merged commit 8cbd7ea into main Feb 26, 2021
@AetherUnbound AetherUnbound deleted the bugfix/name-search branch February 26, 2021 05:53
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.

3 participants