[MM-16772] Migrate Team.RemoveMember to Sync by default (#11565)

* Migrate Team.RemoveMember to Sync by default

* Fixed typo in user_store.go

* Resolved merge conflicts in storetest/user_store.go

store/sqlstore/team_store.go

@@ -764,13 +764,12 @@ func (s SqlTeamStore) GetChannelUnreadsForTeam(teamId, userId string) ([]*model.
 	return channels, nil
 }
 
-func (s SqlTeamStore) RemoveMember(teamId string, userId string) store.StoreChannel {
+func (s SqlTeamStore) RemoveMember(teamId string, userId string) *model.AppError {
-	return store.Do(func(result *store.StoreResult) {
+	_, err := s.GetMaster().Exec("DELETE FROM TeamMembers WHERE TeamId = :TeamId AND UserId = :UserId", map[string]interface{}{"TeamId": teamId, "UserId": userId})
-		_, err := s.GetMaster().Exec("DELETE FROM TeamMembers WHERE TeamId = :TeamId AND UserId = :UserId", map[string]interface{}{"TeamId": teamId, "UserId": userId})
+	if err != nil {
-		if err != nil {
+		return model.NewAppError("SqlTeamStore.RemoveMember", "store.sql_team.remove_member.app_error", nil, "team_id="+teamId+", user_id="+userId+", "+err.Error(), http.StatusInternalServerError)
-			result.Err = model.NewAppError("SqlTeamStore.RemoveMember", "store.sql_team.remove_member.app_error", nil, "team_id="+teamId+", user_id="+userId+", "+err.Error(), http.StatusInternalServerError)
+	}
-		}
+	return nil
-	})
 }
 
 func (s SqlTeamStore) RemoveAllMembersByTeam(teamId string) store.StoreChannel {

store/store.go

@@ -111,7 +111,7 @@ type TeamStore interface {
 	GetTeamsForUserWithPagination(userId string, page, perPage int) ([]*model.TeamMember, *model.AppError)
 	GetChannelUnreadsForAllTeams(excludeTeamId, userId string) ([]*model.ChannelUnread, *model.AppError)
 	GetChannelUnreadsForTeam(teamId, userId string) ([]*model.ChannelUnread, *model.AppError)
-	RemoveMember(teamId string, userId string) StoreChannel
+	RemoveMember(teamId string, userId string) *model.AppError
 	RemoveAllMembersByTeam(teamId string) StoreChannel
 	RemoveAllMembersByUser(userId string) *model.AppError
 	UpdateLastTeamIconUpdate(teamId string, curTime int64) *model.AppError

store/storetest/group_store.go

@@ -1291,12 +1291,12 @@ func testTeamMemberRemovals(t *testing.T, ss store.Store) {
 	require.Len(t, teamMembers, 3)
 
 	// add users back to groups
-	res := <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserA.Id)
+	res := ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserA.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, res)
-	res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserB.Id)
+	res = ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserB.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, res)
-	res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserC.Id)
+	res = ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserC.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, res)
 	err = ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserA.Id)
 	require.Nil(t, err)
 	err = ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserB.Id)
@@ -1366,12 +1366,12 @@ func testChannelMemberRemovals(t *testing.T, ss store.Store) {
 	require.Len(t, channelMembers, 3)
 
 	// add users back to groups
-	res := <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserA.Id)
+	res := ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserA.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, res)
-	res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserB.Id)
+	res = ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserB.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, res)
-	res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserC.Id)
+	res = ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserC.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, res)
 	err = ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserA.Id)
 	require.Nil(t, err)
 	err = ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserB.Id)

store/storetest/mocks/TeamStore.go

@@ -696,15 +696,15 @@ func (_m *TeamStore) RemoveAllMembersByUser(userId string) *model.AppError {
 }
 
 // RemoveMember provides a mock function with given fields: teamId, userId
