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

Feet/bulk group api #2484

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
31 changes: 30 additions & 1 deletion docs/v3-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1086,14 +1086,37 @@ paths:
content:
application/json:
schema:
$ref: '#/components/schemas/UserGroupMember'
oneOf:
- $ref: '#/components/schemas/UserGroupMember'
- $ref: '#/components/schemas/UserGroupMembers'
Akira-256 marked this conversation as resolved.
Show resolved Hide resolved
description: ''
tags:
- group
operationId: addUserGroupMember
description: |-
指定したグループにメンバーを追加します。
対象のユーザーグループの管理者権限が必要です。
delete:
summary: グループメンバーを一括削除
responses:
'204':
description: |-
No Content
グループから全てのユーザーが削除されました。
'403':
description: |-
Forbidden
ユーザーグループを操作する権限がありません。
'404':
description: |-
Not Found
ユーザーグループが見つかりません。
tags:
- group
operationId: removeUserGroupMembers
description: |-
指定したグループから全てのメンバーを削除します。
対象のユーザーグループの管理者権限が必要です。
'/groups/{groupId}/members/{userId}':
parameters:
- $ref: '#/components/parameters/groupIdInPath'
Expand Down Expand Up @@ -5095,6 +5118,12 @@ components:
required:
- id
- role
UserGroupMembers:
title: UserGroupMembers
type: array
description: ユーザーグループメンバーの配列
items:
$ref: '#/components/schemas/UserGroupMember'
UserStats:
title: UserStats
type: object
Expand Down
62 changes: 62 additions & 0 deletions repository/gorm/user_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,51 @@ func (repo *Repository) AddUserToGroup(userID, groupID uuid.UUID, role string) e
return nil
}

// AddUsersToGroup implements UserGroupRepository interface.
func (repo *Repository) AddUsersToGroup(users []model.UserGroupMember, groupID uuid.UUID) error {
if groupID == uuid.Nil {
return repository.ErrNilID
}

err := repo.db.Transaction(func(tx *gorm.DB) error {
var g model.UserGroup
if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}).Preload("Members").First(&g, &model.UserGroup{ID: groupID}).Error; err != nil {
return convertError(err)
}
tx.Clauses(clause.OnConflict{
Columns: []clause.Column{{Name: "group_id"}, {Name: "user_id"}},
DoUpdates: clause.AssignmentColumns([]string{"role"}),
}).Create(&users)
Copy link
Contributor

Choose a reason for hiding this comment

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

何らかの不具合があった時のために↓の様にエラーハンドリングあった方が良さそうです。OnConflictがあるので、既にグループに居るユーザーを追加しようとしてもエラーで止まることはないです。

		if err := tx.Clauses(clause.OnConflict{
			Columns:   []clause.Column{{Name: "group_id"}, {Name: "user_id"}},
			DoUpdates: clause.AssignmentColumns([]string{"role"}),
		}).Create(&users).Error; err != nil {
			return convertError(err)
		}


for _, user := range users {
if g.IsMember(user.UserID) {
repo.hub.Publish(hub.Message{
Name: event.UserGroupMemberUpdated,
Fields: hub.Fields{
"group_id": user.GroupID,
"user_id": user.UserID,
},
})
} else {
repo.hub.Publish(hub.Message{
Name: event.UserGroupMemberAdded,
Fields: hub.Fields{
"group_id": user.GroupID,
"user_id": user.UserID,
},
})
}
}

return tx.Model(&g).UpdateColumn("updated_at", time.Now()).Error
})
if err != nil {
return err
}

return nil
}

