From b13c5eabff07191b6c07283d6d77660639cd46b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Villablanca=20V=C3=A1squez?= Date: Fri, 14 Jun 2019 12:18:45 -0400 Subject: [PATCH] Migrate Team.GetActiveMembersCount to Sync by default (#11146) * Migrate Team.GetActiveMembersCount to Sync by default * Requested change * Requested change * Fix merge * Added a new key to i18n: store.sql_team.get_active_member_count.app_error --- app/team.go | 15 ++++++++----- i18n/en.json | 4 ++++ store/sqlstore/team_store.go | 35 ++++++++++++++---------------- store/store.go | 2 +- store/storetest/mocks/TeamStore.go | 19 +++++++++++----- store/storetest/team_store.go | 12 +++++----- 6 files changed, 50 insertions(+), 37 deletions(-) diff --git a/app/team.go b/app/team.go index a44c88c614..3bc30be8fa 100644 --- a/app/team.go +++ b/app/team.go @@ -523,12 +523,12 @@ func (a *App) joinUserToTeam(team *model.Team, user *model.User) (*model.TeamMem return rtm, true, nil } - membersCount := <-a.Srv.Store.Team().GetActiveMemberCount(tm.TeamId) - if membersCount.Err != nil { - return nil, false, membersCount.Err + membersCount, err := a.Srv.Store.Team().GetActiveMemberCount(tm.TeamId) + if err != nil { + return nil, false, err } - if membersCount.Data.(int64) >= int64(*a.Config().TeamSettings.MaxUsersPerTeam) { + if membersCount >= int64(*a.Config().TeamSettings.MaxUsersPerTeam) { return nil, false, model.NewAppError("joinUserToTeam", "app.team.join_user_to_team.max_accounts.app_error", nil, "teamId="+tm.TeamId, http.StatusBadRequest) } @@ -1149,7 +1149,12 @@ func (a *App) GetTeamStats(teamId string) (*model.TeamStats, *model.AppError) { tchan <- store.StoreResult{Data: totalMemberCount, Err: err} close(tchan) }() - achan := a.Srv.Store.Team().GetActiveMemberCount(teamId) + achan := make(chan store.StoreResult, 1) + go func() { + memberCount, err := a.Srv.Store.Team().GetActiveMemberCount(teamId) + achan <- store.StoreResult{Data: memberCount, Err: err} + close(achan) + }() stats := &model.TeamStats{} stats.TeamId = teamId diff --git a/i18n/en.json b/i18n/en.json index 34ce359b1b..80f0531394 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -6530,6 +6530,10 @@ "id": "store.sql_team.get.finding.app_error", "translation": "We encountered an error finding the team" }, + { + "id": "store.sql_team.get_active_member_count.app_error", + "translation": "Unable to count the team members" + }, { "id": "store.sql_team.get_all.app_error", "translation": "We could not get all teams" diff --git a/store/sqlstore/team_store.go b/store/sqlstore/team_store.go index 45a9d30fc4..8a3780175b 100644 --- a/store/sqlstore/team_store.go +++ b/store/sqlstore/team_store.go @@ -652,26 +652,23 @@ func (s SqlTeamStore) GetTotalMemberCount(teamId string) (int64, *model.AppError return count, nil } -func (s SqlTeamStore) GetActiveMemberCount(teamId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - count, err := s.GetReplica().SelectInt(` - SELECT - count(*) - FROM - TeamMembers, - Users - WHERE - TeamMembers.UserId = Users.Id - AND TeamMembers.TeamId = :TeamId - AND TeamMembers.DeleteAt = 0 - AND Users.DeleteAt = 0`, map[string]interface{}{"TeamId": teamId}) - if err != nil { - result.Err = model.NewAppError("SqlTeamStore.GetActiveMemberCount", "store.sql_team.get_member_count.app_error", nil, "teamId="+teamId+" "+err.Error(), http.StatusInternalServerError) - return - } +func (s SqlTeamStore) GetActiveMemberCount(teamId string) (int64, *model.AppError) { + count, err := s.GetReplica().SelectInt(` + SELECT + count(*) + FROM + TeamMembers, + Users + WHERE + TeamMembers.UserId = Users.Id + AND TeamMembers.TeamId = :TeamId + AND TeamMembers.DeleteAt = 0 + AND Users.DeleteAt = 0`, map[string]interface{}{"TeamId": teamId}) + if err != nil { + return 0, model.NewAppError("SqlTeamStore.GetActiveMemberCount", "store.sql_team.get_active_member_count.app_error", nil, "teamId="+teamId+" "+err.Error(), http.StatusInternalServerError) + } - result.Data = count - }) + return count, nil } func (s SqlTeamStore) GetMembersByIds(teamId string, userIds []string, restrictions *model.ViewUsersRestrictions) ([]*model.TeamMember, *model.AppError) { diff --git a/store/store.go b/store/store.go index 98af39e625..f477bc0734 100644 --- a/store/store.go +++ b/store/store.go @@ -106,7 +106,7 @@ type TeamStore interface { 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) GetTotalMemberCount(teamId string) (int64, *model.AppError) - GetActiveMemberCount(teamId string) StoreChannel + GetActiveMemberCount(teamId string) (int64, *model.AppError) GetTeamsForUser(userId string) StoreChannel GetTeamsForUserWithPagination(userId string, page, perPage int) StoreChannel GetChannelUnreadsForAllTeams(excludeTeamId, userId string) StoreChannel diff --git a/store/storetest/mocks/TeamStore.go b/store/storetest/mocks/TeamStore.go index 41faeb2e77..ed84790d3e 100644 --- a/store/storetest/mocks/TeamStore.go +++ b/store/storetest/mocks/TeamStore.go @@ -99,19 +99,26 @@ func (_m *TeamStore) Get(id string) (*model.Team, *model.AppError) { } // GetActiveMemberCount provides a mock function with given fields: teamId -func (_m *TeamStore) GetActiveMemberCount(teamId string) store.StoreChannel { +func (_m *TeamStore) GetActiveMemberCount(teamId string) (int64, *model.AppError) { ret := _m.Called(teamId) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 int64 + if rf, ok := ret.Get(0).(func(string) int64); ok { r0 = rf(teamId) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(int64) + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(teamId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) } } - return r0 + return r0, r1 } // GetAll provides a mock function with given fields: diff --git a/store/storetest/team_store.go b/store/storetest/team_store.go index 13fcc772c1..60adfa989f 100644 --- a/store/storetest/team_store.go +++ b/store/storetest/team_store.go @@ -1160,10 +1160,10 @@ func testTeamStoreMemberCount(t *testing.T, ss store.Store) { } } - if result := <-ss.Team().GetActiveMemberCount(teamId1); result.Err != nil { - t.Fatal(result.Err) + if result, err := ss.Team().GetActiveMemberCount(teamId1); err != nil { + t.Fatal(err) } else { - if result.Data.(int64) != 1 { + if result != 1 { t.Fatal("wrong count") } } @@ -1179,10 +1179,10 @@ func testTeamStoreMemberCount(t *testing.T, ss store.Store) { } } - if result := <-ss.Team().GetActiveMemberCount(teamId1); result.Err != nil { - t.Fatal(result.Err) + if result, err := ss.Team().GetActiveMemberCount(teamId1); err != nil { + t.Fatal(err) } else { - if result.Data.(int64) != 1 { + if result != 1 { t.Fatal("wrong count") } }