From 0c9f37dbde6d594ba4addfeb1bc804989215f8c7 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 20 Jul 2025 12:39:24 -0700 Subject: [PATCH] revert unnecessary change in this pull request --- models/fixtures/comment.yml | 9 ----- models/issues/comment.go | 54 ++++------------------------ routers/api/v1/repo/issue_comment.go | 2 +- routers/web/repo/issue_comment.go | 10 ++---- services/issue/comments.go | 24 ++++++------- services/issue/comments_test.go | 40 --------------------- services/issue/issue.go | 3 +- services/user/delete.go | 2 +- 8 files changed, 23 insertions(+), 121 deletions(-) delete mode 100644 services/issue/comments_test.go diff --git a/models/fixtures/comment.yml b/models/fixtures/comment.yml index 7d472cdea40..8fde386e226 100644 --- a/models/fixtures/comment.yml +++ b/models/fixtures/comment.yml @@ -102,12 +102,3 @@ 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 diff --git a/models/issues/comment.go b/models/issues/comment.go index a69d8053998..a1e3a7dff2a 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -1113,20 +1113,20 @@ func UpdateComment(ctx context.Context, c *Comment, contentVersion int, doer *us } // DeleteComment deletes the comment -func DeleteComment(ctx context.Context, comment *Comment) (*Comment, error) { +func DeleteComment(ctx context.Context, comment *Comment) error { if _, err := db.GetEngine(ctx).ID(comment.ID).NoAutoCondition().Delete(comment); err != nil { - return nil, err + return err } if _, err := db.DeleteByBean(ctx, &ContentHistory{ CommentID: comment.ID, }); err != nil { - return nil, err + return err } if comment.Type.CountedAsConversation() { if err := UpdateIssueNumComments(ctx, comment.IssueID); err != nil { - return nil, err + return err } } if _, err := db.GetEngine(ctx).Table("action"). @@ -1134,54 +1134,14 @@ func DeleteComment(ctx context.Context, comment *Comment) (*Comment, error) { Update(map[string]any{ "is_deleted": true, }); err != nil { - return nil, err - } - - var deletedReviewComment *Comment - - // delete review & review comment if the code comment is the last comment of the review - if comment.Type == CommentTypeCode && comment.ReviewID > 0 { - if err := comment.LoadReview(ctx); err != nil { - return nil, err - } - if comment.Review != nil && comment.Review.Type == ReviewTypeComment { - res, err := db.GetEngine(ctx).ID(comment.ReviewID). - Where("NOT EXISTS (SELECT 1 FROM comment WHERE review_id = ? AND `type` = ?)", comment.ReviewID, CommentTypeCode). - Delete(new(Review)) - if err != nil { - return nil, err - } - if res > 0 { - var reviewComment Comment - has, err := db.GetEngine(ctx).Where("review_id = ?", comment.ReviewID). - And("`type` = ?", CommentTypeReview).Get(&reviewComment) - if err != nil { - return nil, err - } - if has && reviewComment.Content == "" { - if err := reviewComment.LoadAttachments(ctx); err != nil { - return nil, err - } - if len(reviewComment.Attachments) == 0 { - if _, err := db.GetEngine(ctx).ID(reviewComment.ID).Delete(new(Comment)); err != nil { - return nil, err - } - deletedReviewComment = &reviewComment - } - } - comment.ReviewID = 0 // reset review ID to 0 for the notification - } - } + return err } if err := comment.neuterCrossReferences(ctx); err != nil { - return nil, err + return err } - if err := DeleteReaction(ctx, &ReactionOptions{CommentID: comment.ID}); err != nil { - return nil, err - } - return deletedReviewComment, nil + return DeleteReaction(ctx, &ReactionOptions{CommentID: comment.ID}) } // UpdateCommentsMigrationsByType updates comments' migrations information via given git service type and original id and poster id diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go index feb9f1da64c..66a5f041292 100644 --- a/routers/api/v1/repo/issue_comment.go +++ b/routers/api/v1/repo/issue_comment.go @@ -726,7 +726,7 @@ func deleteIssueComment(ctx *context.APIContext) { return } - if _, err = issue_service.DeleteComment(ctx, ctx.Doer, comment); err != nil { + if err = issue_service.DeleteComment(ctx, ctx.Doer, comment); err != nil { ctx.APIErrorInternal(err) return } diff --git a/routers/web/repo/issue_comment.go b/routers/web/repo/issue_comment.go index 1ad6c588a77..ecaa9f05e47 100644 --- a/routers/web/repo/issue_comment.go +++ b/routers/web/repo/issue_comment.go @@ -325,18 +325,12 @@ func DeleteComment(ctx *context.Context) { return } - deletedReviewComment, err := issue_service.DeleteComment(ctx, ctx.Doer, comment) - if err != nil { + if err := issue_service.DeleteComment(ctx, ctx.Doer, comment); err != nil { ctx.ServerError("DeleteComment", err) return } - res := map[string]any{} - if deletedReviewComment != nil { - res["deletedReviewCommentHashTag"] = deletedReviewComment.HashTag() - } - - ctx.JSON(http.StatusOK, res) + ctx.Status(http.StatusOK) } // ChangeCommentReaction create a reaction for comment diff --git a/services/issue/comments.go b/services/issue/comments.go index 13e5811975d..153d2ebbd60 100644 --- a/services/issue/comments.go +++ b/services/issue/comments.go @@ -132,42 +132,40 @@ func UpdateComment(ctx context.Context, c *issues_model.Comment, contentVersion } // deleteComment deletes the comment -func deleteComment(ctx context.Context, comment *issues_model.Comment, removeAttachments bool) (*issues_model.Comment, error) { - return db.WithTx2(ctx, func(ctx context.Context) (*issues_model.Comment, error) { +func deleteComment(ctx context.Context, comment *issues_model.Comment, removeAttachments bool) error { + return db.WithTx(ctx, func(ctx context.Context) error { if removeAttachments { // load attachments before deleting the comment if err := comment.LoadAttachments(ctx); err != nil { - return nil, err + return err } } // deletedReviewComment should be a review comment with no content and no attachments - deletedReviewComment, err := issues_model.DeleteComment(ctx, comment) - if err != nil { - return nil, err + if err := issues_model.DeleteComment(ctx, comment); err != nil { + return err } if removeAttachments { // mark comment attachments as deleted if _, err := repo_model.MarkAttachmentsDeleted(ctx, comment.Attachments); err != nil { - return nil, err + return err } } - return deletedReviewComment, nil + return nil }) } -func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) (*issues_model.Comment, error) { - deletedReviewComment, err := deleteComment(ctx, comment, true) - if err != nil { - return nil, err +func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) error { + if err := deleteComment(ctx, comment, true); err != nil { + return err } attachment.AddAttachmentsToCleanQueue(ctx, comment.Attachments) notify_service.DeleteComment(ctx, doer, comment) - return deletedReviewComment, nil + return nil } // LoadCommentPushCommits Load push commits diff --git a/services/issue/comments_test.go b/services/issue/comments_test.go deleted file mode 100644 index 4088bd1020d..00000000000 --- a/services/issue/comments_test.go +++ /dev/null @@ -1,40 +0,0 @@ -// 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 - newAttachment, err := repo_model.GetAttachmentByID(t.Context(), comment.Attachments[0].ID) - assert.Error(t, err) - assert.Nil(t, newAttachment) -} diff --git a/services/issue/issue.go b/services/issue/issue.go index fe369fe1e68..967e0376749 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -315,8 +315,7 @@ func deleteIssue(ctx context.Context, issue *issues_model.Issue, deleteAttachmen } for _, comment := range issue.Comments { - _, err := deleteComment(ctx, comment, deleteAttachments) - if err != nil { + if err := deleteComment(ctx, comment, deleteAttachments); err != nil { return nil, fmt.Errorf("deleteComment [comment_id: %d]: %w", comment.ID, err) } toBeCleanedAttachments = append(toBeCleanedAttachments, comment.Attachments...) diff --git a/services/user/delete.go b/services/user/delete.go index af287a074a3..fb8ea392f94 100644 --- a/services/user/delete.go +++ b/services/user/delete.go @@ -122,7 +122,7 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (toBeCleane return nil, err } - if _, err = issues_model.DeleteComment(ctx, comment); err != nil { + if err = issues_model.DeleteComment(ctx, comment); err != nil { return nil, err }