Fix

services/doctor/review.go

@@ -1,4 +1,4 @@
-// Copyright 2021 The Gitea Authors. All rights reserved.
+// Copyright 2025 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
 package doctor

services/issue/comments.go

@@ -134,11 +134,6 @@ func UpdateComment(ctx context.Context, c *issues_model.Comment, contentVersion
 
 // DeleteComment deletes the comment
 func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) error {
-	err := db.WithTx(ctx, func(ctx context.Context) error {
-		if err := issues_model.DeleteComment(ctx, comment); err != nil {
-			return err
-		}
-
 	if comment.ReviewID > 0 {
 		if err := comment.LoadIssue(ctx); err != nil {
 			return err
@@ -149,6 +144,15 @@ func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_m
 		if err := comment.Issue.LoadPullRequest(ctx); err != nil {
 			return err
 		}
+	}
+
+	if err := db.WithTx(ctx, func(ctx context.Context) error {
+		return issues_model.DeleteComment(ctx, comment)
+	}); err != nil {
+		return err
+	}
+
+	if comment.ReviewID > 0 {
 		if err := git.RemoveRef(ctx, comment.Issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(comment.Issue.PullRequest.Index, comment.ID)); err != nil {
 			log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", comment.Issue.PullRequest.ID, err)
 			// We should not return error here, because the comment has been removed from database.
@@ -157,12 +161,6 @@ func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_m
 		}
 	}
 
-		return nil
-	})
-	if err != nil {
-		return err
-	}
-
 	notify_service.DeleteComment(ctx, doer, comment)
 
 	return nil

services/issue/issue.go

@@ -191,7 +191,7 @@ func DeleteIssue(ctx context.Context, doer *user_model.User, gitRepo *git.Reposi
 	}
 
 	// delete entries in database
-	attachmentPaths, err := deleteIssue(ctx, issue)
+	attachmentPaths, comments, err := deleteIssue(ctx, issue)
 	if err != nil {
 		return err
 	}
@@ -211,6 +211,17 @@ func DeleteIssue(ctx context.Context, doer *user_model.User, gitRepo *git.Reposi
 		}
 	}
 
+	for _, comment := range comments {
+		if comment.ReviewID > 0 {
+			if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID)); err != nil {
+				log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", issue.PullRequest.ID, err)
+				// We should not return error here, because the comment has been removed from database.
+				// users have to delete this ref manually or we should have a synchronize between
+				// database comment table and git refs.
+			}
+		}
+	}
+
 	notify_service.DeleteIssue(ctx, doer, issue)
 
 	return nil
@@ -266,53 +277,48 @@ func GetRefEndNamesAndURLs(issues []*issues_model.Issue, repoLink string) (map[i
 }
 
 // deleteIssue deletes the issue
-func deleteIssue(ctx context.Context, issue *issues_model.Issue) ([]string, error) {
+func deleteIssue(ctx context.Context, issue *issues_model.Issue) ([]string, []*issues_model.Comment, error) {
-	ctx, committer, err := db.TxContext(ctx)
+	var attachmentPaths []string
-	if err != nil {
+	if err := db.WithTx(ctx, func(ctx context.Context) error {
-		return nil, err
-	}
-	defer committer.Close()
-
 		// update the total issue numbers
 		if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil {
-		return nil, err
+			return err
 		}
 		// if the issue is closed, update the closed issue numbers
 		if issue.IsClosed {
 			if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil {
-			return nil, err
+				return err
 			}
 		}
 
 		if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil {
-		return nil, fmt.Errorf("error updating counters for milestone id %d: %w",
+			return fmt.Errorf("error updating counters for milestone id %d: %w",
 				issue.MilestoneID, err)
 		}
 
 		if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID, issue.Index); err != nil {
-		return nil, err
+			return err
 		}
 
 		// find attachments related to this issue and remove them
 		if err := issue.LoadAttachments(ctx); err != nil {
-		return nil, err
+			return err
 		}
 
-	var attachmentPaths []string
 		for i := range issue.Attachments {
 			attachmentPaths = append(attachmentPaths, issue.Attachments[i].RelativePath())
 		}
 
 		// deference all review comments
 		if err := issue.LoadRepo(ctx); err != nil {
-		return nil, err
+			return err
 		}
 		if err := issue.LoadPullRequest(ctx); err != nil {
-		return nil, err
+			return err
 		}
 
 		if err := issue.LoadComments(ctx); err != nil {
-		return nil, err
+			return err
 		}
 
 		// delete all database data still assigned to this issue
@@ -335,32 +341,12 @@ func deleteIssue(ctx context.Context, issue *issues_model.Issue) ([]string, erro
 			&issues_model.Comment{DependentIssueID: issue.ID},
 			&issues_model.IssuePin{IssueID: issue.ID},
 		); err != nil {
-		return nil, err
+			return err
 		}
 
 		for _, comment := range issue.Comments {
 			if err := issues_model.DeleteComment(ctx, comment); err != nil {
-			return nil, err
+				return err
-		}
-
-		if comment.ReviewID > 0 {
-			if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID)); err != nil {
-				log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", issue.PullRequest.ID, err)
-				// We should not return error here, because the comment has been removed from database.
-				// users have to delete this ref manually or we should have a synchronize between
-				// database comment table and git refs.
-			}
-		}
-
-		// delete all attachments related to this comment
-		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)
-			}
 			}
 		}
 
