From 2c9c58ff50cc7cb8dc0967d12d01fc25dc561ebe Mon Sep 17 00:00:00 2001 From: Hossein Ahmadian-Yazdi Date: Mon, 15 Jun 2020 11:21:42 -0400 Subject: [PATCH] [MM-25268] Implement prefix/suffix searching on teams and channel page in system console (#14698) * Implement prefix/suffix search on teams page * Make Channel Page prefix/suffix search as well * address PR comments * add tests * fix styling * take postgres into account * add more api tests * update test * add team store test * write store test for team and channel * fix lint * update description * revert go mod Co-authored-by: mattermod --- api4/channel_test.go | 82 ++++++++++++++++++++- api4/team_test.go | 56 +++++++++++++++ store/sqlstore/channel_store.go | 2 +- store/sqlstore/team_store.go | 47 +++++++++--- store/storetest/channel_store.go | 15 +++- store/storetest/team_store.go | 120 +++++++++++++++++++++++++++++++ 6 files changed, 308 insertions(+), 14 deletions(-) diff --git a/api4/channel_test.go b/api4/channel_test.go index 9e060c54c8..b52a06d13f 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -1152,11 +1152,91 @@ func TestSearchAllChannels(t *testing.T) { defer th.TearDown() Client := th.Client - search := &model.ChannelSearch{Term: th.BasicChannel.Name} + channel := &model.Channel{ + DisplayName: "FOOBAR", + Name: "whatever", + Type: model.CHANNEL_OPEN, + TeamId: th.BasicTeam.Id, + } + + // Testing Mixed Case (Ensure we get results for partial word searches) + + // Search by using display name + foobarchannel, err := th.SystemAdminClient.CreateChannel(channel) + CheckNoError(t, err) + + search := &model.ChannelSearch{Term: "oob"} channels, resp := th.SystemAdminClient.SearchAllChannels(search) CheckNoError(t, resp) + assert.Len(t, *channels, 1) + assert.Equal(t, foobarchannel.Id, (*channels)[0].Id) + + search = &model.ChannelSearch{Term: "foo"} + + channels, resp = th.SystemAdminClient.SearchAllChannels(search) + CheckNoError(t, resp) + + assert.Len(t, *channels, 1) + assert.Equal(t, foobarchannel.Id, (*channels)[0].Id) + + search = &model.ChannelSearch{Term: "bar"} + + channels, resp = th.SystemAdminClient.SearchAllChannels(search) + CheckNoError(t, resp) + + assert.Len(t, *channels, 1) + assert.Equal(t, foobarchannel.Id, (*channels)[0].Id) + + // Search by using Name + search = &model.ChannelSearch{Term: "what"} + + channels, resp = th.SystemAdminClient.SearchAllChannels(search) + CheckNoError(t, resp) + + assert.Len(t, *channels, 1) + assert.Equal(t, foobarchannel.Id, (*channels)[0].Id) + + search = &model.ChannelSearch{Term: "ever"} + + channels, resp = th.SystemAdminClient.SearchAllChannels(search) + CheckNoError(t, resp) + + // Seach by partial word search and testing case sensitivty + assert.Len(t, *channels, 1) + assert.Equal(t, foobarchannel.Id, (*channels)[0].Id) + + search = &model.ChannelSearch{Term: th.BasicChannel.Name[2:14]} + + channels, resp = th.SystemAdminClient.SearchAllChannels(search) + CheckNoError(t, resp) + + assert.Len(t, *channels, 1) + assert.Equal(t, th.BasicChannel.Id, (*channels)[0].Id) + + search = &model.ChannelSearch{Term: strings.ToUpper(th.BasicChannel.Name)} + + channels, resp = th.SystemAdminClient.SearchAllChannels(search) + CheckNoError(t, resp) + + assert.Len(t, *channels, 1) + assert.Equal(t, th.BasicChannel.Id, (*channels)[0].Id) + + search = &model.ChannelSearch{Term: th.BasicChannel.Name[0:2] + strings.ToUpper(th.BasicChannel.Name[2:5]) + th.BasicChannel.Name[5:]} + + channels, resp = th.SystemAdminClient.SearchAllChannels(search) + CheckNoError(t, resp) + + assert.Len(t, *channels, 1) + assert.Equal(t, th.BasicChannel.Id, (*channels)[0].Id) + + // Testing Non-Mixed Case test cases + search = &model.ChannelSearch{Term: th.BasicChannel.Name} + + channels, resp = th.SystemAdminClient.SearchAllChannels(search) + CheckNoError(t, resp) + assert.Len(t, *channels, 1) assert.Equal(t, th.BasicChannel.Id, (*channels)[0].Id) diff --git a/api4/team_test.go b/api4/team_test.go index 9fd0771d12..2312568447 100644 --- a/api4/team_test.go +++ b/api4/team_test.go @@ -1220,18 +1220,74 @@ func TestSearchAllTeamsPaged(t *testing.T) { teams[i] = newTeam } + foobarTeam, err := th.App.CreateTeam(&model.Team{ + DisplayName: "FOOBAR", + Name: "whatever", + Type: model.TEAM_OPEN, + Email: th.GenerateTestEmail(), + }) + require.Nil(t, err) + testCases := []struct { Name string Search *model.TeamSearch ExpectedTeams []string ExpectedTotalCount int64 }{ + { + Name: "Get foobar channel", + Search: &model.TeamSearch{Term: "oob", Page: model.NewInt(0), PerPage: model.NewInt(100)}, + ExpectedTeams: []string{foobarTeam.Id}, + ExpectedTotalCount: 1, + }, + { + Name: "Get foobar channel", + Search: &model.TeamSearch{Term: "foo", Page: model.NewInt(0), PerPage: model.NewInt(100)}, + ExpectedTeams: []string{foobarTeam.Id}, + ExpectedTotalCount: 1, + }, + { + Name: "Get foobar channel", + Search: &model.TeamSearch{Term: "bar", Page: model.NewInt(0), PerPage: model.NewInt(100)}, + ExpectedTeams: []string{foobarTeam.Id}, + ExpectedTotalCount: 1, + }, + { + Name: "Get foobar channel", + Search: &model.TeamSearch{Term: "what", Page: model.NewInt(0), PerPage: model.NewInt(100)}, + ExpectedTeams: []string{foobarTeam.Id}, + ExpectedTotalCount: 1, + }, + { + Name: "Get foobar channel", + Search: &model.TeamSearch{Term: "ever", Page: model.NewInt(0), PerPage: model.NewInt(100)}, + ExpectedTeams: []string{foobarTeam.Id}, + ExpectedTotalCount: 1, + }, { Name: "Get all teams on one page", Search: &model.TeamSearch{Term: commonRandom, Page: model.NewInt(0), PerPage: model.NewInt(100)}, ExpectedTeams: []string{teams[0].Id, teams[1].Id, teams[2].Id}, ExpectedTotalCount: 3, }, + { + Name: "Get all teams on one page with partial word", + Search: &model.TeamSearch{Term: commonRandom[11:18]}, + ExpectedTeams: []string{teams[0].Id, teams[1].Id, teams[2].Id}, + ExpectedTotalCount: 3, + }, + { + Name: "Get all teams on one page with term upper cased", + Search: &model.TeamSearch{Term: strings.ToUpper(commonRandom)}, + ExpectedTeams: []string{teams[0].Id, teams[1].Id, teams[2].Id}, + ExpectedTotalCount: 3, + }, + { + Name: "Get all teams on one page with some of term upper and some lower", + Search: &model.TeamSearch{Term: commonRandom[0:11] + strings.ToUpper(commonRandom[11:18]+commonRandom[18:])}, + ExpectedTeams: []string{teams[0].Id, teams[1].Id, teams[2].Id}, + ExpectedTotalCount: 3, + }, { Name: "Get 2 teams on the first page", Search: &model.TeamSearch{Term: commonRandom, Page: model.NewInt(0), PerPage: model.NewInt(2)}, diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index f8345a0755..b12e18a6dc 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -2610,7 +2610,7 @@ func (s SqlChannelStore) buildLIKEClause(term string, searchColumns string) (lik } likeClause = fmt.Sprintf("(%s)", strings.Join(searchFields, " OR ")) - likeTerm += "%" + likeTerm = wildcardSearchTerm(likeTerm) return } diff --git a/store/sqlstore/team_store.go b/store/sqlstore/team_store.go index 42c98814cb..b734f5ffd2 100644 --- a/store/sqlstore/team_store.go +++ b/store/sqlstore/team_store.go @@ -77,6 +77,10 @@ func teamMemberToSlice(member *model.TeamMember) []interface{} { return resultSlice } +func wildcardSearchTerm(term string) string { + return strings.ToLower("%" + term + "%") +} + type rolesInfo struct { roles []string explicitRoles []string @@ -374,8 +378,14 @@ func (s SqlTeamStore) SearchAll(term string) ([]*model.Team, *model.AppError) { var teams []*model.Team term = sanitizeSearchTerm(term, "\\") + term = wildcardSearchTerm(term) - if _, err := s.GetReplica().Select(&teams, "SELECT * FROM Teams WHERE Name LIKE :Term OR DisplayName LIKE :Term", map[string]interface{}{"Term": term + "%"}); err != nil { + operatorKeyword := "ILIKE" + if s.DriverName() == model.DATABASE_DRIVER_MYSQL { + operatorKeyword = "LIKE" + } + queryString := fmt.Sprintf("SELECT * FROM Teams WHERE Name %[1]s :Term OR DisplayName %[1]s :Term", operatorKeyword) + if _, err := s.GetReplica().Select(&teams, queryString, map[string]interface{}{"Term": term}); err != nil { return nil, model.NewAppError("SqlTeamStore.SearchAll", "store.sql_team.search_all_team.app_error", nil, "term="+term+", "+err.Error(), http.StatusInternalServerError) } @@ -389,12 +399,18 @@ func (s SqlTeamStore) SearchAllPaged(term string, page int, perPage int) ([]*mod offset := page * perPage term = sanitizeSearchTerm(term, "\\") - - if _, err := s.GetReplica().Select(&teams, "SELECT * FROM Teams WHERE Name LIKE :Term OR DisplayName LIKE :Term ORDER BY DisplayName, Name LIMIT :Limit OFFSET :Offset", map[string]interface{}{"Term": term + "%", "Limit": perPage, "Offset": offset}); err != nil { + term = wildcardSearchTerm(term) + operatorKeyword := "ILIKE" + if s.DriverName() == model.DATABASE_DRIVER_MYSQL { + operatorKeyword = "LIKE" + } + queryString := fmt.Sprintf("SELECT * FROM Teams WHERE Name %[1]s :Term OR DisplayName %[1]s :Term ORDER BY DisplayName, Name LIMIT :Limit OFFSET :Offset", operatorKeyword) + if _, err := s.GetReplica().Select(&teams, queryString, map[string]interface{}{"Term": term, "Limit": perPage, "Offset": offset}); err != nil { return nil, 0, model.NewAppError("SqlTeamStore.SearchAllPage", "store.sql_team.search_all_team.app_error", nil, "term="+term+", "+err.Error(), http.StatusInternalServerError) } - totalCount, err := s.GetReplica().SelectInt("SELECT COUNT(*) FROM Teams WHERE Name LIKE :Term OR DisplayName LIKE :Term", map[string]interface{}{"Term": term + "%"}) + queryString = fmt.Sprintf("SELECT COUNT(*) FROM Teams WHERE Name %[1]s :Term OR DisplayName %[1]s :Term", operatorKeyword) + totalCount, err := s.GetReplica().SelectInt(queryString, map[string]interface{}{"Term": term}) if err != nil { return nil, 0, model.NewAppError("SqlTeamStore.SearchAllPage", "store.sql_team.search_all_team.app_error", nil, "term="+term+", "+err.Error(), http.StatusInternalServerError) } @@ -408,8 +424,13 @@ func (s SqlTeamStore) SearchOpen(term string) ([]*model.Team, *model.AppError) { var teams []*model.Team term = sanitizeSearchTerm(term, "\\") - - if _, err := s.GetReplica().Select(&teams, "SELECT * FROM Teams WHERE Type = 'O' AND AllowOpenInvite = true AND (Name LIKE :Term OR DisplayName LIKE :Term)", map[string]interface{}{"Term": term + "%"}); err != nil { + term = wildcardSearchTerm(term) + operatorKeyword := "ILIKE" + if s.DriverName() == model.DATABASE_DRIVER_MYSQL { + operatorKeyword = "LIKE" + } + queryString := fmt.Sprintf("SELECT * FROM Teams WHERE Type = 'O' AND AllowOpenInvite = true AND (Name %[1]s :Term OR DisplayName %[1]s :Term)", operatorKeyword) + if _, err := s.GetReplica().Select(&teams, queryString, map[string]interface{}{"Term": term}); err != nil { return nil, model.NewAppError("SqlTeamStore.SearchOpen", "store.sql_team.search_open_team.app_error", nil, "term="+term+", "+err.Error(), http.StatusInternalServerError) } @@ -422,15 +443,19 @@ func (s SqlTeamStore) SearchPrivate(term string) ([]*model.Team, *model.AppError var teams []*model.Team term = sanitizeSearchTerm(term, "\\") - - query := - `SELECT * + term = wildcardSearchTerm(term) + operatorKeyword := "ILIKE" + if s.DriverName() == model.DATABASE_DRIVER_MYSQL { + operatorKeyword = "LIKE" + } + query := fmt.Sprintf(` + SELECT * FROM Teams WHERE (Type != 'O' OR AllowOpenInvite = false) AND - (Name LIKE :Term OR DisplayName LIKE :Term)` - if _, err := s.GetReplica().Select(&teams, query, map[string]interface{}{"Term": term + "%"}); err != nil { + (Name %[1]s :Term OR DisplayName %[1]s :Term)`, operatorKeyword) + if _, err := s.GetReplica().Select(&teams, query, map[string]interface{}{"Term": term}); err != nil { return nil, model.NewAppError("SqlTeamStore.SearchPrivate", "store.sql_team.search_private_team.app_error", nil, "term="+term+", "+err.Error(), http.StatusInternalServerError) } return teams, nil diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index 453427c5df..4614c6e5af 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -5240,6 +5240,14 @@ func testChannelStoreSearchAllChannels(t *testing.T, ss store.Store) { nErr = ss.Channel().Delete(o13.Id, o13.DeleteAt) require.Nil(t, nErr, "channel should have been deleted") + o14 := model.Channel{ + TeamId: t2.Id, + DisplayName: "FOOBAR", + Name: "whatever", + Type: model.CHANNEL_OPEN, + } + _, nErr = ss.Channel().Save(&o14, -1) + require.Nil(t, nErr) testCases := []struct { Description string Term string @@ -5247,9 +5255,14 @@ func testChannelStoreSearchAllChannels(t *testing.T, ss store.Store) { ExpectedResults *model.ChannelList TotalCount int }{ + {"Search FooBar by display name", "oob", store.ChannelSearchOpts{IncludeDeleted: false}, &model.ChannelList{&o14}, 1}, + {"Search FooBar by display name2", "foo", store.ChannelSearchOpts{IncludeDeleted: false}, &model.ChannelList{&o14}, 1}, + {"Search FooBar by display name3", "bar", store.ChannelSearchOpts{IncludeDeleted: false}, &model.ChannelList{&o14}, 1}, + {"Search FooBar by name", "what", store.ChannelSearchOpts{IncludeDeleted: false}, &model.ChannelList{&o14}, 1}, + {"Search FooBar by name2", "ever", store.ChannelSearchOpts{IncludeDeleted: false}, &model.ChannelList{&o14}, 1}, {"ChannelA", "ChannelA", store.ChannelSearchOpts{IncludeDeleted: false}, &model.ChannelList{&o1, &o2, &o3}, 0}, {"ChannelA, include deleted", "ChannelA", store.ChannelSearchOpts{IncludeDeleted: true}, &model.ChannelList{&o1, &o2, &o3, &o13}, 0}, - {"empty string", "", store.ChannelSearchOpts{IncludeDeleted: false}, &model.ChannelList{&o1, &o2, &o3, &o4, &o5, &o12, &o11, &o8, &o7, &o6, &o10, &o9}, 0}, + {"empty string", "", store.ChannelSearchOpts{IncludeDeleted: false}, &model.ChannelList{&o1, &o2, &o3, &o4, &o5, &o12, &o14, &o11, &o8, &o7, &o6, &o10, &o9}, 0}, {"no matches", "blargh", store.ChannelSearchOpts{IncludeDeleted: false}, &model.ChannelList{}, 0}, {"prefix", "off-", store.ChannelSearchOpts{IncludeDeleted: false}, &model.ChannelList{&o8, &o7, &o6}, 0}, {"full match with dash", "off-topic", store.ChannelSearchOpts{IncludeDeleted: false}, &model.ChannelList{&o6}, 0}, diff --git a/store/storetest/team_store.go b/store/storetest/team_store.go index 537e531422..db2871319c 100644 --- a/store/storetest/team_store.go +++ b/store/storetest/team_store.go @@ -231,12 +231,52 @@ func testTeamStoreSearchAll(t *testing.T, ss store.Store) { _, err = ss.Team().Save(&p) require.Nil(t, err) + q := model.Team{} + q.DisplayName = "CHOCOLATE" + q.Name = "ilovecake" + q.Email = MakeEmail() + q.Type = model.TEAM_OPEN + q.AllowOpenInvite = false + + _, err = ss.Team().Save(&q) + require.Nil(t, err) + testCases := []struct { Name string Term string ExpectedLenth int ExpectedFirstId string }{ + { + "Search chocolate by display name", + "ocola", + 1, + q.Id, + }, + { + "Search chocolate by display name", + "choc", + 1, + q.Id, + }, + { + "Search chocolate by display name", + "late", + 1, + q.Id, + }, + { + "Search chocolate by name", + "ilov", + 1, + q.Id, + }, + { + "Search chocolate by name", + "ecake", + 1, + q.Id, + }, { "Search for open team name", o.Name, @@ -302,12 +342,52 @@ func testTeamStoreSearchOpen(t *testing.T, ss store.Store) { _, err = ss.Team().Save(&p) require.Nil(t, err) + q := model.Team{} + q.DisplayName = "PINEAPPLEPIE" + q.Name = "ihadsomepineapplepiewithstrawberry" + q.Email = MakeEmail() + q.Type = model.TEAM_OPEN + q.AllowOpenInvite = true + + _, err = ss.Team().Save(&q) + require.Nil(t, err) + testCases := []struct { Name string Term string ExpectedLength int ExpectedFirstId string }{ + { + "Search PINEAPPLEPIE by display name", + "neapplep", + 1, + q.Id, + }, + { + "Search PINEAPPLEPIE by display name", + "pine", + 1, + q.Id, + }, + { + "Search PINEAPPLEPIE by display name", + "epie", + 1, + q.Id, + }, + { + "Search PINEAPPLEPIE by name", + "ihadsome", + 1, + q.Id, + }, + { + "Search PINEAPPLEPIE by name", + "pineapplepiewithstrawberry", + 1, + q.Id, + }, { "Search for open team name", o.Name, @@ -368,12 +448,52 @@ func testTeamStoreSearchPrivate(t *testing.T, ss store.Store) { _, err = ss.Team().Save(&p) require.Nil(t, err) + q := model.Team{} + q.DisplayName = "FOOBAR" + q.Name = "whatever" + q.Email = MakeEmail() + q.Type = model.TEAM_OPEN + q.AllowOpenInvite = false + + _, err = ss.Team().Save(&q) + require.Nil(t, err) + testCases := []struct { Name string Term string ExpectedLength int ExpectedFirstId string }{ + { + "Search FooBar by display name from text in the middle of display name", + "ooba", + 1, + q.Id, + }, + { + "Search FooBar by display name from text at the beginning of display name", + "foo", + 1, + q.Id, + }, + { + "Search FooBar by display name from text at the end of display name", + "bar", + 1, + q.Id, + }, + { + "Search FooBar by name from text at the beginning name", + "what", + 1, + q.Id, + }, + { + "Search FooBar by name from text at the end of name", + "ever", + 1, + q.Id, + }, { "Search for private team name", p.Name,