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

Add ability to search history by conversation content, not just name (issue #67) #76

Merged
merged 8 commits into from
Sep 11, 2015

Conversation

douglasrapp
Copy link

Primary change is moving search logic from the client (in-browser mini-mongo db) to the server, in order to get around pub-sub issues encountered when trying to search all messages of all the user's joined rooms.

Modified the search logic to also search the contents of the rooms of which the user is a member when a flag is set. Added a checkbox option to the webpage for setting and clearing that flag.

Added 'debounce' function to the the 'keyup' event to try to protect the server from too many calls. Otherwise, an RPC call would get sent to the server every time a user hit a key. Now, it will wait until after the user has stopped typing for 200ms before sending the search request to the server.

Also fixed an issue where clicking on a search result item did not route to that room.

Douglas Rapp added 4 commits August 27, 2015 13:34
Added throttle logic to user input to protect server
Added option to search room contents in addition to name
Fixed issue where clicking history item did nothing
@douglasrapp
Copy link
Author

Previously was going to move history search back to the client rather than the server after subscription change introduced in issue #68 (pull request #88). However, on further analysis, due to issues with subscribing to the messages collection, as well as concerns about synchronizing ALL message history of ALL rooms of which the user is a member with the browser (could be very large), I think it makes sense to leave the search function on the server.

Therefore, this PR is once again ready for review.

@rwakida
Copy link

rwakida commented Sep 8, 2015

Direct message search history result only shows @ in title

@douglasrapp
Copy link
Author

Ah, ok. Looks like a discrepancy from how Rocket was searching subscriptions on the client, and this is now searching rooms on the server. Since direct message rooms do not have a 'name' field, this is showing up blank.

@douglasrapp
Copy link
Author

Looks like it can be fixed by searching subscriptions instead of rooms. Originally, I was under the impression that we'd need to search rooms for the use case of a user "leaving" a room, but later wanting to find it again.

But, looking closer, I see that idea of "leaving" a room only (currently) applies to channels, and when you do leave a channel, not only is the subscription removed, but the user is pulled from the channel membership. Since I'm currently searching rooms based on membership, it looks like it's not gaining anything.

Are there any other cases that might make us want to search rooms vs subscriptions??

This ensures we have a 'name' to show for direct messages
@douglasrapp
Copy link
Author

I went ahead and made the change to search the ChatSubscription collection instead of the ChatRoom collection for matching rooms. So the issue with direct messages showing up as just an '@' symbol is fixed.

@rwakida
Copy link

rwakida commented Sep 10, 2015

👍 I can see the user's name in the title.

douglasrapp pushed a commit that referenced this pull request Sep 11, 2015
Add ability to search history by conversation content, not just name (issue #67)
@douglasrapp douglasrapp merged commit b0407e7 into master Sep 11, 2015
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.

2 participants