[MM-23017] Add check for groups to SendNotifications (#14039)

* MM-23017 Check group mentions as part of notification logic * Add nil groups to existing test cases * MM-23017 Add tests for insertGroupMention and addGroupMention * MM-23017 Add tests for getExplicitMentions that have groups * Add tests for group store GetMemberUsersNotInChannel * MM-23017 Add tests for AllowGroupMentions * MM-23017 Fix error message name * MM-23017 Swap Checks to Name * MM-23017 Code review fixes * Rename var and fix allowGroupMentions test * MM-23017 Use GetMemberUsersInTeam inside of insertGroupMentions * MM-23017 use group mentions permission * Actually call GetMemberUsersInTeam * Remove unnecessary new line * Uncomment filter allow reference * MM-23017 Fix group channel notifications * Update store layer * MM-23017 Improve test coverage for group channels * Trigger CI * Trigger CI
2020-04-17 19:22:54 -04:00
--- a/store/opentracing_layer.go
+++ b/store/opentracing_layer.go
@@ -3337,6 +3337,42 @@ func (s *OpenTracingLayerGroupStore) GetMemberUsers(groupID string) ([]*model.Us
 	return resultVar0, resultVar1
 }

+func (s *OpenTracingLayerGroupStore) GetMemberUsersInTeam(groupID string, teamID string) ([]*model.User, *model.AppError) {
+	origCtx := s.Root.Store.Context()
+	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "GroupStore.GetMemberUsersInTeam")
+	s.Root.Store.SetContext(newCtx)
+	defer func() {
+		s.Root.Store.SetContext(origCtx)
+	}()
+
+	defer span.Finish()
+	resultVar0, resultVar1 := s.GroupStore.GetMemberUsersInTeam(groupID, teamID)
+	if resultVar1 != nil {
+		span.LogFields(spanlog.Error(resultVar1))
+		ext.Error.Set(span, true)
+	}
+
+	return resultVar0, resultVar1
+}
+
+func (s *OpenTracingLayerGroupStore) GetMemberUsersNotInChannel(groupID string, channelID string) ([]*model.User, *model.AppError) {
+	origCtx := s.Root.Store.Context()
+	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "GroupStore.GetMemberUsersNotInChannel")
+	s.Root.Store.SetContext(newCtx)
+	defer func() {
+		s.Root.Store.SetContext(origCtx)
+	}()
+
+	defer span.Finish()
+	resultVar0, resultVar1 := s.GroupStore.GetMemberUsersNotInChannel(groupID, channelID)
+	if resultVar1 != nil {
+		span.LogFields(spanlog.Error(resultVar1))
+		ext.Error.Set(span, true)
+	}
+
+	return resultVar0, resultVar1
+}
+
 func (s *OpenTracingLayerGroupStore) GetMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, *model.AppError) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "GroupStore.GetMemberUsersPage")
