Skip to content

Commit f9653f0

Browse files
Gustedearl-warren
authored andcommitted
[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) (cherry picked from commit 634c5604d430b1b531467783bc70bb4efbee023d) (cherry picked from commit 492ff6379adab7dba1f55e6037fd7be7d8804a0d) (cherry picked from commit 496b04f8e7dfe5e52fa1ab5c81591c548b588897) (cherry picked from commit 3c6e5fca849ef6621bd5e2767f78139abcf44f2a) (cherry picked from commit b309d0b54914a73dec2c075265dc2b5a70e62ac2)
1 parent cd38789 commit f9653f0

File tree

9 files changed

+60
-13
lines changed

9 files changed

+60
-13
lines changed

models/fixtures/issue.yml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,3 +338,20 @@
338338
created_unix: 978307210
339339
updated_unix: 978307210
340340
is_locked: false
341+
342+
-
343+
id: 21
344+
repo_id: 10
345+
index: 2
346+
poster_id: 8
347+
original_author_id: 0
348+
name: issue for pr
349+
content: content
350+
milestone_id: 0
351+
priority: 0
352+
is_closed: false
353+
is_pull: false
354+
num_comments: 0
355+
created_unix: 946684830
356+
updated_unix: 978307200
357+
is_locked: false

models/fixtures/issue_index.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
max_index: 2
1010
-
1111
group_id: 10
12-
max_index: 1
12+
max_index: 2
1313
-
1414
group_id: 32
1515
max_index: 2

models/fixtures/repository.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@
283283
num_watches: 0
284284
num_stars: 0
285285
num_forks: 1
286-
num_issues: 0
286+
num_issues: 1
287287
num_closed_issues: 0
288288
num_pulls: 1
289289
num_closed_pulls: 0

options/locale/locale_en-US.ini

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2823,7 +2823,7 @@ users.cannot_delete_self = "You cannot delete yourself"
28232823
users.still_own_repo = This user still owns one or more repositories. Delete or transfer these repositories first.
28242824
users.still_has_org = This user is a member of an organization. Remove the user from any organizations first.
28252825
users.purge = Purge User
2826-
users.purge_help = Forcibly delete user and any repositories, organizations, and packages owned by the user. All comments will be deleted too.
2826+
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.
28272827
users.still_own_packages = This user still owns one or more packages, delete these packages first.
28282828
users.deletion_success = The user account has been deleted.
28292829
users.reset_2fa = Reset 2FA

services/user/delete.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
repo_model "code.gitea.io/gitea/models/repo"
2424
user_model "code.gitea.io/gitea/models/user"
2525
"code.gitea.io/gitea/modules/setting"
26+
issue_service "code.gitea.io/gitea/services/issue"
2627

2728
"xorm.io/builder"
2829
)
@@ -127,6 +128,31 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
127128
}
128129
}
129130

