From 8ee4133c5d115391fb5fc8c459be2c563b967b0d Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Mon, 5 May 2025 16:13:22 -0400 Subject: [PATCH 1/6] add endpoint to get managed users --- api/dbv1/full_managers.go | 47 ----------------- api/dbv1/get_events.sql.go | 2 +- api/dbv1/get_grants.sql.go | 28 +++++----- api/dbv1/managers.go | 92 +++++++++++++++++++++++++++++++++ api/dbv1/queries/get_grants.sql | 14 +++-- api/server.go | 1 + api/v1_users_managed_users.go | 41 +++++++++++++++ 7 files changed, 154 insertions(+), 71 deletions(-) delete mode 100644 api/dbv1/full_managers.go create mode 100644 api/dbv1/managers.go create mode 100644 api/v1_users_managed_users.go diff --git a/api/dbv1/full_managers.go b/api/dbv1/full_managers.go deleted file mode 100644 index a23e2c49..00000000 --- a/api/dbv1/full_managers.go +++ /dev/null @@ -1,47 +0,0 @@ -package dbv1 - -import ( - "context" -) - -type FullGrant struct { - GetGrantsForUserIdRow - GranteeUserID *struct{} `json:"grantee_user_id,omitempty"` -} - -type FullManager struct { - Manager FullUser `json:"manager"` - Grant FullGrant `json:"grant"` -} - -func (q *Queries) FullManagers(ctx context.Context, params GetGrantsForUserIdParams) ([]FullManager, error) { - - grants, err := q.GetGrantsForUserId(ctx, params) - if err != nil { - return nil, err - } - - user_ids := make([]int32, len(grants)) - for i, grant := range grants { - user_ids[i] = int32(grant.GranteeUserID) - } - - users, err := q.FullUsersKeyed(ctx, GetUsersParams{ - Ids: user_ids, - MyID: params.UserID, - }) - - if err != nil { - return nil, err - } - - managers := make([]FullManager, len(grants)) - for i, grant := range grants { - managers[i] = FullManager{ - Manager: users[int32(grant.GranteeUserID)], - Grant: FullGrant{GetGrantsForUserIdRow: grant}, - } - } - - return managers, nil -} diff --git a/api/dbv1/get_events.sql.go b/api/dbv1/get_events.sql.go index 4251b086..cffde73d 100644 --- a/api/dbv1/get_events.sql.go +++ b/api/dbv1/get_events.sql.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.28.0 +// sqlc v1.29.0 // source: get_events.sql package dbv1 diff --git a/api/dbv1/get_grants.sql.go b/api/dbv1/get_grants.sql.go index c48554fa..513acacb 100644 --- a/api/dbv1/get_grants.sql.go +++ b/api/dbv1/get_grants.sql.go @@ -13,7 +13,7 @@ import ( "github.com/jackc/pgx/v5/pgtype" ) -const getGrantsForGranteeAddress = `-- name: GetGrantsForGranteeAddress :many +const getGrantsForGranteeUserId = `-- name: GetGrantsForGranteeUserId :many SELECT g.user_id, g.grantee_address, @@ -22,22 +22,21 @@ SELECT g.created_at, g.updated_at, u.user_id as grantee_user_id -FROM grants g -JOIN users u ON u.wallet = g.grantee_address -WHERE g.grantee_address = $1 +FROM users u +JOIN grants g ON g.grantee_address = u.wallet +WHERE u.user_id = $1::int AND g.is_current = true AND g.is_revoked = $2 AND $3::boolean IS NULL OR g.is_approved = $3 -ORDER BY g.created_at DESC ` -type GetGrantsForGranteeAddressParams struct { - GranteeAddress string `json:"grantee_address"` - IsRevoked bool `json:"is_revoked"` - IsApproved pgtype.Bool `json:"is_approved"` +type GetGrantsForGranteeUserIdParams struct { + UserID int32 `json:"user_id"` + IsRevoked bool `json:"is_revoked"` + IsApproved pgtype.Bool `json:"is_approved"` } -type GetGrantsForGranteeAddressRow struct { +type GetGrantsForGranteeUserIdRow struct { UserID trashid.HashId `json:"user_id"` GranteeAddress string `json:"grantee_address"` IsRevoked bool `json:"is_revoked"` @@ -47,15 +46,15 @@ type GetGrantsForGranteeAddressRow struct { GranteeUserID trashid.HashId `json:"grantee_user_id"` } -func (q *Queries) GetGrantsForGranteeAddress(ctx context.Context, arg GetGrantsForGranteeAddressParams) ([]GetGrantsForGranteeAddressRow, error) { - rows, err := q.db.Query(ctx, getGrantsForGranteeAddress, arg.GranteeAddress, arg.IsRevoked, arg.IsApproved) +func (q *Queries) GetGrantsForGranteeUserId(ctx context.Context, arg GetGrantsForGranteeUserIdParams) ([]GetGrantsForGranteeUserIdRow, error) { + rows, err := q.db.Query(ctx, getGrantsForGranteeUserId, arg.UserID, arg.IsRevoked, arg.IsApproved) if err != nil { return nil, err } defer rows.Close() - var items []GetGrantsForGranteeAddressRow + var items []GetGrantsForGranteeUserIdRow for rows.Next() { - var i GetGrantsForGranteeAddressRow + var i GetGrantsForGranteeUserIdRow if err := rows.Scan( &i.UserID, &i.GranteeAddress, @@ -90,7 +89,6 @@ WHERE g.user_id = $1::int AND g.is_revoked = $2 AND g.is_current = true AND $3::boolean IS NULL OR g.is_approved = $3 -ORDER BY g.created_at DESC ` type GetGrantsForUserIdParams struct { diff --git a/api/dbv1/managers.go b/api/dbv1/managers.go new file mode 100644 index 00000000..9832294c --- /dev/null +++ b/api/dbv1/managers.go @@ -0,0 +1,92 @@ +package dbv1 + +import ( + "context" +) + +type FullManagerGrant struct { + GetGrantsForUserIdRow + GranteeUserID *struct{} `json:"grantee_user_id,omitempty"` +} + +type FullManagedUserGrant struct { + GetGrantsForGranteeUserIdRow + GranteeUserID *struct{} `json:"grantee_user_id,omitempty"` +} + +type FullManager struct { + Manager FullUser `json:"manager"` + Grant FullManagerGrant `json:"grant"` +} + +type FullManagedUser struct { + User FullUser `json:"user"` + Grant FullManagedUserGrant `json:"grant"` +} + +func (q *Queries) FullManagers(ctx context.Context, params GetGrantsForUserIdParams) ([]FullManager, error) { + + grants, err := q.GetGrantsForUserId(ctx, params) + if err != nil { + return nil, err + } + + user_ids := make([]int32, len(grants)) + for i, grant := range grants { + user_ids[i] = int32(grant.GranteeUserID) + } + + users, err := q.FullUsersKeyed(ctx, GetUsersParams{ + Ids: user_ids, + MyID: params.UserID, + }) + + if err != nil { + return nil, err + } + + managers := make([]FullManager, len(grants)) + for i, grant := range grants { + managers[i] = FullManager{ + Manager: users[int32(grant.GranteeUserID)], + Grant: FullManagerGrant{GetGrantsForUserIdRow: grant}, + } + } + + return managers, nil +} + +func (q *Queries) FullManagedUsers(ctx context.Context, params GetGrantsForGranteeUserIdParams) ([]FullManagedUser, error) { + grants, err := q.GetGrantsForGranteeUserId(ctx, GetGrantsForGranteeUserIdParams{ + IsRevoked: params.IsRevoked, + IsApproved: params.IsApproved, + UserID: params.UserID, + }) + if err != nil { + return nil, err + } + + user_ids := make([]int32, len(grants)) + for i, grant := range grants { + user_ids[i] = int32(grant.UserID) + } + + users, err := q.FullUsersKeyed(ctx, GetUsersParams{ + Ids: user_ids, + MyID: params.UserID, + }) + + if err != nil { + return nil, err + } + + managedUsers := make([]FullManagedUser, len(grants)) + for i, grant := range grants { + managedUsers[i] = FullManagedUser{ + User: users[int32(grant.UserID)], + Grant: FullManagedUserGrant{GetGrantsForGranteeUserIdRow: grant}, + } + } + + return managedUsers, nil +} diff --git a/api/dbv1/queries/get_grants.sql b/api/dbv1/queries/get_grants.sql index f9d5151b..fdd85f69 100644 --- a/api/dbv1/queries/get_grants.sql +++ b/api/dbv1/queries/get_grants.sql @@ -12,10 +12,9 @@ JOIN users u ON u.wallet = g.grantee_address WHERE g.user_id = @user_id::int AND g.is_revoked = @is_revoked AND g.is_current = true - AND sqlc.narg('is_approved')::boolean IS NULL OR g.is_approved = sqlc.narg('is_approved') -ORDER BY g.created_at DESC; + AND sqlc.narg('is_approved')::boolean IS NULL OR g.is_approved = sqlc.narg('is_approved'); --- name: GetGrantsForGranteeAddress :many +-- name: GetGrantsForGranteeUserId :many SELECT g.user_id, g.grantee_address, @@ -24,10 +23,9 @@ SELECT g.created_at, g.updated_at, u.user_id as grantee_user_id -FROM grants g -JOIN users u ON u.wallet = g.grantee_address -WHERE g.grantee_address = @grantee_address +FROM users u +JOIN grants g ON g.grantee_address = u.wallet +WHERE u.user_id = @user_id::int AND g.is_current = true AND g.is_revoked = @is_revoked - AND sqlc.narg('is_approved')::boolean IS NULL OR g.is_approved = sqlc.narg('is_approved') -ORDER BY g.created_at DESC; \ No newline at end of file + AND sqlc.narg('is_approved')::boolean IS NULL OR g.is_approved = sqlc.narg('is_approved'); \ No newline at end of file diff --git a/api/server.go b/api/server.go index 844df62e..6adab77a 100644 --- a/api/server.go +++ b/api/server.go @@ -226,6 +226,7 @@ func NewApiServer(config config.Config) *ApiServer { g.Get("/users/:userId/library/tracks", app.v1UsersLibraryTracks) g.Get("/users/:userId/library/:playlistType", app.v1UsersLibraryPlaylists) g.Get("/users/:userId/managers", app.v1UsersManagers) + g.Get("/users/:userId/managed_users", app.v1UsersManagedUsers) g.Get("/users/:userId/mutuals", app.v1UsersMutuals) g.Get("/users/:userId/reposts", app.v1UsersReposts) g.Get("/users/:userId/related", app.v1UsersRelated) diff --git a/api/v1_users_managed_users.go b/api/v1_users_managed_users.go new file mode 100644 index 00000000..66329974 --- /dev/null +++ b/api/v1_users_managed_users.go @@ -0,0 +1,41 @@ +package api + +import ( + "strconv" + + "bridgerton.audius.co/api/dbv1" + "github.com/gofiber/fiber/v2" + "github.com/jackc/pgx/v5/pgtype" +) + +func (app *ApiServer) v1UsersManagedUsers(c *fiber.Ctx) error { + // Behavior of this field is a little odd. We only want to filter by it + // if it is passed, but otherwise not use a default value for either. + var isApproved *bool + if approvedStr := c.Query("is_approved"); approvedStr != "" { + parsed, err := strconv.ParseBool(approvedStr) + if err != nil { + return fiber.NewError(fiber.StatusBadRequest, "Invalid value for is_approved") + } + isApproved = &parsed + } + + isRevoked, err := strconv.ParseBool(c.Query("is_revoked", "false")) + if err != nil { + return fiber.NewError(fiber.StatusBadRequest, "Invalid value for is_revoked") + } + params := dbv1.GetGrantsForGranteeUserIdParams{ + UserID: int32(c.Locals("userId").(int)), + IsApproved: pgtype.Bool{Bool: isApproved != nil && *isApproved, Valid: isApproved != nil}, + IsRevoked: isRevoked, + } + + managedUsers, err := app.queries.FullManagedUsers(c.Context(), params) + if err != nil { + return err + } + + return c.JSON(fiber.Map{ + "data": managedUsers, + }) +} From c68fb7a9c03cbef8f282217a4a5a5c0da7f01686 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Mon, 5 May 2025 17:17:41 -0400 Subject: [PATCH 2/6] add tests and fix bad queries --- api/dbv1/get_grants.sql.go | 4 +- api/dbv1/queries/get_grants.sql | 4 +- api/testdata/grants_fixtures.csv | 3 ++ api/v1_users_managed_users_test.go | 61 ++++++++++++++++++++++++++++++ api/v1_users_managers_test.go | 4 +- 5 files changed, 70 insertions(+), 6 deletions(-) create mode 100644 api/v1_users_managed_users_test.go diff --git a/api/dbv1/get_grants.sql.go b/api/dbv1/get_grants.sql.go index 513acacb..76d1bf4e 100644 --- a/api/dbv1/get_grants.sql.go +++ b/api/dbv1/get_grants.sql.go @@ -27,7 +27,7 @@ JOIN grants g ON g.grantee_address = u.wallet WHERE u.user_id = $1::int AND g.is_current = true AND g.is_revoked = $2 - AND $3::boolean IS NULL OR g.is_approved = $3 + AND (g.is_approved = $3 OR $3 IS NULL) ` type GetGrantsForGranteeUserIdParams struct { @@ -88,7 +88,7 @@ JOIN users u ON u.wallet = g.grantee_address WHERE g.user_id = $1::int AND g.is_revoked = $2 AND g.is_current = true - AND $3::boolean IS NULL OR g.is_approved = $3 + AND (g.is_approved = $3 OR $3 IS NULL) ` type GetGrantsForUserIdParams struct { diff --git a/api/dbv1/queries/get_grants.sql b/api/dbv1/queries/get_grants.sql index fdd85f69..73793d16 100644 --- a/api/dbv1/queries/get_grants.sql +++ b/api/dbv1/queries/get_grants.sql @@ -12,7 +12,7 @@ JOIN users u ON u.wallet = g.grantee_address WHERE g.user_id = @user_id::int AND g.is_revoked = @is_revoked AND g.is_current = true - AND sqlc.narg('is_approved')::boolean IS NULL OR g.is_approved = sqlc.narg('is_approved'); + AND (g.is_approved = sqlc.narg('is_approved') OR sqlc.narg('is_approved') IS NULL); -- name: GetGrantsForGranteeUserId :many SELECT @@ -28,4 +28,4 @@ JOIN grants g ON g.grantee_address = u.wallet WHERE u.user_id = @user_id::int AND g.is_current = true AND g.is_revoked = @is_revoked - AND sqlc.narg('is_approved')::boolean IS NULL OR g.is_approved = sqlc.narg('is_approved'); \ No newline at end of file + AND (g.is_approved = sqlc.narg('is_approved') OR sqlc.narg('is_approved') IS NULL); \ No newline at end of file diff --git a/api/testdata/grants_fixtures.csv b/api/testdata/grants_fixtures.csv index ac750e8b..de842b1b 100644 --- a/api/testdata/grants_fixtures.csv +++ b/api/testdata/grants_fixtures.csv @@ -3,3 +3,6 @@ user_id,grantee_address,is_approved,is_revoked 1,0xc451c1f8943b575158310552b41230c61844a1c1,false,true 1,0x1234567890abcdef,true,true 1,0x681c616ae836ceca1effe00bd07f2fdbf9a082bc,false,false +2,0x681c616ae836ceca1effe00bd07f2fdbf9a082bc,true,false +3,0x681c616ae836ceca1effe00bd07f2fdbf9a082bc,true,true +4,0x681c616ae836ceca1effe00bd07f2fdbf9a082bc,false,true \ No newline at end of file diff --git a/api/v1_users_managed_users_test.go b/api/v1_users_managed_users_test.go new file mode 100644 index 00000000..ec2c9dd9 --- /dev/null +++ b/api/v1_users_managed_users_test.go @@ -0,0 +1,61 @@ +package api + +import ( + "testing" + + "bridgerton.audius.co/api/dbv1" + "github.com/stretchr/testify/assert" +) + +// Defaults to all approval status and no revoked managers +func TestGetManagedUsersNoParams(t *testing.T) { + var managedUsersResponse struct { + Data []dbv1.FullManagedUser + } + status, _ := testGet(t, "/v1/users/eYZmn/managed_users", &managedUsersResponse) + assert.Equal(t, 200, status) + assert.Equal(t, 2, len(managedUsersResponse.Data)) + + assert.Equal(t, 1, int(managedUsersResponse.Data[0].User.ID)) + assert.Equal(t, false, managedUsersResponse.Data[0].Grant.IsApproved.Bool) + assert.Equal(t, 2, int(managedUsersResponse.Data[1].User.ID)) + assert.Equal(t, true, managedUsersResponse.Data[1].Grant.IsApproved.Bool) +} + +// Should return only approved managers and default to not showing revoked managers +func TestGetManagedUsersApproved(t *testing.T) { + var managedUsersResponse struct { + Data []dbv1.FullManagedUser + } + status, _ := testGet(t, "/v1/users/eYZmn/managed_users?is_approved=true", &managedUsersResponse) + assert.Equal(t, 200, status) + assert.Equal(t, 1, len(managedUsersResponse.Data)) + assert.Equal(t, 2, int(managedUsersResponse.Data[0].User.ID)) + assert.Equal(t, true, managedUsersResponse.Data[0].Grant.IsApproved.Bool) +} + +func TestGetManagedUsersRevoked(t *testing.T) { + var managedUsersResponse struct { + Data []dbv1.FullManagedUser + } + status, _ := testGet(t, "/v1/users/eYZmn/managed_users?is_revoked=true", &managedUsersResponse) + assert.Equal(t, 200, status) + assert.Equal(t, 2, len(managedUsersResponse.Data)) + assert.Equal(t, 3, int(managedUsersResponse.Data[0].User.ID)) + assert.Equal(t, true, managedUsersResponse.Data[0].Grant.IsApproved.Bool) + assert.Equal(t, true, managedUsersResponse.Data[0].Grant.IsRevoked) + assert.Equal(t, 4, int(managedUsersResponse.Data[1].User.ID)) + assert.Equal(t, false, managedUsersResponse.Data[1].Grant.IsApproved.Bool) + assert.Equal(t, true, managedUsersResponse.Data[1].Grant.IsRevoked) +} + +func TestGetManagedUsersInvalidParams(t *testing.T) { + var managedUsersResponse struct { + Data []dbv1.FullManagedUser + } + status, _ := testGet(t, "/v1/users/eYZmn/managed_users?is_approved=invalid", &managedUsersResponse) + assert.Equal(t, 400, status) + + status, _ = testGet(t, "/v1/users/eYZmn/managed_users?is_revoked=invalid", &managedUsersResponse) + assert.Equal(t, 400, status) +} diff --git a/api/v1_users_managers_test.go b/api/v1_users_managers_test.go index 37b56f11..92e2cd65 100644 --- a/api/v1_users_managers_test.go +++ b/api/v1_users_managers_test.go @@ -29,7 +29,7 @@ func TestGetUsersManagersApproved(t *testing.T) { } status, _ := testGet(t, "/v1/users/7eP5n/managers?is_approved=true", &managersResponse) assert.Equal(t, 200, status) - assert.Equal(t, 2, len(managersResponse.Data)) + assert.Equal(t, 1, len(managersResponse.Data)) assert.Equal(t, "0x5f1a372b28956c8363f8bc3a231a6e9e1186ead8", managersResponse.Data[0].Manager.Wallet.String) assert.Equal(t, true, managersResponse.Data[0].Grant.IsApproved.Bool) } @@ -49,7 +49,7 @@ func TestGetUsersManagersRevoked(t *testing.T) { assert.Equal(t, true, managersResponse.Data[1].Grant.IsRevoked) } -func TestInvalidParams(t *testing.T) { +func TestGetUsersManagersInvalidParams(t *testing.T) { var managersResponse struct { Data []dbv1.FullManager } From 7a4a4d763b4d8faf2ec69be895d86de6fa99812a Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Mon, 5 May 2025 17:21:10 -0400 Subject: [PATCH 3/6] 200 test for managed_users endpoint --- api/server_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/api/server_test.go b/api/server_test.go index 2a193c0d..4afc6c1f 100644 --- a/api/server_test.go +++ b/api/server_test.go @@ -119,6 +119,7 @@ func Test200UnAuthed(t *testing.T) { "/v1/full/users/7eP5n/library/albums?type=purchase&sort_method=saves", "/v1/full/users/7eP5n/managers", + "/v1/full/users/7eP5n/managed_users", "/v1/full/users/7eP5n/mutuals", "/v1/full/users/7eP5n/reposts", "/v1/full/users/7eP5n/related", From d242a542b59e1f8a846f5107b208afcfda65c2e6 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Tue, 6 May 2025 10:11:37 -0400 Subject: [PATCH 4/6] PR feedback --- api/dbv1/get_grants.sql.go | 4 +-- api/dbv1/queries/get_grants.sql | 4 +-- api/resolve_middleware.go | 8 ++++++ api/v1_users_managed_users.go | 2 +- api/v1_users_managed_users_test.go | 39 ++++++++++++++++++------------ api/v1_users_managers.go | 2 +- api/v1_users_managers_test.go | 38 +++++++++++++++++------------ 7 files changed, 61 insertions(+), 36 deletions(-) diff --git a/api/dbv1/get_grants.sql.go b/api/dbv1/get_grants.sql.go index 76d1bf4e..c2f6beb8 100644 --- a/api/dbv1/get_grants.sql.go +++ b/api/dbv1/get_grants.sql.go @@ -27,7 +27,7 @@ JOIN grants g ON g.grantee_address = u.wallet WHERE u.user_id = $1::int AND g.is_current = true AND g.is_revoked = $2 - AND (g.is_approved = $3 OR $3 IS NULL) + AND ($3::boolean IS NULL OR g.is_approved = $3) ` type GetGrantsForGranteeUserIdParams struct { @@ -88,7 +88,7 @@ JOIN users u ON u.wallet = g.grantee_address WHERE g.user_id = $1::int AND g.is_revoked = $2 AND g.is_current = true - AND (g.is_approved = $3 OR $3 IS NULL) + AND ($3::boolean IS NULL OR g.is_approved = $3) ` type GetGrantsForUserIdParams struct { diff --git a/api/dbv1/queries/get_grants.sql b/api/dbv1/queries/get_grants.sql index 73793d16..f33b0a57 100644 --- a/api/dbv1/queries/get_grants.sql +++ b/api/dbv1/queries/get_grants.sql @@ -12,7 +12,7 @@ JOIN users u ON u.wallet = g.grantee_address WHERE g.user_id = @user_id::int AND g.is_revoked = @is_revoked AND g.is_current = true - AND (g.is_approved = sqlc.narg('is_approved') OR sqlc.narg('is_approved') IS NULL); + AND (sqlc.narg('is_approved')::boolean IS NULL OR g.is_approved = sqlc.narg('is_approved')); -- name: GetGrantsForGranteeUserId :many SELECT @@ -28,4 +28,4 @@ JOIN grants g ON g.grantee_address = u.wallet WHERE u.user_id = @user_id::int AND g.is_current = true AND g.is_revoked = @is_revoked - AND (g.is_approved = sqlc.narg('is_approved') OR sqlc.narg('is_approved') IS NULL); \ No newline at end of file + AND (sqlc.narg('is_approved')::boolean IS NULL OR g.is_approved = sqlc.narg('is_approved')); \ No newline at end of file diff --git a/api/resolve_middleware.go b/api/resolve_middleware.go index 916e0fb9..bc5e1636 100644 --- a/api/resolve_middleware.go +++ b/api/resolve_middleware.go @@ -33,6 +33,14 @@ func (app *ApiServer) getMyId(c *fiber.Ctx) int32 { return int32(myId.(int)) } +func (app *ApiServer) getUserId(c *fiber.Ctx) int32 { + userId := c.Locals("userId") + if userId == nil { + return 0 + } + return int32(userId.(int)) +} + func (app *ApiServer) requireUserIdMiddleware(c *fiber.Ctx) error { userId, err := trashid.DecodeHashId(c.Params("userId")) if err != nil || userId == 0 { diff --git a/api/v1_users_managed_users.go b/api/v1_users_managed_users.go index 66329974..343f15d8 100644 --- a/api/v1_users_managed_users.go +++ b/api/v1_users_managed_users.go @@ -25,7 +25,7 @@ func (app *ApiServer) v1UsersManagedUsers(c *fiber.Ctx) error { return fiber.NewError(fiber.StatusBadRequest, "Invalid value for is_revoked") } params := dbv1.GetGrantsForGranteeUserIdParams{ - UserID: int32(c.Locals("userId").(int)), + UserID: app.getUserId(c), IsApproved: pgtype.Bool{Bool: isApproved != nil && *isApproved, Valid: isApproved != nil}, IsRevoked: isRevoked, } diff --git a/api/v1_users_managed_users_test.go b/api/v1_users_managed_users_test.go index ec2c9dd9..926c3861 100644 --- a/api/v1_users_managed_users_test.go +++ b/api/v1_users_managed_users_test.go @@ -4,6 +4,7 @@ import ( "testing" "bridgerton.audius.co/api/dbv1" + "bridgerton.audius.co/trashid" "github.com/stretchr/testify/assert" ) @@ -12,14 +13,16 @@ func TestGetManagedUsersNoParams(t *testing.T) { var managedUsersResponse struct { Data []dbv1.FullManagedUser } - status, _ := testGet(t, "/v1/users/eYZmn/managed_users", &managedUsersResponse) + status, body := testGet(t, "/v1/users/eYZmn/managed_users", &managedUsersResponse) assert.Equal(t, 200, status) assert.Equal(t, 2, len(managedUsersResponse.Data)) - assert.Equal(t, 1, int(managedUsersResponse.Data[0].User.ID)) - assert.Equal(t, false, managedUsersResponse.Data[0].Grant.IsApproved.Bool) - assert.Equal(t, 2, int(managedUsersResponse.Data[1].User.ID)) - assert.Equal(t, true, managedUsersResponse.Data[1].Grant.IsApproved.Bool) + jsonAssert(t, body, map[string]string{ + "data.0.user.id": trashid.MustEncodeHashID(1), + "data.0.grant.is_approved": "false", + "data.1.user.id": trashid.MustEncodeHashID(2), + "data.1.grant.is_approved": "true", + }) } // Should return only approved managers and default to not showing revoked managers @@ -27,26 +30,32 @@ func TestGetManagedUsersApproved(t *testing.T) { var managedUsersResponse struct { Data []dbv1.FullManagedUser } - status, _ := testGet(t, "/v1/users/eYZmn/managed_users?is_approved=true", &managedUsersResponse) + status, body := testGet(t, "/v1/users/eYZmn/managed_users?is_approved=true", &managedUsersResponse) assert.Equal(t, 200, status) assert.Equal(t, 1, len(managedUsersResponse.Data)) - assert.Equal(t, 2, int(managedUsersResponse.Data[0].User.ID)) - assert.Equal(t, true, managedUsersResponse.Data[0].Grant.IsApproved.Bool) + + jsonAssert(t, body, map[string]string{ + "data.0.user.id": trashid.MustEncodeHashID(2), + "data.0.grant.is_approved": "true", + }) } func TestGetManagedUsersRevoked(t *testing.T) { var managedUsersResponse struct { Data []dbv1.FullManagedUser } - status, _ := testGet(t, "/v1/users/eYZmn/managed_users?is_revoked=true", &managedUsersResponse) + status, body := testGet(t, "/v1/users/eYZmn/managed_users?is_revoked=true", &managedUsersResponse) assert.Equal(t, 200, status) assert.Equal(t, 2, len(managedUsersResponse.Data)) - assert.Equal(t, 3, int(managedUsersResponse.Data[0].User.ID)) - assert.Equal(t, true, managedUsersResponse.Data[0].Grant.IsApproved.Bool) - assert.Equal(t, true, managedUsersResponse.Data[0].Grant.IsRevoked) - assert.Equal(t, 4, int(managedUsersResponse.Data[1].User.ID)) - assert.Equal(t, false, managedUsersResponse.Data[1].Grant.IsApproved.Bool) - assert.Equal(t, true, managedUsersResponse.Data[1].Grant.IsRevoked) + + jsonAssert(t, body, map[string]string{ + "data.0.user.id": trashid.MustEncodeHashID(3), + "data.0.grant.is_approved": "true", + "data.0.grant.is_revoked": "true", + "data.1.user.id": trashid.MustEncodeHashID(4), + "data.1.grant.is_approved": "false", + "data.1.grant.is_revoked": "true", + }) } func TestGetManagedUsersInvalidParams(t *testing.T) { diff --git a/api/v1_users_managers.go b/api/v1_users_managers.go index 5821ac98..23bd1e5d 100644 --- a/api/v1_users_managers.go +++ b/api/v1_users_managers.go @@ -25,7 +25,7 @@ func (app *ApiServer) v1UsersManagers(c *fiber.Ctx) error { return fiber.NewError(fiber.StatusBadRequest, "Invalid value for is_revoked") } params := dbv1.GetGrantsForUserIdParams{ - UserID: int32(c.Locals("userId").(int)), + UserID: app.getUserId(c), IsApproved: pgtype.Bool{Bool: isApproved != nil && *isApproved, Valid: isApproved != nil}, IsRevoked: isRevoked, } diff --git a/api/v1_users_managers_test.go b/api/v1_users_managers_test.go index 92e2cd65..17d10f8d 100644 --- a/api/v1_users_managers_test.go +++ b/api/v1_users_managers_test.go @@ -12,14 +12,16 @@ func TestGetUsersManagersNoParams(t *testing.T) { var managersResponse struct { Data []dbv1.FullManager } - status, _ := testGet(t, "/v1/users/7eP5n/managers", &managersResponse) + status, body := testGet(t, "/v1/users/7eP5n/managers", &managersResponse) assert.Equal(t, 200, status) assert.Equal(t, 2, len(managersResponse.Data)) - assert.Equal(t, "0x5f1a372b28956c8363f8bc3a231a6e9e1186ead8", managersResponse.Data[0].Manager.Wallet.String) - assert.Equal(t, true, managersResponse.Data[0].Grant.IsApproved.Bool) - assert.Equal(t, "0x681c616ae836ceca1effe00bd07f2fdbf9a082bc", managersResponse.Data[1].Manager.Wallet.String) - assert.Equal(t, false, managersResponse.Data[1].Grant.IsApproved.Bool) + jsonAssert(t, body, map[string]string{ + "data.0.manager.wallet": "0x5f1a372b28956c8363f8bc3a231a6e9e1186ead8", + "data.0.grant.is_approved": "true", + "data.1.manager.wallet": "0x681c616ae836ceca1effe00bd07f2fdbf9a082bc", + "data.1.grant.is_approved": "false", + }) } // Should return only approved managers and default to not showing revoked managers @@ -27,26 +29,32 @@ func TestGetUsersManagersApproved(t *testing.T) { var managersResponse struct { Data []dbv1.FullManager } - status, _ := testGet(t, "/v1/users/7eP5n/managers?is_approved=true", &managersResponse) + status, body := testGet(t, "/v1/users/7eP5n/managers?is_approved=true", &managersResponse) assert.Equal(t, 200, status) assert.Equal(t, 1, len(managersResponse.Data)) - assert.Equal(t, "0x5f1a372b28956c8363f8bc3a231a6e9e1186ead8", managersResponse.Data[0].Manager.Wallet.String) - assert.Equal(t, true, managersResponse.Data[0].Grant.IsApproved.Bool) + + jsonAssert(t, body, map[string]string{ + "data.0.manager.wallet": "0x5f1a372b28956c8363f8bc3a231a6e9e1186ead8", + "data.0.grant.is_approved": "true", + }) } func TestGetUsersManagersRevoked(t *testing.T) { var managersResponse struct { Data []dbv1.FullManager } - status, _ := testGet(t, "/v1/users/7eP5n/managers?is_revoked=true", &managersResponse) + status, body := testGet(t, "/v1/users/7eP5n/managers?is_revoked=true", &managersResponse) assert.Equal(t, 200, status) assert.Equal(t, 2, len(managersResponse.Data)) - assert.Equal(t, "0xc451c1f8943b575158310552b41230c61844a1c1", managersResponse.Data[0].Manager.Wallet.String) - assert.Equal(t, false, managersResponse.Data[0].Grant.IsApproved.Bool) - assert.Equal(t, true, managersResponse.Data[0].Grant.IsRevoked) - assert.Equal(t, "0x1234567890abcdef", managersResponse.Data[1].Manager.Wallet.String) - assert.Equal(t, true, managersResponse.Data[1].Grant.IsApproved.Bool) - assert.Equal(t, true, managersResponse.Data[1].Grant.IsRevoked) + + jsonAssert(t, body, map[string]string{ + "data.0.manager.wallet": "0xc451c1f8943b575158310552b41230c61844a1c1", + "data.0.grant.is_approved": "false", + "data.0.grant.is_revoked": "true", + "data.1.manager.wallet": "0x1234567890abcdef", + "data.1.grant.is_approved": "true", + "data.1.grant.is_revoked": "true", + }) } func TestGetUsersManagersInvalidParams(t *testing.T) { From e3ea04076d29c3aa9c54e98ec46565058207a57e Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Tue, 6 May 2025 10:26:34 -0400 Subject: [PATCH 5/6] optional bool helper --- api/request_helpers.go | 20 ++++++++++++++++++++ api/v1_users_managed_users.go | 13 ++++--------- api/v1_users_managers.go | 13 ++++--------- 3 files changed, 28 insertions(+), 18 deletions(-) create mode 100644 api/request_helpers.go diff --git a/api/request_helpers.go b/api/request_helpers.go new file mode 100644 index 00000000..1ee57847 --- /dev/null +++ b/api/request_helpers.go @@ -0,0 +1,20 @@ +package api + +import ( + "strconv" + + "github.com/gofiber/fiber/v2" + "github.com/jackc/pgx/v5/pgtype" +) + +func getOptionalBool(c *fiber.Ctx, key string) (pgtype.Bool, error) { + var value *bool + if valueStr := c.Query(key); valueStr != "" { + parsed, err := strconv.ParseBool(valueStr) + if err != nil { + return pgtype.Bool{}, err + } + value = &parsed + } + return pgtype.Bool{Bool: value != nil && *value, Valid: value != nil}, nil +} diff --git a/api/v1_users_managed_users.go b/api/v1_users_managed_users.go index 343f15d8..01534439 100644 --- a/api/v1_users_managed_users.go +++ b/api/v1_users_managed_users.go @@ -5,19 +5,14 @@ import ( "bridgerton.audius.co/api/dbv1" "github.com/gofiber/fiber/v2" - "github.com/jackc/pgx/v5/pgtype" ) func (app *ApiServer) v1UsersManagedUsers(c *fiber.Ctx) error { // Behavior of this field is a little odd. We only want to filter by it // if it is passed, but otherwise not use a default value for either. - var isApproved *bool - if approvedStr := c.Query("is_approved"); approvedStr != "" { - parsed, err := strconv.ParseBool(approvedStr) - if err != nil { - return fiber.NewError(fiber.StatusBadRequest, "Invalid value for is_approved") - } - isApproved = &parsed + isApproved, err := getOptionalBool(c, "is_approved") + if err != nil { + return fiber.NewError(fiber.StatusBadRequest, "Invalid value for is_approved") } isRevoked, err := strconv.ParseBool(c.Query("is_revoked", "false")) @@ -26,7 +21,7 @@ func (app *ApiServer) v1UsersManagedUsers(c *fiber.Ctx) error { } params := dbv1.GetGrantsForGranteeUserIdParams{ UserID: app.getUserId(c), - IsApproved: pgtype.Bool{Bool: isApproved != nil && *isApproved, Valid: isApproved != nil}, + IsApproved: isApproved, IsRevoked: isRevoked, } diff --git a/api/v1_users_managers.go b/api/v1_users_managers.go index 23bd1e5d..1a2f0b24 100644 --- a/api/v1_users_managers.go +++ b/api/v1_users_managers.go @@ -5,19 +5,14 @@ import ( "bridgerton.audius.co/api/dbv1" "github.com/gofiber/fiber/v2" - "github.com/jackc/pgx/v5/pgtype" ) func (app *ApiServer) v1UsersManagers(c *fiber.Ctx) error { // Behavior of this field is a little odd. We only want to filter by it // if it is passed, but otherwise not use a default value for either. - var isApproved *bool - if approvedStr := c.Query("is_approved"); approvedStr != "" { - parsed, err := strconv.ParseBool(approvedStr) - if err != nil { - return fiber.NewError(fiber.StatusBadRequest, "Invalid value for is_approved") - } - isApproved = &parsed + isApproved, err := getOptionalBool(c, "is_approved") + if err != nil { + return fiber.NewError(fiber.StatusBadRequest, "Invalid value for is_approved") } isRevoked, err := strconv.ParseBool(c.Query("is_revoked", "false")) @@ -26,7 +21,7 @@ func (app *ApiServer) v1UsersManagers(c *fiber.Ctx) error { } params := dbv1.GetGrantsForUserIdParams{ UserID: app.getUserId(c), - IsApproved: pgtype.Bool{Bool: isApproved != nil && *isApproved, Valid: isApproved != nil}, + IsApproved: isApproved, IsRevoked: isRevoked, } From 12dd70a6b5f559fa92ab05be9451ce259a9cb87a Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Tue, 6 May 2025 10:30:02 -0400 Subject: [PATCH 6/6] fix tests with new jsonAssert --- api/v1_users_managed_users_test.go | 20 ++++++++++---------- api/v1_users_managers_test.go | 20 ++++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/api/v1_users_managed_users_test.go b/api/v1_users_managed_users_test.go index 926c3861..82e3f928 100644 --- a/api/v1_users_managed_users_test.go +++ b/api/v1_users_managed_users_test.go @@ -17,11 +17,11 @@ func TestGetManagedUsersNoParams(t *testing.T) { assert.Equal(t, 200, status) assert.Equal(t, 2, len(managedUsersResponse.Data)) - jsonAssert(t, body, map[string]string{ + jsonAssert(t, body, map[string]any{ "data.0.user.id": trashid.MustEncodeHashID(1), - "data.0.grant.is_approved": "false", + "data.0.grant.is_approved": false, "data.1.user.id": trashid.MustEncodeHashID(2), - "data.1.grant.is_approved": "true", + "data.1.grant.is_approved": true, }) } @@ -34,9 +34,9 @@ func TestGetManagedUsersApproved(t *testing.T) { assert.Equal(t, 200, status) assert.Equal(t, 1, len(managedUsersResponse.Data)) - jsonAssert(t, body, map[string]string{ + jsonAssert(t, body, map[string]any{ "data.0.user.id": trashid.MustEncodeHashID(2), - "data.0.grant.is_approved": "true", + "data.0.grant.is_approved": true, }) } @@ -48,13 +48,13 @@ func TestGetManagedUsersRevoked(t *testing.T) { assert.Equal(t, 200, status) assert.Equal(t, 2, len(managedUsersResponse.Data)) - jsonAssert(t, body, map[string]string{ + jsonAssert(t, body, map[string]any{ "data.0.user.id": trashid.MustEncodeHashID(3), - "data.0.grant.is_approved": "true", - "data.0.grant.is_revoked": "true", + "data.0.grant.is_approved": true, + "data.0.grant.is_revoked": true, "data.1.user.id": trashid.MustEncodeHashID(4), - "data.1.grant.is_approved": "false", - "data.1.grant.is_revoked": "true", + "data.1.grant.is_approved": false, + "data.1.grant.is_revoked": true, }) } diff --git a/api/v1_users_managers_test.go b/api/v1_users_managers_test.go index 17d10f8d..c5efd54e 100644 --- a/api/v1_users_managers_test.go +++ b/api/v1_users_managers_test.go @@ -16,11 +16,11 @@ func TestGetUsersManagersNoParams(t *testing.T) { assert.Equal(t, 200, status) assert.Equal(t, 2, len(managersResponse.Data)) - jsonAssert(t, body, map[string]string{ + jsonAssert(t, body, map[string]any{ "data.0.manager.wallet": "0x5f1a372b28956c8363f8bc3a231a6e9e1186ead8", - "data.0.grant.is_approved": "true", + "data.0.grant.is_approved": true, "data.1.manager.wallet": "0x681c616ae836ceca1effe00bd07f2fdbf9a082bc", - "data.1.grant.is_approved": "false", + "data.1.grant.is_approved": false, }) } @@ -33,9 +33,9 @@ func TestGetUsersManagersApproved(t *testing.T) { assert.Equal(t, 200, status) assert.Equal(t, 1, len(managersResponse.Data)) - jsonAssert(t, body, map[string]string{ + jsonAssert(t, body, map[string]any{ "data.0.manager.wallet": "0x5f1a372b28956c8363f8bc3a231a6e9e1186ead8", - "data.0.grant.is_approved": "true", + "data.0.grant.is_approved": true, }) } @@ -47,13 +47,13 @@ func TestGetUsersManagersRevoked(t *testing.T) { assert.Equal(t, 200, status) assert.Equal(t, 2, len(managersResponse.Data)) - jsonAssert(t, body, map[string]string{ + jsonAssert(t, body, map[string]any{ "data.0.manager.wallet": "0xc451c1f8943b575158310552b41230c61844a1c1", - "data.0.grant.is_approved": "false", - "data.0.grant.is_revoked": "true", + "data.0.grant.is_approved": false, + "data.0.grant.is_revoked": true, "data.1.manager.wallet": "0x1234567890abcdef", - "data.1.grant.is_approved": "true", - "data.1.grant.is_revoked": "true", + "data.1.grant.is_approved": true, + "data.1.grant.is_revoked": true, }) }