diff --git a/app/user.go b/app/user.go index 0fe1191251..2768ea03df 100644 --- a/app/user.go +++ b/app/user.go @@ -447,11 +447,7 @@ func (a *App) GetUserByAuth(authData *string, authService string) (*model.User, } func (a *App) GetUsers(options *model.UserGetOptions) ([]*model.User, *model.AppError) { - result := <-a.Srv.Store.User().GetAllProfiles(options) - if result.Err != nil { - return nil, result.Err - } - return result.Data.([]*model.User), nil + return a.Srv.Store.User().GetAllProfiles(options) } func (a *App) GetUsersPage(options *model.UserGetOptions, asAdmin bool) ([]*model.User, *model.AppError) { diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 99fd843997..4ea4374429 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -392,39 +392,35 @@ func (s SqlUserStore) GetEtagForAllProfiles() store.StoreChannel { }) } -func (us SqlUserStore) GetAllProfiles(options *model.UserGetOptions) store.StoreChannel { +func (us SqlUserStore) GetAllProfiles(options *model.UserGetOptions) ([]*model.User, *model.AppError) { isPostgreSQL := us.DriverName() == model.DATABASE_DRIVER_POSTGRES - return store.Do(func(result *store.StoreResult) { - query := us.usersQuery. - OrderBy("u.Username ASC"). - Offset(uint64(options.Page * options.PerPage)).Limit(uint64(options.PerPage)) + query := us.usersQuery. + OrderBy("u.Username ASC"). + Offset(uint64(options.Page * options.PerPage)).Limit(uint64(options.PerPage)) - query = applyViewRestrictionsFilter(query, options.ViewRestrictions, true) + query = applyViewRestrictionsFilter(query, options.ViewRestrictions, true) - query = applyRoleFilter(query, options.Role, isPostgreSQL) + query = applyRoleFilter(query, options.Role, isPostgreSQL) - if options.Inactive { - query = query.Where("u.DeleteAt != 0") - } + if options.Inactive { + query = query.Where("u.DeleteAt != 0") + } - queryString, args, err := query.ToSql() - if err != nil { - result.Err = model.NewAppError("SqlUserStore.GetAllProfiles", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + queryString, args, err := query.ToSql() + if err != nil { + return nil, model.NewAppError("SqlUserStore.GetAllProfiles", "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.GetAllProfiles", "store.sql_user.get_profiles.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + var users []*model.User + if _, err := us.GetReplica().Select(&users, queryString, args...); err != nil { + return nil, model.NewAppError("SqlUserStore.GetAllProfiles", "store.sql_user.get_profiles.app_error", nil, err.Error(), http.StatusInternalServerError) + } - for _, u := range users { - u.Sanitize(map[string]bool{}) - } + for _, u := range users { + u.Sanitize(map[string]bool{}) + } - result.Data = users - }) + return users, nil } func applyRoleFilter(query sq.SelectBuilder, role string, isPostgreSQL bool) sq.SelectBuilder { diff --git a/store/store.go b/store/store.go index 4f2bc62c15..c635948f43 100644 --- a/store/store.go +++ b/store/store.go @@ -269,7 +269,7 @@ type UserStore interface { GetProfilesNotInChannel(teamId string, channelId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) StoreChannel GetProfilesWithoutTeam(offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) StoreChannel GetProfilesByUsernames(usernames []string, viewRestrictions *model.ViewUsersRestrictions) StoreChannel - GetAllProfiles(options *model.UserGetOptions) StoreChannel + GetAllProfiles(options *model.UserGetOptions) ([]*model.User, *model.AppError) GetProfiles(options *model.UserGetOptions) StoreChannel GetProfileByIds(userIds []string, options *UserGetByIdsOpts, allowFromCache bool) ([]*model.User, *model.AppError) GetProfileByGroupChannelIdsForUser(userId string, channelIds []string) (map[string][]*model.User, *model.AppError) diff --git a/store/storetest/mocks/UserStore.go b/store/storetest/mocks/UserStore.go index 5f5dc71b4f..2cb034605e 100644 --- a/store/storetest/mocks/UserStore.go +++ b/store/storetest/mocks/UserStore.go @@ -193,19 +193,28 @@ func (_m *UserStore) GetAllAfter(limit int, afterId string) ([]*model.User, *mod } // GetAllProfiles provides a mock function with given fields: options -func (_m *UserStore) GetAllProfiles(options *model.UserGetOptions) store.StoreChannel { +func (_m *UserStore) GetAllProfiles(options *model.UserGetOptions) ([]*model.User, *model.AppError) { ret := _m.Called(options) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.UserGetOptions) store.StoreChannel); ok { + var r0 []*model.User + if rf, ok := ret.Get(0).(func(*model.UserGetOptions) []*model.User); ok { r0 = rf(options) } 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(*model.UserGetOptions) *model.AppError); ok { + r1 = rf(options) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetAllProfilesInChannel provides a mock function with given fields: channelId, allowFromCache diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index ffb4044402..3b3f990d79 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -415,10 +415,9 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { t.Run("get offset 0, limit 100", func(t *testing.T) { options := &model.UserGetOptions{Page: 0, PerPage: 100} - result := <-ss.User().GetAllProfiles(options) - require.Nil(t, result.Err) + actual, err := ss.User().GetAllProfiles(options) + require.Nil(t, err) - actual := result.Data.([]*model.User) require.Equal(t, []*model.User{ sanitized(u1), sanitized(u2), @@ -431,12 +430,11 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { }) t.Run("get offset 0, limit 1", func(t *testing.T) { - result := <-ss.User().GetAllProfiles(&model.UserGetOptions{ + actual, err := ss.User().GetAllProfiles(&model.UserGetOptions{ Page: 0, PerPage: 1, }) - require.Nil(t, result.Err) - actual := result.Data.([]*model.User) + require.Nil(t, err) require.Equal(t, []*model.User{ sanitized(u1), }, actual) @@ -476,13 +474,12 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { }) t.Run("filter to system_admin role", func(t *testing.T) { - result := <-ss.User().GetAllProfiles(&model.UserGetOptions{ + actual, err := ss.User().GetAllProfiles(&model.UserGetOptions{ Page: 0, PerPage: 10, Role: "system_admin", }) - require.Nil(t, result.Err) - actual := result.Data.([]*model.User) + require.Nil(t, err) require.Equal(t, []*model.User{ sanitized(u5), sanitized(u6), @@ -490,27 +487,25 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { }) t.Run("filter to system_admin role, inactive", func(t *testing.T) { - result := <-ss.User().GetAllProfiles(&model.UserGetOptions{ + actual, err := ss.User().GetAllProfiles(&model.UserGetOptions{ Page: 0, PerPage: 10, Role: "system_admin", Inactive: true, }) - require.Nil(t, result.Err) - actual := result.Data.([]*model.User) + require.Nil(t, err) require.Equal(t, []*model.User{ sanitized(u6), }, actual) }) t.Run("filter to inactive", func(t *testing.T) { - result := <-ss.User().GetAllProfiles(&model.UserGetOptions{ + actual, err := ss.User().GetAllProfiles(&model.UserGetOptions{ Page: 0, PerPage: 10, Inactive: true, }) - require.Nil(t, result.Err) - actual := result.Data.([]*model.User) + require.Nil(t, err) require.Equal(t, []*model.User{ sanitized(u6), sanitized(u7),