-func (_m *TeamStore) RemoveMember(teamId string, userId string) store.StoreChannel {
+func (_m *TeamStore) RemoveMember(teamId string, userId string) *model.AppError {
 	ret := _m.Called(teamId, userId)
 
-	var r0 store.StoreChannel
+	var r0 *model.AppError
-	if rf, ok := ret.Get(0).(func(string, string) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(string, string) *model.AppError); ok {
 		r0 = rf(teamId, userId)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.AppError)
 		}
 	}
 

store/storetest/team_store.go

@@ -802,8 +802,8 @@ func testTeamMembers(t *testing.T, ss store.Store) {
 		require.Equal(t, m1.TeamId, ms[0].TeamId)
 	}
 
-	if r1 := <-ss.Team().RemoveMember(teamId1, m1.UserId); r1.Err != nil {
+	if err := ss.Team().RemoveMember(teamId1, m1.UserId); err != nil {
-		t.Fatal(r1.Err)
+		t.Fatal(err)
 	}
 
 	if ms, err := ss.Team().GetMembers(teamId1, 0, 100, nil); err != nil {
@@ -872,8 +872,8 @@ func testTeamMembersWithPagination(t *testing.T, ss store.Store) {
 	require.Len(t, ms, 1)
 	require.Equal(t, m1.TeamId, ms[0].TeamId)
 
-	r1 = <-ss.Team().RemoveMember(teamId1, m1.UserId)
+	e := ss.Team().RemoveMember(teamId1, m1.UserId)
-	require.Nil(t, r1.Err)
+	require.Nil(t, e)
 
 	ms, err := ss.Team().GetMembers(teamId1, 0, 100, nil)
 	require.Nil(t, err)
@@ -937,7 +937,7 @@ func testSaveTeamMemberMaxMembers(t *testing.T, ss store.Store) {
 		}, maxUsersPerTeam))
 
 		defer func(userId string) {
-			<-ss.Team().RemoveMember(team.Id, userId)
+			ss.Team().RemoveMember(team.Id, userId)
 		}(userIds[i])
 	}
 
@@ -989,7 +989,7 @@ func testSaveTeamMemberMaxMembers(t *testing.T, ss store.Store) {
 		t.Fatal("should've been able to save new member after deleting one", result.Err)
 	} else {
 		defer func(userId string) {
-			<-ss.Team().RemoveMember(team.Id, userId)
+			ss.Team().RemoveMember(team.Id, userId)
 		}(newUserId)
 	}
 
@@ -1016,7 +1016,7 @@ func testSaveTeamMemberMaxMembers(t *testing.T, ss store.Store) {
 		t.Fatal("should've been able to save new member after deleting one", result.Err)
 	} else {
 		defer func(userId string) {
-			<-ss.Team().RemoveMember(team.Id, userId)
+			ss.Team().RemoveMember(team.Id, userId)
 		}(newUserId2)
 	}
 }

store/storetest/user_store.go

@@ -3477,8 +3477,11 @@ func testUserStoreGetProfilesNotInTeam(t *testing.T, ss store.Store) {
 	// Ensure update at timestamp changes
 	time.Sleep(time.Millisecond * 10)
 
-	store.Must(ss.Team().RemoveMember(teamId, u1.Id))
+	e := ss.Team().RemoveMember(teamId, u1.Id)
-	store.Must(ss.Team().RemoveMember(teamId, u2.Id))
+	require.Nil(t, e)
+	e = ss.Team().RemoveMember(teamId, u2.Id)
+	require.Nil(t, e)
+
 	u1.UpdateAt, err = ss.User().UpdateUpdateAt(u1.Id)
 	require.Nil(t, err)
 	u2.UpdateAt, err = ss.User().UpdateUpdateAt(u2.Id)
@@ -3919,8 +3922,8 @@ func testUserStoreGetTeamGroupUsers(t *testing.T, ss store.Store) {
 	requireNUsers(2)
 
 	// delete team membership of allowed user
-	res = <-ss.Team().RemoveMember(team.Id, userGroupA.Id)
+	err = ss.Team().RemoveMember(team.Id, userGroupA.Id)
-	require.Nil(t, res.Err)
+	require.Nil(t, err)
 
 	// ensure removed allowed member still returned by query
 	requireNUsers(2)