Migrate Groups store to sync by default (Add/Remove Channel/Team Members) (#10997)

* Migrate Groups store to sync by default (Add/Remove Channel/Team Members)

* Addressing PR review comments

app/group.go

@@ -134,35 +134,19 @@ func (a *App) DeleteGroupSyncable(groupID string, syncableID string, syncableTyp
 }
 
 func (a *App) TeamMembersToAdd(since int64) ([]*model.UserTeamIDPair, *model.AppError) {
-	result := <-a.Srv.Store.Group().TeamMembersToAdd(since)
+	return a.Srv.Store.Group().TeamMembersToAdd(since)
-	if result.Err != nil {
-		return nil, result.Err
-	}
-	return result.Data.([]*model.UserTeamIDPair), nil
 }
 
 func (a *App) ChannelMembersToAdd(since int64) ([]*model.UserChannelIDPair, *model.AppError) {
-	result := <-a.Srv.Store.Group().ChannelMembersToAdd(since)
+	return a.Srv.Store.Group().ChannelMembersToAdd(since)
-	if result.Err != nil {
-		return nil, result.Err
-	}
-	return result.Data.([]*model.UserChannelIDPair), nil
 }
 
 func (a *App) TeamMembersToRemove() ([]*model.TeamMember, *model.AppError) {
-	result := <-a.Srv.Store.Group().TeamMembersToRemove()
+	return a.Srv.Store.Group().TeamMembersToRemove()
-	if result.Err != nil {
-		return nil, result.Err
-	}
-	return result.Data.([]*model.TeamMember), nil
 }
 
 func (a *App) ChannelMembersToRemove() ([]*model.ChannelMember, *model.AppError) {
-	result := <-a.Srv.Store.Group().ChannelMembersToRemove()
+	return a.Srv.Store.Group().ChannelMembersToRemove()
-	if result.Err != nil {
-		return nil, result.Err
-	}
-	return result.Data.([]*model.ChannelMember), nil
 }
 
 func (a *App) GetGroupsByChannel(channelId string, opts model.GroupSearchOpts) ([]*model.Group, int, *model.AppError) {

store/sqlstore/group_supplier.go

@@ -701,10 +701,8 @@ func (s *SqlGroupStore) DeleteGroupSyncable(groupID string, syncableID string, s
 // based on the groups configurations.
 //
 // Typically since will be the last successful group sync time.
-func (s *SqlGroupStore) TeamMembersToAdd(since int64) store.StoreChannel {
+func (s *SqlGroupStore) TeamMembersToAdd(since int64) ([]*model.UserTeamIDPair, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	sql := `
-
-		sql := `
 		SELECT
 			GroupMembers.UserId, GroupTeams.TeamId
 		FROM
@@ -727,27 +725,22 @@ func (s *SqlGroupStore) TeamMembersToAdd(since int64) store.StoreChannel {
 			AND (GroupMembers.CreateAt >= :Since
 			OR GroupTeams.UpdateAt >= :Since)`
 
-		var teamMembers []*model.UserTeamIDPair
+	var teamMembers []*model.UserTeamIDPair
 
-		_, err := s.GetReplica().Select(&teamMembers, sql, map[string]interface{}{"Since": since})
+	_, err := s.GetReplica().Select(&teamMembers, sql, map[string]interface{}{"Since": since})
-		if err != nil {
+	if err != nil {
-			result.Err = model.NewAppError("SqlGroupStore.TeamMembersToAdd", "store.select_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, model.NewAppError("SqlGroupStore.TeamMembersToAdd", "store.select_error", nil, err.Error(), http.StatusInternalServerError)
-		}
+	}
 
-		result.Data = teamMembers
+	return teamMembers, nil
-
-		return
-	})
 }
 
 // ChannelMembersToAdd returns a slice of UserChannelIDPair that need newly created memberships
 // based on the groups configurations.
 //
 // Typically since will be the last successful group sync time.
-func (s *SqlGroupStore) ChannelMembersToAdd(since int64) store.StoreChannel {
+func (s *SqlGroupStore) ChannelMembersToAdd(since int64) ([]*model.UserChannelIDPair, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	sql := `
-
-		sql := `
 		SELECT
 			GroupMembers.UserId, GroupChannels.ChannelId
 		FROM
@@ -770,17 +763,14 @@ func (s *SqlGroupStore) ChannelMembersToAdd(since int64) store.StoreChannel {
 			AND (GroupMembers.CreateAt >= :Since
 			OR GroupChannels.UpdateAt >= :Since)`
 
-		var channelMembers []*model.UserChannelIDPair
+	var channelMembers []*model.UserChannelIDPair
 
-		_, err := s.GetReplica().Select(&channelMembers, sql, map[string]interface{}{"Since": since})
+	_, err := s.GetReplica().Select(&channelMembers, sql, map[string]interface{}{"Since": since})
-		if err != nil {
+	if err != nil {
-			result.Err = model.NewAppError("SqlGroupStore.ChannelMembersToAdd", "store.select_error", nil, "", http.StatusInternalServerError)
+		return nil, model.NewAppError("SqlGroupStore.ChannelMembersToAdd", "store.select_error", nil, "", http.StatusInternalServerError)
-		}
+	}
 
-		result.Data = channelMembers
+	return channelMembers, nil
-
-		return
-	})
 }
 
 func groupSyncableToGroupTeam(groupSyncable *model.GroupSyncable) *groupTeam {
@@ -798,10 +788,8 @@ func groupSyncableToGroupChannel(groupSyncable *model.GroupSyncable) *groupChann
 }
 
 // TeamMembersToRemove returns all team members that should be removed based on group constraints.
-func (s *SqlGroupStore) TeamMembersToRemove() store.StoreChannel {
+func (s *SqlGroupStore) TeamMembersToRemove() ([]*model.TeamMember, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	sql := `
-
-		sql := `
 		SELECT
 			TeamMembers.TeamId,
 			TeamMembers.UserId,
@@ -838,17 +826,14 @@ func (s *SqlGroupStore) TeamMembersToRemove() store.StoreChannel {
 					Teams.Id,
 					GroupMembers.UserId)`
 
-		var teamMembers []*model.TeamMember
+	var teamMembers []*model.TeamMember
 
-		_, err := s.GetReplica().Select(&teamMembers, sql)
+	_, err := s.GetReplica().Select(&teamMembers, sql)
-		if err != nil {
+	if err != nil {
-			result.Err = model.NewAppError("SqlGroupStore.TeamMembersToRemove", "store.select_error", nil, "", http.StatusInternalServerError)
+		return nil, model.NewAppError("SqlGroupStore.TeamMembersToRemove", "store.select_error", nil, "", http.StatusInternalServerError)
-		}
+	}
 
-		result.Data = teamMembers
+	return teamMembers, nil
-
-		return
-	})
 }
 
 func (s *SqlGroupStore) CountGroupsByChannel(channelId string, opts model.GroupSearchOpts) store.StoreChannel {
@@ -905,10 +890,8 @@ func (s *SqlGroupStore) GetGroupsByChannel(channelId string, opts model.GroupSea
 }
 
 // ChannelMembersToRemove returns all channel members that should be removed based on group constraints.
-func (s *SqlGroupStore) ChannelMembersToRemove() store.StoreChannel {
+func (s *SqlGroupStore) ChannelMembersToRemove() ([]*model.ChannelMember, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	sql := `
-
-		sql := `
 		SELECT
 			ChannelMembers.ChannelId,
 			ChannelMembers.UserId,
@@ -948,17 +931,14 @@ func (s *SqlGroupStore) ChannelMembersToRemove() store.StoreChannel {
 					Channels.Id,
 					GroupMembers.UserId)`
 
-		var channelMembers []*model.ChannelMember
+	var channelMembers []*model.ChannelMember
 
-		_, err := s.GetReplica().Select(&channelMembers, sql)
+	_, err := s.GetReplica().Select(&channelMembers, sql)
-		if err != nil {
+	if err != nil {
-			result.Err = model.NewAppError("SqlGroupStore.ChannelMembersToRemove", "store.select_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, model.NewAppError("SqlGroupStore.ChannelMembersToRemove", "store.select_error", nil, err.Error(), http.StatusInternalServerError)
-		}
+	}
 
-		result.Data = channelMembers
+	return channelMembers, nil
-
-		return
-	})
 }
 
 func (s *SqlGroupStore) groupsBySyncableBaseQuery(st model.GroupSyncableType, t selectType, syncableID string, opts model.GroupSearchOpts) squirrel.SelectBuilder {

store/store.go

@@ -588,11 +588,11 @@ type GroupStore interface {
 	UpdateGroupSyncable(groupSyncable *model.GroupSyncable) StoreChannel
 	DeleteGroupSyncable(groupID string, syncableID string, syncableType model.GroupSyncableType) StoreChannel
 
-	TeamMembersToAdd(since int64) StoreChannel
+	TeamMembersToAdd(since int64) ([]*model.UserTeamIDPair, *model.AppError)
-	ChannelMembersToAdd(since int64) StoreChannel
+	ChannelMembersToAdd(since int64) ([]*model.UserChannelIDPair, *model.AppError)
 
-	TeamMembersToRemove() StoreChannel
+	TeamMembersToRemove() ([]*model.TeamMember, *model.AppError)
-	ChannelMembersToRemove() StoreChannel
+	ChannelMembersToRemove() ([]*model.ChannelMember, *model.AppError)
 
 	GetGroupsByChannel(channelId string, opts model.GroupSearchOpts) StoreChannel
 	CountGroupsByChannel(channelId string, opts model.GroupSearchOpts) StoreChannel

store/storetest/group_supplier.go

@@ -937,26 +937,25 @@ func testPendingAutoAddTeamMembers(t *testing.T, ss store.Store) {
 	syncable := res.Data.(*model.GroupSyncable)
 
 	// Time before syncable was created
-	res = <-ss.Group().TeamMembersToAdd(syncable.CreateAt - 1)
+	teamMembers, err := ss.Group().TeamMembersToAdd(syncable.CreateAt - 1)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	teamMembers := res.Data.([]*model.UserTeamIDPair)
 	require.Len(t, teamMembers, 1)
 	require.Equal(t, user.Id, teamMembers[0].UserID)
 	require.Equal(t, team.Id, teamMembers[0].TeamID)
 
 	// Time after syncable was created
-	res = <-ss.Group().TeamMembersToAdd(syncable.CreateAt + 1)
+	teamMembers, err = ss.Group().TeamMembersToAdd(syncable.CreateAt + 1)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, teamMembers, 0)
 
 	// Delete and restore GroupMember should return result
 	res = <-ss.Group().DeleteMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
 	res = <-ss.Group().CreateOrRestoreMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().TeamMembersToAdd(syncable.CreateAt + 1)
+	teamMembers, err = ss.Group().TeamMembersToAdd(syncable.CreateAt + 1)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 1)
+	require.Len(t, teamMembers, 1)
 
 	pristineSyncable := *syncable
 
@@ -964,89 +963,88 @@ func testPendingAutoAddTeamMembers(t *testing.T, ss store.Store) {
 	require.Nil(t, res.Err)
 
 	// Time before syncable was updated
-	res = <-ss.Group().TeamMembersToAdd(syncable.UpdateAt - 1)
+	teamMembers, err = ss.Group().TeamMembersToAdd(syncable.UpdateAt - 1)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	teamMembers = res.Data.([]*model.UserTeamIDPair)
 	require.Len(t, teamMembers, 1)
 	require.Equal(t, user.Id, teamMembers[0].UserID)
 	require.Equal(t, team.Id, teamMembers[0].TeamID)
 
 	// Time after syncable was updated
-	res = <-ss.Group().TeamMembersToAdd(syncable.UpdateAt + 1)
+	teamMembers, err = ss.Group().TeamMembersToAdd(syncable.UpdateAt + 1)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, teamMembers, 0)
 
 	// Only includes if auto-add
 	syncable.AutoAdd = false
 	res = <-ss.Group().UpdateGroupSyncable(syncable)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().TeamMembersToAdd(0)
+	teamMembers, err = ss.Group().TeamMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, teamMembers, 0)
 
 	// reset state of syncable and verify
 	res = <-ss.Group().UpdateGroupSyncable(&pristineSyncable)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().TeamMembersToAdd(0)
+	teamMembers, err = ss.Group().TeamMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 1)
+	require.Len(t, teamMembers, 1)
 
 	// No result if Group deleted
 	res = <-ss.Group().Delete(group.Id)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().TeamMembersToAdd(0)
+	teamMembers, err = ss.Group().TeamMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, teamMembers, 0)
 
 	// reset state of group and verify
 	group.DeleteAt = 0
 	res = <-ss.Group().Update(group)
-	res = <-ss.Group().TeamMembersToAdd(0)
+	teamMembers, err = ss.Group().TeamMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 1)
+	require.Len(t, teamMembers, 1)
 
 	// No result if Team deleted
 	team.DeleteAt = model.GetMillis()
 	team, err = ss.Team().Update(team)
 	require.Nil(t, err)
