-
Notifications
You must be signed in to change notification settings - Fork 11k
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
[NEW] Global search #7628
Closed
Closed
[NEW] Global search #7628
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
472ba85
feat: add global search while searching message closes # 1615
cyberhck 83dc184
check user permission to access the room
cyberhck 41a3618
merge develop
savikko 5b7594f
add translations for global search
savikko e5e0e8e
add channel names to search results
savikko da26796
fix eslint
savikko 7e36607
Update README.md
savikko File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This rely's on the username on the room object. I'm not sure if we plan to keep here.
I do worry about how heavy this could be...
Our community server for example we have 145k users. I can only imagine this plus then querying across all rooms in... could be very heavy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about if global search would be admin-enableable feature?
I see that on large installations this needs highly more optimized search function but on smaller environments current implementation could be acceptable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RocketChat/core what do you guys think? Should we have this as something that can be turned on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not in a position to test this feature, nor do I understand what this bit of code does. Is the proposed new search always doing a global search? Perhaps defaulting to searching the current channel, and requiring the user to select a global search option would result in very few global searches being done, ie only when necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, in large environments global search could be used as DoS-attack by normal users.
Therefore allowing user to enable global search while searching should be only secondary, admin should decide this primarily.
On smaller environments (depends on multiple factors I think) your point still stands.
If time permits, I'll add option to admin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That works fine for us, our environment is small, and we will enable the option. I would hate to be in a big environment and denied access to global search just because it might be abused.
Compromises could include allowing users to specify up to, say, 5 channels to include in their search, or a search string syntax that can specify up to 5 channels to search. Eg "channel:general,random".
Or, as I suggested earlier, an option to repeat the current search on the next channel in the list.