Migrate Team.UpdateMember to Sync by default (#11140)

* Team.UpdateMember is sync by default

* Fix go vet: declaration of err shadows declaration at line 261

* Fix shadowing variables

* fix undefined err

* fix shadowing variables

api4/apitestlib.go

@@ -767,9 +767,9 @@ func (me *TestHelper) UpdateUserToTeamAdmin(user *model.User, team *model.Team)
 
 	if tm, err := me.App.Srv.Store.Team().GetMember(team.Id, user.Id); err == nil {
 		tm.SchemeAdmin = true
-		if sr := <-me.App.Srv.Store.Team().UpdateMember(tm); sr.Err != nil {
+		if _, err = me.App.Srv.Store.Team().UpdateMember(tm); err != nil {
 			utils.EnableDebugLogForTest()
-			panic(sr.Err)
+			panic(err)
 		}
 	} else {
 		utils.EnableDebugLogForTest()
@@ -787,9 +787,9 @@ func (me *TestHelper) UpdateUserToNonTeamAdmin(user *model.User, team *model.Tea
 
 	if tm, err := me.App.Srv.Store.Team().GetMember(team.Id, user.Id); err == nil {
 		tm.SchemeAdmin = false
-		if sr := <-me.App.Srv.Store.Team().UpdateMember(tm); sr.Err != nil {
+		if _, err = me.App.Srv.Store.Team().UpdateMember(tm); err != nil {
 			utils.EnableDebugLogForTest()
-			panic(sr.Err)
+			panic(err)
 		}
 	} else {
 		utils.EnableDebugLogForTest()

app/team.go

@@ -270,7 +270,8 @@ func (a *App) UpdateTeamMemberRoles(teamId string, userId string, newRoles strin
 	member.SchemeAdmin = false
 
 	for _, roleName := range strings.Fields(newRoles) {
-		role, err := a.GetRoleByName(roleName)
+		var role *model.Role
+		role, err = a.GetRoleByName(roleName)
 		if err != nil {
 			err.StatusCode = http.StatusBadRequest
 			return nil, err
@@ -304,11 +305,10 @@ func (a *App) UpdateTeamMemberRoles(teamId string, userId string, newRoles strin
 
 	member.ExplicitRoles = strings.Join(newExplicitRoles, " ")
 
-	result := <-a.Srv.Store.Team().UpdateMember(member)
+	member, err = a.Srv.Store.Team().UpdateMember(member)
-	if result.Err != nil {
+	if err != nil {
-		return nil, result.Err
+		return nil, err
 	}
-	member = result.Data.(*model.TeamMember)
 
 	a.ClearSessionCacheForUser(userId)
 
@@ -336,11 +336,10 @@ func (a *App) UpdateTeamMemberSchemeRoles(teamId string, userId string, isScheme
 		member.ExplicitRoles = RemoveRoles([]string{model.TEAM_GUEST_ROLE_ID, model.TEAM_USER_ROLE_ID, model.TEAM_ADMIN_ROLE_ID}, member.ExplicitRoles)
 	}
 
-	result := <-a.Srv.Store.Team().UpdateMember(member)
+	member, err = a.Srv.Store.Team().UpdateMember(member)
-	if result.Err != nil {
+	if err != nil {
-		return nil, result.Err
+		return nil, err
 	}
-	member = result.Data.(*model.TeamMember)
 
 	a.ClearSessionCacheForUser(userId)
 
@@ -532,12 +531,12 @@ func (a *App) joinUserToTeam(team *model.Team, user *model.User) (*model.TeamMem
 		return nil, false, model.NewAppError("joinUserToTeam", "app.team.join_user_to_team.max_accounts.app_error", nil, "teamId="+tm.TeamId, http.StatusBadRequest)
 	}
 
-	tmr := <-a.Srv.Store.Team().UpdateMember(tm)
+	member, err := a.Srv.Store.Team().UpdateMember(tm)
-	if tmr.Err != nil {
+	if err != nil {
-		return nil, false, tmr.Err
+		return nil, false, err
 	}
 
-	return tmr.Data.(*model.TeamMember), false, nil
+	return member, false, nil
 }
 
 func (a *App) JoinUserToTeam(team *model.Team, user *model.User, userRequestorId string) *model.AppError {
@@ -886,8 +885,8 @@ func (a *App) LeaveTeam(team *model.Team, user *model.User, requestorId string)
 	teamMember.Roles = ""
 	teamMember.DeleteAt = model.GetMillis()
 
-	if result := <-a.Srv.Store.Team().UpdateMember(teamMember); result.Err != nil {
+	if _, err := a.Srv.Store.Team().UpdateMember(teamMember); err != nil {
-		return result.Err
+		return err
 	}
 
 	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {

store/sqlstore/team_store.go

@@ -553,41 +553,35 @@ func (s SqlTeamStore) SaveMember(member *model.TeamMember, maxUsersPerTeam int)
 	})
 }
 
-func (s SqlTeamStore) UpdateMember(member *model.TeamMember) store.StoreChannel {
+func (s SqlTeamStore) UpdateMember(member *model.TeamMember) (*model.TeamMember, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	member.PreUpdate()
-		member.PreUpdate()
 
-		if result.Err = member.IsValid(); result.Err != nil {
+	if err := member.IsValid(); err != nil {
-			return
+		return nil, err
+	}
+
+	if _, err := s.GetMaster().Update(NewTeamMemberFromModel(member)); err != nil {
+		return nil, model.NewAppError("SqlTeamStore.UpdateMember", "store.sql_team.save_member.save.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+
+	query := s.getTeamMembersWithSchemeSelectQuery().
+		Where(sq.Eq{"TeamMembers.TeamId": member.TeamId}).
+		Where(sq.Eq{"TeamMembers.UserId": member.UserId})
+
+	queryString, args, err := query.ToSql()
+	if err != nil {
+		return nil, model.NewAppError("SqlTeamStore.UpdateMember", "store.sql_team.get_member.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+
+	var retrievedMember teamMemberWithSchemeRoles
+	if err := s.GetMaster().SelectOne(&retrievedMember, queryString, args...); err != nil {
+		if err == sql.ErrNoRows {
+			return nil, model.NewAppError("SqlTeamStore.UpdateMember", "store.sql_team.get_member.missing.app_error", nil, "team_id="+member.TeamId+"user_id="+member.UserId+","+err.Error(), http.StatusNotFound)
 		}
+		return nil, model.NewAppError("SqlTeamStore.UpdateMember", "store.sql_team.get_member.app_error", nil, "team_id="+member.TeamId+"user_id="+member.UserId+","+err.Error(), http.StatusInternalServerError)
+	}
 
-		if _, err := s.GetMaster().Update(NewTeamMemberFromModel(member)); err != nil {
+	return retrievedMember.ToModel(), nil
-			result.Err = model.NewAppError("SqlTeamStore.UpdateMember", "store.sql_team.save_member.save.app_error", nil, err.Error(), http.StatusInternalServerError)
-			return
-		}
-
-		query := s.getTeamMembersWithSchemeSelectQuery().
-			Where(sq.Eq{"TeamMembers.TeamId": member.TeamId}).
-			Where(sq.Eq{"TeamMembers.UserId": member.UserId})
-
-		queryString, args, err := query.ToSql()
-		if err != nil {
-			result.Err = model.NewAppError("SqlTeamStore.UpdateMember", "store.sql_team.get_member.app_error", nil, err.Error(), http.StatusInternalServerError)
-			return
-		}
-
-		var retrievedMember teamMemberWithSchemeRoles
-		if err := s.GetMaster().SelectOne(&retrievedMember, queryString, args...); err != nil {
-			if err == sql.ErrNoRows {
-				result.Err = model.NewAppError("SqlTeamStore.UpdateMember", "store.sql_team.get_member.missing.app_error", nil, "team_id="+member.TeamId+"user_id="+member.UserId+","+err.Error(), http.StatusNotFound)
-				return
-			}
-			result.Err = model.NewAppError("SqlTeamStore.UpdateMember", "store.sql_team.get_member.app_error", nil, "team_id="+member.TeamId+"user_id="+member.UserId+","+err.Error(), http.StatusInternalServerError)
-			return
-		}
-
-		result.Data = retrievedMember.ToModel()
-	})
 }
 
 func (s SqlTeamStore) GetMember(teamId string, userId string) (*model.TeamMember, *model.AppError) {

store/store.go

@@ -101,7 +101,7 @@ type TeamStore interface {
 	PermanentDelete(teamId string) StoreChannel
 	AnalyticsTeamCount() (int64, *model.AppError)
 	SaveMember(member *model.TeamMember, maxUsersPerTeam int) StoreChannel
-	UpdateMember(member *model.TeamMember) StoreChannel
+	UpdateMember(member *model.TeamMember) (*model.TeamMember, *model.AppError)
 	GetMember(teamId string, userId string) (*model.TeamMember, *model.AppError)
 	GetMembers(teamId string, offset int, limit int, restrictions *model.ViewUsersRestrictions) ([]*model.TeamMember, *model.AppError)
 	GetMembersByIds(teamId string, userIds []string, restrictions *model.ViewUsersRestrictions) ([]*model.TeamMember, *model.AppError)

store/storetest/mocks/TeamStore.go

@@ -818,19 +818,28 @@ func (_m *TeamStore) UpdateLastTeamIconUpdate(teamId string, curTime int64) stor
 }
 
 // UpdateMember provides a mock function with given fields: member
-func (_m *TeamStore) UpdateMember(member *model.TeamMember) store.StoreChannel {
+func (_m *TeamStore) UpdateMember(member *model.TeamMember) (*model.TeamMember, *model.AppError) {
 	ret := _m.Called(member)
 
-	var r0 store.StoreChannel
+	var r0 *model.TeamMember
-	if rf, ok := ret.Get(0).(func(*model.TeamMember) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(*model.TeamMember) *model.TeamMember); ok {
 		r0 = rf(member)
 	} 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.TeamMember) *model.AppError); ok {
+		r1 = rf(member)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // UserBelongsToTeams provides a mock function with given fields: userId, teamIds

store/storetest/team_store.go

@@ -985,11 +985,13 @@ func testSaveTeamMemberMaxMembers(t *testing.T, ss store.Store) {
 	}
 
 	// Leaving the team from the UI sets DeleteAt instead of using TeamStore.RemoveMember
-	store.Must(ss.Team().UpdateMember(&model.TeamMember{
+	if _, err := ss.Team().UpdateMember(&model.TeamMember{
 		TeamId:   team.Id,
 		UserId:   userIds[0],
 		DeleteAt: 1234,
-	}))
+	}); err != nil {
+		panic(err)
+	}
 
 	if totalMemberCount, err := ss.Team().GetTotalMemberCount(team.Id); err != nil {
 		t.Fatal(err)