MM-45723: Include DM/GM channels for user indexing (#21391)

There were 2 separate issues here:

1. We were excluding DM/GM channels for user indexing.
Not sure why. This prevented users to be shown as
channel members in autocomplete results in both
Bleve and Elasticsearch.

2. We were incorrectly deciding to search across
the team based on if the current channel
is in the list of allowed channels or not. But we
were incorrectly wiping the list of allowed channels
even if a channelid was passed. This makes it work
same as a DB search.

One catch is that for the correct results to appear,
a re-indexing is required.

```release-note
Autocompletion results using Elasticsearch or Bleve will
correctly show a user as a channel member in DM/GM channels.
To force this change to appear, a re-indexing will be
necessary.
```
Этот коммит содержится в:
Agniva De Sarker
2022-10-19 20:51:00 +05:30
коммит произвёл GitHub
родитель 28698abb9f
Коммит 10a627fac1
3 изменённых файлов: 54 добавлений и 5 удалений

Просмотреть файл

@@ -37,7 +37,7 @@ func (s *SearchUserStore) deleteUserIndex(user *model.User) {
func (s *SearchUserStore) Search(teamId, term string, options *model.UserSearchOptions) ([]*model.User, error) {
for _, engine := range s.rootStore.searchEngine.GetActiveEngines() {
if engine.IsSearchEnabled() {
listOfAllowedChannels, nErr := s.getListOfAllowedChannelsForTeam(teamId, options.ViewRestrictions)
listOfAllowedChannels, nErr := s.getListOfAllowedChannels(teamId, "", options.ViewRestrictions)
if nErr != nil {
mlog.Warn("Encountered error on Search.", mlog.String("search_engine", engine.GetName()), mlog.Err(nErr))
continue
@@ -148,7 +148,7 @@ func (s *SearchUserStore) autocompleteUsersInChannelByEngine(engine searchengine
return autocomplete, nil
}
// getListOfAllowedChannelsForTeam return the list of allowed channels to search user based on the
// getListOfAllowedChannels return the list of allowed channels to search user based on the
// next scenarios:
// - If there isn't view restrictions (team or channel) and no team id to filter them, then all
// channels are allowed (nil return)
@@ -159,7 +159,7 @@ func (s *SearchUserStore) autocompleteUsersInChannelByEngine(engine searchengine
// - If we receive channels restrictions we get:
// - If we don't have team id, we get those restricted channels (guest accounts and quick search)
// - If we have a team id then we only return those restricted channels that belongs to that team
func (s *SearchUserStore) getListOfAllowedChannelsForTeam(teamId string, viewRestrictions *model.ViewUsersRestrictions) ([]string, error) {
func (s *SearchUserStore) getListOfAllowedChannels(teamId, channelId string, viewRestrictions *model.ViewUsersRestrictions) ([]string, error) {
var listOfAllowedChannels []string
if viewRestrictions == nil && teamId == "" {
// nil return without error means all channels are allowed
@@ -174,6 +174,20 @@ func (s *SearchUserStore) getListOfAllowedChannelsForTeam(teamId string, viewRes
for _, channel := range channels {
listOfAllowedChannels = append(listOfAllowedChannels, channel.Id)
}
if channelId != "" {
ch, err := s.rootStore.Channel().Get(channelId, true)
if err != nil {
return nil, errors.Wrapf(err, "failed to get channel with id: %s", channelId)
}
// Check if DM/GM channel, and add to the list.
// This is because GetTeamChannels does not return DM/GM channels.
// And since the channelId is passed from the API layer, it is already
// auth checked to confirm that the user has permission.
if ch.IsGroupOrDirect() {
listOfAllowedChannels = append(listOfAllowedChannels, channelId)
}
}
return listOfAllowedChannels, nil
}
@@ -196,7 +210,7 @@ func (s *SearchUserStore) getListOfAllowedChannelsForTeam(teamId string, viewRes
func (s *SearchUserStore) AutocompleteUsersInChannel(teamId, channelId, term string, options *model.UserSearchOptions) (*model.UserAutocompleteInChannel, error) {
for _, engine := range s.rootStore.searchEngine.GetActiveEngines() {
if engine.IsAutocompletionEnabled() {
listOfAllowedChannels, nErr := s.getListOfAllowedChannelsForTeam(teamId, options.ViewRestrictions)
listOfAllowedChannels, nErr := s.getListOfAllowedChannels(teamId, channelId, options.ViewRestrictions)
if nErr != nil {
mlog.Warn("Encountered error on AutocompleteUsersInChannel.", mlog.String("search_engine", engine.GetName()), mlog.Err(nErr))
continue
@@ -205,6 +219,7 @@ func (s *SearchUserStore) AutocompleteUsersInChannel(teamId, channelId, term str
return &model.UserAutocompleteInChannel{}, nil
}
options.ListOfAllowedChannels = listOfAllowedChannels
autocomplete, nErr := s.autocompleteUsersInChannelByEngine(engine, teamId, channelId, term, options)
if nErr != nil {
mlog.Warn("Encountered error on AutocompleteUsersInChannel.", mlog.String("search_engine", engine.GetName()), mlog.Err(nErr))

Просмотреть файл

@@ -1805,7 +1805,16 @@ func (us SqlUserStore) GetUsersBatchForIndexing(startTime int64, startFileID str
`).
From("ChannelMembers cm").
Join("Channels c ON cm.ChannelId = c.Id").
Where(sq.Eq{"c.Type": model.ChannelTypeOpen, "cm.UserId": userIds}).
Where(sq.And{
sq.Eq{
"cm.UserId": userIds,
},
sq.Or{
sq.Eq{"c.Type": model.ChannelTypeOpen},
sq.Eq{"c.Type": model.ChannelTypeDirect},
sq.Eq{"c.Type": model.ChannelTypeGroup},
},
}).
ToSql()
if err != nil {
return nil, errors.Wrap(err, "GetUsersBatchForIndexing_ToSql2")

Просмотреть файл

@@ -4912,10 +4912,35 @@ func testUserStoreGetUsersBatchForIndexing(t *testing.T, ss store.Store) {
})
require.NoError(t, err)
cDM := &model.Channel{
Name: model.NewId() + "__" + model.NewId(),
Type: model.ChannelTypeDirect,
}
cm1 := &model.ChannelMember{
UserId: u3.Id,
ChannelId: cDM.Id,
NotifyProps: model.GetDefaultChannelNotifyProps(),
}
cm2 := &model.ChannelMember{
UserId: u2.Id,
ChannelId: cDM.Id,
NotifyProps: model.GetDefaultChannelNotifyProps(),
}
cDM, nErr = ss.Channel().SaveDirectChannel(cDM, cm1, cm2)
require.NoError(t, nErr)
// Getting all users
res1List, err := ss.User().GetUsersBatchForIndexing(u1.CreateAt-1, "", 100)
require.NoError(t, err)
assert.Len(t, res1List, 3)
for _, user := range res1List {
switch user.Id {
case u2.Id:
assert.ElementsMatch(t, user.ChannelsIds, []string{cPub1.Id, cPub2.Id, cDM.Id})
case u3.Id:
assert.ElementsMatch(t, user.ChannelsIds, []string{cPub2.Id, cDM.Id})
}
}
// Testing pagination
res2List, err := ss.User().GetUsersBatchForIndexing(u1.CreateAt-1, "", 1)