From 3ff1e1bc8daee576dde0e288d8411a2b68861a67 Mon Sep 17 00:00:00 2001 From: Pugma Date: Thu, 23 Jan 2025 16:08:29 +0900 Subject: [PATCH 1/5] test impl: bulk insert for adding group members --- router/v3/user_groups_test.go | 66 ++++++++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 5 deletions(-) diff --git a/router/v3/user_groups_test.go b/router/v3/user_groups_test.go index 42c8f9524..f99680ffb 100644 --- a/router/v3/user_groups_test.go +++ b/router/v3/user_groups_test.go @@ -502,7 +502,7 @@ func TestHandlers_AddUserGroupMember(t *testing.T) { s := env.S(t, user.GetID()) - t.Run("not logged in", func(t *testing.T) { + t.Run("not logged in - single", func(t *testing.T) { t.Parallel() e := env.R(t) e.POST(path, ug.ID). @@ -510,8 +510,16 @@ func TestHandlers_AddUserGroupMember(t *testing.T) { Expect(). Status(http.StatusUnauthorized) }) + t.Run("not logged in - multiple", func(t *testing.T) { + t.Parallel() + e := env.R(t) + e.POST(path, ug.ID). + WithJSON(&[]PostUserGroupMemberRequest{{ID: user.GetID()}}). + Expect(). + Status(http.StatusUnauthorized) + }) - t.Run("bad request", func(t *testing.T) { + t.Run("bad request - single", func(t *testing.T) { t.Parallel() e := env.R(t) e.POST(path, ug.ID). @@ -520,8 +528,17 @@ func TestHandlers_AddUserGroupMember(t *testing.T) { Expect(). Status(http.StatusBadRequest) }) + t.Run("bad request - multiple", func(t *testing.T) { + t.Parallel() + e := env.R(t) + e.POST(path, ug.ID). + WithCookie(session.CookieName, s). + WithJSON(&[]PostUserGroupMemberRequest{{ID: uuid.Must(uuid.NewV4())}}). + Expect(). + Status(http.StatusBadRequest) + }) - t.Run("forbidden", func(t *testing.T) { + t.Run("forbidden - single", func(t *testing.T) { t.Parallel() e := env.R(t) e.POST(path, ug2.ID). @@ -530,8 +547,17 @@ func TestHandlers_AddUserGroupMember(t *testing.T) { Expect(). Status(http.StatusForbidden) }) + t.Run("forbidden - multiple", func(t *testing.T) { + t.Parallel() + e := env.R(t) + e.POST(path, ug2.ID). + WithCookie(session.CookieName, s). + WithJSON(&[]PostUserGroupMemberRequest{{ID: user.GetID()}}). + Expect(). + Status(http.StatusForbidden) + }) - t.Run("not found", func(t *testing.T) { + t.Run("not found - single", func(t *testing.T) { t.Parallel() e := env.R(t) e.POST(path, uuid.Must(uuid.NewV4())). @@ -540,8 +566,17 @@ func TestHandlers_AddUserGroupMember(t *testing.T) { Expect(). Status(http.StatusNotFound) }) + t.Run("not found - multiple", func(t *testing.T) { + t.Parallel() + e := env.R(t) + e.POST(path, uuid.Must(uuid.NewV4())). + WithCookie(session.CookieName, s). + WithJSON(&[]PostUserGroupMemberRequest{{ID: user.GetID()}}). + Expect(). + Status(http.StatusNotFound) + }) - t.Run("success", func(t *testing.T) { + t.Run("success - single", func(t *testing.T) { t.Parallel() e := env.R(t) e.POST(path, ug.ID). @@ -558,6 +593,27 @@ func TestHandlers_AddUserGroupMember(t *testing.T) { assert.EqualValues(t, m.Role, "") } }) + t.Run("success - multiple", func(t *testing.T) { + t.Parallel() + e := env.R(t) + e.POST(path, ug.ID). + WithCookie(session.CookieName, s). + WithJSON(&[]PostUserGroupMemberRequest{{ID: user.GetID()}, {ID: user2.GetID()}}). + Expect(). + Status(http.StatusNoContent) + + ug, err := env.Repository.GetUserGroup(ug.ID) + require.NoError(t, err) + if assert.Len(t, ug.Members, 2) { + m := ug.Members[0] + assert.EqualValues(t, m.UserID, user.GetID()) + assert.EqualValues(t, m.Role, "") + + m = ug.Members[1] + assert.EqualValues(t, m.UserID, user2.GetID()) + assert.EqualValues(t, m.Role, "") + } + }) } func TestPatchUserGroupMemberRequest_Validate(t *testing.T) { From 291b0909cdc8c5771f79a7cbe53a97781fbd7cb7 Mon Sep 17 00:00:00 2001 From: Pugma Date: Fri, 24 Jan 2025 01:43:54 +0900 Subject: [PATCH 2/5] test impl: bulk deletion for user group members --- router/v3/user_groups_test.go | 70 +++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/router/v3/user_groups_test.go b/router/v3/user_groups_test.go index f99680ffb..b64b53f3f 100644 --- a/router/v3/user_groups_test.go +++ b/router/v3/user_groups_test.go @@ -808,6 +808,76 @@ func TestHandlers_RemoveUserGroupMember(t *testing.T) { }) } +func TestHandlers_RemoveUserGroupMembers(t *testing.T) { + t.Parallel() + + path := "/api/v3/groups/{groupId}/members" + env := Setup(t, common1) + user := env.CreateUser(t, rand) + user2 := env.CreateUser(t, rand) + + ug := env.CreateUserGroup(t, rand, "", "", user.GetID()) + ug2 := env.CreateUserGroup(t, rand, "", "", user2.GetID()) + env.AddUserToUserGroup(t, user.GetID(), ug.ID, "") + env.AddUserToUserGroup(t, user.GetID(), ug2.ID, "") + + s := env.S(t, user.GetID()) + + t.Run("not logged in", func(t *testing.T) { + t.Parallel() + e := env.R(t) + e.DELETE(path, ug.ID). + Expect(). + Status(http.StatusUnauthorized) + }) + + t.Run("forbidden", func(t *testing.T) { + t.Parallel() + e := env.R(t) + e.DELETE(path, ug2.ID). + WithCookie(session.CookieName, s). + Expect(). + Status(http.StatusForbidden) + }) + + t.Run("user group not found", func(t *testing.T) { + t.Parallel() + e := env.R(t) + e.DELETE(path, uuid.Must(uuid.NewV4())). + WithCookie(session.CookieName, s). + Expect(). + Status(http.StatusNotFound) + }) + + t.Run("already removed", func(t *testing.T) { + t.Parallel() + e := env.R(t) + e.DELETE(path, ug.ID). + WithCookie(session.CookieName, s). + Expect(). + Status(http.StatusNoContent) + }) + + t.Run("success", func(t *testing.T) { + t.Parallel() + e := env.R(t) + e.DELETE(path, ug.ID). + WithCookie(session.CookieName, s). + Expect(). + Status(http.StatusNoContent) + + ug, err := env.Repository.GetUserGroup(ug.ID) + require.NoError(t, err) + assert.Len(t, ug.Members, 0) + + // already removed + e.DELETE(path, ug.ID). + WithCookie(session.CookieName, s). + Expect(). + Status(http.StatusNoContent) + }) +} + func TestHandlers_GetUserGroupAdmins(t *testing.T) { t.Parallel() From 2235a4da58c6053f30566d3dfe16cba19358d624 Mon Sep 17 00:00:00 2001 From: Pugma Date: Fri, 24 Jan 2025 02:48:15 +0900 Subject: [PATCH 3/5] fix: bulk deletion --- repository/gorm/user_group.go | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/repository/gorm/user_group.go b/repository/gorm/user_group.go index 0bba8e82c..578433521 100644 --- a/repository/gorm/user_group.go +++ b/repository/gorm/user_group.go @@ -318,24 +318,27 @@ func (repo *Repository) RemoveUsersFromGroup(groupID uuid.UUID) error { if groupID == uuid.Nil { return repository.ErrNilID } + + var removedUsers []model.UserGroupMember + err := repo.db.Transaction(func(tx *gorm.DB) error { - var removedUsers []model.UserGroupMember - if err := tx.Where("group_id = ?", groupID).Find(&model.UserGroupMember{}).Error; err != nil { + if err := tx.Where("group_id = ?", groupID).Find(&removedUsers).Delete(&model.UserGroupMember{}).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, - }, - }) - } return nil }) if err != nil { - return nil + return err + } + + for _, userID := range removedUsers { + repo.hub.Publish(hub.Message{ + Name: event.UserGroupMemberRemoved, + Fields: hub.Fields{ + "group_id": groupID, + "user_id": userID, + }, + }) } return nil } From 81a6d138af6c58db964b07acf306c987a80ccd58 Mon Sep 17 00:00:00 2001 From: Pugma Date: Fri, 24 Jan 2025 03:13:57 +0900 Subject: [PATCH 4/5] fix: separated test cases into single ones and multiple ones --- router/v3/user_groups_test.go | 101 ++++++++++++++++++++-------------- 1 file changed, 60 insertions(+), 41 deletions(-) diff --git a/router/v3/user_groups_test.go b/router/v3/user_groups_test.go index b64b53f3f..085b225a4 100644 --- a/router/v3/user_groups_test.go +++ b/router/v3/user_groups_test.go @@ -502,7 +502,7 @@ func TestHandlers_AddUserGroupMember(t *testing.T) { s := env.S(t, user.GetID()) - t.Run("not logged in - single", func(t *testing.T) { + t.Run("not logged in", func(t *testing.T) { t.Parallel() e := env.R(t) e.POST(path, ug.ID). @@ -510,16 +510,8 @@ func TestHandlers_AddUserGroupMember(t *testing.T) { Expect(). Status(http.StatusUnauthorized) }) - t.Run("not logged in - multiple", func(t *testing.T) { - t.Parallel() - e := env.R(t) - e.POST(path, ug.ID). - WithJSON(&[]PostUserGroupMemberRequest{{ID: user.GetID()}}). - Expect(). - Status(http.StatusUnauthorized) - }) - t.Run("bad request - single", func(t *testing.T) { + t.Run("bad request", func(t *testing.T) { t.Parallel() e := env.R(t) e.POST(path, ug.ID). @@ -528,17 +520,8 @@ func TestHandlers_AddUserGroupMember(t *testing.T) { Expect(). Status(http.StatusBadRequest) }) - t.Run("bad request - multiple", func(t *testing.T) { - t.Parallel() - e := env.R(t) - e.POST(path, ug.ID). - WithCookie(session.CookieName, s). - WithJSON(&[]PostUserGroupMemberRequest{{ID: uuid.Must(uuid.NewV4())}}). - Expect(). - Status(http.StatusBadRequest) - }) - t.Run("forbidden - single", func(t *testing.T) { + t.Run("forbidden", func(t *testing.T) { t.Parallel() e := env.R(t) e.POST(path, ug2.ID). @@ -547,17 +530,8 @@ func TestHandlers_AddUserGroupMember(t *testing.T) { Expect(). Status(http.StatusForbidden) }) - t.Run("forbidden - multiple", func(t *testing.T) { - t.Parallel() - e := env.R(t) - e.POST(path, ug2.ID). - WithCookie(session.CookieName, s). - WithJSON(&[]PostUserGroupMemberRequest{{ID: user.GetID()}}). - Expect(). - Status(http.StatusForbidden) - }) - t.Run("not found - single", func(t *testing.T) { + t.Run("not found", func(t *testing.T) { t.Parallel() e := env.R(t) e.POST(path, uuid.Must(uuid.NewV4())). @@ -566,17 +540,8 @@ func TestHandlers_AddUserGroupMember(t *testing.T) { Expect(). Status(http.StatusNotFound) }) - t.Run("not found - multiple", func(t *testing.T) { - t.Parallel() - e := env.R(t) - e.POST(path, uuid.Must(uuid.NewV4())). - WithCookie(session.CookieName, s). - WithJSON(&[]PostUserGroupMemberRequest{{ID: user.GetID()}}). - Expect(). - Status(http.StatusNotFound) - }) - t.Run("success - single", func(t *testing.T) { + t.Run("success", func(t *testing.T) { t.Parallel() e := env.R(t) e.POST(path, ug.ID). @@ -593,7 +558,61 @@ func TestHandlers_AddUserGroupMember(t *testing.T) { assert.EqualValues(t, m.Role, "") } }) - t.Run("success - multiple", func(t *testing.T) { +} + +func TestHandlers_AddUsersGroupMember(t *testing.T) { + t.Parallel() + + path := "/api/v3/groups/{groupId}/members" + env := Setup(t, common1) + user := env.CreateUser(t, rand) + user2 := env.CreateUser(t, rand) + + ug := env.CreateUserGroup(t, rand, "", "", user.GetID()) + ug2 := env.CreateUserGroup(t, rand, "", "", user2.GetID()) + + s := env.S(t, user.GetID()) + + t.Run("not logged in", func(t *testing.T) { + t.Parallel() + e := env.R(t) + e.POST(path, ug.ID). + WithJSON(&[]PostUserGroupMemberRequest{{ID: user.GetID()}}). + Expect(). + Status(http.StatusUnauthorized) + }) + + t.Run("bad request", func(t *testing.T) { + t.Parallel() + e := env.R(t) + e.POST(path, ug.ID). + WithCookie(session.CookieName, s). + WithJSON(&[]PostUserGroupMemberRequest{{ID: uuid.Must(uuid.NewV4())}}). + Expect(). + Status(http.StatusBadRequest) + }) + + t.Run("forbidden", func(t *testing.T) { + t.Parallel() + e := env.R(t) + e.POST(path, ug2.ID). + WithCookie(session.CookieName, s). + WithJSON(&[]PostUserGroupMemberRequest{{ID: user.GetID()}}). + Expect(). + Status(http.StatusForbidden) + }) + + t.Run("not found", func(t *testing.T) { + t.Parallel() + e := env.R(t) + e.POST(path, uuid.Must(uuid.NewV4())). + WithCookie(session.CookieName, s). + WithJSON(&[]PostUserGroupMemberRequest{{ID: user.GetID()}}). + Expect(). + Status(http.StatusNotFound) + }) + + t.Run("success", func(t *testing.T) { t.Parallel() e := env.R(t) e.POST(path, ug.ID). From 1227b77e47f7ab659c065a132b5159fc2d6eb208 Mon Sep 17 00:00:00 2001 From: Pugma Date: Fri, 24 Jan 2025 03:17:29 +0900 Subject: [PATCH 5/5] formatted --- router/v3/user_groups_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/router/v3/user_groups_test.go b/router/v3/user_groups_test.go index 085b225a4..87583d2ce 100644 --- a/router/v3/user_groups_test.go +++ b/router/v3/user_groups_test.go @@ -581,7 +581,7 @@ func TestHandlers_AddUsersGroupMember(t *testing.T) { Expect(). Status(http.StatusUnauthorized) }) - + t.Run("bad request", func(t *testing.T) { t.Parallel() e := env.R(t) @@ -591,7 +591,7 @@ func TestHandlers_AddUsersGroupMember(t *testing.T) { Expect(). Status(http.StatusBadRequest) }) - + t.Run("forbidden", func(t *testing.T) { t.Parallel() e := env.R(t) @@ -601,7 +601,7 @@ func TestHandlers_AddUsersGroupMember(t *testing.T) { Expect(). Status(http.StatusForbidden) }) - + t.Run("not found", func(t *testing.T) { t.Parallel() e := env.R(t) @@ -611,7 +611,7 @@ func TestHandlers_AddUsersGroupMember(t *testing.T) { Expect(). Status(http.StatusNotFound) }) - + t.Run("success", func(t *testing.T) { t.Parallel() e := env.R(t)