From f1b5014efdbc0b26dfef93905fc630eb6442d2ba Mon Sep 17 00:00:00 2001 From: Alexander Zobnin Date: Mon, 9 Jan 2023 11:54:33 +0300 Subject: [PATCH] Preferences: Add pagination to org configuration page (#60896) * Add auth labels and access control metadata to org users search results * Fix search result JSON model * Org users: Use API for pagination * Fix default page size * Refactor: UsersListPage to functional component * Refactor: update UsersTable component code style * Add pagination to the /orgs/{org_id}/users endpoint * Use pagination on the AdminEditOrgPage * Add /orgs/{org_id}/users/search endpoint to prevent breaking API * Use existing search store method * Remove unnecessary error * Remove unused * Add query param to search endpoint * Fix endpoint docs * Minor refactor * Fix number of pages calculation * Use SearchOrgUsers for all org users methods * Refactor: GetOrgUsers as a service method * Minor refactor: rename orgId => orgID * Fix integration tests * Fix tests --- pkg/api/api.go | 1 + pkg/api/common_test.go | 1 + pkg/api/org_users.go | 150 +++++++++++------- pkg/api/org_users_test.go | 18 ++- pkg/services/org/model.go | 14 +- pkg/services/org/orgimpl/org.go | 14 +- pkg/services/org/orgimpl/store.go | 115 +++----------- pkg/services/org/orgimpl/store_test.go | 52 +++--- .../provisioning/plugins/mocks/Store.go | 4 +- .../app/features/admin/AdminEditOrgPage.tsx | 115 +++++++------- .../features/users/UsersActionBar.test.tsx | 8 +- public/app/features/users/UsersActionBar.tsx | 107 ++++++------- .../app/features/users/UsersListPage.test.tsx | 11 +- public/app/features/users/UsersListPage.tsx | 129 ++++++--------- public/app/features/users/UsersTable.test.tsx | 2 +- public/app/features/users/UsersTable.tsx | 7 +- .../app/features/users/__mocks__/userMocks.ts | 15 +- public/app/features/users/state/actions.ts | 36 ++++- .../app/features/users/state/reducers.test.ts | 12 +- public/app/features/users/state/reducers.ts | 46 +++++- public/app/features/users/state/selectors.ts | 1 - public/app/types/user.ts | 6 +- 22 files changed, 454 insertions(+), 410 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index 943a6156ebd..a1a689978cd 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -367,6 +367,7 @@ func (hs *HTTPServer) registerRoutes() { orgsRoute.Put("/address", authorizeInOrg(reqGrafanaAdmin, ac.UseOrgFromContextParams, ac.EvalPermission(ac.ActionOrgsWrite)), routing.Wrap(hs.UpdateOrgAddress)) orgsRoute.Delete("/", authorizeInOrg(reqGrafanaAdmin, ac.UseOrgFromContextParams, ac.EvalPermission(ac.ActionOrgsDelete)), routing.Wrap(hs.DeleteOrgByID)) orgsRoute.Get("/users", authorizeInOrg(reqGrafanaAdmin, ac.UseOrgFromContextParams, ac.EvalPermission(ac.ActionOrgUsersRead)), routing.Wrap(hs.GetOrgUsers)) + orgsRoute.Get("/users/search", authorizeInOrg(reqGrafanaAdmin, ac.UseOrgFromContextParams, ac.EvalPermission(ac.ActionOrgUsersRead)), routing.Wrap(hs.SearchOrgUsers)) orgsRoute.Post("/users", authorizeInOrg(reqGrafanaAdmin, ac.UseOrgFromContextParams, ac.EvalPermission(ac.ActionOrgUsersAdd, ac.ScopeUsersAll)), routing.Wrap(hs.AddOrgUser)) orgsRoute.Patch("/users/:userId", authorizeInOrg(reqGrafanaAdmin, ac.UseOrgFromContextParams, ac.EvalPermission(ac.ActionOrgUsersWrite, userIDScope)), routing.Wrap(hs.UpdateOrgUser)) orgsRoute.Delete("/users/:userId", authorizeInOrg(reqGrafanaAdmin, ac.UseOrgFromContextParams, ac.EvalPermission(ac.ActionOrgUsersRemove, userIDScope)), routing.Wrap(hs.RemoveOrgUser)) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index 4dec04f21ef..65468edd6da 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -398,6 +398,7 @@ func setupHTTPServerWithCfgDb( userMock.ExpectedUser = &user.User{ID: 1} orgMock := orgtest.NewOrgServiceFake() orgMock.ExpectedOrg = &org.Org{} + orgMock.ExpectedSearchOrgUsersResult = &org.SearchOrgUsersQueryResult{} // Defining the accesscontrol service has to be done before registering routes if useFakeAccessControl { diff --git a/pkg/api/org_users.go b/pkg/api/org_users.go index 7d229c6e33c..a4798619824 100644 --- a/pkg/api/org_users.go +++ b/pkg/api/org_users.go @@ -115,18 +115,18 @@ func (hs *HTTPServer) addOrgUserHelper(c *models.ReqContext, cmd org.AddOrgUserC // 403: forbiddenError // 500: internalServerError func (hs *HTTPServer) GetOrgUsersForCurrentOrg(c *models.ReqContext) response.Response { - result, err := hs.getOrgUsersHelper(c, &org.GetOrgUsersQuery{ + result, err := hs.searchOrgUsersHelper(c, &org.SearchOrgUsersQuery{ OrgID: c.OrgID, Query: c.Query("query"), Limit: c.QueryInt("limit"), User: c.SignedInUser, - }, c.SignedInUser) + }) if err != nil { return response.Error(500, "Failed to get users for current organization", err) } - return response.JSON(http.StatusOK, result) + return response.JSON(http.StatusOK, result.OrgUsers) } // swagger:route GET /org/users/lookup org getOrgUsersForCurrentOrgLookup @@ -144,13 +144,13 @@ func (hs *HTTPServer) GetOrgUsersForCurrentOrg(c *models.ReqContext) response.Re // 500: internalServerError func (hs *HTTPServer) GetOrgUsersForCurrentOrgLookup(c *models.ReqContext) response.Response { - orgUsers, err := hs.getOrgUsersHelper(c, &org.GetOrgUsersQuery{ + orgUsersResult, err := hs.searchOrgUsersHelper(c, &org.SearchOrgUsersQuery{ OrgID: c.OrgID, Query: c.Query("query"), Limit: c.QueryInt("limit"), User: c.SignedInUser, DontEnforceAccessControl: !hs.License.FeatureEnabled("accesscontrol.enforcement"), - }, c.SignedInUser) + }) if err != nil { return response.Error(500, "Failed to get users for current organization", err) @@ -158,7 +158,7 @@ func (hs *HTTPServer) GetOrgUsersForCurrentOrgLookup(c *models.ReqContext) respo result := make([]*dtos.UserLookupDTO, 0) - for _, u := range orgUsers { + for _, u := range orgUsersResult.OrgUsers { result = append(result, &dtos.UserLookupDTO{ UserID: u.UserID, Login: u.Login, @@ -190,12 +190,58 @@ func (hs *HTTPServer) GetOrgUsers(c *models.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "orgId is invalid", err) } - result, err := hs.getOrgUsersHelper(c, &org.GetOrgUsersQuery{ + result, err := hs.searchOrgUsersHelper(c, &org.SearchOrgUsersQuery{ OrgID: orgId, Query: "", Limit: 0, User: c.SignedInUser, - }, c.SignedInUser) + }) + + if err != nil { + return response.Error(500, "Failed to get users for organization", err) + } + + return response.JSON(http.StatusOK, result.OrgUsers) +} + +// swagger:route GET /orgs/{org_id}/users/search orgs searchOrgUsers +// +// Search Users in Organization. +// +// If you are running Grafana Enterprise and have Fine-grained access control enabled +// you need to have a permission with action: `org.users:read` with scope `users:*`. +// +// Security: +// - basic: +// +// Responses: +// 200: searchOrgUsersResponse +// 401: unauthorisedError +// 403: forbiddenError +// 500: internalServerError +func (hs *HTTPServer) SearchOrgUsers(c *models.ReqContext) response.Response { + orgID, err := strconv.ParseInt(web.Params(c.Req)[":orgId"], 10, 64) + if err != nil { + return response.Error(http.StatusBadRequest, "orgId is invalid", err) + } + + perPage := c.QueryInt("perpage") + if perPage <= 0 { + perPage = 1000 + } + page := c.QueryInt("page") + + if page < 1 { + page = 1 + } + + result, err := hs.searchOrgUsersHelper(c, &org.SearchOrgUsersQuery{ + OrgID: orgID, + Query: c.Query("query"), + Page: page, + Limit: perPage, + User: c.SignedInUser, + }) if err != nil { return response.Error(500, "Failed to get users for organization", err) @@ -204,17 +250,46 @@ func (hs *HTTPServer) GetOrgUsers(c *models.ReqContext) response.Response { return response.JSON(http.StatusOK, result) } -func (hs *HTTPServer) getOrgUsersHelper(c *models.ReqContext, query *org.GetOrgUsersQuery, signedInUser *user.SignedInUser) ([]*org.OrgUserDTO, error) { - result, err := hs.orgService.GetOrgUsers(c.Req.Context(), query) +// SearchOrgUsersWithPaging is an HTTP handler to search for org users with paging. +// GET /api/org/users/search +func (hs *HTTPServer) SearchOrgUsersWithPaging(c *models.ReqContext) response.Response { + perPage := c.QueryInt("perpage") + if perPage <= 0 { + perPage = 1000 + } + page := c.QueryInt("page") + + if page < 1 { + page = 1 + } + + query := &org.SearchOrgUsersQuery{ + OrgID: c.OrgID, + Query: c.Query("query"), + Page: page, + Limit: perPage, + User: c.SignedInUser, + } + + result, err := hs.searchOrgUsersHelper(c, query) + if err != nil { + return response.Error(500, "Failed to get users for current organization", err) + } + + return response.JSON(http.StatusOK, result) +} + +func (hs *HTTPServer) searchOrgUsersHelper(c *models.ReqContext, query *org.SearchOrgUsersQuery) (*org.SearchOrgUsersQueryResult, error) { + result, err := hs.orgService.SearchOrgUsers(c.Req.Context(), query) if err != nil { return nil, err } - filteredUsers := make([]*org.OrgUserDTO, 0, len(result)) + filteredUsers := make([]*org.OrgUserDTO, 0, len(result.OrgUsers)) userIDs := map[string]bool{} - authLabelsUserIDs := make([]int64, 0, len(result)) - for _, user := range result { - if dtos.IsHiddenUser(user.Login, signedInUser, hs.Cfg) { + authLabelsUserIDs := make([]int64, 0, len(result.OrgUsers)) + for _, user := range result.OrgUsers { + if dtos.IsHiddenUser(user.Login, c.SignedInUser, hs.Cfg) { continue } user.AvatarURL = dtos.GetGravatarUrl(user.Email) @@ -241,51 +316,10 @@ func (hs *HTTPServer) getOrgUsersHelper(c *models.ReqContext, query *org.GetOrgU } } - return filteredUsers, nil -} - -// SearchOrgUsersWithPaging is an HTTP handler to search for org users with paging. -// GET /api/org/users/search -func (hs *HTTPServer) SearchOrgUsersWithPaging(c *models.ReqContext) response.Response { - ctx := c.Req.Context() - perPage := c.QueryInt("perpage") - if perPage <= 0 { - perPage = 1000 - } - page := c.QueryInt("page") - - if page < 1 { - page = 1 - } - - query := &org.SearchOrgUsersQuery{ - OrgID: c.OrgID, - Query: c.Query("query"), - Page: page, - Limit: perPage, - User: c.SignedInUser, - } - - result, err := hs.orgService.SearchOrgUsers(ctx, query) - if err != nil { - return response.Error(500, "Failed to get users for current organization", err) - } - - filteredUsers := make([]*org.OrgUserDTO, 0, len(result.OrgUsers)) - for _, user := range result.OrgUsers { - if dtos.IsHiddenUser(user.Login, c.SignedInUser, hs.Cfg) { - continue - } - user.AvatarURL = dtos.GetGravatarUrl(user.Email) - - filteredUsers = append(filteredUsers, user) - } - result.OrgUsers = filteredUsers - result.Page = page - result.PerPage = perPage - - return response.JSON(http.StatusOK, result) + result.Page = query.Page + result.PerPage = query.Limit + return result, nil } // swagger:route PATCH /org/users/{user_id} org updateOrgUserForCurrentOrg diff --git a/pkg/api/org_users_test.go b/pkg/api/org_users_test.go index b86adfc5b29..a035242e3bf 100644 --- a/pkg/api/org_users_test.go +++ b/pkg/api/org_users_test.go @@ -63,12 +63,15 @@ func TestOrgUsersAPIEndpoint_userLoggedIn(t *testing.T) { orgService.ExpectedSearchOrgUsersResult = &org.SearchOrgUsersQueryResult{} hs.orgService = orgService mock := mockstore.NewSQLStoreMock() + loggedInUserScenario(t, "When calling GET on", "api/org/users", "api/org/users", func(sc *scenarioContext) { setUpGetOrgUsersDB(t, sqlStore) - orgService.ExpectedOrgUsers = []*org.OrgUserDTO{ - {Login: testUserLogin, Email: "testUser@grafana.com"}, - {Login: "user1", Email: "user1@grafana.com"}, - {Login: "user2", Email: "user2@grafana.com"}, + orgService.ExpectedSearchOrgUsersResult = &org.SearchOrgUsersQueryResult{ + OrgUsers: []*org.OrgUserDTO{ + {Login: testUserLogin, Email: "testUser@grafana.com"}, + {Login: "user1", Email: "user1@grafana.com"}, + {Login: "user2", Email: "user2@grafana.com"}, + }, } sc.handlerFunc = hs.GetOrgUsersForCurrentOrg sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() @@ -153,6 +156,13 @@ func TestOrgUsersAPIEndpoint_userLoggedIn(t *testing.T) { loggedInUserScenario(t, "When calling GET on", "api/org/users", "api/org/users", func(sc *scenarioContext) { setUpGetOrgUsersDB(t, sqlStore) + orgService.ExpectedSearchOrgUsersResult = &org.SearchOrgUsersQueryResult{ + OrgUsers: []*org.OrgUserDTO{ + {Login: testUserLogin, Email: "testUser@grafana.com"}, + {Login: "user1", Email: "user1@grafana.com"}, + {Login: "user2", Email: "user2@grafana.com"}, + }, + } sc.handlerFunc = hs.GetOrgUsersForCurrentOrg sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() diff --git a/pkg/services/org/model.go b/pkg/services/org/model.go index f5ab6c8f9b7..d000d1a54b7 100644 --- a/pkg/services/org/model.go +++ b/pkg/services/org/model.go @@ -162,6 +162,7 @@ type GetOrgUsersQuery struct { UserID int64 `xorm:"user_id"` OrgID int64 `xorm:"org_id"` Query string + Page int Limit int // Flag used to allow oss edition to query users without access control DontEnforceAccessControl bool @@ -170,17 +171,20 @@ type GetOrgUsersQuery struct { } type SearchOrgUsersQuery struct { - OrgID int64 `xorm:"org_id"` - Query string - Page int - Limit int + UserID int64 `xorm:"user_id"` + OrgID int64 `xorm:"org_id"` + Query string + Page int + Limit int + // Flag used to allow oss edition to query users without access control + DontEnforceAccessControl bool User *user.SignedInUser } type SearchOrgUsersQueryResult struct { TotalCount int64 `json:"totalCount"` - OrgUsers []*OrgUserDTO `json:"OrgUsers"` + OrgUsers []*OrgUserDTO `json:"orgUsers"` Page int `json:"page"` PerPage int `json:"perPage"` } diff --git a/pkg/services/org/orgimpl/org.go b/pkg/services/org/orgimpl/org.go index 9e89ea7cd5e..05a296c1b24 100644 --- a/pkg/services/org/orgimpl/org.go +++ b/pkg/services/org/orgimpl/org.go @@ -194,7 +194,19 @@ func (s *Service) RemoveOrgUser(ctx context.Context, cmd *org.RemoveOrgUserComma // TODO: refactor service to call store CRUD method func (s *Service) GetOrgUsers(ctx context.Context, query *org.GetOrgUsersQuery) ([]*org.OrgUserDTO, error) { - return s.store.GetOrgUsers(ctx, query) + result, err := s.store.SearchOrgUsers(ctx, &org.SearchOrgUsersQuery{ + UserID: query.UserID, + OrgID: query.OrgID, + Query: query.Query, + Page: query.Page, + Limit: query.Limit, + DontEnforceAccessControl: query.DontEnforceAccessControl, + User: query.User, + }) + if err != nil { + return nil, err + } + return result.OrgUsers, nil } // TODO: refactor service to call store CRUD method diff --git a/pkg/services/org/orgimpl/store.go b/pkg/services/org/orgimpl/store.go index 29af8a66ae0..9e3a775d3e7 100644 --- a/pkg/services/org/orgimpl/store.go +++ b/pkg/services/org/orgimpl/store.go @@ -39,7 +39,6 @@ type store interface { CreateWithMember(context.Context, *org.CreateOrgCommand) (*org.Org, error) AddOrgUser(context.Context, *org.AddOrgUserCommand) error UpdateOrgUser(context.Context, *org.UpdateOrgUserCommand) error - GetOrgUsers(context.Context, *org.GetOrgUsersQuery) ([]*org.OrgUserDTO, error) GetByID(context.Context, *org.GetOrgByIDQuery) (*org.Org, error) GetByName(context.Context, *org.GetOrgByNameQuery) (*org.Org, error) SearchOrgUsers(context.Context, *org.SearchOrgUsersQuery) (*org.SearchOrgUsersQueryResult, error) @@ -512,8 +511,29 @@ func validateOneAdminLeftInOrg(orgID int64, sess *db.Session) error { return err } -func (ss *sqlStore) GetOrgUsers(ctx context.Context, query *org.GetOrgUsersQuery) ([]*org.OrgUserDTO, error) { - result := make([]*org.OrgUserDTO, 0) +func (ss *sqlStore) GetByID(ctx context.Context, query *org.GetOrgByIDQuery) (*org.Org, error) { + var orga org.Org + err := ss.db.WithDbSession(ctx, func(dbSession *db.Session) error { + exists, err := dbSession.ID(query.ID).Get(&orga) + if err != nil { + return err + } + + if !exists { + return org.ErrOrgNotFound + } + return nil + }) + if err != nil { + return nil, err + } + return &orga, nil +} + +func (ss *sqlStore) SearchOrgUsers(ctx context.Context, query *org.SearchOrgUsersQuery) (*org.SearchOrgUsersQueryResult, error) { + result := org.SearchOrgUsersQueryResult{ + OrgUsers: make([]*org.OrgUserDTO, 0), + } err := ss.db.WithDbSession(ctx, func(dbSession *db.Session) error { sess := dbSession.Table("org_user") sess.Join("INNER", ss.dialect.Quote("user"), fmt.Sprintf("org_user.user_id=%s.id", ss.dialect.Quote("user"))) @@ -556,7 +576,8 @@ func (ss *sqlStore) GetOrgUsers(ctx context.Context, query *org.GetOrgUsersQuery } if query.Limit > 0 { - sess.Limit(query.Limit, 0) + offset := query.Limit * (query.Page - 1) + sess.Limit(query.Limit, offset) } sess.Cols( @@ -573,92 +594,6 @@ func (ss *sqlStore) GetOrgUsers(ctx context.Context, query *org.GetOrgUsersQuery ) sess.Asc("user.email", "user.login") - if err := sess.Find(&result); err != nil { - return err - } - - for _, user := range result { - user.LastSeenAtAge = util.GetAgeString(user.LastSeenAt) - } - - return nil - }) - if err != nil { - return nil, err - } - return result, nil -} - -func (ss *sqlStore) GetByID(ctx context.Context, query *org.GetOrgByIDQuery) (*org.Org, error) { - var orga org.Org - err := ss.db.WithDbSession(ctx, func(dbSession *db.Session) error { - exists, err := dbSession.ID(query.ID).Get(&orga) - if err != nil { - return err - } - - if !exists { - return org.ErrOrgNotFound - } - return nil - }) - if err != nil { - return nil, err - } - return &orga, nil -} - -func (ss *sqlStore) SearchOrgUsers(ctx context.Context, query *org.SearchOrgUsersQuery) (*org.SearchOrgUsersQueryResult, error) { - result := org.SearchOrgUsersQueryResult{ - OrgUsers: make([]*org.OrgUserDTO, 0), - } - err := ss.db.WithDbSession(ctx, func(dbSession *db.Session) error { - sess := dbSession.Table("org_user") - sess.Join("INNER", ss.dialect.Quote("user"), fmt.Sprintf("org_user.user_id=%s.id", ss.dialect.Quote("user"))) - - whereConditions := make([]string, 0) - whereParams := make([]interface{}, 0) - - whereConditions = append(whereConditions, "org_user.org_id = ?") - whereParams = append(whereParams, query.OrgID) - - whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = %s", ss.dialect.Quote("user"), ss.dialect.BooleanStr(false))) - - if !accesscontrol.IsDisabled(ss.cfg) { - acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "users:id:", accesscontrol.ActionOrgUsersRead) - if err != nil { - return err - } - whereConditions = append(whereConditions, acFilter.Where) - whereParams = append(whereParams, acFilter.Args...) - } - - if query.Query != "" { - queryWithWildcards := "%" + query.Query + "%" - whereConditions = append(whereConditions, "(email "+ss.dialect.LikeStr()+" ? OR name "+ss.dialect.LikeStr()+" ? OR login "+ss.dialect.LikeStr()+" ?)") - whereParams = append(whereParams, queryWithWildcards, queryWithWildcards, queryWithWildcards) - } - - if len(whereConditions) > 0 { - sess.Where(strings.Join(whereConditions, " AND "), whereParams...) - } - - if query.Limit > 0 { - offset := query.Limit * (query.Page - 1) - sess.Limit(query.Limit, offset) - } - - sess.Cols( - "org_user.org_id", - "org_user.user_id", - "user.email", - "user.name", - "user.login", - "org_user.role", - "user.last_seen_at", - ) - sess.Asc("user.email", "user.login") - if err := sess.Find(&result.OrgUsers); err != nil { return err } diff --git a/pkg/services/org/orgimpl/store_test.go b/pkg/services/org/orgimpl/store_test.go index 3859d8f84b6..32239f60a83 100644 --- a/pkg/services/org/orgimpl/store_test.go +++ b/pkg/services/org/orgimpl/store_test.go @@ -327,35 +327,35 @@ func TestIntegrationOrgUserDataAccess(t *testing.T) { err = orgUserStore.UpdateOrgUser(context.Background(), &updateCmd) require.NoError(t, err) - orgUsersQuery := org.GetOrgUsersQuery{ + orgUsersQuery := org.SearchOrgUsersQuery{ OrgID: ac1.OrgID, User: &user.SignedInUser{ OrgID: ac1.OrgID, Permissions: map[int64]map[string][]string{ac1.OrgID: {accesscontrol.ActionOrgUsersRead: {accesscontrol.ScopeUsersAll}}}, }, } - result, err := orgUserStore.GetOrgUsers(context.Background(), &orgUsersQuery) + result, err := orgUserStore.SearchOrgUsers(context.Background(), &orgUsersQuery) require.NoError(t, err) - require.EqualValues(t, result[1].Role, org.RoleAdmin) + require.EqualValues(t, result.OrgUsers[1].Role, org.RoleAdmin) }) t.Run("Can get organization users", func(t *testing.T) { - query := org.GetOrgUsersQuery{ + query := org.SearchOrgUsersQuery{ OrgID: ac1.OrgID, User: &user.SignedInUser{ OrgID: ac1.OrgID, Permissions: map[int64]map[string][]string{ac1.OrgID: {accesscontrol.ActionOrgUsersRead: {accesscontrol.ScopeUsersAll}}}, }, } - result, err := orgUserStore.GetOrgUsers(context.Background(), &query) + result, err := orgUserStore.SearchOrgUsers(context.Background(), &query) require.NoError(t, err) - require.Equal(t, len(result), 2) - require.Equal(t, result[0].Role, "Admin") + require.Equal(t, len(result.OrgUsers), 2) + require.Equal(t, result.OrgUsers[0].Role, "Admin") }) t.Run("Can get organization users with query", func(t *testing.T) { - query := org.GetOrgUsersQuery{ + query := org.SearchOrgUsersQuery{ OrgID: ac1.OrgID, Query: "ac1", User: &user.SignedInUser{ @@ -363,14 +363,14 @@ func TestIntegrationOrgUserDataAccess(t *testing.T) { Permissions: map[int64]map[string][]string{ac1.OrgID: {accesscontrol.ActionOrgUsersRead: {accesscontrol.ScopeUsersAll}}}, }, } - result, err := orgUserStore.GetOrgUsers(context.Background(), &query) + result, err := orgUserStore.SearchOrgUsers(context.Background(), &query) require.NoError(t, err) - require.Equal(t, len(result), 1) - require.Equal(t, result[0].Email, ac1.Email) + require.Equal(t, len(result.OrgUsers), 1) + require.Equal(t, result.OrgUsers[0].Email, ac1.Email) }) t.Run("Can get organization users with query and limit", func(t *testing.T) { - query := org.GetOrgUsersQuery{ + query := org.SearchOrgUsersQuery{ OrgID: ac1.OrgID, Query: "ac", Limit: 1, @@ -379,11 +379,11 @@ func TestIntegrationOrgUserDataAccess(t *testing.T) { Permissions: map[int64]map[string][]string{ac1.OrgID: {accesscontrol.ActionOrgUsersRead: {accesscontrol.ScopeUsersAll}}}, }, } - result, err := orgUserStore.GetOrgUsers(context.Background(), &query) + result, err := orgUserStore.SearchOrgUsers(context.Background(), &query) require.NoError(t, err) - require.Equal(t, len(result), 1) - require.Equal(t, result[0].Email, ac1.Email) + require.Equal(t, len(result.OrgUsers), 1) + require.Equal(t, result.OrgUsers[0].Email, ac1.Email) }) t.Run("Cannot update role so no one is admin user", func(t *testing.T) { remCmd := org.RemoveOrgUserCommand{OrgID: ac1.OrgID, UserID: ac2.ID, ShouldDeleteOrphanedUser: true} @@ -532,12 +532,12 @@ func TestIntegration_SQLStore_GetOrgUsers(t *testing.T) { } tests := []struct { desc string - query *org.GetOrgUsersQuery + query *org.SearchOrgUsersQuery expectedNumUsers int }{ { desc: "should return all users", - query: &org.GetOrgUsersQuery{ + query: &org.SearchOrgUsersQuery{ OrgID: 1, User: &user.SignedInUser{ OrgID: 1, @@ -548,7 +548,7 @@ func TestIntegration_SQLStore_GetOrgUsers(t *testing.T) { }, { desc: "should return no users", - query: &org.GetOrgUsersQuery{ + query: &org.SearchOrgUsersQuery{ OrgID: 1, User: &user.SignedInUser{ OrgID: 1, @@ -559,7 +559,7 @@ func TestIntegration_SQLStore_GetOrgUsers(t *testing.T) { }, { desc: "should return some users", - query: &org.GetOrgUsersQuery{ + query: &org.SearchOrgUsersQuery{ OrgID: 1, User: &user.SignedInUser{ OrgID: 1, @@ -589,12 +589,12 @@ func TestIntegration_SQLStore_GetOrgUsers(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - result, err := orgUserStore.GetOrgUsers(context.Background(), tt.query) + result, err := orgUserStore.SearchOrgUsers(context.Background(), tt.query) require.NoError(t, err) - require.Len(t, result, tt.expectedNumUsers) + require.Len(t, result.OrgUsers, tt.expectedNumUsers) if !hasWildcardScope(tt.query.User, accesscontrol.ActionOrgUsersRead) { - for _, u := range result { + for _, u := range result.OrgUsers { assert.Contains(t, tt.query.User.Permissions[tt.query.User.OrgID][accesscontrol.ActionOrgUsersRead], fmt.Sprintf("users:id:%d", u.UserID)) } } @@ -675,7 +675,7 @@ func TestIntegration_SQLStore_GetOrgUsers_PopulatesCorrectly(t *testing.T) { }) require.NoError(t, err) - query := &org.GetOrgUsersQuery{ + query := &org.SearchOrgUsersQuery{ OrgID: 1, UserID: newUser.ID, User: &user.SignedInUser{ @@ -683,11 +683,11 @@ func TestIntegration_SQLStore_GetOrgUsers_PopulatesCorrectly(t *testing.T) { Permissions: map[int64]map[string][]string{1: {accesscontrol.ActionOrgUsersRead: {accesscontrol.ScopeUsersAll}}}, }, } - result, err := orgUserStore.GetOrgUsers(context.Background(), query) + result, err := orgUserStore.SearchOrgUsers(context.Background(), query) require.NoError(t, err) - require.Len(t, result, 1) + require.Len(t, result.OrgUsers, 1) - actual := result[0] + actual := result.OrgUsers[0] assert.Equal(t, int64(1), actual.OrgID) assert.Equal(t, int64(1), actual.UserID) assert.Equal(t, "viewer@localhost", actual.Email) diff --git a/pkg/services/provisioning/plugins/mocks/Store.go b/pkg/services/provisioning/plugins/mocks/Store.go index 4247e875bc3..698da31a9b3 100644 --- a/pkg/services/provisioning/plugins/mocks/Store.go +++ b/pkg/services/provisioning/plugins/mocks/Store.go @@ -6,8 +6,8 @@ import ( context "context" models "github.com/grafana/grafana/pkg/models" - mock "github.com/stretchr/testify/mock" "github.com/grafana/grafana/pkg/services/org" + mock "github.com/stretchr/testify/mock" ) // Store is an autogenerated mock type for the Store type @@ -16,7 +16,7 @@ type Store struct { } // GetOrgByNameHandler provides a mock function with given fields: ctx, query -func (_m *Store) GetOrgByNameHandler(ctx context.Context, query * org.GetOrgByNameQuery) error { +func (_m *Store) GetOrgByNameHandler(ctx context.Context, query *org.GetOrgByNameQuery) error { ret := _m.Called(ctx, query) var r0 error diff --git a/public/app/features/admin/AdminEditOrgPage.tsx b/public/app/features/admin/AdminEditOrgPage.tsx index 79e56287ec8..e4359481f87 100644 --- a/public/app/features/admin/AdminEditOrgPage.tsx +++ b/public/app/features/admin/AdminEditOrgPage.tsx @@ -1,69 +1,90 @@ -import { css } from '@emotion/css'; import React, { useState, useEffect } from 'react'; import { useAsyncFn } from 'react-use'; import { NavModelItem, UrlQueryValue } from '@grafana/data'; import { getBackendSrv } from '@grafana/runtime'; -import { Form, Field, Input, Button, Legend, Alert } from '@grafana/ui'; +import { Form, Field, Input, Button, Legend, Alert, VerticalGroup, HorizontalGroup, Pagination } from '@grafana/ui'; import { Page } from 'app/core/components/Page/Page'; import { contextSrv } from 'app/core/core'; import { GrafanaRouteComponentProps } from 'app/core/navigation/types'; import { accessControlQueryParam } from 'app/core/utils/accessControl'; -import { OrgUser, AccessControlAction } from 'app/types'; +import { OrgUser, AccessControlAction, OrgRole } from 'app/types'; -import UsersTable from '../users/UsersTable'; +import { UsersTable } from '../users/UsersTable'; + +const perPage = 30; interface OrgNameDTO { orgName: string; } const getOrg = async (orgId: UrlQueryValue) => { - return await getBackendSrv().get('/api/orgs/' + orgId); + return await getBackendSrv().get(`/api/orgs/${orgId}`); }; -const getOrgUsers = async (orgId: UrlQueryValue) => { +const getOrgUsers = async (orgId: UrlQueryValue, page: number) => { if (contextSrv.hasPermission(AccessControlAction.OrgUsersRead)) { - return await getBackendSrv().get(`/api/orgs/${orgId}/users`, accessControlQueryParam()); + return getBackendSrv().get(`/api/orgs/${orgId}/users/search`, accessControlQueryParam({ perpage: perPage, page })); } - return []; + return { orgUsers: [] }; }; -const updateOrgUserRole = async (orgUser: OrgUser, orgId: UrlQueryValue) => { - await getBackendSrv().patch('/api/orgs/' + orgId + '/users/' + orgUser.userId, orgUser); +const updateOrgUserRole = (orgUser: OrgUser, orgId: UrlQueryValue) => { + return getBackendSrv().patch(`/api/orgs/${orgId}/users/${orgUser.userId}`, orgUser); }; -const removeOrgUser = async (orgUser: OrgUser, orgId: UrlQueryValue) => { - return await getBackendSrv().delete('/api/orgs/' + orgId + '/users/' + orgUser.userId); +const removeOrgUser = (orgUser: OrgUser, orgId: UrlQueryValue) => { + return getBackendSrv().delete(`/api/orgs/${orgId}/users/${orgUser.userId}`); }; interface Props extends GrafanaRouteComponentProps<{ id: string }> {} -export default function AdminEditOrgPage({ match }: Props) { +const AdminEditOrgPage = ({ match }: Props) => { const orgId = parseInt(match.params.id, 10); const canWriteOrg = contextSrv.hasPermission(AccessControlAction.OrgsWrite); const canReadUsers = contextSrv.hasPermission(AccessControlAction.OrgUsersRead); const [users, setUsers] = useState([]); + const [page, setPage] = useState(1); + const [totalPages, setTotalPages] = useState(1); const [orgState, fetchOrg] = useAsyncFn(() => getOrg(orgId), []); - const [, fetchOrgUsers] = useAsyncFn(() => getOrgUsers(orgId), []); + const [, fetchOrgUsers] = useAsyncFn(async (page) => { + const result = await getOrgUsers(orgId, page); + const totalPages = result?.perPage !== 0 ? Math.ceil(result.totalCount / result.perPage) : 0; + setTotalPages(totalPages); + setUsers(result.orgUsers); + return result.orgUsers; + }, []); useEffect(() => { fetchOrg(); - fetchOrgUsers().then((res) => setUsers(res)); - }, [fetchOrg, fetchOrgUsers]); + fetchOrgUsers(page); + }, [fetchOrg, fetchOrgUsers, page]); const updateOrgName = async (name: string) => { - return await getBackendSrv().put('/api/orgs/' + orgId, { ...orgState.value, name }); + return await getBackendSrv().put(`/api/orgs/${orgId}`, { ...orgState.value, name }); }; - const renderMissingUserListRightsMessage = () => { - return ( - - You do not have permission to see users in this organization. To update this organization, contact your server - administrator. - - ); + const renderMissingPermissionMessage = () => ( + + You do not have permission to see users in this organization. To update this organization, contact your server + administrator. + + ); + + const onPageChange = (toPage: number) => { + setPage(toPage); + }; + + const onRemoveUser = async (orgUser: OrgUser) => { + await removeOrgUser(orgUser, orgId); + fetchOrgUsers(page); + }; + + const onRoleChange = async (role: OrgRole, orgUser: OrgUser) => { + await updateOrgUserRole({ ...orgUser, role }, orgId); + fetchOrgUsers(page); }; const pageNav: NavModelItem = { @@ -81,7 +102,7 @@ export default function AdminEditOrgPage({ match }: Props) { {orgState.value && (
await updateOrgName(values.orgName)} + onSubmit={(values: OrgNameDTO) => updateOrgName(values.orgName)} > {({ register, errors }) => ( <> @@ -96,39 +117,27 @@ export default function AdminEditOrgPage({ match }: Props) {
)} -
+
Organization users - {!canReadUsers && renderMissingUserListRightsMessage()} + {!canReadUsers && renderMissingPermissionMessage()} {canReadUsers && !!users.length && ( - { - updateOrgUserRole({ ...orgUser, role }, orgId); - setUsers( - users.map((user) => { - if (orgUser.userId === user.userId) { - return { ...orgUser, role }; - } - return user; - }) - ); - fetchOrgUsers(); - }} - onRemoveUser={(orgUser) => { - removeOrgUser(orgUser, orgId); - setUsers(users.filter((user) => orgUser.userId !== user.userId)); - fetchOrgUsers(); - }} - /> + + + + + + )}
); -} +}; + +export default AdminEditOrgPage; diff --git a/public/app/features/users/UsersActionBar.test.tsx b/public/app/features/users/UsersActionBar.test.tsx index 1e8d2d44a73..d366e61e155 100644 --- a/public/app/features/users/UsersActionBar.test.tsx +++ b/public/app/features/users/UsersActionBar.test.tsx @@ -2,8 +2,8 @@ import { render, screen } from '@testing-library/react'; import React from 'react'; import { mockToolkitActionCreator } from 'test/core/redux/mocks'; -import { Props, UsersActionBar } from './UsersActionBar'; -import { setUsersSearchQuery } from './state/reducers'; +import { Props, UsersActionBarUnconnected } from './UsersActionBar'; +import { searchQueryChanged } from './state/reducers'; jest.mock('app/core/core', () => ({ contextSrv: { @@ -15,7 +15,7 @@ jest.mock('app/core/core', () => ({ const setup = (propOverrides?: object) => { const props: Props = { searchQuery: '', - setUsersSearchQuery: mockToolkitActionCreator(setUsersSearchQuery), + changeSearchQuery: mockToolkitActionCreator(searchQueryChanged), onShowInvites: jest.fn(), pendingInvitesCount: 0, canInvite: false, @@ -26,7 +26,7 @@ const setup = (propOverrides?: object) => { Object.assign(props, propOverrides); - const { rerender } = render(); + const { rerender } = render(); return { rerender, props }; }; diff --git a/public/app/features/users/UsersActionBar.tsx b/public/app/features/users/UsersActionBar.tsx index b9dc2906313..24bd045ad7d 100644 --- a/public/app/features/users/UsersActionBar.tsx +++ b/public/app/features/users/UsersActionBar.tsx @@ -1,5 +1,5 @@ -import React, { PureComponent } from 'react'; -import { connect } from 'react-redux'; +import React from 'react'; +import { connect, ConnectedProps } from 'react-redux'; import { RadioButtonGroup, LinkButton, FilterInput } from '@grafana/ui'; import { contextSrv } from 'app/core/core'; @@ -7,61 +7,12 @@ import { AccessControlAction, StoreState } from 'app/types'; import { selectTotal } from '../invites/state/selectors'; -import { setUsersSearchQuery } from './state/reducers'; +import { changeSearchQuery } from './state/actions'; import { getUsersSearchQuery } from './state/selectors'; -export interface Props { - searchQuery: string; - setUsersSearchQuery: typeof setUsersSearchQuery; - onShowInvites: () => void; - pendingInvitesCount: number; - canInvite: boolean; +export interface OwnProps { showInvites: boolean; - externalUserMngLinkUrl: string; - externalUserMngLinkName: string; -} - -export class UsersActionBar extends PureComponent { - render() { - const { - canInvite, - externalUserMngLinkName, - externalUserMngLinkUrl, - searchQuery, - pendingInvitesCount, - setUsersSearchQuery, - onShowInvites, - showInvites, - } = this.props; - const options = [ - { label: 'Users', value: 'users' }, - { label: `Pending Invites (${pendingInvitesCount})`, value: 'invites' }, - ]; - const canAddToOrg: boolean = contextSrv.hasAccess(AccessControlAction.OrgUsersAdd, canInvite); - - return ( -
-
- -
- {pendingInvitesCount > 0 && ( -
- -
- )} - {canAddToOrg && Invite} - {externalUserMngLinkUrl && ( - - {externalUserMngLinkName} - - )} -
- ); - } + onShowInvites: () => void; } function mapStateToProps(state: StoreState) { @@ -75,7 +26,51 @@ function mapStateToProps(state: StoreState) { } const mapDispatchToProps = { - setUsersSearchQuery, + changeSearchQuery, }; -export default connect(mapStateToProps, mapDispatchToProps)(UsersActionBar); +const connector = connect(mapStateToProps, mapDispatchToProps); + +export type Props = ConnectedProps & OwnProps; + +export const UsersActionBarUnconnected = ({ + canInvite, + externalUserMngLinkName, + externalUserMngLinkUrl, + searchQuery, + pendingInvitesCount, + changeSearchQuery, + onShowInvites, + showInvites, +}: Props): JSX.Element => { + const options = [ + { label: 'Users', value: 'users' }, + { label: `Pending Invites (${pendingInvitesCount})`, value: 'invites' }, + ]; + const canAddToOrg: boolean = contextSrv.hasAccess(AccessControlAction.OrgUsersAdd, canInvite); + + return ( +
+
+ +
+ {pendingInvitesCount > 0 && ( +
+ +
+ )} + {canAddToOrg && Invite} + {externalUserMngLinkUrl && ( + + {externalUserMngLinkName} + + )} +
+ ); +}; + +export const UsersActionBar = connector(UsersActionBarUnconnected); diff --git a/public/app/features/users/UsersListPage.test.tsx b/public/app/features/users/UsersListPage.test.tsx index f4e8c2d3b8d..57a50a0ac0a 100644 --- a/public/app/features/users/UsersListPage.test.tsx +++ b/public/app/features/users/UsersListPage.test.tsx @@ -7,7 +7,7 @@ import { configureStore } from 'app/store/configureStore'; import { Invitee, OrgUser } from 'app/types'; import { Props, UsersListPageUnconnected } from './UsersListPage'; -import { setUsersSearchPage, setUsersSearchQuery } from './state/reducers'; +import { pageChanged } from './state/reducers'; jest.mock('../../core/app_events', () => ({ emit: jest.fn(), @@ -27,15 +27,16 @@ const setup = (propOverrides?: object) => { users: [] as OrgUser[], invitees: [] as Invitee[], searchQuery: '', - searchPage: 1, + page: 1, + totalPages: 1, + perPage: 30, externalUserMngInfo: '', fetchInvitees: jest.fn(), loadUsers: jest.fn(), updateUser: jest.fn(), removeUser: jest.fn(), - setUsersSearchQuery: mockToolkitActionCreator(setUsersSearchQuery), - setUsersSearchPage: mockToolkitActionCreator(setUsersSearchPage), - hasFetched: false, + changePage: mockToolkitActionCreator(pageChanged), + isLoading: false, }; Object.assign(props, propOverrides); diff --git a/public/app/features/users/UsersListPage.tsx b/public/app/features/users/UsersListPage.tsx index c30b74326d8..b8c4723ed18 100644 --- a/public/app/features/users/UsersListPage.tsx +++ b/public/app/features/users/UsersListPage.tsx @@ -1,4 +1,4 @@ -import React, { PureComponent } from 'react'; +import React, { useEffect, useState } from 'react'; import { connect, ConnectedProps } from 'react-redux'; import { renderMarkdown } from '@grafana/data'; @@ -11,29 +11,29 @@ import InviteesTable from '../invites/InviteesTable'; import { fetchInvitees } from '../invites/state/actions'; import { selectInvitesMatchingQuery } from '../invites/state/selectors'; -import UsersActionBar from './UsersActionBar'; -import UsersTable from './UsersTable'; -import { loadUsers, removeUser, updateUser } from './state/actions'; -import { setUsersSearchQuery, setUsersSearchPage } from './state/reducers'; -import { getUsers, getUsersSearchQuery, getUsersSearchPage } from './state/selectors'; +import { UsersActionBar } from './UsersActionBar'; +import { UsersTable } from './UsersTable'; +import { loadUsers, removeUser, updateUser, changePage } from './state/actions'; +import { getUsers, getUsersSearchQuery } from './state/selectors'; function mapStateToProps(state: StoreState) { const searchQuery = getUsersSearchQuery(state.users); return { users: getUsers(state.users), searchQuery: getUsersSearchQuery(state.users), - searchPage: getUsersSearchPage(state.users), + page: state.users.page, + totalPages: state.users.totalPages, + perPage: state.users.perPage, invitees: selectInvitesMatchingQuery(state.invites, searchQuery), externalUserMngInfo: state.users.externalUserMngInfo, - hasFetched: state.users.hasFetched, + isLoading: state.users.isLoading, }; } const mapDispatchToProps = { loadUsers, fetchInvitees, - setUsersSearchQuery, - setUsersSearchPage, + changePage, updateUser, removeUser, }; @@ -46,73 +46,51 @@ export interface State { showInvites: boolean; } -const pageLimit = 30; +export const UsersListPageUnconnected = ({ + users, + page, + totalPages, + invitees, + externalUserMngInfo, + isLoading, + loadUsers, + fetchInvitees, + changePage, + updateUser, + removeUser, +}: Props): JSX.Element => { + const [showInvites, setShowInvites] = useState(false); + const externalUserMngInfoHtml = externalUserMngInfo ? renderMarkdown(externalUserMngInfo) : ''; -export class UsersListPageUnconnected extends PureComponent { - declare externalUserMngInfoHtml: string; + useEffect(() => { + loadUsers(); + fetchInvitees(); + }, [fetchInvitees, loadUsers]); - constructor(props: Props) { - super(props); - - if (this.props.externalUserMngInfo) { - this.externalUserMngInfoHtml = renderMarkdown(this.props.externalUserMngInfo); - } - - this.state = { - showInvites: false, - }; - } - - componentDidMount() { - this.fetchUsers(); - this.fetchInvitees(); - } - - async fetchUsers() { - return await this.props.loadUsers(); - } - - async fetchInvitees() { - return await this.props.fetchInvitees(); - } - - onRoleChange = (role: OrgRole, user: OrgUser) => { - const updatedUser = { ...user, role: role }; - - this.props.updateUser(updatedUser); + const onRoleChange = (role: OrgRole, user: OrgUser) => { + updateUser({ ...user, role: role }); }; - onShowInvites = () => { - this.setState((prevState) => ({ - showInvites: !prevState.showInvites, - })); + const onShowInvites = () => { + setShowInvites(!showInvites); }; - getPaginatedUsers = (users: OrgUser[]) => { - const offset = (this.props.searchPage - 1) * pageLimit; - return users.slice(offset, offset + pageLimit); - }; - - renderTable() { - const { invitees, users, setUsersSearchPage } = this.props; - const paginatedUsers = this.getPaginatedUsers(users); - const totalPages = Math.ceil(users.length / pageLimit); - - if (this.state.showInvites) { + const renderTable = () => { + if (showInvites) { return ; } else { return ( this.onRoleChange(role, user)} - onRemoveUser={(user) => this.props.removeUser(user.userId)} + onRoleChange={(role, user) => onRoleChange(role, user)} + onRemoveUser={(user) => removeUser(user.userId)} /> @@ -120,23 +98,18 @@ export class UsersListPageUnconnected extends PureComponent { ); } - } + }; - render() { - const { hasFetched } = this.props; - const externalUserMngInfoHtml = this.externalUserMngInfoHtml; - - return ( - - - {externalUserMngInfoHtml && ( -
- )} - {hasFetched && this.renderTable()} - - ); - } -} + return ( + + + {externalUserMngInfoHtml && ( +
+ )} + {isLoading && renderTable()} + + ); +}; export const UsersListPageContent = connector(UsersListPageUnconnected); diff --git a/public/app/features/users/UsersTable.test.tsx b/public/app/features/users/UsersTable.test.tsx index d7ef2b4181d..0db1c7b1511 100644 --- a/public/app/features/users/UsersTable.test.tsx +++ b/public/app/features/users/UsersTable.test.tsx @@ -4,7 +4,7 @@ import React from 'react'; import { OrgUser } from 'app/types'; -import UsersTable, { Props } from './UsersTable'; +import { UsersTable, Props } from './UsersTable'; import { getMockUsers } from './__mocks__/userMocks'; jest.mock('app/core/core', () => ({ diff --git a/public/app/features/users/UsersTable.tsx b/public/app/features/users/UsersTable.tsx index fa437a41145..ac8f6c7c917 100644 --- a/public/app/features/users/UsersTable.tsx +++ b/public/app/features/users/UsersTable.tsx @@ -1,4 +1,4 @@ -import React, { FC, useEffect, useState } from 'react'; +import React, { useEffect, useState } from 'react'; import { OrgRole } from '@grafana/data'; import { Button, ConfirmModal } from '@grafana/ui'; @@ -17,8 +17,7 @@ export interface Props { onRemoveUser: (user: OrgUser) => void; } -const UsersTable: FC = (props) => { - const { users, orgId, onRoleChange, onRemoveUser } = props; +export const UsersTable = ({ users, orgId, onRoleChange, onRemoveUser }: Props) => { const [userToRemove, setUserToRemove] = useState(null); const [roleOptions, setRoleOptions] = useState([]); @@ -148,5 +147,3 @@ const UsersTable: FC = (props) => { ); }; - -export default UsersTable; diff --git a/public/app/features/users/__mocks__/userMocks.ts b/public/app/features/users/__mocks__/userMocks.ts index abdb3c7f8bf..e494b74fa2f 100644 --- a/public/app/features/users/__mocks__/userMocks.ts +++ b/public/app/features/users/__mocks__/userMocks.ts @@ -1,6 +1,19 @@ import { OrgRole, OrgUser } from 'app/types'; -export const getMockUsers = (amount: number) => { +import { UsersFetchResult, initialState } from '../state/reducers'; + +export const getFetchUsersMock = (amount: number): UsersFetchResult => { + const users = getMockUsers(amount); + + return { + orgUsers: users as OrgUser[], + perPage: initialState.perPage, + page: initialState.page, + totalCount: initialState.totalPages, + }; +}; + +export const getMockUsers = (amount: number): OrgUser[] => { const users = []; for (let i = 0; i <= amount; i++) { diff --git a/public/app/features/users/state/actions.ts b/public/app/features/users/state/actions.ts index 259d0d1829d..3578144c863 100644 --- a/public/app/features/users/state/actions.ts +++ b/public/app/features/users/state/actions.ts @@ -1,18 +1,30 @@ +import { debounce } from 'lodash'; + import { getBackendSrv } from '@grafana/runtime'; import { accessControlQueryParam } from 'app/core/utils/accessControl'; import { OrgUser } from 'app/types'; import { ThunkResult } from '../../../types'; -import { usersLoaded } from './reducers'; +import { usersLoaded, pageChanged, usersFetchBegin, usersFetchEnd, searchQueryChanged } from './reducers'; export function loadUsers(): ThunkResult { - return async (dispatch) => { - const users = await getBackendSrv().get('/api/org/users', accessControlQueryParam()); - dispatch(usersLoaded(users)); + return async (dispatch, getState) => { + try { + const { perPage, page, searchQuery } = getState().users; + const users = await getBackendSrv().get( + `/api/org/users/search`, + accessControlQueryParam({ perpage: perPage, page, query: searchQuery }) + ); + dispatch(usersLoaded(users)); + } catch (error) { + usersFetchEnd(); + } }; } +const fetchUsersWithDebounce = debounce((dispatch) => dispatch(loadUsers()), 300); + export function updateUser(user: OrgUser): ThunkResult { return async (dispatch) => { await getBackendSrv().patch(`/api/org/users/${user.userId}`, { role: user.role }); @@ -26,3 +38,19 @@ export function removeUser(userId: number): ThunkResult { dispatch(loadUsers()); }; } + +export function changePage(page: number): ThunkResult { + return async (dispatch) => { + dispatch(usersFetchBegin()); + dispatch(pageChanged(page)); + dispatch(loadUsers()); + }; +} + +export function changeSearchQuery(query: string): ThunkResult { + return async (dispatch) => { + dispatch(usersFetchBegin()); + dispatch(searchQueryChanged(query)); + fetchUsersWithDebounce(dispatch); + }; +} diff --git a/public/app/features/users/state/reducers.test.ts b/public/app/features/users/state/reducers.test.ts index c5634126f0b..e1759deb9b8 100644 --- a/public/app/features/users/state/reducers.test.ts +++ b/public/app/features/users/state/reducers.test.ts @@ -1,28 +1,28 @@ import { reducerTester } from '../../../../test/core/redux/reducerTester'; import { UsersState } from '../../../types'; -import { getMockUsers } from '../__mocks__/userMocks'; +import { getMockUsers, getFetchUsersMock } from '../__mocks__/userMocks'; -import { initialState, setUsersSearchQuery, usersLoaded, usersReducer } from './reducers'; +import { initialState, searchQueryChanged, usersLoaded, usersReducer } from './reducers'; describe('usersReducer', () => { describe('when usersLoaded is dispatched', () => { it('then state should be correct', () => { reducerTester() .givenReducer(usersReducer, { ...initialState }) - .whenActionIsDispatched(usersLoaded(getMockUsers(1))) + .whenActionIsDispatched(usersLoaded(getFetchUsersMock(1))) .thenStateShouldEqual({ ...initialState, users: getMockUsers(1), - hasFetched: true, + isLoading: true, }); }); }); - describe('when setUsersSearchQuery is dispatched', () => { + describe('when searchQueryChanged is dispatched', () => { it('then state should be correct', () => { reducerTester() .givenReducer(usersReducer, { ...initialState }) - .whenActionIsDispatched(setUsersSearchQuery('a query')) + .whenActionIsDispatched(searchQueryChanged('a query')) .thenStateShouldEqual({ ...initialState, searchQuery: 'a query', diff --git a/public/app/features/users/state/reducers.ts b/public/app/features/users/state/reducers.ts index 8b77465357b..1dfeec92a91 100644 --- a/public/app/features/users/state/reducers.ts +++ b/public/app/features/users/state/reducers.ts @@ -6,32 +6,62 @@ import { OrgUser, UsersState } from 'app/types'; export const initialState: UsersState = { users: [] as OrgUser[], searchQuery: '', - searchPage: 1, + page: 0, + perPage: 30, + totalPages: 1, canInvite: !config.externalUserMngLinkName, externalUserMngInfo: config.externalUserMngInfo, externalUserMngLinkName: config.externalUserMngLinkName, externalUserMngLinkUrl: config.externalUserMngLinkUrl, - hasFetched: false, + isLoading: false, }; +export interface UsersFetchResult { + orgUsers: OrgUser[]; + perPage: number; + page: number; + totalCount: number; +} + const usersSlice = createSlice({ name: 'users', initialState, reducers: { - usersLoaded: (state, action: PayloadAction): UsersState => { - return { ...state, hasFetched: true, users: action.payload }; + usersLoaded: (state, action: PayloadAction): UsersState => { + const { totalCount, perPage, page, orgUsers } = action.payload; + const totalPages = Math.ceil(totalCount / perPage); + + return { + ...state, + isLoading: true, + users: orgUsers, + perPage, + page, + totalPages, + }; }, - setUsersSearchQuery: (state, action: PayloadAction): UsersState => { + searchQueryChanged: (state, action: PayloadAction): UsersState => { // reset searchPage otherwise search results won't appear - return { ...state, searchQuery: action.payload, searchPage: initialState.searchPage }; + return { ...state, searchQuery: action.payload, page: initialState.page }; }, setUsersSearchPage: (state, action: PayloadAction): UsersState => { - return { ...state, searchPage: action.payload }; + return { ...state, page: action.payload }; + }, + pageChanged: (state, action: PayloadAction) => ({ + ...state, + page: action.payload, + }), + usersFetchBegin: (state) => { + return { ...state, isLoading: true }; + }, + usersFetchEnd: (state) => { + return { ...state, isLoading: false }; }, }, }); -export const { setUsersSearchQuery, setUsersSearchPage, usersLoaded } = usersSlice.actions; +export const { searchQueryChanged, setUsersSearchPage, usersLoaded, usersFetchBegin, usersFetchEnd, pageChanged } = + usersSlice.actions; export const usersReducer = usersSlice.reducer; diff --git a/public/app/features/users/state/selectors.ts b/public/app/features/users/state/selectors.ts index 19a200c69ff..2d80444a47d 100644 --- a/public/app/features/users/state/selectors.ts +++ b/public/app/features/users/state/selectors.ts @@ -9,4 +9,3 @@ export const getUsers = (state: UsersState) => { }; export const getUsersSearchQuery = (state: UsersState) => state.searchQuery; -export const getUsersSearchPage = (state: UsersState) => state.searchPage; diff --git a/public/app/types/user.ts b/public/app/types/user.ts index f8e0a7349bb..6177c311897 100644 --- a/public/app/types/user.ts +++ b/public/app/types/user.ts @@ -68,12 +68,14 @@ export interface Invitee { export interface UsersState { users: OrgUser[]; searchQuery: string; - searchPage: number; canInvite: boolean; externalUserMngLinkUrl: string; externalUserMngLinkName: string; externalUserMngInfo: string; - hasFetched: boolean; + isLoading: boolean; + page: number; + perPage: number; + totalPages: number; } export interface UserSession {