From 3a8bd3767c84a31626461d8b9c72c5fd4e403452 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cec=C3=ADlia=20Rom=C3=A3o?= Date: Tue, 21 Nov 2023 18:01:52 -0300 Subject: [PATCH 1/8] feat: sort users according to query params --- internal/data/query_params.go | 2 + internal/serve/httphandler/user_handler.go | 9 +++- .../serve/httphandler/user_handler_test.go | 9 +++- .../serve/validators/user_query_validator.go | 21 ++++++++++ stellar-auth/pkg/auth/auth.go | 7 ++-- stellar-auth/pkg/auth/auth_test.go | 41 +++++++++++++++---- stellar-auth/pkg/auth/authenticator.go | 17 ++++++-- stellar-auth/pkg/auth/authenticator_test.go | 20 +++++++-- stellar-auth/pkg/auth/mocks.go | 9 ++-- 9 files changed, 111 insertions(+), 24 deletions(-) create mode 100644 internal/serve/validators/user_query_validator.go diff --git a/internal/data/query_params.go b/internal/data/query_params.go index c24cd4811..bb91a9403 100644 --- a/internal/data/query_params.go +++ b/internal/data/query_params.go @@ -20,6 +20,8 @@ type SortField string const ( SortFieldName SortField = "name" + SortFieldEmail SortField = "email" + SortFieldIsActive SortField = "is_active" SortFieldCreatedAt SortField = "created_at" SortFieldUpdatedAt SortField = "updated_at" ) diff --git a/internal/serve/httphandler/user_handler.go b/internal/serve/httphandler/user_handler.go index c940f1eee..8dc4241f8 100644 --- a/internal/serve/httphandler/user_handler.go +++ b/internal/serve/httphandler/user_handler.go @@ -275,6 +275,13 @@ func (h UserHandler) UpdateUserRoles(rw http.ResponseWriter, req *http.Request) } func (h UserHandler) GetAllUsers(rw http.ResponseWriter, req *http.Request) { + validator := validators.NewUserQueryValidator() + queryParams := validator.ParseParametersFromRequest(req) + if validator.HasErrors() { + httperror.BadRequest("request invalid", nil, validator.Errors).Render(rw) + return + } + ctx := req.Context() token, ok := ctx.Value(middleware.TokenContextKey).(string) @@ -284,7 +291,7 @@ func (h UserHandler) GetAllUsers(rw http.ResponseWriter, req *http.Request) { return } - users, err := h.AuthManager.GetAllUsers(ctx, token) + users, err := h.AuthManager.GetAllUsers(ctx, token, queryParams) if err != nil { if errors.Is(err, auth.ErrInvalidToken) { httperror.Unauthorized("", err, nil).Render(rw) diff --git a/internal/serve/httphandler/user_handler_test.go b/internal/serve/httphandler/user_handler_test.go index ede35bd76..df78fcaa3 100644 --- a/internal/serve/httphandler/user_handler_test.go +++ b/internal/serve/httphandler/user_handler_test.go @@ -1246,7 +1246,14 @@ func Test_UserHandler_GetAllUsers(t *testing.T) { Once() authenticatorMock. - On("GetAllUsers", req.Context()). + On("GetAllUsers", req.Context(), &data.QueryParams{ + Query: "", + Page: 1, + PageLimit: 20, + SortBy: data.SortFieldEmail, + SortOrder: data.SortOrderASC, + Filters: map[data.FilterKey]interface{}{}, + }). Return([]auth.User{ { ID: "user1-ID", diff --git a/internal/serve/validators/user_query_validator.go b/internal/serve/validators/user_query_validator.go new file mode 100644 index 000000000..eb5bd07b1 --- /dev/null +++ b/internal/serve/validators/user_query_validator.go @@ -0,0 +1,21 @@ +package validators + +import ( + "github.com/stellar/stellar-disbursement-platform-backend/stellar-auth/pkg/auth" +) + +type UserQueryValidator struct { + QueryValidator +} + +// NewUserQueryValidator creates a new UserQueryValidator with the provided configuration. +func NewUserQueryValidator() *UserQueryValidator { + return &UserQueryValidator{ + QueryValidator: QueryValidator{ + Validator: NewValidator(), + DefaultSortField: auth.DefaultUserSortField, + DefaultSortOrder: auth.DefaultUserSortOrder, + AllowedSortFields: auth.AllowedUserSorts, + }, + } +} diff --git a/stellar-auth/pkg/auth/auth.go b/stellar-auth/pkg/auth/auth.go index 361e0a84a..04de4c7d0 100644 --- a/stellar-auth/pkg/auth/auth.go +++ b/stellar-auth/pkg/auth/auth.go @@ -7,6 +7,7 @@ import ( "fmt" "time" + "github.com/stellar/stellar-disbursement-platform-backend/internal/data" "github.com/stellar/stellar-disbursement-platform-backend/stellar-auth/internal/db" ) @@ -25,7 +26,7 @@ type AuthManager interface { UpdatePassword(ctx context.Context, token, currentPassword, newPassword string) error GetUser(ctx context.Context, tokenString string) (*User, error) GetUserID(ctx context.Context, tokenString string) (string, error) - GetAllUsers(ctx context.Context, tokenString string) ([]User, error) + GetAllUsers(ctx context.Context, tokenString string, queryParams *data.QueryParams) ([]User, error) UpdateUserRoles(ctx context.Context, tokenString, userID string, roles []string) error DeactivateUser(ctx context.Context, tokenString, userID string) error ActivateUser(ctx context.Context, tokenString, userID string) error @@ -283,7 +284,7 @@ func (am *defaultAuthManager) UpdateUserRoles(ctx context.Context, tokenString, return nil } -func (am *defaultAuthManager) GetAllUsers(ctx context.Context, tokenString string) ([]User, error) { +func (am *defaultAuthManager) GetAllUsers(ctx context.Context, tokenString string, queryParams *data.QueryParams) ([]User, error) { isValid, err := am.ValidateToken(ctx, tokenString) if err != nil { return nil, fmt.Errorf("validating token: %w", err) @@ -293,7 +294,7 @@ func (am *defaultAuthManager) GetAllUsers(ctx context.Context, tokenString strin return nil, ErrInvalidToken } - users, err := am.authenticator.GetAllUsers(ctx) + users, err := am.authenticator.GetAllUsers(ctx, queryParams) if err != nil { return nil, fmt.Errorf("error getting all users: %w", err) } diff --git a/stellar-auth/pkg/auth/auth_test.go b/stellar-auth/pkg/auth/auth_test.go index dffafb711..207fbf5c6 100644 --- a/stellar-auth/pkg/auth/auth_test.go +++ b/stellar-auth/pkg/auth/auth_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/stellar/stellar-disbursement-platform-backend/internal/data" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -1108,7 +1109,10 @@ func Test_AuthManager_GetAllUsers(t *testing.T) { Return(false, errUnexpectedError). Once() - users, err := authManager.GetAllUsers(ctx, token) + users, err := authManager.GetAllUsers(ctx, token, &data.QueryParams{ + SortBy: data.SortFieldEmail, + SortOrder: data.SortOrderASC, + }) assert.Nil(t, users) assert.EqualError(t, err, "validating token: validating token: unexpected error") @@ -1122,7 +1126,10 @@ func Test_AuthManager_GetAllUsers(t *testing.T) { Return(false, nil). Once() - users, err := authManager.GetAllUsers(ctx, token) + users, err := authManager.GetAllUsers(ctx, token, &data.QueryParams{ + SortBy: data.SortFieldEmail, + SortOrder: data.SortOrderASC, + }) assert.Nil(t, users) assert.EqualError(t, err, "invalid token") @@ -1137,11 +1144,17 @@ func Test_AuthManager_GetAllUsers(t *testing.T) { Once() authenticatorMock. - On("GetAllUsers", ctx). + On("GetAllUsers", ctx, &data.QueryParams{ + SortBy: data.SortFieldEmail, + SortOrder: data.SortOrderASC, + }). Return(nil, errUnexpectedError). Once() - users, err := authManager.GetAllUsers(ctx, token) + users, err := authManager.GetAllUsers(ctx, token, &data.QueryParams{ + SortBy: data.SortFieldEmail, + SortOrder: data.SortOrderASC, + }) assert.EqualError(t, err, "error getting all users: unexpected error") assert.Nil(t, users) @@ -1156,11 +1169,17 @@ func Test_AuthManager_GetAllUsers(t *testing.T) { Twice() authenticatorMock. - On("GetAllUsers", ctx). + On("GetAllUsers", ctx, &data.QueryParams{ + SortBy: data.SortFieldEmail, + SortOrder: data.SortOrderASC, + }). Return([]User{}, nil). Once() - users, err := authManager.GetAllUsers(ctx, token) + users, err := authManager.GetAllUsers(ctx, token, &data.QueryParams{ + SortBy: data.SortFieldEmail, + SortOrder: data.SortOrderASC, + }) require.NoError(t, err) assert.Empty(t, users) @@ -1186,11 +1205,17 @@ func Test_AuthManager_GetAllUsers(t *testing.T) { } authenticatorMock. - On("GetAllUsers", ctx). + On("GetAllUsers", ctx, &data.QueryParams{ + SortBy: data.SortFieldEmail, + SortOrder: data.SortOrderASC, + }). Return(expectedUsers, nil). Once() - users, err = authManager.GetAllUsers(ctx, token) + users, err = authManager.GetAllUsers(ctx, token, &data.QueryParams{ + SortBy: data.SortFieldEmail, + SortOrder: data.SortOrderASC, + }) require.NoError(t, err) assert.Equal(t, expectedUsers, users) }) diff --git a/stellar-auth/pkg/auth/authenticator.go b/stellar-auth/pkg/auth/authenticator.go index 25ddce24b..39db13622 100644 --- a/stellar-auth/pkg/auth/authenticator.go +++ b/stellar-auth/pkg/auth/authenticator.go @@ -11,6 +11,7 @@ import ( "time" "github.com/lib/pq" + "github.com/stellar/stellar-disbursement-platform-backend/internal/data" "github.com/stellar/stellar-disbursement-platform-backend/stellar-auth/internal/db" "github.com/stellar/stellar-disbursement-platform-backend/stellar-auth/pkg/utils" ) @@ -28,6 +29,12 @@ const ( resetTokenLength = 10 ) +var ( + DefaultUserSortField = data.SortFieldEmail + DefaultUserSortOrder = data.SortOrderASC + AllowedUserSorts = []data.SortField{data.SortFieldEmail, data.SortFieldIsActive} +) + type Authenticator interface { ValidateCredentials(ctx context.Context, email, password string) (*User, error) // CreateUser creates a new user it receives a user object and the password @@ -38,7 +45,7 @@ type Authenticator interface { ForgotPassword(ctx context.Context, email string) (string, error) ResetPassword(ctx context.Context, resetToken, password string) error UpdatePassword(ctx context.Context, user *User, currentPassword, newPassword string) error - GetAllUsers(ctx context.Context) ([]User, error) + GetAllUsers(ctx context.Context, queryParams *data.QueryParams) ([]User, error) GetUser(ctx context.Context, userID string) (*User, error) } @@ -404,8 +411,8 @@ func (a *defaultAuthenticator) invalidateResetPasswordToken(ctx context.Context, return nil } -func (a *defaultAuthenticator) GetAllUsers(ctx context.Context) ([]User, error) { - const query = ` +func (a *defaultAuthenticator) GetAllUsers(ctx context.Context, queryParams *data.QueryParams) ([]User, error) { + query := fmt.Sprintf(` SELECT id, first_name, @@ -416,7 +423,8 @@ func (a *defaultAuthenticator) GetAllUsers(ctx context.Context) ([]User, error) is_active FROM auth_users - ` + ORDER BY %s %s + `, queryParams.SortBy, queryParams.SortOrder) dbUsers := []struct { ID string `db:"id"` @@ -429,6 +437,7 @@ func (a *defaultAuthenticator) GetAllUsers(ctx context.Context) ([]User, error) }{} err := a.dbConnectionPool.SelectContext(ctx, &dbUsers, query) if err != nil { + fmt.Println(queryParams) return nil, fmt.Errorf("error querying all users in the database: %w", err) } diff --git a/stellar-auth/pkg/auth/authenticator_test.go b/stellar-auth/pkg/auth/authenticator_test.go index 582a5aac2..5a32657c9 100644 --- a/stellar-auth/pkg/auth/authenticator_test.go +++ b/stellar-auth/pkg/auth/authenticator_test.go @@ -4,9 +4,11 @@ import ( "context" "database/sql" "errors" + "sort" "testing" "time" + "github.com/stellar/stellar-disbursement-platform-backend/internal/data" "github.com/stellar/stellar-disbursement-platform-backend/stellar-auth/internal/db" "github.com/stellar/stellar-disbursement-platform-backend/stellar-auth/internal/db/dbtest" "github.com/stretchr/testify/assert" @@ -16,6 +18,12 @@ import ( var errUnexpectedError = errors.New("unexpected error") +type UserSorter []User + +func (a UserSorter) Len() int { return len(a) } +func (a UserSorter) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a UserSorter) Less(i, j int) bool { return a[i].Email < a[j].Email} + func assertUserIsActive(t *testing.T, ctx context.Context, dbConnectionPool db.DBConnectionPool, userID string, expectedIsActive bool) { const query = "SELECT is_active FROM auth_users WHERE id = $1" @@ -612,7 +620,10 @@ func Test_DefaultAuthenticator_GetAllUsers(t *testing.T) { ctx := context.Background() t.Run("returns an empty array if no users are registered", func(t *testing.T) { - users, err := authenticator.GetAllUsers(ctx) + users, err := authenticator.GetAllUsers(ctx, &data.QueryParams{ + SortBy: data.SortFieldEmail, + SortOrder: data.SortOrderASC, + }) require.NoError(t, err) assert.Empty(t, users) @@ -627,7 +638,10 @@ func Test_DefaultAuthenticator_GetAllUsers(t *testing.T) { randUser2 := CreateRandomAuthUserFixture(t, ctx, dbConnectionPool, passwordEncrypterMock, true, "role1", "role2") randUser3 := CreateRandomAuthUserFixture(t, ctx, dbConnectionPool, passwordEncrypterMock, false, "role3") - users, err := authenticator.GetAllUsers(ctx) + users, err := authenticator.GetAllUsers(ctx, &data.QueryParams{ + SortBy: data.SortFieldEmail, + SortOrder: data.SortOrderASC, + }) require.NoError(t, err) expectedUsers := []User{ @@ -635,7 +649,7 @@ func Test_DefaultAuthenticator_GetAllUsers(t *testing.T) { *randUser2.ToUser(), *randUser3.ToUser(), } - + sort.Sort(UserSorter(expectedUsers)) assert.Equal(t, expectedUsers, users) }) diff --git a/stellar-auth/pkg/auth/mocks.go b/stellar-auth/pkg/auth/mocks.go index eaf22ebcd..842f511d4 100644 --- a/stellar-auth/pkg/auth/mocks.go +++ b/stellar-auth/pkg/auth/mocks.go @@ -4,6 +4,7 @@ import ( "context" "time" + "github.com/stellar/stellar-disbursement-platform-backend/internal/data" "github.com/stretchr/testify/mock" ) @@ -105,8 +106,8 @@ func (am *AuthenticatorMock) UpdatePassword(ctx context.Context, user *User, cur return args.Error(0) } -func (am *AuthenticatorMock) GetAllUsers(ctx context.Context) ([]User, error) { - args := am.Called(ctx) +func (am *AuthenticatorMock) GetAllUsers(ctx context.Context, queryParams *data.QueryParams) ([]User, error) { + args := am.Called(ctx, queryParams) if args.Get(0) == nil { return nil, args.Error(1) } @@ -255,8 +256,8 @@ func (am *AuthManagerMock) GetUserID(ctx context.Context, tokenString string) (s return args.Get(0).(string), args.Error(1) } -func (am *AuthManagerMock) GetAllUsers(ctx context.Context, tokenString string) ([]User, error) { - args := am.Called(ctx, tokenString) +func (am *AuthManagerMock) GetAllUsers(ctx context.Context, tokenString string, queryParams *data.QueryParams) ([]User, error) { + args := am.Called(ctx, tokenString, queryParams) if args.Get(0) == nil { return nil, args.Error(1) } From ebefade95b96edbd2834d978e3ed96debbfe5afb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cec=C3=ADlia=20Rom=C3=A3o?= Date: Tue, 21 Nov 2023 18:09:45 -0300 Subject: [PATCH 2/8] Update authenticator_test.go --- stellar-auth/pkg/auth/authenticator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stellar-auth/pkg/auth/authenticator_test.go b/stellar-auth/pkg/auth/authenticator_test.go index 5a32657c9..bd8eea876 100644 --- a/stellar-auth/pkg/auth/authenticator_test.go +++ b/stellar-auth/pkg/auth/authenticator_test.go @@ -22,7 +22,7 @@ type UserSorter []User func (a UserSorter) Len() int { return len(a) } func (a UserSorter) Swap(i, j int) { a[i], a[j] = a[j], a[i] } -func (a UserSorter) Less(i, j int) bool { return a[i].Email < a[j].Email} +func (a UserSorter) Less(i, j int) bool { return a[i].Email < a[j].Email } func assertUserIsActive(t *testing.T, ctx context.Context, dbConnectionPool db.DBConnectionPool, userID string, expectedIsActive bool) { const query = "SELECT is_active FROM auth_users WHERE id = $1" From 7070eec91d8ff5a5a2b69ec18602e0bf2204719b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cec=C3=ADlia=20Rom=C3=A3o?= Date: Tue, 21 Nov 2023 18:20:59 -0300 Subject: [PATCH 3/8] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d285f99e..5e7fde4f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Add the ability to filter supported assets by wallets. [#35](https://github.com/stellar/stellar-disbursement-platform-backend/pull/35) - Add wallets filtering by `enabled` flag [#72](https://github.com/stellar/stellar-disbursement-platform-backend/pull/72) - Return SMS templates in `GET /organization` endpoint. [#63](https://github.com/stellar/stellar-disbursement-platform-backend/pull/63) + - Add sorting to `GET /users` endpoint [#104](https://github.com/stellar/stellar-disbursement-platform-backend/pull/104) ### Fixed From 9335837772d322181209a1e6b87a436cbcfc606d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cec=C3=ADlia=20Rom=C3=A3o?= Date: Fri, 24 Nov 2023 10:29:21 -0300 Subject: [PATCH 4/8] Update CHANGELOG.md --- CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e7fde4f9..b0b28eccd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Unreleased -> Place unreleased changes here. +- Add sorting to `GET /users` endpoint [#104](https://github.com/stellar/stellar-disbursement-platform-backend/pull/104) ## [1.0.0](https://github.com/stellar/stellar-disbursement-platform-backend/compare/1.0.0-rc2...1.0.0) @@ -41,7 +41,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Add the ability to filter supported assets by wallets. [#35](https://github.com/stellar/stellar-disbursement-platform-backend/pull/35) - Add wallets filtering by `enabled` flag [#72](https://github.com/stellar/stellar-disbursement-platform-backend/pull/72) - Return SMS templates in `GET /organization` endpoint. [#63](https://github.com/stellar/stellar-disbursement-platform-backend/pull/63) - - Add sorting to `GET /users` endpoint [#104](https://github.com/stellar/stellar-disbursement-platform-backend/pull/104) ### Fixed From e5ff7e761d3bb65fd88d3d2f692ca4182c169e5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cec=C3=ADlia=20Rom=C3=A3o?= Date: Tue, 28 Nov 2023 14:56:48 -0300 Subject: [PATCH 5/8] Update stellar-auth/pkg/auth/authenticator.go Co-authored-by: Caio Teixeira --- stellar-auth/pkg/auth/authenticator.go | 1 - 1 file changed, 1 deletion(-) diff --git a/stellar-auth/pkg/auth/authenticator.go b/stellar-auth/pkg/auth/authenticator.go index 39db13622..537fd82e0 100644 --- a/stellar-auth/pkg/auth/authenticator.go +++ b/stellar-auth/pkg/auth/authenticator.go @@ -437,7 +437,6 @@ func (a *defaultAuthenticator) GetAllUsers(ctx context.Context, queryParams *dat }{} err := a.dbConnectionPool.SelectContext(ctx, &dbUsers, query) if err != nil { - fmt.Println(queryParams) return nil, fmt.Errorf("error querying all users in the database: %w", err) } From 79978881a1efdd1b0b0fa8b09c60de56584008e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cec=C3=ADlia=20Rom=C3=A3o?= Date: Tue, 28 Nov 2023 17:43:22 -0300 Subject: [PATCH 6/8] fix: stellar-auth / internal dependency --- internal/serve/httphandler/user_handler.go | 32 +- .../serve/httphandler/user_handler_test.go | 289 +++++++++++++++++- stellar-auth/pkg/auth/auth.go | 7 +- stellar-auth/pkg/auth/auth_test.go | 41 +-- stellar-auth/pkg/auth/authenticator.go | 11 +- stellar-auth/pkg/auth/authenticator_test.go | 20 +- stellar-auth/pkg/auth/mocks.go | 9 +- 7 files changed, 328 insertions(+), 81 deletions(-) diff --git a/internal/serve/httphandler/user_handler.go b/internal/serve/httphandler/user_handler.go index 8dc4241f8..36bc14b79 100644 --- a/internal/serve/httphandler/user_handler.go +++ b/internal/serve/httphandler/user_handler.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "net/url" + "sort" "github.com/stellar/go/support/http/httpdecode" "github.com/stellar/go/support/log" @@ -32,6 +33,18 @@ type UserActivationRequest struct { IsActive *bool `json:"is_active"` } +type UserSorterByEmail []auth.User + +func (a UserSorterByEmail) Len() int { return len(a) } +func (a UserSorterByEmail) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a UserSorterByEmail) Less(i, j int) bool { return a[i].Email < a[j].Email } + +type UserSorterByIsActive []auth.User + +func (a UserSorterByIsActive) Len() int { return len(a) } +func (a UserSorterByIsActive) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a UserSorterByIsActive) Less(i, j int) bool { return a[i].IsActive } + func (uar UserActivationRequest) validate() *httperror.HTTPError { validator := validators.NewValidator() @@ -291,16 +304,31 @@ func (h UserHandler) GetAllUsers(rw http.ResponseWriter, req *http.Request) { return } - users, err := h.AuthManager.GetAllUsers(ctx, token, queryParams) + users, err := h.AuthManager.GetAllUsers(ctx, token) if err != nil { if errors.Is(err, auth.ErrInvalidToken) { httperror.Unauthorized("", err, nil).Render(rw) return } - httperror.InternalError(ctx, "Cannot get all users", err, nil).Render(rw) return } + // Order users + switch queryParams.SortBy { + case data.SortFieldEmail: + if queryParams.SortOrder == data.SortOrderDESC { + sort.Sort(sort.Reverse(UserSorterByEmail(users))) + } else { + sort.Sort(UserSorterByEmail(users)) + } + case data.SortFieldIsActive: + if queryParams.SortOrder == data.SortOrderDESC { + sort.Sort(sort.Reverse(UserSorterByIsActive(users))) + } else { + sort.Sort(UserSorterByIsActive(users)) + } + } + httpjson.RenderStatus(rw, http.StatusOK, users, httpjson.JSON) } diff --git a/internal/serve/httphandler/user_handler_test.go b/internal/serve/httphandler/user_handler_test.go index df78fcaa3..34201069d 100644 --- a/internal/serve/httphandler/user_handler_test.go +++ b/internal/serve/httphandler/user_handler_test.go @@ -1232,12 +1232,17 @@ func Test_UserHandler_GetAllUsers(t *testing.T) { assert.Contains(t, buf.String(), "Cannot get all users") }) - t.Run("returns all users successfully", func(t *testing.T) { + const orderByEmailAscURL = "/users?sort=email&direction=ASC" + const orderByEmailDescURL = "/users?sort=email&direction=DESC" + const orderByIsActiveAscURL = "/users?sort=is_active&direction=ASC" + const orderByIsActiveDescURL = "/users?sort=is_active&direction=DESC" + + t.Run("returns all users ordered by email ASC", func(t *testing.T) { token := "mytoken" ctx := context.WithValue(context.Background(), middleware.TokenContextKey, token) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, orderByEmailAscURL, nil) require.NoError(t, err) jwtManagerMock. @@ -1246,20 +1251,13 @@ func Test_UserHandler_GetAllUsers(t *testing.T) { Once() authenticatorMock. - On("GetAllUsers", req.Context(), &data.QueryParams{ - Query: "", - Page: 1, - PageLimit: 20, - SortBy: data.SortFieldEmail, - SortOrder: data.SortOrderASC, - Filters: map[data.FilterKey]interface{}{}, - }). + On("GetAllUsers", req.Context()). Return([]auth.User{ { ID: "user1-ID", FirstName: "First", LastName: "Last", - Email: "user1@email.com", + Email: "userA@email.com", IsOwner: false, IsActive: false, Roles: []string{data.BusinessUserRole.String()}, @@ -1268,7 +1266,16 @@ func Test_UserHandler_GetAllUsers(t *testing.T) { ID: "user2-ID", FirstName: "First", LastName: "Last", - Email: "user2@email.com", + Email: "userC@email.com", + IsOwner: true, + IsActive: true, + Roles: []string{data.OwnerUserRole.String()}, + }, + { + ID: "user3-ID", + FirstName: "First", + LastName: "Last", + Email: "userB@email.com", IsOwner: true, IsActive: true, Roles: []string{data.OwnerUserRole.String()}, @@ -1291,17 +1298,27 @@ func Test_UserHandler_GetAllUsers(t *testing.T) { "id": "user1-ID", "first_name": "First", "last_name": "Last", - "email": "user1@email.com", + "email": "userA@email.com", "is_active": false, "roles": [ "business" ] }, + { + "id": "user3-ID", + "first_name": "First", + "last_name": "Last", + "email": "userB@email.com", + "is_active": true, + "roles": [ + "owner" + ] + }, { "id": "user2-ID", "first_name": "First", "last_name": "Last", - "email": "user2@email.com", + "email": "userC@email.com", "is_active": true, "roles": [ "owner" @@ -1313,4 +1330,248 @@ func Test_UserHandler_GetAllUsers(t *testing.T) { assert.Equal(t, http.StatusOK, resp.StatusCode) assert.JSONEq(t, wantsBody, string(respBody)) }) + + t.Run("returns all users ordered by email DESC", func(t *testing.T) { + token := "mytoken" + + ctx := context.WithValue(context.Background(), middleware.TokenContextKey, token) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, orderByEmailDescURL, nil) + require.NoError(t, err) + + jwtManagerMock. + On("ValidateToken", req.Context(), token). + Return(true, nil). + Once() + + authenticatorMock. + On("GetAllUsers", req.Context()). + Return([]auth.User{ + { + ID: "user1-ID", + FirstName: "First", + LastName: "Last", + Email: "userA@email.com", + IsOwner: false, + IsActive: false, + Roles: []string{data.BusinessUserRole.String()}, + }, + { + ID: "user2-ID", + FirstName: "First", + LastName: "Last", + Email: "userC@email.com", + IsOwner: true, + IsActive: true, + Roles: []string{data.OwnerUserRole.String()}, + }, + { + ID: "user3-ID", + FirstName: "First", + LastName: "Last", + Email: "userB@email.com", + IsOwner: true, + IsActive: true, + Roles: []string{data.OwnerUserRole.String()}, + }, + }, nil). + Once() + + w := httptest.NewRecorder() + + http.HandlerFunc(handler.GetAllUsers).ServeHTTP(w, req) + + resp := w.Result() + + respBody, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + wantsBody := ` + [ + { + "id": "user2-ID", + "first_name": "First", + "last_name": "Last", + "email": "userC@email.com", + "is_active": true, + "roles": [ + "owner" + ] + }, + { + "id": "user3-ID", + "first_name": "First", + "last_name": "Last", + "email": "userB@email.com", + "is_active": true, + "roles": [ + "owner" + ] + }, + { + "id": "user1-ID", + "first_name": "First", + "last_name": "Last", + "email": "userA@email.com", + "is_active": false, + "roles": [ + "business" + ] + } + ] + ` + + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.JSONEq(t, wantsBody, string(respBody)) + }) + + t.Run("returns all users ordered by is_active ASC", func(t *testing.T) { + token := "mytoken" + + ctx := context.WithValue(context.Background(), middleware.TokenContextKey, token) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, orderByIsActiveAscURL, nil) + require.NoError(t, err) + + jwtManagerMock. + On("ValidateToken", req.Context(), token). + Return(true, nil). + Once() + + authenticatorMock. + On("GetAllUsers", req.Context()). + Return([]auth.User{ + { + ID: "user1-ID", + FirstName: "First", + LastName: "Last", + Email: "userA@email.com", + IsOwner: false, + IsActive: false, + Roles: []string{data.BusinessUserRole.String()}, + }, + { + ID: "user2-ID", + FirstName: "First", + LastName: "Last", + Email: "userB@email.com", + IsOwner: true, + IsActive: true, + Roles: []string{data.OwnerUserRole.String()}, + }, + }, nil). + Once() + + w := httptest.NewRecorder() + + http.HandlerFunc(handler.GetAllUsers).ServeHTTP(w, req) + + resp := w.Result() + + respBody, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + wantsBody := ` + [ + { + "id": "user2-ID", + "first_name": "First", + "last_name": "Last", + "email": "userB@email.com", + "is_active": true, + "roles": [ + "owner" + ] + }, + { + "id": "user1-ID", + "first_name": "First", + "last_name": "Last", + "email": "userA@email.com", + "is_active": false, + "roles": [ + "business" + ] + } + ] + ` + + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.JSONEq(t, wantsBody, string(respBody)) + }) + + t.Run("returns all users ordered by is_active DESC", func(t *testing.T) { + token := "mytoken" + + ctx := context.WithValue(context.Background(), middleware.TokenContextKey, token) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, orderByIsActiveDescURL, nil) + require.NoError(t, err) + + jwtManagerMock. + On("ValidateToken", req.Context(), token). + Return(true, nil). + Once() + + authenticatorMock. + On("GetAllUsers", req.Context()). + Return([]auth.User{ + { + ID: "user1-ID", + FirstName: "First", + LastName: "Last", + Email: "userA@email.com", + IsOwner: false, + IsActive: false, + Roles: []string{data.BusinessUserRole.String()}, + }, + { + ID: "user2-ID", + FirstName: "First", + LastName: "Last", + Email: "userB@email.com", + IsOwner: true, + IsActive: true, + Roles: []string{data.OwnerUserRole.String()}, + }, + }, nil). + Once() + + w := httptest.NewRecorder() + + http.HandlerFunc(handler.GetAllUsers).ServeHTTP(w, req) + + resp := w.Result() + + respBody, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + wantsBody := ` + [ + { + "id": "user1-ID", + "first_name": "First", + "last_name": "Last", + "email": "userA@email.com", + "is_active": false, + "roles": [ + "business" + ] + }, + { + "id": "user2-ID", + "first_name": "First", + "last_name": "Last", + "email": "userB@email.com", + "is_active": true, + "roles": [ + "owner" + ] + } + ] + ` + + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.JSONEq(t, wantsBody, string(respBody)) + }) } diff --git a/stellar-auth/pkg/auth/auth.go b/stellar-auth/pkg/auth/auth.go index 04de4c7d0..361e0a84a 100644 --- a/stellar-auth/pkg/auth/auth.go +++ b/stellar-auth/pkg/auth/auth.go @@ -7,7 +7,6 @@ import ( "fmt" "time" - "github.com/stellar/stellar-disbursement-platform-backend/internal/data" "github.com/stellar/stellar-disbursement-platform-backend/stellar-auth/internal/db" ) @@ -26,7 +25,7 @@ type AuthManager interface { UpdatePassword(ctx context.Context, token, currentPassword, newPassword string) error GetUser(ctx context.Context, tokenString string) (*User, error) GetUserID(ctx context.Context, tokenString string) (string, error) - GetAllUsers(ctx context.Context, tokenString string, queryParams *data.QueryParams) ([]User, error) + GetAllUsers(ctx context.Context, tokenString string) ([]User, error) UpdateUserRoles(ctx context.Context, tokenString, userID string, roles []string) error DeactivateUser(ctx context.Context, tokenString, userID string) error ActivateUser(ctx context.Context, tokenString, userID string) error @@ -284,7 +283,7 @@ func (am *defaultAuthManager) UpdateUserRoles(ctx context.Context, tokenString, return nil } -func (am *defaultAuthManager) GetAllUsers(ctx context.Context, tokenString string, queryParams *data.QueryParams) ([]User, error) { +func (am *defaultAuthManager) GetAllUsers(ctx context.Context, tokenString string) ([]User, error) { isValid, err := am.ValidateToken(ctx, tokenString) if err != nil { return nil, fmt.Errorf("validating token: %w", err) @@ -294,7 +293,7 @@ func (am *defaultAuthManager) GetAllUsers(ctx context.Context, tokenString strin return nil, ErrInvalidToken } - users, err := am.authenticator.GetAllUsers(ctx, queryParams) + users, err := am.authenticator.GetAllUsers(ctx) if err != nil { return nil, fmt.Errorf("error getting all users: %w", err) } diff --git a/stellar-auth/pkg/auth/auth_test.go b/stellar-auth/pkg/auth/auth_test.go index 207fbf5c6..dffafb711 100644 --- a/stellar-auth/pkg/auth/auth_test.go +++ b/stellar-auth/pkg/auth/auth_test.go @@ -6,7 +6,6 @@ import ( "testing" "time" - "github.com/stellar/stellar-disbursement-platform-backend/internal/data" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -1109,10 +1108,7 @@ func Test_AuthManager_GetAllUsers(t *testing.T) { Return(false, errUnexpectedError). Once() - users, err := authManager.GetAllUsers(ctx, token, &data.QueryParams{ - SortBy: data.SortFieldEmail, - SortOrder: data.SortOrderASC, - }) + users, err := authManager.GetAllUsers(ctx, token) assert.Nil(t, users) assert.EqualError(t, err, "validating token: validating token: unexpected error") @@ -1126,10 +1122,7 @@ func Test_AuthManager_GetAllUsers(t *testing.T) { Return(false, nil). Once() - users, err := authManager.GetAllUsers(ctx, token, &data.QueryParams{ - SortBy: data.SortFieldEmail, - SortOrder: data.SortOrderASC, - }) + users, err := authManager.GetAllUsers(ctx, token) assert.Nil(t, users) assert.EqualError(t, err, "invalid token") @@ -1144,17 +1137,11 @@ func Test_AuthManager_GetAllUsers(t *testing.T) { Once() authenticatorMock. - On("GetAllUsers", ctx, &data.QueryParams{ - SortBy: data.SortFieldEmail, - SortOrder: data.SortOrderASC, - }). + On("GetAllUsers", ctx). Return(nil, errUnexpectedError). Once() - users, err := authManager.GetAllUsers(ctx, token, &data.QueryParams{ - SortBy: data.SortFieldEmail, - SortOrder: data.SortOrderASC, - }) + users, err := authManager.GetAllUsers(ctx, token) assert.EqualError(t, err, "error getting all users: unexpected error") assert.Nil(t, users) @@ -1169,17 +1156,11 @@ func Test_AuthManager_GetAllUsers(t *testing.T) { Twice() authenticatorMock. - On("GetAllUsers", ctx, &data.QueryParams{ - SortBy: data.SortFieldEmail, - SortOrder: data.SortOrderASC, - }). + On("GetAllUsers", ctx). Return([]User{}, nil). Once() - users, err := authManager.GetAllUsers(ctx, token, &data.QueryParams{ - SortBy: data.SortFieldEmail, - SortOrder: data.SortOrderASC, - }) + users, err := authManager.GetAllUsers(ctx, token) require.NoError(t, err) assert.Empty(t, users) @@ -1205,17 +1186,11 @@ func Test_AuthManager_GetAllUsers(t *testing.T) { } authenticatorMock. - On("GetAllUsers", ctx, &data.QueryParams{ - SortBy: data.SortFieldEmail, - SortOrder: data.SortOrderASC, - }). + On("GetAllUsers", ctx). Return(expectedUsers, nil). Once() - users, err = authManager.GetAllUsers(ctx, token, &data.QueryParams{ - SortBy: data.SortFieldEmail, - SortOrder: data.SortOrderASC, - }) + users, err = authManager.GetAllUsers(ctx, token) require.NoError(t, err) assert.Equal(t, expectedUsers, users) }) diff --git a/stellar-auth/pkg/auth/authenticator.go b/stellar-auth/pkg/auth/authenticator.go index 537fd82e0..71164cb1f 100644 --- a/stellar-auth/pkg/auth/authenticator.go +++ b/stellar-auth/pkg/auth/authenticator.go @@ -45,7 +45,7 @@ type Authenticator interface { ForgotPassword(ctx context.Context, email string) (string, error) ResetPassword(ctx context.Context, resetToken, password string) error UpdatePassword(ctx context.Context, user *User, currentPassword, newPassword string) error - GetAllUsers(ctx context.Context, queryParams *data.QueryParams) ([]User, error) + GetAllUsers(ctx context.Context) ([]User, error) GetUser(ctx context.Context, userID string) (*User, error) } @@ -411,8 +411,8 @@ func (a *defaultAuthenticator) invalidateResetPasswordToken(ctx context.Context, return nil } -func (a *defaultAuthenticator) GetAllUsers(ctx context.Context, queryParams *data.QueryParams) ([]User, error) { - query := fmt.Sprintf(` +func (a *defaultAuthenticator) GetAllUsers(ctx context.Context) ([]User, error) { + const query = ` SELECT id, first_name, @@ -422,9 +422,8 @@ func (a *defaultAuthenticator) GetAllUsers(ctx context.Context, queryParams *dat is_owner, is_active FROM - auth_users - ORDER BY %s %s - `, queryParams.SortBy, queryParams.SortOrder) + auth_users + ` dbUsers := []struct { ID string `db:"id"` diff --git a/stellar-auth/pkg/auth/authenticator_test.go b/stellar-auth/pkg/auth/authenticator_test.go index bd8eea876..582a5aac2 100644 --- a/stellar-auth/pkg/auth/authenticator_test.go +++ b/stellar-auth/pkg/auth/authenticator_test.go @@ -4,11 +4,9 @@ import ( "context" "database/sql" "errors" - "sort" "testing" "time" - "github.com/stellar/stellar-disbursement-platform-backend/internal/data" "github.com/stellar/stellar-disbursement-platform-backend/stellar-auth/internal/db" "github.com/stellar/stellar-disbursement-platform-backend/stellar-auth/internal/db/dbtest" "github.com/stretchr/testify/assert" @@ -18,12 +16,6 @@ import ( var errUnexpectedError = errors.New("unexpected error") -type UserSorter []User - -func (a UserSorter) Len() int { return len(a) } -func (a UserSorter) Swap(i, j int) { a[i], a[j] = a[j], a[i] } -func (a UserSorter) Less(i, j int) bool { return a[i].Email < a[j].Email } - func assertUserIsActive(t *testing.T, ctx context.Context, dbConnectionPool db.DBConnectionPool, userID string, expectedIsActive bool) { const query = "SELECT is_active FROM auth_users WHERE id = $1" @@ -620,10 +612,7 @@ func Test_DefaultAuthenticator_GetAllUsers(t *testing.T) { ctx := context.Background() t.Run("returns an empty array if no users are registered", func(t *testing.T) { - users, err := authenticator.GetAllUsers(ctx, &data.QueryParams{ - SortBy: data.SortFieldEmail, - SortOrder: data.SortOrderASC, - }) + users, err := authenticator.GetAllUsers(ctx) require.NoError(t, err) assert.Empty(t, users) @@ -638,10 +627,7 @@ func Test_DefaultAuthenticator_GetAllUsers(t *testing.T) { randUser2 := CreateRandomAuthUserFixture(t, ctx, dbConnectionPool, passwordEncrypterMock, true, "role1", "role2") randUser3 := CreateRandomAuthUserFixture(t, ctx, dbConnectionPool, passwordEncrypterMock, false, "role3") - users, err := authenticator.GetAllUsers(ctx, &data.QueryParams{ - SortBy: data.SortFieldEmail, - SortOrder: data.SortOrderASC, - }) + users, err := authenticator.GetAllUsers(ctx) require.NoError(t, err) expectedUsers := []User{ @@ -649,7 +635,7 @@ func Test_DefaultAuthenticator_GetAllUsers(t *testing.T) { *randUser2.ToUser(), *randUser3.ToUser(), } - sort.Sort(UserSorter(expectedUsers)) + assert.Equal(t, expectedUsers, users) }) diff --git a/stellar-auth/pkg/auth/mocks.go b/stellar-auth/pkg/auth/mocks.go index 842f511d4..eaf22ebcd 100644 --- a/stellar-auth/pkg/auth/mocks.go +++ b/stellar-auth/pkg/auth/mocks.go @@ -4,7 +4,6 @@ import ( "context" "time" - "github.com/stellar/stellar-disbursement-platform-backend/internal/data" "github.com/stretchr/testify/mock" ) @@ -106,8 +105,8 @@ func (am *AuthenticatorMock) UpdatePassword(ctx context.Context, user *User, cur return args.Error(0) } -func (am *AuthenticatorMock) GetAllUsers(ctx context.Context, queryParams *data.QueryParams) ([]User, error) { - args := am.Called(ctx, queryParams) +func (am *AuthenticatorMock) GetAllUsers(ctx context.Context) ([]User, error) { + args := am.Called(ctx) if args.Get(0) == nil { return nil, args.Error(1) } @@ -256,8 +255,8 @@ func (am *AuthManagerMock) GetUserID(ctx context.Context, tokenString string) (s return args.Get(0).(string), args.Error(1) } -func (am *AuthManagerMock) GetAllUsers(ctx context.Context, tokenString string, queryParams *data.QueryParams) ([]User, error) { - args := am.Called(ctx, tokenString, queryParams) +func (am *AuthManagerMock) GetAllUsers(ctx context.Context, tokenString string) ([]User, error) { + args := am.Called(ctx, tokenString) if args.Get(0) == nil { return nil, args.Error(1) } From cd0b7e097203c9a50ea98325fb760a6d2ee4e690 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cec=C3=ADlia=20Rom=C3=A3o?= Date: Tue, 28 Nov 2023 22:19:34 -0300 Subject: [PATCH 7/8] Update authenticator.go --- stellar-auth/pkg/auth/authenticator.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/stellar-auth/pkg/auth/authenticator.go b/stellar-auth/pkg/auth/authenticator.go index 71164cb1f..25ddce24b 100644 --- a/stellar-auth/pkg/auth/authenticator.go +++ b/stellar-auth/pkg/auth/authenticator.go @@ -11,7 +11,6 @@ import ( "time" "github.com/lib/pq" - "github.com/stellar/stellar-disbursement-platform-backend/internal/data" "github.com/stellar/stellar-disbursement-platform-backend/stellar-auth/internal/db" "github.com/stellar/stellar-disbursement-platform-backend/stellar-auth/pkg/utils" ) @@ -29,12 +28,6 @@ const ( resetTokenLength = 10 ) -var ( - DefaultUserSortField = data.SortFieldEmail - DefaultUserSortOrder = data.SortOrderASC - AllowedUserSorts = []data.SortField{data.SortFieldEmail, data.SortFieldIsActive} -) - type Authenticator interface { ValidateCredentials(ctx context.Context, email, password string) (*User, error) // CreateUser creates a new user it receives a user object and the password @@ -422,7 +415,7 @@ func (a *defaultAuthenticator) GetAllUsers(ctx context.Context) ([]User, error) is_owner, is_active FROM - auth_users + auth_users ` dbUsers := []struct { From 80c00268cdaaa2f527c65d9c50fc6517bdfca20f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cec=C3=ADlia=20Rom=C3=A3o?= Date: Tue, 28 Nov 2023 22:25:19 -0300 Subject: [PATCH 8/8] Update user_query_validator.go --- internal/serve/validators/user_query_validator.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/serve/validators/user_query_validator.go b/internal/serve/validators/user_query_validator.go index eb5bd07b1..5425596a9 100644 --- a/internal/serve/validators/user_query_validator.go +++ b/internal/serve/validators/user_query_validator.go @@ -1,21 +1,27 @@ package validators import ( - "github.com/stellar/stellar-disbursement-platform-backend/stellar-auth/pkg/auth" + "github.com/stellar/stellar-disbursement-platform-backend/internal/data" ) type UserQueryValidator struct { QueryValidator } +var ( + DefaultUserSortField = data.SortFieldEmail + DefaultUserSortOrder = data.SortOrderASC + AllowedUserSorts = []data.SortField{data.SortFieldEmail, data.SortFieldIsActive} +) + // NewUserQueryValidator creates a new UserQueryValidator with the provided configuration. func NewUserQueryValidator() *UserQueryValidator { return &UserQueryValidator{ QueryValidator: QueryValidator{ Validator: NewValidator(), - DefaultSortField: auth.DefaultUserSortField, - DefaultSortOrder: auth.DefaultUserSortOrder, - AllowedSortFields: auth.AllowedUserSorts, + DefaultSortField: DefaultUserSortField, + DefaultSortOrder: DefaultUserSortOrder, + AllowedSortFields: AllowedUserSorts, }, } }