Commit 49e04737 authored by Jesús Espino's avatar Jesús Espino Committed by Carlos Tadeu Panato Junior

MM-11567: Autocomplete search in: for DMs and GMs (#9430)

* MM-11567: Autocomplete search in: for DMs and GMs

* Adding unit tests

* Allowing to search Direct Messages in the autocompletion

* Fix it in TE
parent 89852d04
......@@ -2109,9 +2109,11 @@ func TestAutocompleteChannels(t *testing.T) {
[]string{"town"},
},
} {
if channels, resp := th.Client.AutocompleteChannelsForTeam(tc.teamId, tc.fragment); resp.Error != nil {
t.Fatal("Test case " + tc.description + " failed. Err: " + resp.Error.Error())
} else {
t.Run(tc.description, func(t *testing.T) {
channels, resp := th.Client.AutocompleteChannelsForTeam(tc.teamId, tc.fragment)
if resp.Error != nil {
t.Fatal("Err: " + resp.Error.Error())
}
for _, expectedInclude := range tc.expectedIncludes {
found := false
for _, channel := range *channels {
......@@ -2121,17 +2123,140 @@ func TestAutocompleteChannels(t *testing.T) {
}
}
if !found {
t.Fatal("Test case " + tc.description + " failed. Expected but didn't find channel: " + expectedInclude)
t.Fatal("Expected but didn't find channel: " + expectedInclude)
}
}
for _, expectedExclude := range tc.expectedExcludes {
for _, channel := range *channels {
if channel.Name == expectedExclude {
t.Fatal("Test case " + tc.description + " failed. Found channel we didn't want: " + expectedExclude)
t.Fatal("Found channel we didn't want: " + expectedExclude)
}
}
}
}
})
}
}
func TestAutocompleteChannelsForSearch(t *testing.T) {
th := Setup().InitBasic().InitSystemAdmin()
defer th.TearDown()
th.LoginSystemAdminWithClient(th.SystemAdminClient)
th.LoginBasicWithClient(th.Client)
u1 := th.CreateUserWithClient(th.SystemAdminClient)
u2 := th.CreateUserWithClient(th.SystemAdminClient)
u3 := th.CreateUserWithClient(th.SystemAdminClient)
u4 := th.CreateUserWithClient(th.SystemAdminClient)
// A private channel to make sure private channels are not used
utils.DisableDebugLogForTest()
ptown, _ := th.SystemAdminClient.CreateChannel(&model.Channel{
DisplayName: "Town",
Name: "town",
Type: model.CHANNEL_PRIVATE,
TeamId: th.BasicTeam.Id,
})
defer func() {
th.Client.DeleteChannel(ptown.Id)
}()
mypriv, _ := th.Client.CreateChannel(&model.Channel{
DisplayName: "My private town",
Name: "townpriv",
Type: model.CHANNEL_PRIVATE,
TeamId: th.BasicTeam.Id,
})
defer func() {
th.Client.DeleteChannel(mypriv.Id)
}()
utils.EnableDebugLogForTest()
dc1, resp := th.Client.CreateDirectChannel(th.BasicUser.Id, u1.Id)
CheckNoError(t, resp)
defer func() {
th.Client.DeleteChannel(dc1.Id)
}()
dc2, resp := th.SystemAdminClient.CreateDirectChannel(u2.Id, u3.Id)
CheckNoError(t, resp)
defer func() {
th.SystemAdminClient.DeleteChannel(dc2.Id)
}()
gc1, resp := th.Client.CreateGroupChannel([]string{th.BasicUser.Id, u2.Id, u3.Id})
CheckNoError(t, resp)
defer func() {
th.Client.DeleteChannel(gc1.Id)
}()
gc2, resp := th.SystemAdminClient.CreateGroupChannel([]string{u2.Id, u3.Id, u4.Id})
CheckNoError(t, resp)
defer func() {
th.SystemAdminClient.DeleteChannel(gc2.Id)
}()
for _, tc := range []struct {
description string
teamId string
fragment string
expectedIncludes []string
expectedExcludes []string
}{
{
"Basic town-square",
th.BasicTeam.Id,
"town",
[]string{"town-square", "townpriv"},
[]string{"off-topic", "town"},
},
{
"Basic off-topic",
th.BasicTeam.Id,
"off-to",
[]string{"off-topic"},
[]string{"town-square", "town", "townpriv"},
},
{
"Basic town square and off topic",
th.BasicTeam.Id,
"to",
[]string{"off-topic", "town-square", "townpriv"},
[]string{"town"},
},
{
"Direct and group messages",
th.BasicTeam.Id,
"fakeuser",
[]string{dc1.Name, gc1.Name},
[]string{dc2.Name, gc2.Name},
},
} {
t.Run(tc.description, func(t *testing.T) {
channels, resp := th.Client.AutocompleteChannelsForTeamForSearch(tc.teamId, tc.fragment)
if resp.Error != nil {
t.Fatal("Err: " + resp.Error.Error())
}
for _, expectedInclude := range tc.expectedIncludes {
found := false
for _, channel := range *channels {
if channel.Name == expectedInclude {
found = true
break
}
}
if !found {
t.Fatal("Expected but didn't find channel: " + expectedInclude + " Channels: " + fmt.Sprintf("%v", channels))
}
}
for _, expectedExclude := range tc.expectedExcludes {
for _, channel := range *channels {
if channel.Name == expectedExclude {
t.Fatal("Found channel we didn't want: " + expectedExclude)
}
}
}
})
}
}
......
......@@ -616,6 +616,43 @@ func (a *App) DeletePostFiles(post *model.Post) {
}
}
func (a *App) parseAndFetchChannelIdByNameFromInFilter(channelName, userId, teamId string, includeDeleted bool) (*model.Channel, error) {
if strings.HasPrefix(channelName, "@") && strings.Contains(channelName, ",") {
var userIds []string
users, err := a.GetUsersByUsernames(strings.Split(channelName[1:], ","), false)
if err != nil {
return nil, err
}
for _, user := range users {
userIds = append(userIds, user.Id)
}
channel, err := a.GetGroupChannel(userIds)
if err != nil {
return nil, err
}
return channel, nil
}
if strings.HasPrefix(channelName, "@") && !strings.Contains(channelName, ",") {
user, err := a.GetUserByUsername(channelName[1:])
if err != nil {
return nil, err
}
channel, err := a.GetDirectChannel(userId, user.Id)
if err != nil {
return nil, err
}
return channel, nil
}
channel, err := a.GetChannelByName(channelName, teamId, includeDeleted)
if err != nil {
return nil, err
}
return channel, nil
}
func (a *App) SearchPostsInTeam(terms string, userId string, teamId string, isOrSearch bool, includeDeletedChannels bool, timeZoneOffset int, page, perPage int) (*model.PostSearchResults, *model.AppError) {
paramsList := model.ParseSearchParams(terms, timeZoneOffset)
includeDeleted := includeDeletedChannels && *a.Config().TeamSettings.ExperimentalViewArchivedChannels
......@@ -630,11 +667,12 @@ func (a *App) SearchPostsInTeam(terms string, userId string, teamId string, isOr
if params.Terms != "*" {
// Convert channel names to channel IDs
for idx, channelName := range params.InChannels {
if channel, err := a.GetChannelByName(channelName, teamId, includeDeleted); err != nil {
channel, err := a.parseAndFetchChannelIdByNameFromInFilter(channelName, userId, teamId, includeDeletedChannels)
if err != nil {
mlog.Error(fmt.Sprint(err))
} else {
params.InChannels[idx] = channel.Id
continue
}
params.InChannels[idx] = channel.Id
}
// Convert usernames to user IDs
......@@ -695,6 +733,16 @@ func (a *App) SearchPostsInTeam(terms string, userId string, teamId string, isOr
params.OrTerms = isOrSearch
// don't allow users to search for everything
if params.Terms != "*" {
for idx, channelName := range params.InChannels {
if strings.HasPrefix(channelName, "@") {
channel, err := a.parseAndFetchChannelIdByNameFromInFilter(channelName, userId, teamId, includeDeletedChannels)
if err != nil {
mlog.Error(fmt.Sprint(err))
continue
}
params.InChannels[idx] = channel.Name
}
}
channels = append(channels, a.Srv.Store.Post().Search(teamId, userId, params))
}
}
......
......@@ -1691,14 +1691,14 @@ func (s SqlChannelStore) AutocompleteInTeam(teamId string, term string, includeD
var channels model.ChannelList
if likeClause, likeTerm := s.buildLIKEClause(term); likeClause == "" {
if likeClause, likeTerm := s.buildLIKEClause(term, "Name, DisplayName, Purpose"); likeClause == "" {
if _, err := s.GetReplica().Select(&channels, fmt.Sprintf(queryFormat, ""), map[string]interface{}{"TeamId": teamId}); err != nil {
result.Err = model.NewAppError("SqlChannelStore.AutocompleteInTeam", "store.sql_channel.search.app_error", nil, "term="+term+", "+", "+err.Error(), http.StatusInternalServerError)
}
} else {
// Using a UNION results in index_merge and fulltext queries and is much faster than the ref
// query you would get using an OR of the LIKE and full-text clauses.
fulltextClause, fulltextTerm := s.buildFulltextClause(term)
fulltextClause, fulltextTerm := s.buildFulltextClause(term, "Name, DisplayName, Purpose")
likeQuery := fmt.Sprintf(queryFormat, "AND "+likeClause)
fulltextQuery := fmt.Sprintf(queryFormat, "AND "+fulltextClause)
query := fmt.Sprintf("(%v) UNION (%v) LIMIT 50", likeQuery, fulltextQuery)
......@@ -1730,7 +1730,7 @@ func (s SqlChannelStore) AutocompleteInTeamForSearch(teamId string, userId strin
JOIN
ChannelMembers AS CM ON CM.ChannelId = C.Id
WHERE
C.TeamId = :TeamId
(C.TeamId = :TeamId OR (C.TeamId = '' AND C.Type = 'G'))
AND CM.UserId = :UserId
` + deleteFilter + `
%v
......@@ -1738,14 +1738,14 @@ func (s SqlChannelStore) AutocompleteInTeamForSearch(teamId string, userId strin
var channels model.ChannelList
if likeClause, likeTerm := s.buildLIKEClause(term); likeClause == "" {
if likeClause, likeTerm := s.buildLIKEClause(term, "Name, DisplayName, Purpose"); likeClause == "" {
if _, err := s.GetReplica().Select(&channels, fmt.Sprintf(queryFormat, ""), map[string]interface{}{"TeamId": teamId, "UserId": userId}); err != nil {
result.Err = model.NewAppError("SqlChannelStore.AutocompleteInTeamForSearch", "store.sql_channel.search.app_error", nil, "term="+term+", "+", "+err.Error(), http.StatusInternalServerError)
}
} else {
// Using a UNION results in index_merge and fulltext queries and is much faster than the ref
// query you would get using an OR of the LIKE and full-text clauses.
fulltextClause, fulltextTerm := s.buildFulltextClause(term)
fulltextClause, fulltextTerm := s.buildFulltextClause(term, "Name, DisplayName, Purpose")
likeQuery := fmt.Sprintf(queryFormat, "AND "+likeClause)
fulltextQuery := fmt.Sprintf(queryFormat, "AND "+fulltextClause)
query := fmt.Sprintf("(%v) UNION (%v) LIMIT 50", likeQuery, fulltextQuery)
......@@ -1755,6 +1755,14 @@ func (s SqlChannelStore) AutocompleteInTeamForSearch(teamId string, userId strin
}
}
directChannels, err := s.autocompleteInTeamForSearchDirectMessages(userId, term)
if err != nil {
result.Err = err
return
}
channels = append(channels, directChannels...)
sort.Slice(channels, func(a, b int) bool {
return strings.ToLower(channels[a].DisplayName) < strings.ToLower(channels[b].DisplayName)
})
......@@ -1762,6 +1770,48 @@ func (s SqlChannelStore) AutocompleteInTeamForSearch(teamId string, userId strin
})
}
func (s SqlChannelStore) autocompleteInTeamForSearchDirectMessages(userId string, term string) ([]*model.Channel, *model.AppError) {
queryFormat := `
SELECT
C.*,
OtherUsers.Username as DisplayName
FROM
Channels AS C
JOIN
ChannelMembers AS CM ON CM.ChannelId = C.Id
INNER JOIN (
SELECT
ICM.ChannelId AS ChannelId, IU.Username AS Username
FROM
Users as IU
JOIN
ChannelMembers AS ICM ON ICM.UserId = IU.Id
WHERE
IU.Id != :UserId
%v
) AS OtherUsers ON OtherUsers.ChannelId = C.Id
WHERE
C.Type = 'D'
AND CM.UserId = :UserId
LIMIT 50`
var channels model.ChannelList
if likeClause, likeTerm := s.buildLIKEClause(term, "IU.Username, IU.Nickname"); likeClause == "" {
if _, err := s.GetReplica().Select(&channels, fmt.Sprintf(queryFormat, ""), map[string]interface{}{"UserId": userId}); err != nil {
return nil, model.NewAppError("SqlChannelStore.AutocompleteInTeamForSearch", "store.sql_channel.search.app_error", nil, "term="+term+", "+", "+err.Error(), http.StatusInternalServerError)
}
} else {
query := fmt.Sprintf(queryFormat, "AND "+likeClause)
if _, err := s.GetReplica().Select(&channels, query, map[string]interface{}{"UserId": userId, "LikeTerm": likeTerm}); err != nil {
return nil, model.NewAppError("SqlChannelStore.AutocompleteInTeamForSearch", "store.sql_channel.search.app_error", nil, "term="+term+", "+", "+err.Error(), http.StatusInternalServerError)
}
}
return channels, nil
}
func (s SqlChannelStore) SearchInTeam(teamId string, term string, includeDeleted bool) store.StoreChannel {
return store.Do(func(result *store.StoreResult) {
deleteFilter := "AND DeleteAt = 0"
......@@ -1814,9 +1864,8 @@ func (s SqlChannelStore) SearchMore(userId string, teamId string, term string) s
})
}
func (s SqlChannelStore) buildLIKEClause(term string) (likeClause, likeTerm string) {
func (s SqlChannelStore) buildLIKEClause(term string, searchColumns string) (likeClause, likeTerm string) {
likeTerm = term
searchColumns := "Name, DisplayName, Purpose"
// These chars must be removed from the like query.
for _, c := range ignoreLikeSearchChar {
......@@ -1847,12 +1896,10 @@ func (s SqlChannelStore) buildLIKEClause(term string) (likeClause, likeTerm stri
return
}
func (s SqlChannelStore) buildFulltextClause(term string) (fulltextClause, fulltextTerm string) {
func (s SqlChannelStore) buildFulltextClause(term string, searchColumns string) (fulltextClause, fulltextTerm string) {
// Copy the terms as we will need to prepare them differently for each search type.
fulltextTerm = term
searchColumns := "Name, DisplayName, Purpose"
// These chars must be treated as spaces in the fulltext query.
for _, c := range spaceFulltextSearchChar {
fulltextTerm = strings.Replace(fulltextTerm, c, " ", -1)
......@@ -1891,13 +1938,13 @@ func (s SqlChannelStore) buildFulltextClause(term string) (fulltextClause, fullt
func (s SqlChannelStore) performSearch(searchQuery string, term string, parameters map[string]interface{}) store.StoreResult {
result := store.StoreResult{}
likeClause, likeTerm := s.buildLIKEClause(term)
likeClause, likeTerm := s.buildLIKEClause(term, "Name, DisplayName, Purpose")
if likeTerm == "" {
// If the likeTerm is empty after preparing, then don't bother searching.
searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "", 1)
} else {
parameters["LikeTerm"] = likeTerm
fulltextClause, fulltextTerm := s.buildFulltextClause(term)
fulltextClause, fulltextTerm := s.buildFulltextClause(term, "Name, DisplayName, Purpose")
parameters["FulltextTerm"] = fulltextTerm
searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "AND ("+likeClause+" OR "+fulltextClause+")", 1)
}
......
......@@ -555,14 +555,14 @@ func (s SqlChannelStoreExperimental) AutocompleteInTeam(teamId string, term stri
var channels model.ChannelList
if likeClause, likeTerm := s.buildLIKEClause(term); likeClause == "" {
if likeClause, likeTerm := s.buildLIKEClause(term, "c.Name, c.DisplayName, c.Purpose"); likeClause == "" {
if _, err := s.GetReplica().Select(&channels, fmt.Sprintf(queryFormat, ""), map[string]interface{}{"TeamId": teamId}); err != nil {
result.Err = model.NewAppError("SqlChannelStore.AutocompleteInTeam", "store.sql_channel.search.app_error", nil, "term="+term+", "+", "+err.Error(), http.StatusInternalServerError)
}
} else {
// Using a UNION results in index_merge and fulltext queries and is much faster than the ref
// query you would get using an OR of the LIKE and full-text clauses.
fulltextClause, fulltextTerm := s.buildFulltextClause(term)
fulltextClause, fulltextTerm := s.buildFulltextClause(term, "c.Name, c.DisplayName, c.Purpose")
likeQuery := fmt.Sprintf(queryFormat, "AND "+likeClause)
fulltextQuery := fmt.Sprintf(queryFormat, "AND "+fulltextClause)
query := fmt.Sprintf("(%v) UNION (%v) LIMIT 50", likeQuery, fulltextQuery)
......@@ -647,13 +647,12 @@ func (s SqlChannelStoreExperimental) SearchMore(userId string, teamId string, te
})
}
func (s SqlChannelStoreExperimental) buildLIKEClause(term string) (likeClause, likeTerm string) {
func (s SqlChannelStoreExperimental) buildLIKEClause(term string, searchColumns string) (likeClause, likeTerm string) {
if !s.IsExperimentalPublicChannelsMaterializationEnabled() {
return s.SqlChannelStore.buildLIKEClause(term)
return s.SqlChannelStore.buildLIKEClause(term, searchColumns)
}
likeTerm = term
searchColumns := "c.Name, c.DisplayName, c.Purpose"
// These chars must be removed from the like query.
for _, c := range ignoreLikeSearchChar {
......@@ -684,16 +683,14 @@ func (s SqlChannelStoreExperimental) buildLIKEClause(term string) (likeClause, l
return
}
func (s SqlChannelStoreExperimental) buildFulltextClause(term string) (fulltextClause, fulltextTerm string) {
func (s SqlChannelStoreExperimental) buildFulltextClause(term string, searchColumns string) (fulltextClause, fulltextTerm string) {
if !s.IsExperimentalPublicChannelsMaterializationEnabled() {
return s.SqlChannelStore.buildFulltextClause(term)
return s.SqlChannelStore.buildFulltextClause(term, searchColumns)
}
// Copy the terms as we will need to prepare them differently for each search type.
fulltextTerm = term
searchColumns := "c.Name, c.DisplayName, c.Purpose"
// These chars must be treated as spaces in the fulltext query.
for _, c := range spaceFulltextSearchChar {
fulltextTerm = strings.Replace(fulltextTerm, c, " ", -1)
......@@ -736,13 +733,13 @@ func (s SqlChannelStoreExperimental) performSearch(searchQuery string, term stri
result := store.StoreResult{}
likeClause, likeTerm := s.buildLIKEClause(term)
likeClause, likeTerm := s.buildLIKEClause(term, "c.Name, c.DisplayName, c.Purpose")
if likeTerm == "" {
// If the likeTerm is empty after preparing, then don't bother searching.
searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "", 1)
} else {
parameters["LikeTerm"] = likeTerm
fulltextClause, fulltextTerm := s.buildFulltextClause(term)
fulltextClause, fulltextTerm := s.buildFulltextClause(term, "c.Name, c.DisplayName, c.Purpose")
parameters["FulltextTerm"] = fulltextTerm
searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "AND ("+likeClause+" OR "+fulltextClause+")", 1)
}
......
......@@ -2023,6 +2023,30 @@ func testChannelStoreSearchInTeam(t *testing.T, ss store.Store) {
}
func testChannelStoreAutocompleteInTeamForSearch(t *testing.T, ss store.Store) {
u1 := &model.User{}
u1.Email = MakeEmail()
u1.Username = "user1" + model.NewId()
u1.Nickname = model.NewId()
store.Must(ss.User().Save(u1))
u2 := &model.User{}
u2.Email = MakeEmail()
u2.Username = "user2" + model.NewId()
u2.Nickname = model.NewId()
store.Must(ss.User().Save(u2))
u3 := &model.User{}
u3.Email = MakeEmail()
u3.Username = "user3" + model.NewId()
u3.Nickname = model.NewId()
store.Must(ss.User().Save(u3))
u4 := &model.User{}
u4.Email = MakeEmail()
u4.Username = "user4" + model.NewId()
u4.Nickname = model.NewId()
store.Must(ss.User().Save(u4))
o1 := model.Channel{}
o1.TeamId = model.NewId()
o1.DisplayName = "ChannelA"
......@@ -2032,7 +2056,7 @@ func testChannelStoreAutocompleteInTeamForSearch(t *testing.T, ss store.Store) {
m1 := model.ChannelMember{}
m1.ChannelId = o1.Id
m1.UserId = model.NewId()
m1.UserId = u1.Id
m1.NotifyProps = model.GetDefaultChannelNotifyProps()
store.Must(ss.Channel().SaveMember(&m1))
......@@ -2084,17 +2108,22 @@ func testChannelStoreAutocompleteInTeamForSearch(t *testing.T, ss store.Store) {
o5.Type = model.CHANNEL_PRIVATE
store.Must(ss.Channel().Save(&o5, -1))
store.Must(ss.Channel().CreateDirectChannel(u1.Id, u2.Id))
store.Must(ss.Channel().CreateDirectChannel(u2.Id, u3.Id))
tt := []struct {
name string
term string
includeDeleted bool
expectedMatches int
}{
{"Empty search (list all)", "", false, 3},
{"Empty search (list all)", "", false, 4},
{"Narrow search", "ChannelA", false, 2},
{"Wide search", "Cha", false, 3},
{"Direct messages", "user", false, 1},
{"Wide search with archived channels", "Cha", true, 4},
{"Narrow with archived channels", "ChannelA", true, 3},
{"Direct messages with archived channels", "user", true, 1},
{"Search without results", "blarg", true, 0},
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment