Skip to content

Commit 006291b

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 219073f5c5558e7712039a83754f68b092689963) (cherry picked from commit 32893dbab139e2d238db8c3d7878321c6bdd0cd3) (cherry picked from commit 0ef40cfb5a23d9f654e093ade2668d82ce8d333a) (cherry picked from commit e535409cab3c276fd8db6b402f85934ef5127491) (cherry picked from commit 29059f611b5617d275737996b9e4076a3b0b667e) (cherry picked from commit cd480c5b8b99feed11a3797ab36a697cd0dcc91b) (cherry picked from commit 340e6573924dbd8d69843c69243e6b027c66f166) (cherry picked from commit 3a7a5564d02bde767cb14cbc30e3ca816808f7d6) (cherry picked from commit 1dbcaca726f3cbd777a4965b7414d6b60050ba54) (cherry picked from commit c491c439e28f04fdd0f002ec1403b19933afc7c3) (cherry picked from commit c8fe2140cc15dcffa9bb7c966493707ac23cdb74) (cherry picked from commit c72564e3ee1bd9972d4f7d7a7e1dc34bb8d81299) (cherry picked from commit 2084f3fa113bca751be0689f53dfeb7d059ffb8f) (cherry picked from commit 918e65327da011303ba7dfb3ff6970b8c83f5319)
1 parent a485566 commit 006291b

File tree

8 files changed

+59
-12
lines changed

8 files changed

+59
-12
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
@@ -2875,7 +2875,7 @@ users.cannot_delete_self = "You cannot delete yourself"
28752875
users.still_own_repo = This user still owns one or more repositories. Delete or transfer these repositories first.
28762876
users.still_has_org = This user is a member of an organization. Remove the user from any organizations first.
28772877
users.purge = Purge User
2878-
users.purge_help = Forcibly delete user and any repositories, organizations, and packages owned by the user. All comments will be deleted too.
2878+
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.
28792879
users.still_own_packages = This user still owns one or more packages, delete these packages first.
28802880
users.deletion_success = The user account has been deleted.
28812881
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
@@ -368,7 +368,7 @@ func TestAPISearchIssues(t *testing.T) {
368368
defer tests.PrepareTestEnv(t)()
369369

370370
// as this API was used in the frontend, it uses UI page size
371-
expectedIssueCount := 18 // from the fixtures
371+
expectedIssueCount := 19 // from the fixtures
372372
if expectedIssueCount > setting.UI.IssuePagingNum {
373373
expectedIssueCount = setting.UI.IssuePagingNum
374374
}
@@ -392,7 +392,7 @@ func TestAPISearchIssues(t *testing.T) {
392392
req = NewRequest(t, "GET", link.String()).AddTokenAuth(token)
393393
resp = MakeRequest(t, req, http.StatusOK)
394394
DecodeJSON(t, resp, &apiIssues)
395-
assert.Len(t, apiIssues, 11)
395+
assert.Len(t, apiIssues, 12)
396396
query.Del("since")
397397
query.Del("before")
398398

@@ -408,15 +408,15 @@ func TestAPISearchIssues(t *testing.T) {
408408
req = NewRequest(t, "GET", link.String()).AddTokenAuth(token)
409409
resp = MakeRequest(t, req, http.StatusOK)
410410
DecodeJSON(t, resp, &apiIssues)
411-
assert.EqualValues(t, "20", resp.Header().Get("X-Total-Count"))
412-
assert.Len(t, apiIssues, 20)
411+
assert.EqualValues(t, "21", resp.Header().Get("X-Total-Count"))
412+
assert.Len(t, apiIssues, 21)
413413

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

422422
query = url.Values{"assigned": {"true"}, "state": {"all"}}
@@ -466,7 +466,7 @@ func TestAPISearchIssuesWithLabels(t *testing.T) {
466466
defer tests.PrepareTestEnv(t)()
467467

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

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)