From ff89b2c8e186b21d60b71b32eea305bea393bfa8 Mon Sep 17 00:00:00 2001 From: Mounica Paladugu <49451510+mounicapaladugu@users.noreply.github.com> Date: Tue, 9 Jul 2019 09:21:18 -0700 Subject: [PATCH] [MM-16170] Migrate Team.GetTeamsByUserId to Sync by default (#11100) * [MM-16170] Migrate Team.GetTeamsByUserId to Sync by default * Fixing comments --- app/file.go | 7 +++---- app/notification_email.go | 7 +++---- app/team.go | 6 +----- app/user.go | 8 ++++---- cmd/mattermost/commands/team_test.go | 5 ++--- store/sqlstore/team_store.go | 15 ++++++--------- store/store.go | 2 +- store/storetest/mocks/TeamStore.go | 19 ++++++++++++++----- store/storetest/team_store.go | 5 ++--- 9 files changed, 36 insertions(+), 38 deletions(-) diff --git a/app/file.go b/app/file.go index 4dc438e7f9..bcff96a437 100644 --- a/app/file.go +++ b/app/file.go @@ -192,13 +192,12 @@ func (a *App) FindTeamIdForFilename(post *model.Post, filename string) string { name, _ := url.QueryUnescape(split[4]) // This post is in a direct channel so we need to figure out what team the files are stored under. - result := <-a.Srv.Store.Team().GetTeamsByUserId(post.UserId) - if result.Err != nil { - mlog.Error(fmt.Sprintf("Unable to get teams when migrating post to use FileInfo, err=%v", result.Err), mlog.String("post_id", post.Id)) + teams, err := a.Srv.Store.Team().GetTeamsByUserId(post.UserId) + if err != nil { + mlog.Error(fmt.Sprintf("Unable to get teams when migrating post to use FileInfo, err=%v", err), mlog.String("post_id", post.Id)) return "" } - teams := result.Data.([]*model.Team) if len(teams) == 1 { // The user has only one team so the post must've been sent from it return teams[0].Id diff --git a/app/notification_email.go b/app/notification_email.go index e1c897a118..db478b7b1c 100644 --- a/app/notification_email.go +++ b/app/notification_email.go @@ -22,13 +22,12 @@ func (a *App) sendNotificationEmail(notification *postNotification, user *model. post := notification.post if channel.IsGroupOrDirect() { - result := <-a.Srv.Store.Team().GetTeamsByUserId(user.Id) - if result.Err != nil { - return result.Err + teams, err := a.Srv.Store.Team().GetTeamsByUserId(user.Id) + if err != nil { + return err } // if the recipient isn't in the current user's team, just pick one - teams := result.Data.([]*model.Team) found := false for i := range teams { diff --git a/app/team.go b/app/team.go index a654d8413e..fa96d5d814 100644 --- a/app/team.go +++ b/app/team.go @@ -654,11 +654,7 @@ func (a *App) SearchPrivateTeams(term string) ([]*model.Team, *model.AppError) { } func (a *App) GetTeamsForUser(userId string) ([]*model.Team, *model.AppError) { - result := <-a.Srv.Store.Team().GetTeamsByUserId(userId) - if result.Err != nil { - return nil, result.Err - } - return result.Data.([]*model.Team), nil + return a.Srv.Store.Team().GetTeamsByUserId(userId) } func (a *App) GetTeamMember(teamId, userId string) (*model.TeamMember, *model.AppError) { diff --git a/app/user.go b/app/user.go index 828ddb91d7..9f8525657d 100644 --- a/app/user.go +++ b/app/user.go @@ -189,13 +189,13 @@ func (a *App) IsFirstUserAccount() bool { // indexUser fetches the required information to index a user from the database and // calls the elasticsearch interface method func (a *App) indexUser(user *model.User) *model.AppError { - userTeams := <-a.Srv.Store.Team().GetTeamsByUserId(user.Id) - if userTeams.Err != nil { - return userTeams.Err + userTeams, err := a.Srv.Store.Team().GetTeamsByUserId(user.Id) + if err != nil { + return err } userTeamsIds := []string{} - for _, team := range userTeams.Data.([]*model.Team) { + for _, team := range userTeams { userTeamsIds = append(userTeamsIds, team.Id) } diff --git a/cmd/mattermost/commands/team_test.go b/cmd/mattermost/commands/team_test.go index 13987c2b66..1afe8da244 100644 --- a/cmd/mattermost/commands/team_test.go +++ b/cmd/mattermost/commands/team_test.go @@ -71,9 +71,8 @@ func TestLeaveTeam(t *testing.T) { t.Fatal("profile should not be on team") } - if result := <-th.App.Srv.Store.Team().GetTeamsByUserId(th.BasicUser.Id); result.Err != nil { - teamMembers := result.Data.([]*model.TeamMember) - if len(teamMembers) > 0 { + if teams, err := th.App.Srv.Store.Team().GetTeamsByUserId(th.BasicUser.Id); err != nil { + if len(teams) > 0 { t.Fatal("Shouldn't be in team") } } diff --git a/store/sqlstore/team_store.go b/store/sqlstore/team_store.go index f8249b7aab..f569c6ea17 100644 --- a/store/sqlstore/team_store.go +++ b/store/sqlstore/team_store.go @@ -376,16 +376,13 @@ func (s SqlTeamStore) GetAllPage(offset int, limit int) ([]*model.Team, *model.A return teams, nil } -func (s SqlTeamStore) GetTeamsByUserId(userId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - var data []*model.Team - if _, err := s.GetReplica().Select(&data, "SELECT Teams.* FROM Teams, TeamMembers WHERE TeamMembers.TeamId = Teams.Id AND TeamMembers.UserId = :UserId AND TeamMembers.DeleteAt = 0 AND Teams.DeleteAt = 0", map[string]interface{}{"UserId": userId}); err != nil { - result.Err = model.NewAppError("SqlTeamStore.GetTeamsByUserId", "store.sql_team.get_all.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } +func (s SqlTeamStore) GetTeamsByUserId(userId string) ([]*model.Team, *model.AppError) { + var teams []*model.Team + if _, err := s.GetReplica().Select(&teams, "SELECT Teams.* FROM Teams, TeamMembers WHERE TeamMembers.TeamId = Teams.Id AND TeamMembers.UserId = :UserId AND TeamMembers.DeleteAt = 0 AND Teams.DeleteAt = 0", map[string]interface{}{"UserId": userId}); err != nil { + return nil, model.NewAppError("SqlTeamStore.GetTeamsByUserId", "store.sql_team.get_all.app_error", nil, err.Error(), http.StatusInternalServerError) + } - result.Data = data - }) + return teams, nil } func (s SqlTeamStore) GetAllPrivateTeamListing() ([]*model.Team, *model.AppError) { diff --git a/store/store.go b/store/store.go index d500bc6b6b..db8ae3d8a9 100644 --- a/store/store.go +++ b/store/store.go @@ -96,7 +96,7 @@ type TeamStore interface { GetAllPrivateTeamPageListing(offset int, limit int) ([]*model.Team, *model.AppError) GetAllTeamListing() ([]*model.Team, *model.AppError) GetAllTeamPageListing(offset int, limit int) ([]*model.Team, *model.AppError) - GetTeamsByUserId(userId string) StoreChannel + GetTeamsByUserId(userId string) ([]*model.Team, *model.AppError) GetByInviteId(inviteId string) (*model.Team, *model.AppError) PermanentDelete(teamId string) *model.AppError AnalyticsTeamCount() (int64, *model.AppError) diff --git a/store/storetest/mocks/TeamStore.go b/store/storetest/mocks/TeamStore.go index 2ccc85b893..e957eb473d 100644 --- a/store/storetest/mocks/TeamStore.go +++ b/store/storetest/mocks/TeamStore.go @@ -522,19 +522,28 @@ func (_m *TeamStore) GetTeamsByScheme(schemeId string, offset int, limit int) ([ } // GetTeamsByUserId provides a mock function with given fields: userId -func (_m *TeamStore) GetTeamsByUserId(userId string) store.StoreChannel { +func (_m *TeamStore) GetTeamsByUserId(userId string) ([]*model.Team, *model.AppError) { ret := _m.Called(userId) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 []*model.Team + if rf, ok := ret.Get(0).(func(string) []*model.Team); ok { r0 = rf(userId) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).([]*model.Team) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(userId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetTeamsForUser provides a mock function with given fields: userId diff --git a/store/storetest/team_store.go b/store/storetest/team_store.go index f34f054e9f..113be6cae8 100644 --- a/store/storetest/team_store.go +++ b/store/storetest/team_store.go @@ -431,10 +431,9 @@ func testTeamStoreByUserId(t *testing.T, ss store.Store) { m1 := &model.TeamMember{TeamId: o1.Id, UserId: model.NewId()} store.Must(ss.Team().SaveMember(m1, -1)) - if r1 := <-ss.Team().GetTeamsByUserId(m1.UserId); r1.Err != nil { - t.Fatal(r1.Err) + if teams, err := ss.Team().GetTeamsByUserId(m1.UserId); err != nil { + t.Fatal(err) } else { - teams := r1.Data.([]*model.Team) if len(teams) == 0 { t.Fatal("Should return a team") }