diff --git a/app/user.go b/app/user.go index c2dcdbfec4..c10d825df7 100644 --- a/app/user.go +++ b/app/user.go @@ -476,11 +476,7 @@ func (a *App) GetUsersInTeam(options *model.UserGetOptions) ([]*model.User, *mod } func (a *App) GetUsersNotInTeam(teamId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) { - result := <-a.Srv.Store.User().GetProfilesNotInTeam(teamId, groupConstrained, offset, limit, viewRestrictions) - if result.Err != nil { - return nil, result.Err - } - return result.Data.([]*model.User), nil + return a.Srv.Store.User().GetProfilesNotInTeam(teamId, groupConstrained, offset, limit, viewRestrictions) } func (a *App) GetUsersInTeamPage(options *model.UserGetOptions, asAdmin bool) ([]*model.User, *model.AppError) { diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 170e96a394..7c32513a78 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -1435,38 +1435,33 @@ func (us SqlUserStore) AnalyticsGetSystemAdminCount() (int64, *model.AppError) { return count, nil } -func (us SqlUserStore) GetProfilesNotInTeam(teamId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - query := us.usersQuery. - LeftJoin("TeamMembers tm ON ( tm.UserId = u.Id AND tm.DeleteAt = 0 AND tm.TeamId = ? )", teamId). - Where("tm.UserId IS NULL"). - OrderBy("u.Username ASC"). - Offset(uint64(offset)).Limit(uint64(limit)) +func (us SqlUserStore) GetProfilesNotInTeam(teamId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) { + var users []*model.User + query := us.usersQuery. + LeftJoin("TeamMembers tm ON ( tm.UserId = u.Id AND tm.DeleteAt = 0 AND tm.TeamId = ? )", teamId). + Where("tm.UserId IS NULL"). + OrderBy("u.Username ASC"). + Offset(uint64(offset)).Limit(uint64(limit)) - query = applyViewRestrictionsFilter(query, viewRestrictions, true) + query = applyViewRestrictionsFilter(query, viewRestrictions, true) - if groupConstrained { - query = applyTeamGroupConstrainedFilter(query, teamId) - } + if groupConstrained { + query = applyTeamGroupConstrainedFilter(query, teamId) + } - queryString, args, err := query.ToSql() - if err != nil { - result.Err = model.NewAppError("SqlUserStore.GetProfilesNotInTeam", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + queryString, args, err := query.ToSql() + if err != nil { + return nil, model.NewAppError("SqlUserStore.GetProfilesNotInTeam", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError) + } - var users []*model.User - if _, err := us.GetReplica().Select(&users, queryString, args...); err != nil { - result.Err = model.NewAppError("SqlUserStore.GetProfilesNotInTeam", "store.sql_user.get_profiles.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + if _, err := us.GetReplica().Select(&users, queryString, args...); err != nil { + return nil, model.NewAppError("SqlUserStore.GetProfilesNotInTeam", "store.sql_user.get_profiles.app_error", nil, err.Error(), http.StatusInternalServerError) + } - for _, u := range users { - u.Sanitize(map[string]bool{}) - } - - result.Data = users - }) + for _, u := range users { + u.Sanitize(map[string]bool{}) + } + return users, nil } func (us SqlUserStore) GetEtagForProfilesNotInTeam(teamId string) store.StoreChannel { diff --git a/store/store.go b/store/store.go index 94afe17ab3..864ae56ec9 100644 --- a/store/store.go +++ b/store/store.go @@ -295,7 +295,7 @@ type UserStore interface { SearchWithoutTeam(term string, options *model.UserSearchOptions) ([]*model.User, *model.AppError) AnalyticsGetInactiveUsersCount() (int64, *model.AppError) AnalyticsGetSystemAdminCount() (int64, *model.AppError) - GetProfilesNotInTeam(teamId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) StoreChannel + GetProfilesNotInTeam(teamId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) GetEtagForProfilesNotInTeam(teamId string) StoreChannel ClearAllCustomRoleAssignments() StoreChannel InferSystemInstallDate() StoreChannel diff --git a/store/storetest/mocks/UserStore.go b/store/storetest/mocks/UserStore.go index 4f66a4de7a..8f83f32ed8 100644 --- a/store/storetest/mocks/UserStore.go +++ b/store/storetest/mocks/UserStore.go @@ -567,19 +567,28 @@ func (_m *UserStore) GetProfilesNotInChannel(teamId string, channelId string, gr } // GetProfilesNotInTeam provides a mock function with given fields: teamId, groupConstrained, offset, limit, viewRestrictions -func (_m *UserStore) GetProfilesNotInTeam(teamId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) store.StoreChannel { +func (_m *UserStore) GetProfilesNotInTeam(teamId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) { ret := _m.Called(teamId, groupConstrained, offset, limit, viewRestrictions) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, bool, int, int, *model.ViewUsersRestrictions) store.StoreChannel); ok { + var r0 []*model.User + if rf, ok := ret.Get(0).(func(string, bool, int, int, *model.ViewUsersRestrictions) []*model.User); ok { r0 = rf(teamId, groupConstrained, offset, limit, viewRestrictions) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).([]*model.User) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string, bool, int, int, *model.ViewUsersRestrictions) *model.AppError); ok { + r1 = rf(teamId, groupConstrained, offset, limit, viewRestrictions) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetProfilesWithoutTeam provides a mock function with given fields: offset, limit, viewRestrictions diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index 449be169ad..9c1f97b7cc 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -3342,29 +3342,29 @@ func testUserStoreGetProfilesNotInTeam(t *testing.T, ss store.Store) { }) t.Run("get not in team 1, offset 0, limit 100000", func(t *testing.T) { - result := <-ss.User().GetProfilesNotInTeam(teamId, false, 0, 100000, nil) - require.Nil(t, result.Err) + users, userErr := ss.User().GetProfilesNotInTeam(teamId, false, 0, 100000, nil) + require.Nil(t, userErr) assert.Equal(t, []*model.User{ sanitized(u2), sanitized(u3), - }, result.Data.([]*model.User)) + }, users) }) t.Run("get not in team 1, offset 1, limit 1", func(t *testing.T) { - result := <-ss.User().GetProfilesNotInTeam(teamId, false, 1, 1, nil) - require.Nil(t, result.Err) + users, userErr := ss.User().GetProfilesNotInTeam(teamId, false, 1, 1, nil) + require.Nil(t, userErr) assert.Equal(t, []*model.User{ sanitized(u3), - }, result.Data.([]*model.User)) + }, users) }) t.Run("get not in team 2, offset 0, limit 100", func(t *testing.T) { - result := <-ss.User().GetProfilesNotInTeam(teamId2, false, 0, 100, nil) - require.Nil(t, result.Err) + users, userErr := ss.User().GetProfilesNotInTeam(teamId2, false, 0, 100, nil) + require.Nil(t, userErr) assert.Equal(t, []*model.User{ sanitized(u1), sanitized(u3), - }, result.Data.([]*model.User)) + }, users) }) // Ensure update at timestamp changes @@ -3382,11 +3382,11 @@ func testUserStoreGetProfilesNotInTeam(t *testing.T, ss store.Store) { }) t.Run("get not in team 1, offset 0, limit 100000 after update", func(t *testing.T) { - result := <-ss.User().GetProfilesNotInTeam(teamId, false, 0, 100000, nil) - require.Nil(t, result.Err) + users, userErr := ss.User().GetProfilesNotInTeam(teamId, false, 0, 100000, nil) + require.Nil(t, userErr) assert.Equal(t, []*model.User{ sanitized(u3), - }, result.Data.([]*model.User)) + }, users) }) // Ensure update at timestamp changes @@ -3406,13 +3406,13 @@ func testUserStoreGetProfilesNotInTeam(t *testing.T, ss store.Store) { }) t.Run("get not in team 1, offset 0, limit 100000 after second update", func(t *testing.T) { - result := <-ss.User().GetProfilesNotInTeam(teamId, false, 0, 100000, nil) - require.Nil(t, result.Err) + users, userErr := ss.User().GetProfilesNotInTeam(teamId, false, 0, 100000, nil) + require.Nil(t, userErr) assert.Equal(t, []*model.User{ sanitized(u1), sanitized(u2), sanitized(u3), - }, result.Data.([]*model.User)) + }, users) }) // Ensure update at timestamp changes @@ -3450,9 +3450,9 @@ func testUserStoreGetProfilesNotInTeam(t *testing.T, ss store.Store) { }) t.Run("get not in team 1, offset 0, limit 100000 after second update, setting group constrained when it's not", func(t *testing.T) { - result := <-ss.User().GetProfilesNotInTeam(teamId, true, 0, 100000, nil) - require.Nil(t, result.Err) - assert.Empty(t, result.Data.([]*model.User)) + users, userErr := ss.User().GetProfilesNotInTeam(teamId, true, 0, 100000, nil) + require.Nil(t, userErr) + assert.Empty(t, users) }) // create a group @@ -3479,12 +3479,12 @@ func testUserStoreGetProfilesNotInTeam(t *testing.T, ss store.Store) { require.Nil(t, err) t.Run("get not in team 1, offset 0, limit 100000 after second update, setting group constrained", func(t *testing.T) { - result := <-ss.User().GetProfilesNotInTeam(teamId, true, 0, 100000, nil) - require.Nil(t, result.Err) + users, userErr := ss.User().GetProfilesNotInTeam(teamId, true, 0, 100000, nil) + require.Nil(t, userErr) assert.Equal(t, []*model.User{ sanitized(u1), sanitized(u2), - }, result.Data.([]*model.User)) + }, users) }) }