From 8fe003876d9cd4a459a1b9a5d4ea4a05dc476bae Mon Sep 17 00:00:00 2001 From: Farhan Munshi <3207297+fmunshi@users.noreply.github.com> Date: Fri, 8 May 2020 16:32:48 -0400 Subject: [PATCH] [MM-24828] Take into account group synced state of team and channel when sending notifications (#14477) * MM-24828 Take into account group synced state of team and channel when getting groups for mentions * Update app/notification_test.go * i18n-extract * Revert "i18n-extract" This reverts commit dcb0426b98afa4646c26870c0e3a1236f99fda17. * Trigger CI Co-authored-by: mattermod --- app/notification.go | 41 +++++++++++++--- app/notification_test.go | 103 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 7 deletions(-) diff --git a/app/notification.go b/app/notification.go index d1712b85d8..87bf89fb78 100644 --- a/app/notification.go +++ b/app/notification.go @@ -40,13 +40,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod if a.allowGroupMentions(post) { gchan = make(chan store.StoreResult, 1) go func() { - groups, err := a.Srv().Store.Group().GetGroups(0, 0, model.GroupSearchOpts{FilterAllowReference: true}) - groupsMap := make(map[string]*model.Group) - if err == nil { - for _, group := range groups { - groupsMap[group.Name] = group - } - } + groupsMap, err := a.getGroupsAllowedForReferenceInChannel(channel, team) gchan <- store.StoreResult{Data: groupsMap, Err: err} close(gchan) }() @@ -755,6 +749,39 @@ func (a *App) allowGroupMentions(post *model.Post) bool { return true } +// getGroupsAllowedForReferenceInChannel returns a map of groups allowed for reference in a given channel and team. +func (a *App) getGroupsAllowedForReferenceInChannel(channel *model.Channel, team *model.Team) (map[string]*model.Group, *model.AppError) { + var err *model.AppError + groupsMap := make(map[string]*model.Group) + opts := model.GroupSearchOpts{FilterAllowReference: true} + + if channel.IsGroupConstrained() || team.IsGroupConstrained() { + var groups []*model.GroupWithSchemeAdmin + if channel.IsGroupConstrained() { + groups, err = a.Srv().Store.Group().GetGroupsByChannel(channel.Id, opts) + } else { + groups, err = a.Srv().Store.Group().GetGroupsByTeam(team.Id, opts) + } + if err != nil { + return nil, err + } + for _, group := range groups { + groupsMap[group.Group.Name] = &group.Group + } + return groupsMap, nil + } + + groups, err := a.Srv().Store.Group().GetGroups(0, 0, opts) + if err != nil { + return nil, err + } + for _, group := range groups { + groupsMap[group.Name] = group + } + + return groupsMap, nil +} + // Given a map of user IDs to profiles, returns a list of mention // keywords for all users in the channel. func (a *App) getMentionKeywordsInChannel(profiles map[string]*model.User, allowChannelMentions bool, channelMemberNotifyPropsMap map[string]model.StringMap) map[string][]string { diff --git a/app/notification_test.go b/app/notification_test.go index 03bae9bc75..7bb630ceb8 100644 --- a/app/notification_test.go +++ b/app/notification_test.go @@ -2417,3 +2417,106 @@ func TestInsertGroupMentions(t *testing.T) { require.Equal(t, found, true) }) } + +func TestGetGroupsAllowedForReferenceInChannel(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + var err *model.AppError + var groupsMap map[string]*model.Group + + team := th.BasicTeam + channel := th.BasicChannel + group1 := th.CreateGroup() + + t.Run("should return empty map when no groups with allow reference", func(t *testing.T) { + groupsMap, err = th.App.getGroupsAllowedForReferenceInChannel(channel, team) + require.Nil(t, err) + require.Len(t, groupsMap, 0) + }) + + group1.AllowReference = true + group1, err = th.App.UpdateGroup(group1) + require.Nil(t, err) + + group2 := th.CreateGroup() + t.Run("should only return groups with allow reference", func(t *testing.T) { + groupsMap, err = th.App.getGroupsAllowedForReferenceInChannel(channel, team) + require.Nil(t, err) + require.Len(t, groupsMap, 1) + require.Nil(t, groupsMap[group2.Name]) + require.Equal(t, groupsMap[group1.Name], group1) + }) + + group2.AllowReference = true + group2, err = th.App.UpdateGroup(group2) + require.Nil(t, err) + + // Sync first group to constrained channel + constrainedChannel := th.CreateChannel(th.BasicTeam) + constrainedChannel.GroupConstrained = model.NewBool(true) + constrainedChannel, err = th.App.UpdateChannel(constrainedChannel) + require.Nil(t, err) + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ + GroupId: group1.Id, + Type: model.GroupSyncableTypeChannel, + SyncableId: constrainedChannel.Id, + }) + require.Nil(t, err) + + t.Run("should return only groups synced to channel if channel is group constrained", func(t *testing.T) { + groupsMap, err = th.App.getGroupsAllowedForReferenceInChannel(constrainedChannel, team) + require.Nil(t, err) + require.Len(t, groupsMap, 1) + require.Nil(t, groupsMap[group2.Name]) + require.Equal(t, groupsMap[group1.Name], group1) + }) + + // Create a third group not synced with a team or channel + group3 := th.CreateGroup() + group3.AllowReference = true + group3, err = th.App.UpdateGroup(group3) + require.Nil(t, err) + + // Sync group2 to the team + team.GroupConstrained = model.NewBool(true) + team, err = th.App.UpdateTeam(team) + require.Nil(t, err) + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ + GroupId: group2.Id, + Type: model.GroupSyncableTypeTeam, + SyncableId: team.Id, + }) + require.Nil(t, err) + + t.Run("should return union of groups synced to team and any channels if team is group constrained", func(t *testing.T) { + groupsMap, err = th.App.getGroupsAllowedForReferenceInChannel(channel, team) + require.Nil(t, err) + require.Len(t, groupsMap, 2) + require.Nil(t, groupsMap[group3.Name]) + require.Equal(t, groupsMap[group2.Name], group2) + require.Equal(t, groupsMap[group1.Name], group1) + }) + + t.Run("should return only subset of groups synced to channel for group constrained channel when team is also group constrained", func(t *testing.T) { + groupsMap, err = th.App.getGroupsAllowedForReferenceInChannel(constrainedChannel, team) + require.Nil(t, err) + require.Len(t, groupsMap, 1) + require.Nil(t, groupsMap[group3.Name]) + require.Nil(t, groupsMap[group2.Name]) + require.Equal(t, groupsMap[group1.Name], group1) + }) + + team.GroupConstrained = model.NewBool(false) + team, err = th.App.UpdateTeam(team) + require.Nil(t, err) + + t.Run("should return all groups when team and channel are not group constrained", func(t *testing.T) { + groupsMap, err = th.App.getGroupsAllowedForReferenceInChannel(channel, team) + require.Nil(t, err) + require.Len(t, groupsMap, 3) + require.Equal(t, groupsMap[group1.Name], group1) + require.Equal(t, groupsMap[group2.Name], group2) + require.Equal(t, groupsMap[group3.Name], group3) + }) +}