@@ -369,28 +355,29 @@ func deleteIssue(ctx context.Context, issue *issues_model.Issue) ([]string, erro
 			// Delete scheduled auto merges
 			if _, err := db.GetEngine(ctx).Where("pull_id=?", issue.PullRequest.ID).
 				Delete(&pull_model.AutoMerge{}); err != nil {
-			return nil, err
+				return err
 			}
 
 			// Delete review states
 			if _, err := db.GetEngine(ctx).Where("pull_id=?", issue.PullRequest.ID).
 				Delete(&pull_model.ReviewState{}); err != nil {
-			return nil, err
+				return err
 			}
 
 			if _, err := db.GetEngine(ctx).ID(issue.PullRequest.ID).Delete(&issues_model.PullRequest{}); err != nil {
-			return nil, err
+				return err
 			}
 		}
 
 		if _, err := db.GetEngine(ctx).ID(issue.ID).NoAutoCondition().Delete(issue); err != nil {
-		return nil, err
+			return err
+		}
+		return nil
+	}); err != nil {
+		return nil, nil, err
 	}
 
-	if err := committer.Commit(); err != nil {
+	return attachmentPaths, issue.Comments, nil
-		return nil, err
-	}
-	return attachmentPaths, nil
 }
 
 // DeleteOrphanedIssues delete issues without a repo
@@ -438,11 +425,22 @@ func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths []
 		}
 
 		for _, issue := range issues {
-			issueAttachPaths, err := deleteIssue(ctx, issue)
+			issueAttachPaths, comments, err := deleteIssue(ctx, issue)
 			if err != nil {
 				return nil, fmt.Errorf("deleteIssue [issue_id: %d]: %w", issue.ID, err)
 			}
 
+			for _, comment := range comments {
+				if comment.ReviewID > 0 {
+					if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID)); err != nil {
+						log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", issue.PullRequest.ID, err)
+						// We should not return error here, because the comment has been removed from database.
+						// users have to delete this ref manually or we should have a synchronize between
+						// database comment table and git refs.
+					}
+				}
+			}
+
 			attachmentPaths = append(attachmentPaths, issueAttachPaths...)
 		}
 	}

services/issue/issue_test.go

@@ -41,7 +41,7 @@ func TestIssue_DeleteIssue(t *testing.T) {
 
 	issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: issueIDs[2]})
 
-	_, err = deleteIssue(db.DefaultContext, issue)
+	_, _, err = deleteIssue(db.DefaultContext, issue)
 	assert.NoError(t, err)
 	issueIDs, err = issues_model.GetIssueIDsByRepoID(db.DefaultContext, 1)
 	assert.NoError(t, err)
@@ -52,7 +52,7 @@ func TestIssue_DeleteIssue(t *testing.T) {
 	assert.NoError(t, err)
 	issue, err = issues_model.GetIssueByID(db.DefaultContext, 4)
 	assert.NoError(t, err)
-	_, err = deleteIssue(db.DefaultContext, issue)
+	_, _, err = deleteIssue(db.DefaultContext, issue)
 	assert.NoError(t, err)
 	assert.Len(t, attachments, 2)
 	for i := range attachments {
@@ -75,7 +75,7 @@ func TestIssue_DeleteIssue(t *testing.T) {
 	assert.NoError(t, err)
 	assert.False(t, left)
 
-	_, err = deleteIssue(db.DefaultContext, issue2)
+	_, _, err = deleteIssue(db.DefaultContext, issue2)
 	assert.NoError(t, err)
 	left, err = issues_model.IssueNoDependenciesLeft(db.DefaultContext, issue1)
 	assert.NoError(t, err)