-	res = <-ss.Group().TeamMembersToAdd(0)
+	teamMembers, err = ss.Group().TeamMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, teamMembers, 0)
 
 	// reset state of team and verify
 	team.DeleteAt = 0
 	team, err = ss.Team().Update(team)
 	require.Nil(t, err)
-	res = <-ss.Group().TeamMembersToAdd(0)
+	teamMembers, err = ss.Group().TeamMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 1)
+	require.Len(t, teamMembers, 1)
 
 	// No result if GroupTeam deleted
 	res = <-ss.Group().DeleteGroupSyncable(group.Id, team.Id, model.GroupSyncableTypeTeam)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().TeamMembersToAdd(0)
+	teamMembers, err = ss.Group().TeamMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, teamMembers, 0)
 
 	// reset GroupTeam and verify
 	res = <-ss.Group().UpdateGroupSyncable(&pristineSyncable)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().TeamMembersToAdd(0)
+	teamMembers, err = ss.Group().TeamMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 1)
+	require.Len(t, teamMembers, 1)
 
 	// No result if GroupMember deleted
 	res = <-ss.Group().DeleteMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().TeamMembersToAdd(0)
+	teamMembers, err = ss.Group().TeamMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, teamMembers, 0)
 
 	// restore group member and verify
 	res = <-ss.Group().CreateOrRestoreMember(group.Id, user.Id)
