Fix deleting code comment bug

Author: lunny

models/issues/comment.go

@@ -1122,36 +1122,66 @@ func UpdateComment(ctx context.Context, c *Comment, contentVersion int, doer *us
 }
 
 // DeleteComment deletes the comment
-func DeleteComment(ctx context.Context, comment *Comment) error {
+func DeleteComment(ctx context.Context, comment *Comment) (*Comment, error) {
 	e := db.GetEngine(ctx)
 	if _, err := e.ID(comment.ID).NoAutoCondition().Delete(comment); err != nil {
-		return err
+		return nil, err
 	}
 
 	if _, err := db.DeleteByBean(ctx, &ContentHistory{
 		CommentID: comment.ID,
 	}); err != nil {
-		return err
+		return nil, err
 	}
 
 	if comment.Type.CountedAsConversation() {
 		if err := UpdateIssueNumComments(ctx, comment.IssueID); err != nil {
-			return err
+			return nil, err
 		}
 	}
 	if _, err := e.Table("action").
 		Where("comment_id = ?", comment.ID).
 		Update(map[string]any{
 			"is_deleted": true,
 		}); err != nil {
-		return err
+		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 {
+		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 := 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
+		}
 	}
 
 	if err := comment.neuterCrossReferences(ctx); err != nil {
-		return err
+		return nil, err
 	}
 
-	return DeleteReaction(ctx, &ReactionOptions{CommentID: comment.ID})
+	if err := DeleteReaction(ctx, &ReactionOptions{CommentID: comment.ID}); err != nil {
+		return nil, err
+	}
+	return deletedReviewComment, nil
 }
 
 // UpdateCommentsMigrationsByType updates comments' migrations information via given git service type and original id and poster id

models/issues/comment_test.go

@@ -124,3 +124,18 @@ 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})
+}

routers/api/v1/repo/issue_comment.go

@@ -721,12 +721,12 @@ func deleteIssueComment(ctx *context.APIContext) {
 	if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull)) {
 		ctx.Status(http.StatusForbidden)
 		return
-	} else if comment.Type != issues_model.CommentTypeComment {
+	} else if comment.Type != issues_model.CommentTypeComment && comment.Type == issues_model.CommentTypeCode {
 		ctx.Status(http.StatusNoContent)
 		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
 	}

routers/web/repo/issue_comment.go

@@ -325,12 +325,18 @@ func DeleteComment(ctx *context.Context) {
 		return
 	}
 
-	if err = issue_service.DeleteComment(ctx, ctx.Doer, comment); err != nil {
+	deletedReviewComment, err := issue_service.DeleteComment(ctx, ctx.Doer, comment)
+	if err != nil {
 		ctx.ServerError("DeleteComment", err)
 		return
 	}
 
-	ctx.Status(http.StatusOK)
+	res := map[string]any{}
+	if deletedReviewComment != nil {
+		res["deletedReviewCommentHashTag"] = deletedReviewComment.HashTag()
+	}
+
+	ctx.JSON(http.StatusOK, res)
 }
 
 // ChangeCommentReaction create a reaction for comment

services/issue/comments.go

@@ -15,6 +15,8 @@ import (
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/json"
+	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/storage"
 	"code.gitea.io/gitea/modules/timeutil"
 	git_service "code.gitea.io/gitea/services/git"
 	notify_service "code.gitea.io/gitea/services/notify"
@@ -131,17 +133,40 @@ 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 {
-		return issues_model.DeleteComment(ctx, comment)
+func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) (*issues_model.Comment, error) {
+	deletedReviewComment, err := db.WithTx2(ctx, func(ctx context.Context) (*issues_model.Comment, error) {
+		if err := comment.LoadAttachments(ctx); err != nil {
+			return nil, err
+		}
+
+		deletedReviewComment, err := issues_model.DeleteComment(ctx, comment)
+		if err != nil {
+			return nil, err
+		}
+
+		// delete comment attachments
+		if _, err := repo_model.DeleteAttachments(ctx, comment.Attachments, true); err != nil {
+			return nil, 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)
+			}
+		}
+		return deletedReviewComment, nil
 	})
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	notify_service.DeleteComment(ctx, doer, comment)
 
-	return nil
+	return deletedReviewComment, nil
 }
 
 // LoadCommentPushCommits Load push commits

services/user/delete.go

@@ -117,7 +117,7 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
 			}
 
 			for _, comment := range comments {
-				if err = issues_model.DeleteComment(ctx, comment); err != nil {
+				if _, err = issues_model.DeleteComment(ctx, comment); err != nil {
 					return err
 				}
 			}

web_src/js/features/repo-issue.ts

@@ -150,6 +150,12 @@ export function initRepoIssueCommentDelete() {
           counter.textContent = String(num);
         }
 
+        const json: Record<string, any> = await response.json();
+        if (json.errorMessage) throw new Error(json.errorMessage);
+
+        if (json.deletedReviewCommentHashTag) {
+          document.querySelector(`#${json.deletedReviewCommentHashTag}`)?.remove();
+        }
         document.querySelector(`#${deleteButton.getAttribute('data-comment-id')}`)?.remove();
 
         if (conversationHolder && !conversationHolder.querySelector('.comment')) {