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

Changing the backend for search to query the database #71

Merged
merged 5 commits into from
Nov 10, 2023
Merged

Conversation

PaulaPerdomo
Copy link
Contributor

@PaulaPerdomo PaulaPerdomo commented Nov 7, 2023

Fixes #4 for searching results displays.
Fixes #2 for the backend component of keyword search.
Fixes #3 for searching by club (after integrating suggestions)

This PR changes the api/autoggest and api/search endpoints to query the database instead of the mock data it previously contained.

Please run the tests using pytest in the backend/test directory and check the UI as well.

The events are currently being searched in the following way:

  • If the event.title begins in the same way as the query in the Search Bar.
  • If the event.location begins in the way as the query in the Search Bar.
  • If the event.description contains the word/part of the word from the query in its field.

NOTE:

  • I am searching by location because if students want to know what events are held at a specific place like HartHouse, this filtering is useful.
  • With the description, trying to search by the keyword increases the results a lot. For example, let's say there is an event called Andy's Party and another event that uses the word and in its description. Both will appear with the query in the Search Bar being 'and' because we are looking for similar words in the description and event titles that start the same, which in my opinion is not necessary. Please let me know what you think!

Copy link
Contributor

@tmahabir tmahabir 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 that if location is the field that is being matched, then that is what should show up in the dropdown, because it's unintuitive to type a letter that matches the location of an event but have the title show up instead.
For example, let's say you have an event called 'Origami' at location 'SF'.
Currently if you type'S', then 'Origami' will show up in the dropdown because of the match on location, but that's unintuitive to be seeing 'Origami' as it doesn't contain 'S'

So, I think either:

  • When their is a match on location, then that match should show up in the dropdown. In the example above, it would be 'SF' showing up in the dropdown
  • Or, both the title and location should appear in the dropdown. In the example above, it would be 'Origami - SF' showing up in the dropdown. This way, the user can intuitively see why typing 'S' made a match.

@meriam04
Copy link
Contributor

meriam04 commented Nov 7, 2023

I think that if location is the field that is being matched, then that is what should show up in the dropdown, because it's unintuitive to type a letter that matches the location of an event but have the title show up instead. For example, let's say you have an event called 'Origami' at location 'SF'. Currently if you type'S', then 'Origami' will show up in the dropdown because of the match on location, but that's unintuitive to be seeing 'Origami' as it doesn't contain 'S'

So, I think either:

  • When their is a match on location, then that match should show up in the dropdown. In the example above, it would be 'SF' showing up in the dropdown
  • Or, both the title and location should appear in the dropdown. In the example above, it would be 'Origami - SF' showing up in the dropdown. This way, the user can intuitively see why typing 'S' made a match.

I think the first option (showing just the location) would make more sense in this case. However, I would also like to point out that we have a "Location" filter on the side. Do we want to keep that or just use the search bar or both?

Copy link
Contributor

@meriam04 meriam04 left a comment

Choose a reason for hiding this comment

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

I agree that the description might return more search results than necessary, but if we change the title to "contain" a string then we may not need the description. I will leave it up to your discretion.

backend/datalayer_event.py Outdated Show resolved Hide resolved
@meriam04
Copy link
Contributor

meriam04 commented Nov 8, 2023

Also, could we add search results that contain the club name as well?

@PaulaPerdomo
Copy link
Contributor Author

I think that if location is the field that is being matched, then that is what should show up in the dropdown, because it's unintuitive to type a letter that matches the location of an event but have the title show up instead. For example, let's say you have an event called 'Origami' at location 'SF'. Currently if you type'S', then 'Origami' will show up in the dropdown because of the match on location, but that's unintuitive to be seeing 'Origami' as it doesn't contain 'S'
So, I think either:

  • When their is a match on location, then that match should show up in the dropdown. In the example above, it would be 'SF' showing up in the dropdown
  • Or, both the title and location should appear in the dropdown. In the example above, it would be 'Origami - SF' showing up in the dropdown. This way, the user can intuitively see why typing 'S' made a match.

I think the first option (showing just the location) would make more sense in this case. However, I would also like to point out that we have a "Location" filter on the side. Do we want to keep that or just use the search bar or both?

Currently, I eliminated the search for the location since there will be a Location filter for this. However, if it does end up being something that we still want to display I can add it later to prevent clutter of tags for Location.

@PaulaPerdomo
Copy link
Contributor Author

I agree that the description might return more search results than necessary, but if we change the title to "contain" a string then we may not need the description. I will leave it up to your discretion.

Yeah, I deleted the description because it returns a lot of results where sometimes the word matching is not intuitive or descriptive of what the event is. Searching by event title or club name is better.

@meriam04
Copy link
Contributor

meriam04 commented Nov 8, 2023

I think this is better, but I was hoping for the club name that matches to also be displayed in the search results. I think that makes it more usable since the user can see the exact match, rather than thinking the search is malfunctioning.

@PaulaPerdomo
Copy link
Contributor Author

I think this is better, but I was hoping for the club name that matches to also be displayed in the search results. I think that makes it more usable since the user can see the exact match, rather than thinking the search is malfunctioning.

Yes you are right. I changed the code to now include the clubs in the dropdown menu as well.

@meriam04
Copy link
Contributor

meriam04 commented Nov 8, 2023

Thanks! This is better, but I noticed a couple of bugs. I think it is related to case sensitivity, but when i type 'f' I don't see "Fall Career Week" in the dropdown. same with 'y' for YNCN. Could we make the search words case insensitive? this will greatly improve usability.

Screen.Recording.2023-11-08.at.2.14.10.AM.mov

@PaulaPerdomo
Copy link
Contributor Author

Thanks! This is better, but I noticed a couple of bugs. I think it is related to case sensitivity, but when i type 'f' I don't see "Fall Career Week" in the dropdown. same with 'y' for YNCN. Could we make the search words case insensitive? this will greatly improve usability.

Screen.Recording.2023-11-08.at.2.14.10.AM.mov

Yes, I have just added changes for this to include lower case searches. I also noticed that it was displaying incorrect club names for some searches. Both have been fixed now.

Copy link
Contributor

@meriam04 meriam04 left a comment

Choose a reason for hiding this comment

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

Awesome! I tested it out and I think it's much better. I also ran the tests and they all pass on my end. Thanks!

@PaulaPerdomo PaulaPerdomo merged commit 3013487 into main Nov 10, 2023
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.

[1.3] Search result display [1.2] Search by Club [1.1] Keyword Search
3 participants