-	res = <-ss.Group().TeamMembersToAdd(0)
+	teamMembers, err = ss.Group().TeamMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 1)
+	require.Len(t, teamMembers, 1)
 
 	// adding team membership stops returning result
 	res = <-ss.Team().SaveMember(&model.TeamMember{
@@ -1054,9 +1052,9 @@ func testPendingAutoAddTeamMembers(t *testing.T, ss store.Store) {
 		UserId: user.Id,
 	}, 999)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().TeamMembersToAdd(0)
+	teamMembers, err = ss.Group().TeamMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, teamMembers, 0)
 }
 
 func testPendingAutoAddChannelMembers(t *testing.T, ss store.Store) {
@@ -1099,26 +1097,25 @@ func testPendingAutoAddChannelMembers(t *testing.T, ss store.Store) {
 	syncable := res.Data.(*model.GroupSyncable)
 
 	// Time before syncable was created
-	res = <-ss.Group().ChannelMembersToAdd(syncable.CreateAt - 1)
+	channelMembers, err := ss.Group().ChannelMembersToAdd(syncable.CreateAt - 1)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	channelMembers := res.Data.([]*model.UserChannelIDPair)
 	require.Len(t, channelMembers, 1)
 	require.Equal(t, user.Id, channelMembers[0].UserID)
 	require.Equal(t, channel.Id, channelMembers[0].ChannelID)
 
 	// Time after syncable was created
-	res = <-ss.Group().ChannelMembersToAdd(syncable.CreateAt + 1)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(syncable.CreateAt + 1)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, channelMembers, 0)
 
 	// Delete and restore GroupMember should return result
 	res = <-ss.Group().DeleteMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
 	res = <-ss.Group().CreateOrRestoreMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().ChannelMembersToAdd(syncable.CreateAt + 1)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(syncable.CreateAt + 1)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 1)
