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/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"`
 }

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

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
 		}

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)