131+
// ***** START: Issues *****
132+
if purge {
133+
const batchSize = 50
134+
135+
for {
136+
issues := make([]*issues_model.Issue, 0, batchSize)
137+
if err = e.Where("poster_id=?", u.ID).Limit(batchSize, 0).Find(&issues); err != nil {
138+
return err
139+
}
140+
if len(issues) == 0 {
141+
break
142+
}
143+
144+
for _, issue := range issues {
145+
// NOTE: Don't open git repositories just to remove the reference data,
146+
// `git gc` is able to remove that reference which is run as a cron job
147+
// by default. Also use the deleted user as doer to delete the issue.
148+
if err = issue_service.DeleteIssue(ctx, u, nil, issue); err != nil {
149+
return err
150+
}
151+
}
152+
}
153+
}
154+
// ***** END: Issues *****
155+
130156
// ***** START: Branch Protections *****
131157
{
132158
const batchSize = 50

tests/integration/admin_user_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,10 @@ func TestAdminDeleteUser(t *testing.T) {
7575
csrf := GetCSRF(t, session, "/admin/users/8/edit")
7676
req := NewRequestWithValues(t, "POST", "/admin/users/8/delete", map[string]string{
7777
"_csrf": csrf,
78+
"purge": "true",
7879
})
7980
session.MakeRequest(t, req, http.StatusSeeOther)
8081

81-
assertUserDeleted(t, 8)
82+
assertUserDeleted(t, 8, true)
8283
unittest.CheckConsistencyFor(t, &user_model.User{})
8384
}

tests/integration/api_issue_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,7 @@ func TestAPISearchIssues(t *testing.T) {
370370
token := getUserToken(t, "user2", auth_model.AccessTokenScopeReadIssue)
371371

372372
// as this API was used in the frontend, it uses UI page size
373-
expectedIssueCount := 18 // from the fixtures
373+
expectedIssueCount := 19 // from the fixtures
374374
if expectedIssueCount > setting.UI.IssuePagingNum {
375375
expectedIssueCount = setting.UI.IssuePagingNum
376376
}
@@ -394,7 +394,7 @@ func TestAPISearchIssues(t *testing.T) {
394394
req = NewRequest(t, "GET", link.String())
395395
resp = MakeRequest(t, req, http.StatusOK)
396396
DecodeJSON(t, resp, &apiIssues)
397-
assert.Len(t, apiIssues, 11)
397+
assert.Len(t, apiIssues, 12)
398398
query.Del("since")
399399
query.Del("before")
400400

@@ -410,15 +410,15 @@ func TestAPISearchIssues(t *testing.T) {
410410
req = NewRequest(t, "GET", link.String())
411411
resp = MakeRequest(t, req, http.StatusOK)
412412
DecodeJSON(t, resp, &apiIssues)
413-
assert.EqualValues(t, "20", resp.Header().Get("X-Total-Count"))
414-
assert.Len(t, apiIssues, 20)
413+
assert.EqualValues(t, "21", resp.Header().Get("X-Total-Count"))
414+
assert.Len(t, apiIssues, 21)
415415

416416
query.Add("limit", "10")
417417
link.RawQuery = query.Encode()
418418
req = NewRequest(t, "GET", link.String())
419419
resp = MakeRequest(t, req, http.StatusOK)
420420
DecodeJSON(t, resp, &apiIssues)
421-
assert.EqualValues(t, "20", resp.Header().Get("X-Total-Count"))
421+
assert.EqualValues(t, "21", resp.Header().Get("X-Total-Count"))
422422
assert.Len(t, apiIssues, 10)
423423

424424
query = url.Values{"assigned": {"true"}, "state": {"all"}, "token": {token}}
@@ -468,7 +468,7 @@ func TestAPISearchIssuesWithLabels(t *testing.T) {
468468
defer tests.PrepareTestEnv(t)()
469469

470470
// as this API was used in the frontend, it uses UI page size
471-
expectedIssueCount := 18 // from the fixtures
471+
expectedIssueCount := 19 // from the fixtures
472472
if expectedIssueCount > setting.UI.IssuePagingNum {
473473
expectedIssueCount = setting.UI.IssuePagingNum
474474
}

tests/integration/api_nodeinfo_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func TestNodeinfo(t *testing.T) {
3333
assert.True(t, nodeinfo.OpenRegistrations)
3434
assert.Equal(t, "gitea", nodeinfo.Software.Name)
3535
assert.Equal(t, 25, nodeinfo.Usage.Users.Total)
36-
assert.Equal(t, 20, nodeinfo.Usage.LocalPosts)
36+
assert.Equal(t, 21, nodeinfo.Usage.LocalPosts)
3737
assert.Equal(t, 2, nodeinfo.Usage.LocalComments)
3838
})
3939
}

tests/integration/delete_user_test.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import (
1717
"code.gitea.io/gitea/tests"
1818
)
1919

20-
func assertUserDeleted(t *testing.T, userID int64) {
20+
func assertUserDeleted(t *testing.T, userID int64, purged bool) {
2121
unittest.AssertNotExistsBean(t, &user_model.User{ID: userID})
2222
unittest.AssertNotExistsBean(t, &user_model.Follow{UserID: userID})
2323
unittest.AssertNotExistsBean(t, &user_model.Follow{FollowID: userID})
@@ -27,6 +27,9 @@ func assertUserDeleted(t *testing.T, userID int64) {
2727
unittest.AssertNotExistsBean(t, &issues_model.IssueUser{UID: userID})
2828
unittest.AssertNotExistsBean(t, &organization.TeamUser{UID: userID})
2929
unittest.AssertNotExistsBean(t, &repo_model.Star{UID: userID})
30+
if purged {
31+
unittest.AssertNotExistsBean(t, &issues_model.Issue{PosterID: userID})
32+
}
3033
}
3134

3235
func TestUserDeleteAccount(t *testing.T) {
@@ -40,7 +43,7 @@ func TestUserDeleteAccount(t *testing.T) {
4043
})
4144
session.MakeRequest(t, req, http.StatusSeeOther)
4245

43-
assertUserDeleted(t, 8)
46+
assertUserDeleted(t, 8, false)
4447
unittest.CheckConsistencyFor(t, &user_model.User{})
4548
}
4649

0 commit comments

Comments
 (0)