From 5c57959cefc8859b177a5f7906dc1060be1632b3 Mon Sep 17 00:00:00 2001 From: Zackary Lassetter <93090968+zacklassetter@users.noreply.github.com> Date: Sun, 21 Jan 2024 15:04:45 -0500 Subject: [PATCH] SAC-6 Get User GET (#14) Co-authored-by: edwinliiiii Co-authored-by: edwinliiiii <91173669+edwinliiiii@users.noreply.github.com> Co-authored-by: Garrett Ladley <92384606+garrettladley@users.noreply.github.com> Co-authored-by: garrettladley --- .github/workflows/go.yml | 5 +-- backend/src/controllers/user.go | 21 +++++++++ backend/src/models/user.go | 2 +- backend/src/server/server.go | 1 + backend/src/services/user.go | 13 +++++- backend/src/transactions/user.go | 17 +++++++- backend/tests/api/README.md | 12 ++++-- backend/tests/api/category_test.go | 10 +++-- backend/tests/api/health_test.go | 2 +- backend/tests/api/helpers.go | 14 ++++++ backend/tests/api/tag_test.go | 26 +++++------ backend/tests/api/user_test.go | 69 +++++++++++++++++++++++++++++- cli/commands/test.go | 5 +-- 13 files changed, 164 insertions(+), 33 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 34c75bd09..1801e281f 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -69,9 +69,8 @@ jobs: - name: Install Dependencies run: cd backend && go get ./... - name: Migrate DB - run: | - cd backend/src && go run main.go --only-migrate + run: cd backend/src && go run main.go --only-migrate - name: Run Tests with Coverage - run: cd backend && go test -race -coverprofile=coverage.txt -covermode=atomic ./... + run: cd backend && go test -failfast -benchmem -race -coverprofile=coverage.txt ./... - name: Print Coverage run: cd backend && go tool cover -func=coverage.txt diff --git a/backend/src/controllers/user.go b/backend/src/controllers/user.go index c23ce0c0c..57e6802be 100644 --- a/backend/src/controllers/user.go +++ b/backend/src/controllers/user.go @@ -33,3 +33,24 @@ func (u *UserController) GetAllUsers(c *fiber.Ctx) error { return c.Status(fiber.StatusOK).JSON(users) } + +// GetUser godoc +// +// @Summary Gets specific user +// @Description Returns specific user +// @ID get-user +// @Tags user +// @Produce json +// @Success 200 {object} models.User +// @Failure 400 {string} string "failed to validate id" +// @Failure 404 {string} string "failed to find user" +// @Failure 500 {string} string +// @Router /api/v1/users/ [get] +func (u *UserController) GetUser(c *fiber.Ctx) error { + user, err := u.userService.GetUser(c.Params("id")) + if err != nil { + return err + } + + return c.Status(fiber.StatusOK).JSON(user) +} diff --git a/backend/src/models/user.go b/backend/src/models/user.go index 9bf454057..f5e4433e6 100644 --- a/backend/src/models/user.go +++ b/backend/src/models/user.go @@ -38,7 +38,7 @@ const ( type User struct { types.Model - Role UserRole `gorm:"type:varchar(255);" json:"user_role" validate:"required,max=255"` + Role UserRole `gorm:"type:varchar(255);" json:"user_role,omitempty" validate:"required,max=255"` NUID string `gorm:"column:nuid;type:varchar(9);unique" json:"nuid" validate:"required,numeric,len=9"` FirstName string `gorm:"type:varchar(255)" json:"first_name" validate:"required,max=255"` LastName string `gorm:"type:varchar(255)" json:"last_name" validate:"required,max=255"` diff --git a/backend/src/server/server.go b/backend/src/server/server.go index a59011369..1f6a8e20b 100644 --- a/backend/src/server/server.go +++ b/backend/src/server/server.go @@ -65,6 +65,7 @@ func userRoutes(router fiber.Router, userService services.UserServiceInterface) users := router.Group("/users") users.Get("/", userController.GetAllUsers) + users.Get("/:id", userController.GetUser) } func categoryRoutes(router fiber.Router, categoryService services.CategoryServiceInterface) { diff --git a/backend/src/services/user.go b/backend/src/services/user.go index fe27eb731..8e9e60ac1 100644 --- a/backend/src/services/user.go +++ b/backend/src/services/user.go @@ -3,19 +3,30 @@ package services import ( "github.com/GenerateNU/sac/backend/src/models" "github.com/GenerateNU/sac/backend/src/transactions" + "github.com/GenerateNU/sac/backend/src/utilities" "gorm.io/gorm" ) type UserServiceInterface interface { GetAllUsers() ([]models.User, error) + GetUser(string) (*models.User, error) } type UserService struct { DB *gorm.DB } -// Gets all users (including soft deleted users) for testing func (u *UserService) GetAllUsers() ([]models.User, error) { return transactions.GetAllUsers(u.DB) } + +func (u *UserService) GetUser(userID string) (*models.User, error) { + idAsUint, err := utilities.ValidateID(userID) + + if err != nil { + return nil, err + } + + return transactions.GetUser(u.DB, *idAsUint) +} diff --git a/backend/src/transactions/user.go b/backend/src/transactions/user.go index 3d15f1385..54792b068 100644 --- a/backend/src/transactions/user.go +++ b/backend/src/transactions/user.go @@ -1,6 +1,8 @@ package transactions import ( + "errors" + "github.com/GenerateNU/sac/backend/src/models" "github.com/gofiber/fiber/v2" @@ -10,9 +12,22 @@ import ( func GetAllUsers(db *gorm.DB) ([]models.User, error) { var users []models.User - if err := db.Unscoped().Omit("password_hash").Find(&users).Error; err != nil { + if err := db.Omit("password_hash").Find(&users).Error; err != nil { return nil, fiber.NewError(fiber.StatusInternalServerError, "failed to get all users") } return users, nil } + +func GetUser(db *gorm.DB, id uint) (*models.User, error) { + var user models.User + + if err := db.Omit("password_hash").First(&user, id).Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil, fiber.NewError(fiber.StatusNotFound, "failed to find tag") + } + return nil, fiber.NewError(fiber.StatusInternalServerError, "failed to get user") + } + + return &user, nil +} diff --git a/backend/tests/api/README.md b/backend/tests/api/README.md index 6621648c3..317eb2800 100644 --- a/backend/tests/api/README.md +++ b/backend/tests/api/README.md @@ -40,7 +40,7 @@ Say you want to test hitting the `[APP_ADDRESS]/health` endpoint with a GET requ TestRequest{ Method: "GET", Path: "/health", - }.TestOnStatus(t, nil, 200) + }.TestOnStatus(t, nil, 200).Close() ``` ## Testing that a Request Returns a XXX Status Code and Assert Something About the Database @@ -59,7 +59,7 @@ TestRequest{ Status: 201, DBTester: AssertRespCategorySameAsDBCategory, }, - ) + ).Close() ``` ### DBTesters @@ -114,9 +114,13 @@ TestRequest{ Status: 201, DBTester: AssertRespTagSameAsDBTag, }, - ) + ).Close() ``` +### Why Close? + +This closes the connection to the database. This is important because if you don't close the connection, we will run out of available connections and the tests will fail. **Call this on the last test request of a test** + ## Testing that a Request Returns a XXX Status Code, Assert Something About the Message, and Assert Something About the Database Say you want to test a bad request to POST `[APP_ADDRESS]/api/v1/categories/` endpoint returns a `400` status code, the message is `failed to process the request`, and that a category was not created. @@ -136,5 +140,5 @@ TestRequest{ }, DBTester: AssertNoCategories, }, - ) + ).Close() ``` diff --git a/backend/tests/api/category_test.go b/backend/tests/api/category_test.go index 6a11fa7a3..5ba99410c 100644 --- a/backend/tests/api/category_test.go +++ b/backend/tests/api/category_test.go @@ -41,7 +41,7 @@ func CreateSampleCategory(t *testing.T, categoryName string, existingAppAssert * } func TestCreateCategoryWorks(t *testing.T) { - CreateSampleCategory(t, "Science", nil) + CreateSampleCategory(t, "Science", nil).Close() } func TestCreateCategoryIgnoresid(t *testing.T) { @@ -57,7 +57,7 @@ func TestCreateCategoryIgnoresid(t *testing.T) { Status: 201, DBTester: AssertRespCategorySameAsDBCategory, }, - ) + ).Close() } func AssertNoCategories(app TestApp, assert *assert.A, resp *http.Response) { @@ -89,7 +89,7 @@ func TestCreateCategoryFailsIfNameIsNotString(t *testing.T) { }, DBTester: AssertNoCategories, }, - ) + ).Close() } func TestCreateCategoryFailsIfNameIsMissing(t *testing.T) { @@ -105,7 +105,7 @@ func TestCreateCategoryFailsIfNameIsMissing(t *testing.T) { }, DBTester: AssertNoCategories, }, - ) + ).Close() } func TestCreateCategoryFailsIfCategoryWithThatNameAlreadyExists(t *testing.T) { @@ -134,4 +134,6 @@ func TestCreateCategoryFailsIfCategoryWithThatNameAlreadyExists(t *testing.T) { }, ) } + + existingAppAssert.Close() } diff --git a/backend/tests/api/health_test.go b/backend/tests/api/health_test.go index a44d19d2c..71fe536dd 100644 --- a/backend/tests/api/health_test.go +++ b/backend/tests/api/health_test.go @@ -10,5 +10,5 @@ func TestHealthWorks(t *testing.T) { Path: "/health", }.TestOnStatus(t, nil, 200, - ) + ).Close() } diff --git a/backend/tests/api/helpers.go b/backend/tests/api/helpers.go index 7cd5f131c..25d2d5a93 100644 --- a/backend/tests/api/helpers.go +++ b/backend/tests/api/helpers.go @@ -122,6 +122,20 @@ type ExistingAppAssert struct { Assert *assert.A } +func (eaa ExistingAppAssert) Close() { + db, err := eaa.App.Conn.DB() + + if err != nil { + panic(err) + } + + err = db.Close() + + if err != nil { + panic(err) + } +} + type TestRequest struct { Method string Path string diff --git a/backend/tests/api/tag_test.go b/backend/tests/api/tag_test.go index 169152fb4..45c16d94d 100644 --- a/backend/tests/api/tag_test.go +++ b/backend/tests/api/tag_test.go @@ -45,7 +45,7 @@ func CreateSampleTag(t *testing.T, tagName string, categoryName string, existing } func TestCreateTagWorks(t *testing.T) { - CreateSampleTag(t, "Generate", "Science", nil) + CreateSampleTag(t, "Generate", "Science", nil).Close() } var AssertNoTags = func(app TestApp, assert *assert.A, resp *http.Response) { @@ -83,7 +83,7 @@ func TestCreateTagFailsBadRequest(t *testing.T) { }, DBTester: AssertNoTags, }, - ) + ).Close() } } @@ -111,7 +111,7 @@ func TestCreateTagFailsValidation(t *testing.T) { }, DBTester: AssertNoTags, }, - ) + ).Close() } } @@ -126,7 +126,7 @@ func TestGetTagWorks(t *testing.T) { Status: 200, DBTester: AssertRespTagSameAsDBTag, }, - ) + ).Close() } func TestGetTagFailsBadRequest(t *testing.T) { @@ -147,7 +147,7 @@ func TestGetTagFailsBadRequest(t *testing.T) { Status: 400, Message: "failed to validate id", }, - ) + ).Close() } } @@ -160,7 +160,7 @@ func TestGetTagFailsNotFound(t *testing.T) { Status: 404, Message: "failed to find tag", }, - ) + ).Close() } func TestUpdateTagWorksUpdateName(t *testing.T) { @@ -178,7 +178,7 @@ func TestUpdateTagWorksUpdateName(t *testing.T) { Status: 200, DBTester: AssertRespTagSameAsDBTag, }, - ) + ).Close() } func TestUpdateTagWorksUpdateCategory(t *testing.T) { @@ -197,7 +197,7 @@ func TestUpdateTagWorksUpdateCategory(t *testing.T) { Status: 200, DBTester: AssertRespTagSameAsDBTag, }, - ) + ).Close() } func TestUpdateTagWorksWithSameDetails(t *testing.T) { @@ -215,7 +215,7 @@ func TestUpdateTagWorksWithSameDetails(t *testing.T) { Status: 200, DBTester: AssertRespTagSameAsDBTag, }, - ) + ).Close() } func TestUpdateTagFailsBadRequest(t *testing.T) { @@ -243,7 +243,7 @@ func TestUpdateTagFailsBadRequest(t *testing.T) { }, DBTester: AssertNoTags, }, - ) + ).Close() } } @@ -258,7 +258,7 @@ func TestDeleteTagWorks(t *testing.T) { Status: 204, DBTester: AssertNoTags, }, - ) + ).Close() } func TestDeleteTagFailsBadRequest(t *testing.T) { @@ -279,7 +279,7 @@ func TestDeleteTagFailsBadRequest(t *testing.T) { Status: 400, Message: "failed to validate id", }, - ) + ).Close() } } @@ -292,5 +292,5 @@ func TestDeleteTagFailsNotFound(t *testing.T) { Status: 404, Message: "failed to find tag", }, - ) + ).Close() } diff --git a/backend/tests/api/user_test.go b/backend/tests/api/user_test.go index 8307c0dc9..693d93c8e 100644 --- a/backend/tests/api/user_test.go +++ b/backend/tests/api/user_test.go @@ -1,6 +1,7 @@ package tests import ( + "fmt" "net/http" "testing" @@ -43,5 +44,71 @@ func TestGetAllUsersWorks(t *testing.T) { assert.Equal(dbUser, respUser) }, }, - ) + ).Close() +} + +var AssertRespUserSameAsDBUser = func(app TestApp, assert *assert.A, resp *http.Response) { + var respUser models.User + + err := json.NewDecoder(resp.Body).Decode(&respUser) + + assert.NilError(err) + + dbUser, err := transactions.GetUser(app.Conn, respUser.ID) + + assert.NilError(err) + + assert.Equal(dbUser.Role, respUser.Role) + assert.Equal(dbUser.NUID, respUser.NUID) + assert.Equal(dbUser.FirstName, respUser.FirstName) + assert.Equal(dbUser.LastName, respUser.LastName) + assert.Equal(dbUser.Email, respUser.Email) + assert.Equal(dbUser.College, respUser.College) + assert.Equal(dbUser.Year, respUser.Year) +} + +func TestGetUserWorks(t *testing.T) { + TestRequest{ + Method: "GET", + Path: "/api/v1/users/1", + }.TestOnStatusAndDB(t, nil, + DBTesterWithStatus{ + Status: 200, + DBTester: AssertRespUserSameAsDBUser, + }, + ).Close() +} + +func TestGetUserFailsBadRequest(t *testing.T) { + badRequests := []string{ + "0", + "-1", + "1.1", + "foo", + "null", + } + + for _, badRequest := range badRequests { + TestRequest{ + Method: "GET", + Path: fmt.Sprintf("/api/v1/tags/%s", badRequest), + }.TestOnStatusAndMessage(t, nil, + MessageWithStatus{ + Status: 400, + Message: "failed to validate id", + }, + ).Close() + } +} + +func TestGetUserFailsNotFound(t *testing.T) { + TestRequest{ + Method: "GET", + Path: "/api/v1/users/69", + }.TestOnStatusAndMessage(t, nil, + MessageWithStatus{ + Status: 404, + Message: "failed to find tag", + }, + ).Close() } diff --git a/cli/commands/test.go b/cli/commands/test.go index a4ea32ac3..65b5fe4ad 100644 --- a/cli/commands/test.go +++ b/cli/commands/test.go @@ -8,7 +8,6 @@ import ( "github.com/urfave/cli/v2" ) - func TestCommand(backendDir string, frontendDir string) *cli.Command { command := cli.Command{ Name: "test", @@ -36,7 +35,7 @@ func TestCommand(backendDir string, frontendDir string) *cli.Command { } fmt.Println("Frontend", c.String("frontend")) - + folder := c.String("frontend") runFrontend := folder != "" runBackend := c.Bool("backend") @@ -50,7 +49,6 @@ func TestCommand(backendDir string, frontendDir string) *cli.Command { return &command } - func Test(backendDir string, frontendDir string, folder string, runFrontend bool, runBackend bool) error { var wg sync.WaitGroup @@ -106,4 +104,3 @@ func FrontendTest(frontendDir string, folder string) error { return nil } -