--- a/store/sqlstore/group_store.go
+++ b/store/sqlstore/group_store.go
@@ -260,7 +260,7 @@ func (s *SqlGroupStore) GetMemberUsers(groupID string) ([]*model.User, *model.Ap
 			AND GroupId = :GroupId`

 	if _, err := s.GetReplica().Select(&groupMembers, query, map[string]interface{}{"GroupId": groupID}); err != nil {
-		return nil, model.NewAppError("SqlGroupStore.GroupGetAllBySource", "store.select_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, model.NewAppError("SqlGroupStore.GetMemberUsers", "store.select_error", nil, err.Error(), http.StatusInternalServerError)
 	}

 	return groupMembers, nil
@@ -312,6 +312,70 @@ func (s *SqlGroupStore) GetMemberCount(groupID string) (int64, *model.AppError)
 	return count, nil
 }

+func (s *SqlGroupStore) GetMemberUsersInTeam(groupID string, teamID string) ([]*model.User, *model.AppError) {
+	var groupMembers []*model.User
+
+	query := `
+		SELECT
+			Users.*
+		FROM
+			GroupMembers
+			JOIN Users ON Users.Id = GroupMembers.UserId
+		WHERE
+			GroupId = :GroupId
+			AND GroupMembers.UserId IN (
+				SELECT TeamMembers.UserId
+				FROM TeamMembers
+				JOIN Teams ON Teams.Id = :TeamId
+				WHERE TeamMembers.TeamId = Teams.Id
+				AND TeamMembers.DeleteAt = 0
+			)
+			AND GroupMembers.DeleteAt = 0
+			AND Users.DeleteAt = 0
+		`
+
+	if _, err := s.GetReplica().Select(&groupMembers, query, map[string]interface{}{"GroupId": groupID, "TeamId": teamID}); err != nil {
+		return nil, model.NewAppError("SqlGroupStore.GetMemberUsersInTeam", "store.select_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+
+	return groupMembers, nil
+}
+
+func (s *SqlGroupStore) GetMemberUsersNotInChannel(groupID string, channelID string) ([]*model.User, *model.AppError) {
+	var groupMembers []*model.User
+
+	query := `
+		SELECT
+			Users.*
+		FROM
+			GroupMembers
+			JOIN Users ON Users.Id = GroupMembers.UserId
+		WHERE
+			GroupId = :GroupId
+			AND GroupMembers.UserId NOT IN (
+				SELECT ChannelMembers.UserId
+				FROM ChannelMembers
+				WHERE ChannelMembers.ChannelId = :ChannelId
+			)
+			AND GroupMembers.UserId IN (
+				SELECT TeamMembers.UserId
+				FROM TeamMembers
+				JOIN Channels ON Channels.Id = :ChannelId
+				JOIN Teams ON Teams.Id = Channels.TeamId
+				WHERE TeamMembers.TeamId = Teams.Id
+				AND TeamMembers.DeleteAt = 0
+			)
+			AND GroupMembers.DeleteAt = 0
+			AND Users.DeleteAt = 0
+		`
+
+	if _, err := s.GetReplica().Select(&groupMembers, query, map[string]interface{}{"GroupId": groupID, "ChannelId": channelID}); err != nil {
+		return nil, model.NewAppError("SqlGroupStore.GetMemberUsersNotInChannel", "store.select_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+
+	return groupMembers, nil
+}
+
 func (s *SqlGroupStore) UpsertMember(groupID string, userID string) (*model.GroupMember, *model.AppError) {
 	member := &model.GroupMember{
 		GroupId:  groupID,
@@ -1081,10 +1145,14 @@ func (s *SqlGroupStore) GetGroups(page, perPage int, opts model.GroupSearchOpts)
 	groupsQuery = groupsQuery.
 		From("UserGroups g").
 		Where("g.DeleteAt = 0").
-		Limit(uint64(perPage)).
-		Offset(uint64(page * perPage)).
 		OrderBy("g.DisplayName")

+	if perPage != 0 {
+		groupsQuery = groupsQuery.
+			Limit(uint64(perPage)).
+			Offset(uint64(page * perPage))
+	}
+
 	if opts.FilterAllowReference {
 		groupsQuery = groupsQuery.Where("g.AllowReference = true")
 	}
--- a/store/store.go
+++ b/store/store.go
@@ -629,6 +629,10 @@ type GroupStore interface {
 	GetMemberUsers(groupID string) ([]*model.User, *model.AppError)
 	GetMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, *model.AppError)
 	GetMemberCount(groupID string) (int64, *model.AppError)
+
+	GetMemberUsersInTeam(groupID string, teamID string) ([]*model.User, *model.AppError)
+	GetMemberUsersNotInChannel(groupID string, channelID string) ([]*model.User, *model.AppError)
+
 	UpsertMember(groupID string, userID string) (*model.GroupMember, *model.AppError)
 	DeleteMember(groupID string, userID string) (*model.GroupMember, *model.AppError)
 	PermanentDeleteMembersByUser(userId string) *model.AppError
--- a/store/storetest/group_store.go
+++ b/store/storetest/group_store.go
@@ -32,6 +32,10 @@ func TestGroupStore(t *testing.T, ss store.Store) {

 	t.Run("GetMemberUsers", func(t *testing.T) { testGroupGetMemberUsers(t, ss) })
 	t.Run("GetMemberUsersPage", func(t *testing.T) { testGroupGetMemberUsersPage(t, ss) })
+
+	t.Run("GetMemberUsersInTeam", func(t *testing.T) { testGroupGetMemberUsersInTeam(t, ss) })
+	t.Run("GetMemberUsersNotInChannel", func(t *testing.T) { testGroupGetMemberUsersNotInChannel(t, ss) })
+
 	t.Run("UpsertMember", func(t *testing.T) { testUpsertMember(t, ss) })
 	t.Run("DeleteMember", func(t *testing.T) { testGroupDeleteMember(t, ss) })
 	t.Run("PermanentDeleteMembersByUser", func(t *testing.T) { testGroupPermanentDeleteMembersByUser(t, ss) })
@@ -682,6 +686,231 @@ func testGroupGetMemberUsersPage(t *testing.T, ss store.Store) {
 	require.Equal(t, 2, len(groupMembers))
 }

+func testGroupGetMemberUsersInTeam(t *testing.T, ss store.Store) {
+	// Save a team
+	team := &model.Team{
+		DisplayName: "Name",
+		Description: "Some description",
+		CompanyName: "Some company name",
+		Name:        "z-z-" + model.NewId() + "a",
+		Email:       "success+" + model.NewId() + "@simulator.amazonses.com",
+		Type:        model.TEAM_OPEN,
+	}
+	team, err := ss.Team().Save(team)
+	require.Nil(t, err)
+
+	// Save a group
+	g1 := &model.Group{
+		Name:        model.NewId(),
+		DisplayName: model.NewId(),
+		Description: model.NewId(),
+		Source:      model.GroupSourceLdap,
+		RemoteId:    model.NewId(),
+	}
+	group, err := ss.Group().Create(g1)
+	require.Nil(t, err)
+
+	u1 := &model.User{
+		Email:    MakeEmail(),
+		Username: model.NewId(),
+	}
+	user1, err := ss.User().Save(u1)
+	require.Nil(t, err)
+
+	_, err = ss.Group().UpsertMember(group.Id, user1.Id)
+	require.Nil(t, err)
+
+	u2 := &model.User{
+		Email:    MakeEmail(),
+		Username: model.NewId(),
+	}
+	user2, err := ss.User().Save(u2)
+	require.Nil(t, err)
+
+	_, err = ss.Group().UpsertMember(group.Id, user2.Id)
+	require.Nil(t, err)
+
+	u3 := &model.User{
+		Email:    MakeEmail(),
+		Username: model.NewId(),
+	}
+	user3, err := ss.User().Save(u3)
+	require.Nil(t, err)
+
+	_, err = ss.Group().UpsertMember(group.Id, user3.Id)
+	require.Nil(t, err)
+
+	// returns no members when team does not exist
+	groupMembers, err := ss.Group().GetMemberUsersInTeam(group.Id, "non-existant-channel-id")
+	require.Nil(t, err)
+	require.Equal(t, 0, len(groupMembers))
+
+	// returns no members when group has no members in the team
+	groupMembers, err = ss.Group().GetMemberUsersInTeam(group.Id, team.Id)
+	require.Nil(t, err)
+	require.Equal(t, 0, len(groupMembers))
+
+	m1 := &model.TeamMember{TeamId: team.Id, UserId: user1.Id}
+	_, err = ss.Team().SaveMember(m1, -1)
+	require.Nil(t, err)
+
+	// returns single member in team
+	groupMembers, err = ss.Group().GetMemberUsersInTeam(group.Id, team.Id)
+	require.Nil(t, err)
+	require.Equal(t, 1, len(groupMembers))
+
+	m2 := &model.TeamMember{TeamId: team.Id, UserId: user2.Id}
+	m3 := &model.TeamMember{TeamId: team.Id, UserId: user3.Id}
+	_, err = ss.Team().SaveMember(m2, -1)
+	require.Nil(t, err)
+	_, err = ss.Team().SaveMember(m3, -1)
+	require.Nil(t, err)
+
+	// returns all members when all members are in team
+	groupMembers, err = ss.Group().GetMemberUsersInTeam(group.Id, team.Id)
+	require.Nil(t, err)
+	require.Equal(t, 3, len(groupMembers))
+}
+
+func testGroupGetMemberUsersNotInChannel(t *testing.T, ss store.Store) {
+	// Save a team
+	team := &model.Team{
+		DisplayName: "Name",
+		Description: "Some description",
+		CompanyName: "Some company name",
+		Name:        "z-z-" + model.NewId() + "a",
+		Email:       "success+" + model.NewId() + "@simulator.amazonses.com",
+		Type:        model.TEAM_OPEN,
+	}
+	team, err := ss.Team().Save(team)
+	require.Nil(t, err)
+
+	// Save a group
+	g1 := &model.Group{
+		Name:        model.NewId(),
+		DisplayName: model.NewId(),
+		Description: model.NewId(),
+		Source:      model.GroupSourceLdap,
+		RemoteId:    model.NewId(),
+	}
+	group, err := ss.Group().Create(g1)
+	require.Nil(t, err)
+
+	u1 := &model.User{
+		Email:    MakeEmail(),
+		Username: model.NewId(),
+	}
+	user1, err := ss.User().Save(u1)
+	require.Nil(t, err)
+
+	_, err = ss.Group().UpsertMember(group.Id, user1.Id)
+	require.Nil(t, err)
+
+	u2 := &model.User{
+		Email:    MakeEmail(),
+		Username: model.NewId(),
+	}
+	user2, err := ss.User().Save(u2)
+	require.Nil(t, err)
+
+	_, err = ss.Group().UpsertMember(group.Id, user2.Id)
+	require.Nil(t, err)
+
+	u3 := &model.User{
+		Email:    MakeEmail(),
+		Username: model.NewId(),
+	}
+	user3, err := ss.User().Save(u3)
+	require.Nil(t, err)
+
+	_, err = ss.Group().UpsertMember(group.Id, user3.Id)
+	require.Nil(t, err)
+
+	// Create Channel
+	channel := &model.Channel{
+		TeamId:      team.Id,
+		DisplayName: "Channel",
+		Name:        model.NewId(),
+		Type:        model.CHANNEL_OPEN, // Query does not look at type so this shouldn't matter.
+	}
+	channel, err = ss.Channel().Save(channel, 9999)
+	require.Nil(t, err)
+
+	// returns no members when channel does not exist
+	groupMembers, err := ss.Group().GetMemberUsersNotInChannel(group.Id, "non-existant-channel-id")
+	require.Nil(t, err)
+	require.Equal(t, 0, len(groupMembers))
+
+	// returns no members when group has no members in the team that the channel belongs to
+	groupMembers, err = ss.Group().GetMemberUsersNotInChannel(group.Id, channel.Id)
+	require.Nil(t, err)
+	require.Equal(t, 0, len(groupMembers))
+
+	m1 := &model.TeamMember{TeamId: team.Id, UserId: user1.Id}
+	_, err = ss.Team().SaveMember(m1, -1)
+	require.Nil(t, err)
+
+	// returns single member in team and not in channel
+	groupMembers, err = ss.Group().GetMemberUsersNotInChannel(group.Id, channel.Id)
+	require.Nil(t, err)
+	require.Equal(t, 1, len(groupMembers))
+
+	m2 := &model.TeamMember{TeamId: team.Id, UserId: user2.Id}
+	m3 := &model.TeamMember{TeamId: team.Id, UserId: user3.Id}
+	_, err = ss.Team().SaveMember(m2, -1)
+	require.Nil(t, err)
+	_, err = ss.Team().SaveMember(m3, -1)
+	require.Nil(t, err)
+
+	// returns all members when all members are in team and not in channel
+	groupMembers, err = ss.Group().GetMemberUsersNotInChannel(group.Id, channel.Id)
+	require.Nil(t, err)
+	require.Equal(t, 3, len(groupMembers))
+
+	cm1 := &model.ChannelMember{
+		ChannelId:   channel.Id,
+		UserId:      user1.Id,
+		SchemeGuest: false,
+		SchemeUser:  true,
+		SchemeAdmin: false,
+		NotifyProps: model.GetDefaultChannelNotifyProps(),
+	}
+	_, err = ss.Channel().SaveMember(cm1)
+	require.Nil(t, err)
+
+	// returns both members not yet added to channel
+	groupMembers, err = ss.Group().GetMemberUsersNotInChannel(group.Id, channel.Id)
+	require.Nil(t, err)
+	require.Equal(t, 2, len(groupMembers))
+
+	cm2 := &model.ChannelMember{
+		ChannelId:   channel.Id,
+		UserId:      user2.Id,
+		SchemeGuest: false,
+		SchemeUser:  true,
+		SchemeAdmin: false,
+		NotifyProps: model.GetDefaultChannelNotifyProps(),
+	}
+	cm3 := &model.ChannelMember{
+		ChannelId:   channel.Id,
+		UserId:      user3.Id,
+		SchemeGuest: false,
+		SchemeUser:  true,
+		SchemeAdmin: false,
+		NotifyProps: model.GetDefaultChannelNotifyProps(),
+	}
+
+	_, err = ss.Channel().SaveMember(cm2)
+	require.Nil(t, err)
+	_, err = ss.Channel().SaveMember(cm3)
+	require.Nil(t, err)
+
+	// returns none when all members have been added to team and channel
+	groupMembers, err = ss.Group().GetMemberUsersNotInChannel(group.Id, channel.Id)
+	require.Nil(t, err)
+	require.Equal(t, 0, len(groupMembers))
+}
+
 func testUpsertMember(t *testing.T, ss store.Store) {
 	// Create group
 	g1 := &model.Group{
--- a/store/storetest/mocks/GroupStore.go
+++ b/store/storetest/mocks/GroupStore.go
@@ -702,6 +702,56 @@ func (_m *GroupStore) GetMemberUsers(groupID string) ([]*model.User, *model.AppE
 	return r0, r1
 }

+// GetMemberUsersInTeam provides a mock function with given fields: groupID, teamID
+func (_m *GroupStore) GetMemberUsersInTeam(groupID string, teamID string) ([]*model.User, *model.AppError) {
+	ret := _m.Called(groupID, teamID)
+
+	var r0 []*model.User
+	if rf, ok := ret.Get(0).(func(string, string) []*model.User); ok {
+		r0 = rf(groupID, teamID)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).([]*model.User)
+		}
+	}
+
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string, string) *model.AppError); ok {
+		r1 = rf(groupID, teamID)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
+}
+
+// GetMemberUsersNotInChannel provides a mock function with given fields: groupID, channelID
+func (_m *GroupStore) GetMemberUsersNotInChannel(groupID string, channelID string) ([]*model.User, *model.AppError) {
+	ret := _m.Called(groupID, channelID)
+
+	var r0 []*model.User
+	if rf, ok := ret.Get(0).(func(string, string) []*model.User); ok {
+		r0 = rf(groupID, channelID)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).([]*model.User)
+		}
+	}
+
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string, string) *model.AppError); ok {
+		r1 = rf(groupID, channelID)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
+}
+
 // GetMemberUsersPage provides a mock function with given fields: groupID, page, perPage
 func (_m *GroupStore) GetMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, *model.AppError) {
 	ret := _m.Called(groupID, page, perPage)
--- a/store/timer_layer.go
+++ b/store/timer_layer.go
@@ -3054,6 +3054,38 @@ func (s *TimerLayerGroupStore) GetMemberUsers(groupID string) ([]*model.User, *m
 	return resultVar0, resultVar1
 }

+func (s *TimerLayerGroupStore) GetMemberUsersInTeam(groupID string, teamID string) ([]*model.User, *model.AppError) {
+	start := timemodule.Now()
+
+	resultVar0, resultVar1 := s.GroupStore.GetMemberUsersInTeam(groupID, teamID)
+
+	elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second)
+	if s.Root.Metrics != nil {
+		success := "false"
+		if resultVar1 == nil {
+			success = "true"
+		}
+		s.Root.Metrics.ObserveStoreMethodDuration("GroupStore.GetMemberUsersInTeam", success, elapsed)
+	}
+	return resultVar0, resultVar1
+}
+
+func (s *TimerLayerGroupStore) GetMemberUsersNotInChannel(groupID string, channelID string) ([]*model.User, *model.AppError) {
+	start := timemodule.Now()
+
+	resultVar0, resultVar1 := s.GroupStore.GetMemberUsersNotInChannel(groupID, channelID)
+
+	elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second)
+	if s.Root.Metrics != nil {
+		success := "false"
+		if resultVar1 == nil {
+			success = "true"
+		}
+		s.Root.Metrics.ObserveStoreMethodDuration("GroupStore.GetMemberUsersNotInChannel", success, elapsed)
+	}
+	return resultVar0, resultVar1
+}
+
 func (s *TimerLayerGroupStore) GetMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, *model.AppError) {
 	start := timemodule.Now()