+	require.Len(t, channelMembers, 1)
 
 	pristineSyncable := *syncable
 
@@ -1126,131 +1123,127 @@ func testPendingAutoAddChannelMembers(t *testing.T, ss store.Store) {
 	require.Nil(t, res.Err)
 
 	// Time before syncable was updated
-	res = <-ss.Group().ChannelMembersToAdd(syncable.UpdateAt - 1)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(syncable.UpdateAt - 1)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	channelMembers = res.Data.([]*model.UserChannelIDPair)
 	require.Len(t, channelMembers, 1)
 	require.Equal(t, user.Id, channelMembers[0].UserID)
 	require.Equal(t, channel.Id, channelMembers[0].ChannelID)
 
 	// Time after syncable was updated
-	res = <-ss.Group().ChannelMembersToAdd(syncable.UpdateAt + 1)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(syncable.UpdateAt + 1)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, channelMembers, 0)
 
 	// Only includes if auto-add
 	syncable.AutoAdd = false
 	res = <-ss.Group().UpdateGroupSyncable(syncable)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().ChannelMembersToAdd(0)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, channelMembers, 0)
 
 	// reset state of syncable and verify
 	res = <-ss.Group().UpdateGroupSyncable(&pristineSyncable)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().ChannelMembersToAdd(0)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 1)
