From 634c5604d430b1b531467783bc70bb4efbee023d Mon Sep 17 00:00:00 2001
From: Gusted <postmaster@gusted.xyz>
Date: Sat, 26 Aug 2023 23:10:42 +0200
Subject: [PATCH] [MODERATION] Purge issues on user deletion
- Forgejo has the option to delete users, in which all data except
issues and comments are removed, this makes sense in some cases where
users need to be removed cleanly but without removing their existing bug
reports or comments to an discussion. In the case of spammers, admins
have the option to enable purging, where comments are removed.
- Add issues to the list of things to be removed if purge is checked.
- No unit testing, as this gigantic function doesn't have one to begin
with.
- Add integration test.
- Resolves https://codeberg.org/forgejo/forgejo/issues/1268
(cherry picked from commit 3ed381c75826ffc6834fd54943f71579c060c16d)
(cherry picked from commit 44d00650ce77bd4395892a62a64a90829578c81d)
(cherry picked from commit 7f4da82779fa1d761b5fe045d3e0b4b2627638c0)
(cherry picked from commit d629314def8e3e6d0f78184aa584fa57ece18bb1)
Conflicts:
models/fixtures/issue.yml
https://codeberg.org/forgejo/forgejo/pulls/1508
(cherry picked from commit 794dcc218f2c0c53028aaf617407d46bddda57f3)
(cherry picked from commit c433f2ecb60669e5c8748912b30c0433d5fe507a)
(cherry picked from commit bb23683f4b10a504da677843bc2ae2b73ec299c4)
---
models/fixtures/issue.yml | 17 +++++++++++++++++
models/fixtures/issue_index.yml | 2 +-
models/fixtures/repository.yml | 2 +-
options/locale/locale_en-US.ini | 2 +-
services/user/delete.go | 26 ++++++++++++++++++++++++++
tests/integration/admin_user_test.go | 3 ++-
tests/integration/api_issue_test.go | 12 ++++++------
tests/integration/api_nodeinfo_test.go | 2 +-
tests/integration/delete_user_test.go | 7 +++++--
9 files changed, 60 insertions(+), 13 deletions(-)
diff --git a/models/fixtures/issue.yml b/models/fixtures/issue.yml
index 0c9b6ff406..da7db94015 100644
--- a/models/fixtures/issue.yml
+++ b/models/fixtures/issue.yml
@@ -338,3 +338,20 @@
created_unix: 978307210
updated_unix: 978307210
is_locked: false
+
+-
+ id: 21
+ repo_id: 10
+ index: 2
+ poster_id: 8
+ original_author_id: 0
+ name: issue for pr
+ content: content
+ milestone_id: 0
+ priority: 0
+ is_closed: false
+ is_pull: false
+ num_comments: 0
+ created_unix: 946684830
+ updated_unix: 978307200
+ is_locked: false
diff --git a/models/fixtures/issue_index.yml b/models/fixtures/issue_index.yml
index de6e955804..ceae892d99 100644
--- a/models/fixtures/issue_index.yml
+++ b/models/fixtures/issue_index.yml
@@ -9,7 +9,7 @@
max_index: 2
-
group_id: 10
- max_index: 1
+ max_index: 2
-
group_id: 32
max_index: 2
diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml
index bc63969789..382d7c14ac 100644
--- a/models/fixtures/repository.yml
+++ b/models/fixtures/repository.yml
@@ -283,7 +283,7 @@
num_watches: 0
num_stars: 0
num_forks: 1
- num_issues: 0
+ num_issues: 1
num_closed_issues: 0
num_pulls: 1
num_closed_pulls: 0
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index f728ba06fa..76c302de77 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -2851,7 +2851,7 @@ users.cannot_delete_self = "You cannot delete yourself"
users.still_own_repo = This user still owns one or more repositories. Delete or transfer these repositories first.
users.still_has_org = This user is a member of an organization. Remove the user from any organizations first.
users.purge = Purge User
-users.purge_help = Forcibly delete user and any repositories, organizations, and packages owned by the user. All comments will be deleted too.
+users.purge_help = Forcibly delete user and any repositories, organizations, and packages owned by the user. All comments and issues posted by this user will also be deleted.
users.still_own_packages = This user still owns one or more packages, delete these packages first.
users.deletion_success = The user account has been deleted.
users.reset_2fa = Reset 2FA
diff --git a/services/user/delete.go b/services/user/delete.go
index 0f33d712a4..8abba6cb0b 100644
--- a/services/user/delete.go
+++ b/services/user/delete.go
@@ -22,6 +22,7 @@ import (
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/setting"
+ issue_service "code.gitea.io/gitea/services/issue"
"xorm.io/builder"
)
@@ -127,6 +128,31 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
}
}
+ // ***** START: Issues *****
+ if purge {
+ const batchSize = 50
+
+ for {
+ issues := make([]*issues_model.Issue, 0, batchSize)
+ if err = e.Where("poster_id=?", u.ID).Limit(batchSize, 0).Find(&issues); err != nil {
+ return err
+ }
+ if len(issues) == 0 {
+ break
+ }
+
+ for _, issue := range issues {
+ // NOTE: Don't open git repositories just to remove the reference data,
+ // `git gc` is able to remove that reference which is run as a cron job
+ // by default. Also use the deleted user as doer to delete the issue.
+ if err = issue_service.DeleteIssue(ctx, u, nil, issue); err != nil {
+ return err
+ }
+ }
+ }
+ }
+ // ***** END: Issues *****
+
// ***** START: Branch Protections *****
{
const batchSize = 50
diff --git a/tests/integration/admin_user_test.go b/tests/integration/admin_user_test.go
index 669060c787..682d632a0f 100644
--- a/tests/integration/admin_user_test.go
+++ b/tests/integration/admin_user_test.go
@@ -75,9 +75,10 @@ func TestAdminDeleteUser(t *testing.T) {
csrf := GetCSRF(t, session, "/admin/users/8/edit")
req := NewRequestWithValues(t, "POST", "/admin/users/8/delete", map[string]string{
"_csrf": csrf,
+ "purge": "true",
})
session.MakeRequest(t, req, http.StatusSeeOther)
- assertUserDeleted(t, 8)
+ assertUserDeleted(t, 8, true)
unittest.CheckConsistencyFor(t, &user_model.User{})
}
diff --git a/tests/integration/api_issue_test.go b/tests/integration/api_issue_test.go
index e2aa98ad41..248c59ac86 100644
--- a/tests/integration/api_issue_test.go
+++ b/tests/integration/api_issue_test.go
@@ -370,7 +370,7 @@ func TestAPISearchIssues(t *testing.T) {
token := getUserToken(t, "user2", auth_model.AccessTokenScopeReadIssue)
// as this API was used in the frontend, it uses UI page size
- expectedIssueCount := 18 // from the fixtures
+ expectedIssueCount := 19 // from the fixtures
if expectedIssueCount > setting.UI.IssuePagingNum {
expectedIssueCount = setting.UI.IssuePagingNum
}
@@ -394,7 +394,7 @@ func TestAPISearchIssues(t *testing.T) {
req = NewRequest(t, "GET", link.String())
resp = MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues)
- assert.Len(t, apiIssues, 11)
+ assert.Len(t, apiIssues, 12)
query.Del("since")
query.Del("before")
@@ -410,15 +410,15 @@ func TestAPISearchIssues(t *testing.T) {
req = NewRequest(t, "GET", link.String())
resp = MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues)
- assert.EqualValues(t, "20", resp.Header().Get("X-Total-Count"))
- assert.Len(t, apiIssues, 20)
+ assert.EqualValues(t, "21", resp.Header().Get("X-Total-Count"))
+ assert.Len(t, apiIssues, 21)
query.Add("limit", "10")
link.RawQuery = query.Encode()
req = NewRequest(t, "GET", link.String())
resp = MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues)
- assert.EqualValues(t, "20", resp.Header().Get("X-Total-Count"))
+ assert.EqualValues(t, "21", resp.Header().Get("X-Total-Count"))
assert.Len(t, apiIssues, 10)
query = url.Values{"assigned": {"true"}, "state": {"all"}, "token": {token}}
@@ -468,7 +468,7 @@ func TestAPISearchIssuesWithLabels(t *testing.T) {
defer tests.PrepareTestEnv(t)()
// as this API was used in the frontend, it uses UI page size
- expectedIssueCount := 18 // from the fixtures
+ expectedIssueCount := 19 // from the fixtures
if expectedIssueCount > setting.UI.IssuePagingNum {
expectedIssueCount = setting.UI.IssuePagingNum
}
diff --git a/tests/integration/api_nodeinfo_test.go b/tests/integration/api_nodeinfo_test.go
index fb35d72ac2..4fe8ebda9e 100644
--- a/tests/integration/api_nodeinfo_test.go
+++ b/tests/integration/api_nodeinfo_test.go
@@ -33,7 +33,7 @@ func TestNodeinfo(t *testing.T) {
assert.True(t, nodeinfo.OpenRegistrations)
assert.Equal(t, "gitea", nodeinfo.Software.Name)
assert.Equal(t, 25, nodeinfo.Usage.Users.Total)
- assert.Equal(t, 20, nodeinfo.Usage.LocalPosts)
+ assert.Equal(t, 21, nodeinfo.Usage.LocalPosts)
assert.Equal(t, 3, nodeinfo.Usage.LocalComments)
})
}
diff --git a/tests/integration/delete_user_test.go b/tests/integration/delete_user_test.go
index 806b87dc4c..fa407a75ad 100644
--- a/tests/integration/delete_user_test.go
+++ b/tests/integration/delete_user_test.go
@@ -17,7 +17,7 @@ import (
"code.gitea.io/gitea/tests"
)
-func assertUserDeleted(t *testing.T, userID int64) {
+func assertUserDeleted(t *testing.T, userID int64, purged bool) {
unittest.AssertNotExistsBean(t, &user_model.User{ID: userID})
unittest.AssertNotExistsBean(t, &user_model.Follow{UserID: userID})
unittest.AssertNotExistsBean(t, &user_model.Follow{FollowID: userID})
@@ -27,6 +27,9 @@ func assertUserDeleted(t *testing.T, userID int64) {
unittest.AssertNotExistsBean(t, &issues_model.IssueUser{UID: userID})
unittest.AssertNotExistsBean(t, &organization.TeamUser{UID: userID})
unittest.AssertNotExistsBean(t, &repo_model.Star{UID: userID})
+ if purged {
+ unittest.AssertNotExistsBean(t, &issues_model.Issue{PosterID: userID})
+ }
}
func TestUserDeleteAccount(t *testing.T) {
@@ -40,7 +43,7 @@ func TestUserDeleteAccount(t *testing.T) {
})
session.MakeRequest(t, req, http.StatusSeeOther)
- assertUserDeleted(t, 8)
+ assertUserDeleted(t, 8, false)
unittest.CheckConsistencyFor(t, &user_model.User{})
}