Skip to content

Commit

Permalink
Change /groups/{group id}/users route (#115)
Browse files Browse the repository at this point in the history
* fix: inject user id into context in development env

* feat: add ID column to list view of Users

* feat: add role attribute to /groups/{groupId}/users response

a userId could have multiple roles, if it is present in the usergroups table in multiple rows
(with different groupId).

however, since we specify the groupId in the /group/{groupId}/users route, it makes sense that we
only retrieve users that have that groupId.

- in controller, get groupId from context
- for each user obtained from the users table, call GetUserRoleByID function from usergroups model
- store the roles obtained in an array
- pass array to updated ListFrom function from users view, which now include a role attribute
in its JSON response

* fix: DeleteUser in users model

previously, the DeleteUser function was not working as it chained
`.Delete(&user)` after `.First(&user)`. this resulted in a query as
shown, and its corresponding error:

* `ERROR: table name "users" specified more than once (SQLSTATE 42712)
[8.059ms] [rows:1] UPDATE "users" SET "deleted_at"='2024-02-12
20:52:41.02' FROM "users" WHERE id = 4 AND "users"."deleted_at" IS NULL
AND "users"."id" = 4`

the intent of `.First(&user)` was likely to store the user to be deleted
first in the `user` variable with the use of a `SELECT` SQL statement.
however, chaining another method at the end of a finisher method likely
led to some errors (see chaining
[here](https://gorm.io/docs/method_chaining.html)).

thus, this commit attempts to separate the two statements, preserving
the initial intent of storing the user to be deleted before deleting,
and then returning this user.

* feat: function to update row in users model

* feat: function to update row in usergroups model

* feat: add /users/{userID}/role put route

missing validation for parameters, but seemed unnecessary since there is
only one route with parameter validation (creating user)

* fix: package name for update role's params

* Use range iterator for users list view

* Remove unused UpdateUser function

* Prevent self-update of roles

* Fix error message

* Add `UpdateRole` param validation

* Refactor logic

---------

Co-authored-by: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>
  • Loading branch information
zsiggg and RichDom2185 authored May 12, 2024
1 parent 7288818 commit 840e79d
Show file tree
Hide file tree
Showing 8 changed files with 177 additions and 8 deletions.
86 changes: 86 additions & 0 deletions controller/usergroups/update.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package usergroups

import (
"encoding/json"
"fmt"
"net/http"
"strconv"

"github.com/go-chi/chi/v5"
"github.com/sirupsen/logrus"
"github.com/source-academy/stories-backend/controller"
"github.com/source-academy/stories-backend/internal/auth"
"github.com/source-academy/stories-backend/internal/database"
apierrors "github.com/source-academy/stories-backend/internal/errors"
userpermissiongroups "github.com/source-academy/stories-backend/internal/permissiongroups/users"
"github.com/source-academy/stories-backend/model"
usergroupparams "github.com/source-academy/stories-backend/params/usergroups"
userviews "github.com/source-academy/stories-backend/view/users"
)

func HandleUpdateRole(w http.ResponseWriter, r *http.Request) error {
userIDStr := chi.URLParam(r, "userID")
userID, err := strconv.Atoi(userIDStr)
if err != nil {
return apierrors.ClientBadRequestError{
Message: fmt.Sprintf("Invalid userID: %v", err),
}
}

err = auth.CheckPermissions(r, userpermissiongroups.Update(uint(userID)))
if err != nil {
logrus.Error(err)
return apierrors.ClientForbiddenError{
Message: fmt.Sprintf("Error updating user: %v", err),
}
}

var params usergroupparams.UpdateRole
if err := json.NewDecoder(r.Body).Decode(&params); err != nil {
e, ok := err.(*json.UnmarshalTypeError)
if !ok {
logrus.Error(err)
return apierrors.ClientBadRequestError{
Message: fmt.Sprintf("Bad JSON parsing: %v", err),
}
}

// TODO: Investigate if we should use errors.Wrap instead
return apierrors.ClientUnprocessableEntityError{
Message: fmt.Sprintf("Invalid JSON format: %s should be a %s.", e.Field, e.Type),
}
}

err = params.Validate()
if err != nil {
logrus.Error(err)
return apierrors.ClientUnprocessableEntityError{
Message: fmt.Sprintf("JSON validation failed: %v", err),
}
}

updateModel := *params.ToModel(uint(userID))

// Get DB instance
db, err := database.GetDBFrom(r)
if err != nil {
logrus.Error(err)
return err
}

// TODO: Refactor logic
userGroup, err := model.UpdateUserGroupByUserID(db, updateModel.UserID, &updateModel)
if err != nil {
logrus.Error(err)
return err
}

user, err := model.GetUserByID(db, int(userGroup.UserID))
if err != nil {
logrus.Error(err)
return err
}

controller.EncodeJSONResponse(w, userviews.SummaryFrom(user, userGroup))
return nil
}
19 changes: 18 additions & 1 deletion controller/users/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import (
"github.com/source-academy/stories-backend/controller"
"github.com/source-academy/stories-backend/internal/auth"
"github.com/source-academy/stories-backend/internal/database"
groupenums "github.com/source-academy/stories-backend/internal/enums/groups"
apierrors "github.com/source-academy/stories-backend/internal/errors"
userpermissiongroups "github.com/source-academy/stories-backend/internal/permissiongroups/users"
"github.com/source-academy/stories-backend/internal/usergroups"
"github.com/source-academy/stories-backend/model"
userviews "github.com/source-academy/stories-backend/view/users"
)
Expand All @@ -36,6 +38,21 @@ func HandleList(w http.ResponseWriter, r *http.Request) error {
return err
}

controller.EncodeJSONResponse(w, userviews.ListFrom(users))
groupId, err := usergroups.GetGroupIDFrom(r)
if err != nil {
logrus.Error(err)
return err
}

roles := make([]groupenums.Role, len(users))
for i, user := range users {
roles[i], err = model.GetUserRoleByID(db, user.ID, *groupId)
if err != nil {
logrus.Error(err)
return err
}
}

controller.EncodeJSONResponse(w, userviews.ListFrom(users, roles))
return nil
}
5 changes: 4 additions & 1 deletion internal/auth/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ func MakeMiddlewareFrom(conf *config.Config) func(http.Handler) http.Handler {
// Skip auth in development mode
if conf.Environment == envutils.ENV_DEVELOPMENT {
return func(next http.Handler) http.Handler {
return next
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
r = injectUserIDToContext(r, 1)
next.ServeHTTP(w, r)
})
}
}

Expand Down
1 change: 0 additions & 1 deletion internal/permissiongroups/users/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ func Update(userID uint) permissions.PermissionGroup {
Groups: []permissions.PermissionGroup{
userpermissions.
GetRolePermission(userpermissions.CanUpdateUsers),
IsSelf{UserID: userID},
},
}
}
Expand Down
1 change: 1 addition & 0 deletions internal/router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ func Setup(config *config.Config, injectMiddleWares []func(http.Handler) http.Ha
r.Route("/users", func(r chi.Router) {
r.Get("/", handleAPIError(users.HandleList))
r.Get("/{userID}", handleAPIError(users.HandleRead))
r.Put("/{userID}/role", handleAPIError(usergroupscontroller.HandleUpdateRole))
r.Delete("/{userID}", handleAPIError(users.HandleDelete))
r.Post("/", handleAPIError(users.HandleCreate))
r.Post("/batch", handleAPIError(usergroupscontroller.HandleBatchCreate))
Expand Down
28 changes: 28 additions & 0 deletions model/usergroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,31 @@ func CreateUserGroup(db *gorm.DB, userGroup *UserGroup) error {
}
return nil
}

func GetUserRoleByID(db *gorm.DB, userID uint, groupID uint) (groupenums.Role, error) {
userGroup, err := GetUserGroupByID(db, userID, groupID)
if err != nil {
return userGroup.Role, database.HandleDBError(err, "userGroup")
}
return userGroup.Role, nil
}

func UpdateUserGroupByUserID(db *gorm.DB, userID uint, updates *UserGroup) (UserGroup, error) {
var userGroup UserGroup

// Check if the user is trying to update another user's role
if updates.UserID != 0 && updates.UserID != userID {
return userGroup, database.HandleDBError(nil, "userGroup")
}

err := db.Model(&userGroup).
Preload(clause.Associations).
Where(UserGroup{UserID: userID}).
Updates(&userGroup).
Error

if err != nil {
return userGroup, database.HandleDBError(err, "userGroup")
}
return userGroup, nil
}
28 changes: 28 additions & 0 deletions params/usergroups/update.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package usergroupparams

import (
"fmt"

groupenums "github.com/source-academy/stories-backend/internal/enums/groups"
"github.com/source-academy/stories-backend/model"
)

type UpdateRole struct {
Role groupenums.Role `json:"role"`
}

func (params *UpdateRole) ToModel(userID uint) *model.UserGroup {
return &model.UserGroup{
UserID: userID,
Role: params.Role,
}
}

func (params *UpdateRole) Validate() error {
// Extra params won't do anything, e.g. authorID can't be changed.
// TODO: Error on extra params?
if !params.Role.IsValid() {
return fmt.Errorf("Invalid role %s.", params.Role)
}
return nil
}
17 changes: 12 additions & 5 deletions view/users/list.go
Original file line number Diff line number Diff line change
@@ -1,22 +1,29 @@
package userviews

import "github.com/source-academy/stories-backend/model"
import (
groupenums "github.com/source-academy/stories-backend/internal/enums/groups"
"github.com/source-academy/stories-backend/model"
)

type ListView struct {
Name string `json:"name"`
Username string `json:"username"`
LoginProvider string `json:"provider"`
ID uint `json:"id"`
Name string `json:"name"`
Username string `json:"username"`
LoginProvider string `json:"provider"`
Role groupenums.Role `json:"role"`
}

func ListFrom(users []model.User) []ListView {
func ListFrom(users []model.User, roles []groupenums.Role) []ListView {
usersListView := make([]ListView, len(users))
for i, user := range users {
usersListView[i] = ListView{
// Unlike other views, we do not fallback an empty name to
// the username for the users' list view.
ID: user.ID,
Name: user.FullName,
Username: user.Username,
LoginProvider: user.LoginProvider.ToString(),
Role: roles[i],
}
}
return usersListView
Expand Down

0 comments on commit 840e79d

Please sign in to comment.