From b9329d19841fb4e5f6e2f674ddea31d5d1e94cd8 Mon Sep 17 00:00:00 2001 From: Hossein Ahmadian-Yazdi Date: Thu, 24 Oct 2019 17:31:17 -0400 Subject: [PATCH] [MM-13704] Updated CLI "deleter user" to also permanently delete GroupMembers associated to user (#12858) * [MM-13704] Updated CLI to set DeleteAt in groupmemebrs * [MM-13704] Updated Mock Test * [MM-13704] Refactored to use current pattern to write function * [MM-13704] Small text change * [MM-13704] updated translation * [MM-13704] Updated params of error function * [MM-13704] Fixed confusion between channel and group store * [MM-13704] Addressed PR comments * [MM-13704] Added test case * [MM-13704] Fixed err declaration --- app/user.go | 4 ++++ i18n/en.json | 4 ++++ store/sqlstore/channel_store.go | 2 +- store/sqlstore/group_store.go | 7 ++++++ store/store.go | 1 + store/storetest/group_store.go | 37 +++++++++++++++++++++++++++++ store/storetest/mocks/GroupStore.go | 16 +++++++++++++ 7 files changed, 70 insertions(+), 1 deletion(-) diff --git a/app/user.go b/app/user.go index da2e016c21..878ce66a31 100644 --- a/app/user.go +++ b/app/user.go @@ -1456,6 +1456,10 @@ func (a *App) PermanentDeleteUser(user *model.User) *model.AppError { return err } + if err := a.Srv.Store.Group().PermanentDeleteMembersByUser(user.Id); err != nil { + return err + } + if err := a.Srv.Store.Post().PermanentDeleteByUser(user.Id); err != nil { return err } diff --git a/i18n/en.json b/i18n/en.json index fa03c92154..6a89c055f4 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -6126,6 +6126,10 @@ "id": "store.sql_group.no_rows_changed", "translation": "no rows changed" }, + { + "id": "store.sql_group.permanent_delete_members_by_user.app_error", + "translation": "Unable to remove the group members with UserID \"{{.UserId}}\"" + }, { "id": "store.sql_group.unique_constraint", "translation": "a group with that name already exists" diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 308b8bc727..aba5d38d77 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -1790,7 +1790,7 @@ func (s SqlChannelStore) RemoveAllDeactivatedMembers(channelId string) *model.Ap func (s SqlChannelStore) PermanentDeleteMembersByUser(userId string) *model.AppError { if _, err := s.GetMaster().Exec("DELETE FROM ChannelMembers WHERE UserId = :UserId", map[string]interface{}{"UserId": userId}); err != nil { - return model.NewAppError("SqlChannelStore.RemoveMember", "store.sql_channel.permanent_delete_members_by_user.app_error", nil, "user_id="+userId+", "+err.Error(), http.StatusInternalServerError) + return model.NewAppError("SqlChannelStore.ChannelPermanentDeleteMembersByUser", "store.sql_channel.permanent_delete_members_by_user.app_error", nil, "user_id="+userId+", "+err.Error(), http.StatusInternalServerError) } return nil } diff --git a/store/sqlstore/group_store.go b/store/sqlstore/group_store.go index 7ce65b23c4..f0e4555ff3 100644 --- a/store/sqlstore/group_store.go +++ b/store/sqlstore/group_store.go @@ -373,6 +373,13 @@ func (s *SqlGroupStore) DeleteMember(groupID string, userID string) (*model.Grou return retrievedMember, nil } +func (s *SqlGroupStore) PermanentDeleteMembersByUser(userId string) *model.AppError { + if _, err := s.GetMaster().Exec("DELETE FROM GroupMembers WHERE UserId = :UserId", map[string]interface{}{"UserId": userId}); err != nil { + return model.NewAppError("SqlGroupStore.GroupPermanentDeleteMembersByUser", "store.sql_group.permanent_delete_members_by_user.app_error", map[string]interface{}{"UserId": userId}, "", http.StatusInternalServerError) + } + return nil +} + func (s *SqlGroupStore) CreateGroupSyncable(groupSyncable *model.GroupSyncable) (*model.GroupSyncable, *model.AppError) { if err := groupSyncable.IsValid(); err != nil { return nil, err diff --git a/store/store.go b/store/store.go index ab28c0f588..26fb519a7e 100644 --- a/store/store.go +++ b/store/store.go @@ -582,6 +582,7 @@ type GroupStore interface { GetMemberCount(groupID string) (int64, *model.AppError) UpsertMember(groupID string, userID string) (*model.GroupMember, *model.AppError) DeleteMember(groupID string, userID string) (*model.GroupMember, *model.AppError) + PermanentDeleteMembersByUser(userId string) *model.AppError CreateGroupSyncable(groupSyncable *model.GroupSyncable) (*model.GroupSyncable, *model.AppError) GetGroupSyncable(groupID string, syncableID string, syncableType model.GroupSyncableType) (*model.GroupSyncable, *model.AppError) diff --git a/store/storetest/group_store.go b/store/storetest/group_store.go index 0a289cc3ef..125173ef23 100644 --- a/store/storetest/group_store.go +++ b/store/storetest/group_store.go @@ -33,6 +33,7 @@ func TestGroupStore(t *testing.T, ss store.Store) { t.Run("GetMemberUsersPage", func(t *testing.T) { testGroupGetMemberUsersPage(t, ss) }) t.Run("UpsertMember", func(t *testing.T) { testUpsertMember(t, ss) }) t.Run("DeleteMember", func(t *testing.T) { testGroupDeleteMember(t, ss) }) + t.Run("PermanentDeleteMembersByUser", func(t *testing.T) { testGroupPermanentDeleteMembersByUser(t, ss) }) t.Run("CreateGroupSyncable", func(t *testing.T) { testCreateGroupSyncable(t, ss) }) t.Run("GetGroupSyncable", func(t *testing.T) { testGetGroupSyncable(t, ss) }) @@ -762,6 +763,42 @@ func testGroupDeleteMember(t *testing.T, ss store.Store) { require.Equal(t, err.Id, "store.sql_group.no_rows") } +func testGroupPermanentDeleteMembersByUser(t *testing.T, ss store.Store) { + var g *model.Group + var groups []*model.Group + numberOfGroups := 5 + + for i := 0; i < numberOfGroups; i++ { + g = &model.Group{ + Name: model.NewId(), + DisplayName: model.NewId(), + Source: model.GroupSourceLdap, + RemoteId: model.NewId(), + } + group, err := ss.Group().Create(g) + groups = append(groups, group) + require.Nil(t, err) + } + + // Create user + u1 := &model.User{ + Email: MakeEmail(), + Username: model.NewId(), + } + user, err := ss.User().Save(u1) + require.Nil(t, err) + + // Create members + for _, group := range groups { + _, err = ss.Group().UpsertMember(group.Id, user.Id) + require.Nil(t, err) + } + + // Happy path + err = ss.Group().PermanentDeleteMembersByUser(user.Id) + require.Nil(t, err) +} + func testCreateGroupSyncable(t *testing.T, ss store.Store) { // Invalid GroupID _, err := ss.Group().CreateGroupSyncable(model.NewGroupTeam("x", model.NewId(), false)) diff --git a/store/storetest/mocks/GroupStore.go b/store/storetest/mocks/GroupStore.go index e5328e4e50..2c1a7fbefc 100644 --- a/store/storetest/mocks/GroupStore.go +++ b/store/storetest/mocks/GroupStore.go @@ -654,6 +654,22 @@ func (_m *GroupStore) GetMemberUsersPage(groupID string, page int, perPage int) return r0, r1 } +// PermanentDeleteMembersByUser provides a mock function with given fields: userId +func (_m *GroupStore) PermanentDeleteMembersByUser(userId string) *model.AppError { + ret := _m.Called(userId) + + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(string) *model.AppError); ok { + r0 = rf(userId) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.AppError) + } + } + + return r0 +} + // TeamMembersMinusGroupMembers provides a mock function with given fields: teamID, groupIDs, page, perPage func (_m *GroupStore) TeamMembersMinusGroupMembers(teamID string, groupIDs []string, page int, perPage int) ([]*model.UserWithGroups, *model.AppError) { ret := _m.Called(teamID, groupIDs, page, perPage)