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

[GH-1539] Implement searching for the individual words in workspaces #1590

Merged
merged 7 commits into from
Nov 22, 2021

Conversation

De1ain
Copy link
Contributor

@De1ain De1ain commented Oct 18, 2021

Implement searching for the individual words in workspaces

Old behavior:
Simply checking if a search query is a substing of any workspace name, by using includes function
New behavior:
Search now looks for any word (or part of it) from the search query in the names of all workspaces.

Examples:
Have workspaces: 'Workspace 1', 'Workspace 2', and 'Workspace 3'.
Searching for 'Workspace 1' will find all 3 workspaces, because each one contains the word 'workspace'.

Have workspaces: 'Foo Bar Baz'
Seaching for 'foo baz' will find 'Foo Bar Baz'

Update:
Updated first search test name with some description. Updated it's snapshot, according to the new search algorithm changes.
Updated the search filter of the second search test to not include the work workspace, since it will find all 3 workspaces that contain this word.
Add 2 new sample workspace with unique names.
Add 2 new tests.

Fixes #1539

@De1ain De1ain requested a review from a team as a code owner October 18, 2021 16:57
@mattermod
Copy link
Contributor

Hello @De1ain,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

@De1ain De1ain changed the title Implement searching for the individual words in workspaces [GH-1539] Implement searching for the individual words in workspaces Oct 19, 2021
Copy link
Member

@harshilsharma63 harshilsharma63 left a comment

Choose a reason for hiding this comment

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

Awesome! LGTM!

Copy link
Contributor

@hahmadia hahmadia left a comment

Choose a reason for hiding this comment

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

LGTM and Worked Well.

I will have QA test this before merging.

@ogi-m Do you mind testing this before we merge it since it affects a critical part of the code base. Thank you

@hahmadia hahmadia added the 2: QA Review Requires review by a QA tester label Oct 26, 2021
@hahmadia hahmadia requested a review from ogi-m October 26, 2021 15:19
@harshilsharma63
Copy link
Member

@ogi-m bump :)

Copy link
Contributor

@ogi-m ogi-m left a comment

Choose a reason for hiding this comment

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

Thanks @De1ain. One issue I'm seeing is this:

  1. Set up 5 workspaces:
  • Workspace
  • Workspace 1
  • Workspace 11
  • Workspace 2
  • Workspace 3
  1. Search on the dashboard, by typing in "workspace"

Observed: Workspace, Workspace 2, Workspace 3 are returned but Workspace 1 and Workspace 11 are not
Expected: All 5 workspaces are returned

@De1ain
Copy link
Contributor Author

De1ain commented Nov 4, 2021

@ogi-m Thanks for testing
It seems like this change ed0a339 introduced this bug.
Investigating...

@emilyacook
Copy link

Thanks for participating in Hacktoberfest! You can claim your sticker set here: https://get.printfection.com/hacktober21/4144583267 @De1ain

@De1ain
Copy link
Contributor Author

De1ain commented Nov 9, 2021

Fixed in d13d640.

Changed the title search pattern.
Added new test cases and dummy workspaces.

@hahmadia
Copy link
Contributor

/update-branch

@ogi-m
Copy link
Contributor

ogi-m commented Nov 22, 2021

Thanks @De1ain , LGTM 👍🏻

@ogi-m ogi-m removed the 2: QA Review Requires review by a QA tester label Nov 22, 2021
@hahmadia hahmadia merged commit 26e8625 into mattermost-community:main Nov 22, 2021
@hahmadia hahmadia added the 3: Reviews Complete All reviewers have approved the pull request label Nov 22, 2021
@De1ain De1ain deleted the fix_search branch November 23, 2021 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Idea: Dashboard: allow searching for words rather than only substrings
6 participants