// RemoveUserFromGroup implements UserGroupRepository interface.
func (repo *Repository) RemoveUserFromGroup(userID, groupID uuid.UUID) error {
if userID == uuid.Nil || groupID == uuid.Nil {
Expand Down Expand Up @@ -266,6 +311,23 @@ func (repo *Repository) RemoveUserFromGroup(userID, groupID uuid.UUID) error {
return nil
}

// RemoveUsersFromGroup implements UserGroupRepository interface.
func (repo *Repository) RemoveUsersFromGroup(groupID uuid.UUID) error {
if groupID == uuid.Nil {
return repository.ErrNilID
}
err := repo.db.Transaction(func(tx *gorm.DB) error {
if err := tx.Where("group_id = ?", groupID).Delete(&model.UserGroupMember{}).Error; err != nil {
return err
}
return nil
})
if err != nil {
return err
}
return nil
}

Copy link
Contributor

Choose a reason for hiding this comment

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

RemoveUserFromGroupevent.UserGroupMemberRemovedがあるので、RemoveUsersFromGroupでもevent.UserGroupMemberRemovedするべきですね。
各ユーザーの削除を追跡して、それに基づいてイベントを発行する必要があってちょっと大変。

var removedUsers []model.UserGroupMember
if err := tx.Where("group_id = ?", groupID).Find(&members).Error; err != nil {
	return err
}

でユーザー一覧を消す前に取得して、削除に成功したら取得したそれぞれのユーザーに対してイベントの発行

for _, userID := range removedUsers {
repo.hub.Publish(hub.Message{
	Name: event.UserGroupMemberRemoved,
	Fields: hub.Fields{
		"group_id": groupID,
		"user_id":  userID,
	},
})
}

をすれば良さそう。

// AddUserToGroupAdmin implements UserGroupRepository interface.
func (repo *Repository) AddUserToGroupAdmin(userID, groupID uuid.UUID) error {
if userID == uuid.Nil || groupID == uuid.Nil {
Expand Down
16 changes: 15 additions & 1 deletion repository/user_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,27 @@ type UserGroupRepository interface {
// 引数にuuid.Nilを指定した場合、ErrNilIDを返します。
// DBによるエラーを返すことがあります。
AddUserToGroup(userID, groupID uuid.UUID, role string) error
// AddUsersToGroup 指定したグループに指定した複数のユーザーを追加します
//
// 成功した、或いは既に追加されている場合、nilを返します。
// 存在しないグループの場合、ErrNotFoundを返します。
// 引数にuuid.Nilを指定した場合、ErrNilIDを返します。
// DBによるエラーを返すことがあります。
AddUsersToGroup(users []model.UserGroupMember, groupID uuid.UUID) error
// RemoveUserFromGroup 指定したグループから指定したユーザーを削除します
//
// 成功した、或いは既に居ない場合、nilを返します。
// 全員の追加が成功した、或いは既に追加されている場合、nilを返します。
// 存在しないグループの場合、ErrNotFoundを返します。
// 引数にuuid.Nilを指定した場合、ErrNilIDを返します。
// DBによるエラーを返すことがあります。
RemoveUserFromGroup(userID, groupID uuid.UUID) error
// RemoveUsersFromGroup 指定したグループから指定した複数のユーザーを削除します
//
// 全員の削除が成功した、或いは既に削除されている場合、nilを返します。
// 存在しないグループの場合、ErrNotFoundを返します。
// 引数にuuid.Nilを指定した場合、ErrNilIDを返します。
// DBによるエラーを返すことがあります。
RemoveUsersFromGroup(groupID uuid.UUID) error
// AddUserToGroupAdmin 指定したグループの管理者に指定したユーザーを追加します
//
// 成功した、或いは既に追加されている場合、nilを返します。
Expand Down
1 change: 1 addition & 0 deletions router/v3/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ func (h *Handlers) Setup(e *echo.Group) {
{
apiGroupsGIDMembers.GET("", h.GetUserGroupMembers, requires(permission.GetUserGroup))
apiGroupsGIDMembers.POST("", h.AddUserGroupMember, requiresGroupAdminPerm, requires(permission.EditUserGroup))
apiGroupsGIDMembers.DELETE("", h.RemoveUserGroupMembers, requiresGroupAdminPerm, requires(permission.EditUserGroup))
apiGroupsGIDMembersUID := apiGroupsGIDMembers.Group("/:userID", requiresGroupAdminPerm)
{
apiGroupsGIDMembersUID.PATCH("", h.EditUserGroupMember, requires(permission.EditUserGroup))
Expand Down
51 changes: 44 additions & 7 deletions router/v3/user_groups.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
package v3

import (
"bytes"
"context"
"errors"
"io"
"net/http"

vd "github.com/go-ozzo/ozzo-validation/v4"
"github.com/gofrs/uuid"
"github.com/labstack/echo/v4"

"github.com/traPtitech/traQ/model"
"github.com/traPtitech/traQ/repository"
"github.com/traPtitech/traQ/router/consts"
"github.com/traPtitech/traQ/router/extension"
Expand Down Expand Up @@ -170,16 +174,35 @@ func (r PostUserGroupMemberRequest) ValidateWithContext(ctx context.Context) err
// AddUserGroupMember POST /groups/:groupID/members
func (h *Handlers) AddUserGroupMember(c echo.Context) error {
g := getParamGroup(c)

var req PostUserGroupMemberRequest
if err := bindAndValidate(c, &req); err != nil {
bodyBytes, err := io.ReadAll(c.Request().Body)
if err != nil {
return err
}

if err := h.Repo.AddUserToGroup(req.ID, g.ID, req.Role); err != nil {
return herror.InternalServerError(err)
}
var singleReq PostUserGroupMemberRequest
c.Request().Body = io.NopCloser(bytes.NewBuffer(bodyBytes))
if err := bindAndValidate(c, &singleReq); err == nil {
if err := h.Repo.AddUserToGroup(singleReq.ID, g.ID, singleReq.Role); err != nil {
return herror.InternalServerError(err)
}
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

ちょっとネストが深いので、↓のように早期リターンをしたほうが読みやすいかな~と思ったのですが、好みの範疇だと思うのでどちらでもokです。

// AddUserGroupMember POST /groups/:groupID/members
func (h *Handlers) AddUserGroupMember(c echo.Context) error {
	g := getParamGroup(c)
	bodyBytes, err := io.ReadAll(c.Request().Body)
	if err != nil {
		return err
	}

	var singleReq PostUserGroupMemberRequest
	c.Request().Body = io.NopCloser(bytes.NewBuffer(bodyBytes))
	if err := bindAndValidate(c, &singleReq); err == nil {
		if err := h.Repo.AddUserToGroup(singleReq.ID, g.ID, singleReq.Role); err != nil {
			return herror.InternalServerError(err)
		}
		return c.NoContent(http.StatusNoContent)
	}

	var reqs []PostUserGroupMemberRequest
	c.Request().Body = io.NopCloser(bytes.NewBuffer(bodyBytes))
	if err := bindAndValidate(c, &reqs); err != nil {
		return err
	}
	if len(reqs) == 0 {
		return herror.BadRequest(errors.New("no users provided"))
	}
	users := make([]model.UserGroupMember, len(reqs))
	for i, req := range reqs {
		users[i] = model.UserGroupMember{UserID: req.ID, Role: req.Role, GroupID: g.ID}
	}
	if err := h.Repo.AddUsersToGroup(users, g.ID); err != nil {
		return herror.InternalServerError(err)
	}

	return c.NoContent(http.StatusNoContent)
}

var reqs []PostUserGroupMemberRequest
c.Request().Body = io.NopCloser(bytes.NewBuffer(bodyBytes))
if err := bindAndValidate(c, &reqs); err != nil {
return err
}
if len(reqs) == 0 {
return herror.BadRequest(errors.New("no users provided"))
}
users := make([]model.UserGroupMember, len(reqs))
for i, req := range reqs {
users[i] = model.UserGroupMember{UserID: req.ID, Role: req.Role, GroupID: g.ID}
}
if err := h.Repo.AddUsersToGroup(users, g.ID); err != nil {
return herror.InternalServerError(err)
}

}
return c.NoContent(http.StatusNoContent)
}

Expand Down Expand Up @@ -220,14 +243,28 @@ func (h *Handlers) EditUserGroupMember(c echo.Context) error {
func (h *Handlers) RemoveUserGroupMember(c echo.Context) error {
userID := getParamAsUUID(c, consts.ParamUserID)
g := getParamGroup(c)

if userID == uuid.Nil {
if err := h.Repo.RemoveUsersFromGroup(g.ID); err != nil {
return herror.InternalServerError(err)
}
return c.NoContent(http.StatusNoContent)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

RemoveUserGroupMemberで全てのユーザーがグループから削除される挙動は予期されないと思うので、ここは消した方が良さそう。

if err := h.Repo.RemoveUserFromGroup(userID, g.ID); err != nil {
return herror.InternalServerError(err)
}

return c.NoContent(http.StatusNoContent)
}

// RemoveUserGroupMembers DELETE /groups/:groupID/members
func (h *Handlers) RemoveUserGroupMembers(c echo.Context) error {
g := getParamGroup(c)
if err := h.Repo.RemoveUsersFromGroup(g.ID); err != nil {
return herror.InternalServerError(err)
}
return c.NoContent(http.StatusNoContent)
}

// GetUserGroupAdmins GET /groups/:groupID/admins
func (h *Handlers) GetUserGroupAdmins(c echo.Context) error {
return c.JSON(http.StatusOK, getParamGroup(c).AdminIDArray())
Expand Down
Loading