+	require.Len(t, channelMembers, 1)
 
 	// No result if Group deleted
 	res = <-ss.Group().Delete(group.Id)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().ChannelMembersToAdd(0)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, channelMembers, 0)
 
 	// reset state of group and verify
 	group.DeleteAt = 0
 	res = <-ss.Group().Update(group)
-	res = <-ss.Group().ChannelMembersToAdd(0)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 1)
+	require.Len(t, channelMembers, 1)
 
 	// No result if Channel deleted
 	err = ss.Channel().Delete(channel.Id, model.GetMillis())
 	require.Nil(t, err)
-	res = <-ss.Group().ChannelMembersToAdd(0)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, channelMembers, 0)
 
 	// reset state of channel and verify
 	channel.DeleteAt = 0
 	_, err = ss.Channel().Update(channel)
 	require.Nil(t, err)
-	res = <-ss.Group().ChannelMembersToAdd(0)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 1)
+	require.Len(t, channelMembers, 1)
 
 	// No result if GroupChannel deleted
 	res = <-ss.Group().DeleteGroupSyncable(group.Id, channel.Id, model.GroupSyncableTypeChannel)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().ChannelMembersToAdd(0)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, channelMembers, 0)
 
 	// reset GroupChannel and verify
 	res = <-ss.Group().UpdateGroupSyncable(&pristineSyncable)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().ChannelMembersToAdd(0)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 1)
+	require.Len(t, channelMembers, 1)
 
 	// No result if GroupMember deleted
 	res = <-ss.Group().DeleteMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().ChannelMembersToAdd(0)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, channelMembers, 0)
 
 	// restore group member and verify
 	res = <-ss.Group().CreateOrRestoreMember(group.Id, user.Id)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().ChannelMembersToAdd(0)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 1)
+	require.Len(t, channelMembers, 1)
 
 	// Adding Channel (ChannelMemberHistory) should stop returning result
 	res = <-ss.ChannelMemberHistory().LogJoinEvent(user.Id, channel.Id, model.GetMillis())
 	require.Nil(t, res.Err)
-	res = <-ss.Group().ChannelMembersToAdd(0)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, channelMembers, 0)
 
 	// Leaving Channel (ChannelMemberHistory) should still not return result
 	res = <-ss.ChannelMemberHistory().LogLeaveEvent(user.Id, channel.Id, model.GetMillis())
 	require.Nil(t, res.Err)
-	res = <-ss.Group().ChannelMembersToAdd(0)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 0)
+	require.Len(t, channelMembers, 0)
 
 	// Purging ChannelMemberHistory re-returns the result
 	res = <-ss.ChannelMemberHistory().PermanentDeleteBatch(model.GetMillis()+1, 100)
 	require.Nil(t, res.Err)
