Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change /groups/{group id}/users route #115

Merged
merged 17 commits into from
May 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
}
}
Comment on lines +48 to +54
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

N + 1 query problem

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed as an issue: #121.

We can just merge first as it's not critical at the moment.


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
Loading