From 4fbfd849575de6294b2fe00fae9b3f5c65116ea8 Mon Sep 17 00:00:00 2001 From: Arya Khochare <91268931+Aryakoste@users.noreply.github.com> Date: Thu, 13 Mar 2025 19:19:38 +0530 Subject: [PATCH] [MM-61691] Deleting drafts when permanently deleting a user (#30233) * deleting drafts on permanently deleting user * verified count of drafts before deleting * i18n fix * use ExecBuilder * changed error message to specify user --------- Co-authored-by: Mattermost Build Co-authored-by: Harshil Sharma --- server/channels/app/user.go | 4 + .../channels/store/retrylayer/retrylayer.go | 21 +++++ server/channels/store/sqlstore/draft_store.go | 14 ++++ server/channels/store/store.go | 1 + .../channels/store/storetest/draft_store.go | 81 +++++++++++++++++++ .../store/storetest/mocks/DraftStore.go | 18 +++++ .../channels/store/timerlayer/timerlayer.go | 16 ++++ server/i18n/en.json | 4 + 8 files changed, 159 insertions(+) diff --git a/server/channels/app/user.go b/server/channels/app/user.go index ed2f09231ee..e20d5fb4204 100644 --- a/server/channels/app/user.go +++ b/server/channels/app/user.go @@ -1832,6 +1832,10 @@ func (a *App) PermanentDeleteUser(rctx request.CTX, user *model.User) *model.App return model.NewAppError("PermanentDeleteUser", "app.scheduled_post.permanent_delete_by_user.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } + if err := a.Srv().Store().Draft().PermanentDeleteByUser(user.Id); err != nil { + return model.NewAppError("PermanentDeleteUser", "app.drafts.permanent_delete_by_user.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + if err := a.Srv().Store().Bot().PermanentDelete(user.Id); err != nil { var invErr *store.ErrInvalidInput switch { diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index 111509144ab..8f49be7067f 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -4209,6 +4209,27 @@ func (s *RetryLayerDraftStore) GetLastCreateAtAndUserIdValuesForEmptyDraftsMigra } +func (s *RetryLayerDraftStore) PermanentDeleteByUser(userId string) error { + + tries := 0 + for { + err := s.DraftStore.PermanentDeleteByUser(userId) + if err == nil { + return nil + } + if !isRepeatableError(err) { + return err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerDraftStore) Upsert(d *model.Draft) (*model.Draft, error) { tries := 0 diff --git a/server/channels/store/sqlstore/draft_store.go b/server/channels/store/sqlstore/draft_store.go index f3661be5b52..37488e75a5b 100644 --- a/server/channels/store/sqlstore/draft_store.go +++ b/server/channels/store/sqlstore/draft_store.go @@ -181,6 +181,20 @@ func (s *SqlDraftStore) Delete(userID, channelID, rootID string) error { return nil } +func (s *SqlDraftStore) PermanentDeleteByUser(userID string) error { + query := s.getQueryBuilder(). + Delete("Drafts"). + Where(sq.Eq{ + "UserId": userID, + }) + + if _, err := s.GetMaster().ExecBuilder(query); err != nil { + return errors.Wrapf(err, "PermanentDeleteByUser: failed to delete drafts for user: %s", userID) + } + + return nil +} + // DeleteDraftsAssociatedWithPost deletes all drafts associated with a post. func (s *SqlDraftStore) DeleteDraftsAssociatedWithPost(channelID, rootID string) error { query := s.getQueryBuilder(). diff --git a/server/channels/store/store.go b/server/channels/store/store.go index 2d929b65a05..d3f6fbd3823 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -1039,6 +1039,7 @@ type DraftStore interface { GetLastCreateAtAndUserIdValuesForEmptyDraftsMigration(createAt int64, userID string) (int64, string, error) DeleteEmptyDraftsByCreateAtAndUserId(createAt int64, userID string) error DeleteOrphanDraftsByCreateAtAndUserId(createAt int64, userID string) error + PermanentDeleteByUser(userId string) error } type PostAcknowledgementStore interface { diff --git a/server/channels/store/storetest/draft_store.go b/server/channels/store/storetest/draft_store.go index bb305977c72..e20d2ce1c08 100644 --- a/server/channels/store/storetest/draft_store.go +++ b/server/channels/store/storetest/draft_store.go @@ -25,6 +25,7 @@ func TestDraftStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) t.Run("GetLastCreateAtAndUserIdValuesForEmptyDraftsMigration", func(t *testing.T) { testGetLastCreateAtAndUserIDValuesForEmptyDraftsMigration(t, rctx, ss) }) t.Run("DeleteEmptyDraftsByCreateAtAndUserId", func(t *testing.T) { testDeleteEmptyDraftsByCreateAtAndUserID(t, rctx, ss) }) t.Run("DeleteOrphanDraftsByCreateAtAndUserId", func(t *testing.T) { testDeleteOrphanDraftsByCreateAtAndUserID(t, rctx, ss) }) + t.Run("PermanentDeleteByUser", func(t *testing.T) { testPermanentDeleteDraftsByUser(t, rctx, ss) }) } func testSaveDraft(t *testing.T, rctx request.CTX, ss store.Store) { @@ -218,6 +219,86 @@ func testDeleteDraft(t *testing.T, rctx request.CTX, ss store.Store) { }) } +func testPermanentDeleteDraftsByUser(t *testing.T, rctx request.CTX, ss store.Store) { + t.Run("should delete all drafts for a given user", func(t *testing.T) { + userId := model.NewId() + channel1Id := model.NewId() + channel2Id := model.NewId() + + member1 := &model.ChannelMember{ + ChannelId: channel1Id, + UserId: userId, + NotifyProps: model.GetDefaultChannelNotifyProps(), + } + + member2 := &model.ChannelMember{ + ChannelId: channel2Id, + UserId: userId, + NotifyProps: model.GetDefaultChannelNotifyProps(), + } + + _, err := ss.Channel().SaveMember(rctx, member1) + require.NoError(t, err) + + _, err = ss.Channel().SaveMember(rctx, member2) + require.NoError(t, err) + + draft1 := &model.Draft{ + CreateAt: model.GetMillis(), + UpdateAt: model.GetMillis(), + UserId: userId, + ChannelId: channel1Id, + Message: "draft1", + } + + draft2 := &model.Draft{ + CreateAt: model.GetMillis(), + UpdateAt: model.GetMillis(), + UserId: userId, + ChannelId: channel2Id, + Message: "draft2", + } + + _, err = ss.Draft().Upsert(draft1) + require.NoError(t, err) + + _, err = ss.Draft().Upsert(draft2) + require.NoError(t, err) + + draftsResp, err := ss.Draft().GetDraftsForUser(userId, "") + assert.NoError(t, err) + assert.Len(t, draftsResp, 2) + + // Delete draft for the user + err = ss.Draft().PermanentDeleteByUser(userId) + assert.NoError(t, err) + + // Verify that no drafts exist for the user + draftsResp, err = ss.Draft().GetDraftsForUser(userId, "") + assert.NoError(t, err) + assert.Len(t, draftsResp, 0) + }) + + t.Run("should not fail if no drafts exist for the user", func(t *testing.T) { + userId := model.NewId() + + // Attempt to delete drafts for a user with no drafts + err := ss.Draft().PermanentDeleteByUser(userId) + assert.NoError(t, err) + }) + + t.Run("should handle empty user id", func(t *testing.T) { + err := ss.Draft().PermanentDeleteByUser("") + assert.NoError(t, err) + }) + + t.Run("should handle non-existing user id", func(t *testing.T) { + nonExistingUserId := model.NewId() + err := ss.Draft().PermanentDeleteByUser(nonExistingUserId) + assert.NoError(t, err) + }) +} + func testGetDraft(t *testing.T, rctx request.CTX, ss store.Store) { user := &model.User{ Id: model.NewId(), diff --git a/server/channels/store/storetest/mocks/DraftStore.go b/server/channels/store/storetest/mocks/DraftStore.go index 82ff38a2f25..00d6f2df374 100644 --- a/server/channels/store/storetest/mocks/DraftStore.go +++ b/server/channels/store/storetest/mocks/DraftStore.go @@ -181,6 +181,24 @@ func (_m *DraftStore) GetLastCreateAtAndUserIdValuesForEmptyDraftsMigration(crea return r0, r1, r2 } +// PermanentDeleteByUser provides a mock function with given fields: userId +func (_m *DraftStore) PermanentDeleteByUser(userId string) error { + ret := _m.Called(userId) + + if len(ret) == 0 { + panic("no return value specified for PermanentDeleteByUser") + } + + var r0 error + if rf, ok := ret.Get(0).(func(string) error); ok { + r0 = rf(userId) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // Upsert provides a mock function with given fields: d func (_m *DraftStore) Upsert(d *model.Draft) (*model.Draft, error) { ret := _m.Called(d) diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index bdd09444621..3641b4f6c4b 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -3429,6 +3429,22 @@ func (s *TimerLayerDraftStore) GetLastCreateAtAndUserIdValuesForEmptyDraftsMigra return result, resultVar1, err } +func (s *TimerLayerDraftStore) PermanentDeleteByUser(userId string) error { + start := time.Now() + + err := s.DraftStore.PermanentDeleteByUser(userId) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("DraftStore.PermanentDeleteByUser", success, elapsed) + } + return err +} + func (s *TimerLayerDraftStore) Upsert(d *model.Draft) (*model.Draft, error) { start := time.Now() diff --git a/server/i18n/en.json b/server/i18n/en.json index fbe85d6f4d1..8cc3875681e 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -5122,6 +5122,10 @@ "id": "app.draft.save.app_error", "translation": "Unable to save the Draft." }, + { + "id": "app.drafts.permanent_delete_by_user.app_error", + "translation": "Unable to delete drafts for user." + }, { "id": "app.email.no_rate_limiter.app_error", "translation": "Rate limiter is not set up."