-	res = <-ss.Group().ChannelMembersToAdd(0)
+	channelMembers, err = ss.Group().ChannelMembersToAdd(0)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 1)
+	require.Len(t, channelMembers, 1)
 }
 
 func testTeamMemberRemovals(t *testing.T, ss store.Store) {
 	data := pendingMemberRemovalsDataSetup(t, ss)
 
 	// one result when both users are in the group (for user C)
-	res := <-ss.Group().TeamMembersToRemove()
+	teamMembers, err := ss.Group().TeamMembersToRemove()
-
+	require.Nil(t, err)
-	require.Nil(t, res.Err)
+	require.Len(t, teamMembers, 1)
-	require.Len(t, res.Data, 1)
-	teamMembers := res.Data.([]*model.TeamMember)
 	require.Equal(t, data.UserC.Id, teamMembers[0].UserId)
 
-	res = <-ss.Group().DeleteMember(data.Group.Id, data.UserB.Id)
+	res := <-ss.Group().DeleteMember(data.Group.Id, data.UserB.Id)
 	require.Nil(t, res.Err)
 
 	// user b and c should now be returned
-	res = <-ss.Group().TeamMembersToRemove()
+	teamMembers, err = ss.Group().TeamMembersToRemove()
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 2)
+	require.Len(t, teamMembers, 2)
-	teamMembers = res.Data.([]*model.TeamMember)
 
 	var userIDs []string
 	for _, item := range teamMembers {
@@ -1264,13 +1257,13 @@ func testTeamMemberRemovals(t *testing.T, ss store.Store) {
 	res = <-ss.Group().DeleteMember(data.Group.Id, data.UserA.Id)
 	require.Nil(t, res.Err)
 
-	res = <-ss.Group().TeamMembersToRemove()
+	teamMembers, err = ss.Group().TeamMembersToRemove()
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 3)
+	require.Len(t, teamMembers, 3)
 
 	// Make one of them a bot
-	res = <-ss.Group().TeamMembersToRemove()
+	teamMembers, err = ss.Group().TeamMembersToRemove()
-	teamMembers = res.Data.([]*model.TeamMember)
+	require.Nil(t, err)
 	teamMember := teamMembers[0]
 	bot := &model.Bot{
 		UserId:      teamMember.UserId,
@@ -1283,18 +1276,18 @@ func testTeamMemberRemovals(t *testing.T, ss store.Store) {
 	bot = res.Data.(*model.Bot)
 
 	// verify that bot is not returned in results
-	res = <-ss.Group().TeamMembersToRemove()
+	teamMembers, err = ss.Group().TeamMembersToRemove()
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 2)
+	require.Len(t, teamMembers, 2)
 
 	// delete the bot
 	res = <-ss.Bot().PermanentDelete(bot.UserId)
 	require.Nil(t, res.Err)
 
 	// Should be back to 3 users
-	res = <-ss.Group().TeamMembersToRemove()
+	teamMembers, err = ss.Group().TeamMembersToRemove()
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 3)
+	require.Len(t, teamMembers, 3)
 
 	// add users back to groups
 	res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserA.Id)
