improve the test

Author: lunny

models/fixtures/attachment.yml

@@ -153,3 +153,16 @@
   download_count: 0
   size: 0
   created_unix: 946684800
+
+-
+  id: 13
+  uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a23
+  repo_id: 1
+  issue_id: 3
+  release_id: 0
+  uploader_id: 0
+  comment_id: 7
+  name: code_comment_uploaded_attachment.png
+  download_count: 0
+  size: 0
+  created_unix: 946684812

models/fixtures/comment.yml

@@ -102,3 +102,12 @@
   review_id: 22
   assignee_id: 5
   created_unix: 946684817
+
+-
+  id: 12
+  type: 22 # review
+  poster_id: 100
+  issue_id: 3
+  content: ""
+  review_id: 10
+  created_unix: 946684812

models/issues/comment_test.go

@@ -124,18 +124,3 @@ func Test_UpdateIssueNumComments(t *testing.T) {
 	issue2 = unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
 	assert.Equal(t, 1, issue2.NumComments)
 }
-
-func Test_DeleteCommentWithReview(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 7})
-	assert.Equal(t, int64(10), comment.ReviewID)
-	review := unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: comment.ReviewID})
-
-	// FIXME: the test fixtures needs a review type comment to be created
-
-	// since this is the last comment of the review, it should be deleted when the comment is deleted
-	assert.NoError(t, issues_model.DeleteComment(db.DefaultContext, comment))
-
-	unittest.AssertNotExistsBean(t, &issues_model.Review{ID: review.ID})
-}

services/issue/comments_test.go

@@ -0,0 +1,38 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package issue
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/db"
+	issues_model "code.gitea.io/gitea/models/issues"
+	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/models/unittest"
+	user_model "code.gitea.io/gitea/models/user"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_DeleteCommentWithReview(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 7})
+	assert.NoError(t, comment.LoadAttachments(t.Context()))
+	assert.Len(t, comment.Attachments, 1)
+	assert.Equal(t, int64(13), comment.Attachments[0].ID)
+	assert.Equal(t, int64(10), comment.ReviewID)
+	review := unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: comment.ReviewID})
+	user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
+
+	// since this is the last comment of the review, it should be deleted when the comment is deleted
+	deletedReviewComment, err := DeleteComment(db.DefaultContext, user1, comment)
+	assert.NoError(t, err)
+	assert.NotNil(t, deletedReviewComment)
+
+	// the review should be deleted as well
+	unittest.AssertNotExistsBean(t, &issues_model.Review{ID: review.ID})
+	// the attachment should be deleted as well
+	unittest.AssertNotExistsBean(t, &repo_model.Attachment{ID: comment.Attachments[0].ID})
+}

services/user/delete.go

@@ -22,7 +22,9 @@ import (
 	pull_model "code.gitea.io/gitea/models/pull"
 	repo_model "code.gitea.io/gitea/models/repo"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/storage"
 
 	"xorm.io/builder"
 )
@@ -105,7 +107,7 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
 
 	if purge || (setting.Service.UserDeleteWithCommentsMaxTime != 0 &&
 		u.CreatedUnix.AsTime().Add(setting.Service.UserDeleteWithCommentsMaxTime).After(time.Now())) {
-		// Delete Comments
+		// Delete Comments with attachments
 		const batchSize = 50
 		for {
 			comments := make([]*issues_model.Comment, 0, batchSize)
@@ -117,9 +119,29 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
 			}
 
 			for _, comment := range comments {
+				// Delete attachments of the comments
+				if err := comment.LoadAttachments(ctx); err != nil {
+					return err
+				}
+
 				if _, err = issues_model.DeleteComment(ctx, comment); err != nil {
 					return err
 				}
+
+				// delete comment attachments
+				if _, err := repo_model.DeleteAttachments(ctx, comment.Attachments, true); err != nil {
+					return fmt.Errorf("delete attachments: %w", err)
+				}
+
+				for _, attachment := range comment.Attachments {
+					if err := storage.Attachments.Delete(repo_model.AttachmentRelativePath(attachment.UUID)); err != nil {
+						// Even delete files failed, but the attachments has been removed from database, so we
+						// should not return error but only record the error on logs.
+						// users have to delete this attachments manually or we should have a
+						// synchronize between database attachment table and attachment storage
+						log.Error("delete attachment[uuid: %s] failed: %v", attachment.UUID, err)
+					}
+				}
 			}
 		}