@@ -1315,21 +1308,18 @@ func testChannelMemberRemovals(t *testing.T, ss store.Store) {
 	data := pendingMemberRemovalsDataSetup(t, ss)
 
 	// one result when both users are in the group (for user C)
-	res := <-ss.Group().ChannelMembersToRemove()
+	channelMembers, err := ss.Group().ChannelMembersToRemove()
-
+	require.Nil(t, err)
-	require.Nil(t, res.Err)
+	require.Len(t, channelMembers, 1)
-	require.Len(t, res.Data, 1)
-	channelMembers := res.Data.([]*model.ChannelMember)
 	require.Equal(t, data.UserC.Id, channelMembers[0].UserId)
 
-	res = <-ss.Group().DeleteMember(data.Group.Id, data.UserB.Id)
+	res := <-ss.Group().DeleteMember(data.Group.Id, data.UserB.Id)
 	require.Nil(t, res.Err)
 
 	// user b and c should now be returned
-	res = <-ss.Group().ChannelMembersToRemove()
+	channelMembers, err = ss.Group().ChannelMembersToRemove()
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 2)
+	require.Len(t, channelMembers, 2)
-	channelMembers = res.Data.([]*model.ChannelMember)
 
 	var userIDs []string
 	for _, item := range channelMembers {
@@ -1343,13 +1333,13 @@ func testChannelMemberRemovals(t *testing.T, ss store.Store) {
 	res = <-ss.Group().DeleteMember(data.Group.Id, data.UserA.Id)
 	require.Nil(t, res.Err)
 
-	res = <-ss.Group().ChannelMembersToRemove()
+	channelMembers, err = ss.Group().ChannelMembersToRemove()
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 3)
+	require.Len(t, channelMembers, 3)
 
 	// Make one of them a bot
-	res = <-ss.Group().ChannelMembersToRemove()
+	channelMembers, err = ss.Group().ChannelMembersToRemove()
-	channelMembers = res.Data.([]*model.ChannelMember)
+	require.Nil(t, err)
 	channelMember := channelMembers[0]
 	bot := &model.Bot{
 		UserId:      channelMember.UserId,
@@ -1362,18 +1352,18 @@ func testChannelMemberRemovals(t *testing.T, ss store.Store) {
 	bot = res.Data.(*model.Bot)
 
 	// verify that bot is not returned in results
-	res = <-ss.Group().ChannelMembersToRemove()
+	channelMembers, err = ss.Group().ChannelMembersToRemove()
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 2)
+	require.Len(t, channelMembers, 2)
 
 	// delete the bot
 	res = <-ss.Bot().PermanentDelete(bot.UserId)
 	require.Nil(t, res.Err)
 
 	// Should be back to 3 users
-	res = <-ss.Group().ChannelMembersToRemove()
+	channelMembers, err = ss.Group().ChannelMembersToRemove()
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
-	require.Len(t, res.Data, 3)
+	require.Len(t, channelMembers, 3)
 
 	// add users back to groups
 	res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserA.Id)

store/storetest/mocks/GroupStore.go

@@ -14,35 +14,53 @@ type GroupStore struct {
 }
 
 // ChannelMembersToAdd provides a mock function with given fields: since
-func (_m *GroupStore) ChannelMembersToAdd(since int64) store.StoreChannel {
+func (_m *GroupStore) ChannelMembersToAdd(since int64) ([]*model.UserChannelIDPair, *model.AppError) {
 	ret := _m.Called(since)
 
-	var r0 store.StoreChannel
+	var r0 []*model.UserChannelIDPair
-	if rf, ok := ret.Get(0).(func(int64) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(int64) []*model.UserChannelIDPair); ok {
 		r0 = rf(since)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).([]*model.UserChannelIDPair)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(int64) *model.AppError); ok {
+		r1 = rf(since)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // ChannelMembersToRemove provides a mock function with given fields:
-func (_m *GroupStore) ChannelMembersToRemove() store.StoreChannel {
+func (_m *GroupStore) ChannelMembersToRemove() ([]*model.ChannelMember, *model.AppError) {
 	ret := _m.Called()
 
-	var r0 store.StoreChannel
+	var r0 []*model.ChannelMember
-	if rf, ok := ret.Get(0).(func() store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func() []*model.ChannelMember); ok {
 		r0 = rf()
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).([]*model.ChannelMember)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func() *model.AppError); ok {
+		r1 = rf()
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // CountGroupsByChannel provides a mock function with given fields: channelId, opts
@@ -350,35 +368,53 @@ func (_m *GroupStore) GetMemberUsersPage(groupID string, offset int, limit int)
 }
 
 // TeamMembersToAdd provides a mock function with given fields: since
-func (_m *GroupStore) TeamMembersToAdd(since int64) store.StoreChannel {
+func (_m *GroupStore) TeamMembersToAdd(since int64) ([]*model.UserTeamIDPair, *model.AppError) {
 	ret := _m.Called(since)
 
-	var r0 store.StoreChannel
+	var r0 []*model.UserTeamIDPair
-	if rf, ok := ret.Get(0).(func(int64) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(int64) []*model.UserTeamIDPair); ok {
 		r0 = rf(since)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).([]*model.UserTeamIDPair)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(int64) *model.AppError); ok {
+		r1 = rf(since)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // TeamMembersToRemove provides a mock function with given fields:
-func (_m *GroupStore) TeamMembersToRemove() store.StoreChannel {
+func (_m *GroupStore) TeamMembersToRemove() ([]*model.TeamMember, *model.AppError) {
 	ret := _m.Called()
 
-	var r0 store.StoreChannel
+	var r0 []*model.TeamMember
-	if rf, ok := ret.Get(0).(func() store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func() []*model.TeamMember); ok {
 		r0 = rf()
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).([]*model.TeamMember)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func() *model.AppError); ok {
+		r1 = rf()
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // Update provides a mock function with given fields: group