Revert "Replies to outdated code comments should also be outdated (#13217)" (#13439)

This reverts commit 3cab3bee57.

models/issue_comment.go

@@ -712,7 +712,6 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
 		RefAction:        opts.RefAction,
 		RefIsPull:        opts.RefIsPull,
 		IsForcePush:      opts.IsForcePush,
-		Invalidated:      opts.Invalidated,
 	}
 	if _, err = e.Insert(comment); err != nil {
 		return nil, err
@@ -879,7 +878,6 @@ type CreateCommentOptions struct {
 	RefAction        references.XRefAction
 	RefIsPull        bool
 	IsForcePush      bool
-	Invalidated      bool
 }
 
 // CreateComment creates comment of issue or commit.
@@ -955,8 +953,6 @@ type FindCommentsOptions struct {
 	ReviewID int64
 	Since    int64
 	Before   int64
-	Line     int64
-	TreePath string
 	Type     CommentType
 }
 
@@ -980,12 +976,6 @@ func (opts *FindCommentsOptions) toConds() builder.Cond {
 	if opts.Type != CommentTypeUnknown {
 		cond = cond.And(builder.Eq{"comment.type": opts.Type})
 	}
-	if opts.Line > 0 {
-		cond = cond.And(builder.Eq{"comment.line": opts.Line})
-	}
-	if len(opts.TreePath) > 0 {
-		cond = cond.And(builder.Eq{"comment.tree_path": opts.TreePath})
-	}
 	return cond
 }
 
@@ -1000,8 +990,6 @@ func findComments(e Engine, opts FindCommentsOptions) ([]*Comment, error) {
 		sess = opts.setSessionPagination(sess)
 	}
 
-	// WARNING: If you change this order you will need to fix createCodeComment
-
 	return comments, sess.
 		Asc("comment.created_unix").
 		Asc("comment.id").

models/migrations/migrations.go

@@ -250,8 +250,6 @@ var migrations = []Migration{
 	NewMigration("fix publisher ID for tag releases", fixPublisherIDforTagReleases),
 	// v157 -> v158
 	NewMigration("ensure repo topics are up-to-date", fixRepoTopics),
-	// v158 -> v159
-	NewMigration("code comment replies should have the commitID of the review they are replying to", updateCodeCommentReplies),
 }
 
 // GetCurrentDBVersion returns the current db version

models/migrations/v158.go

@@ -1,78 +0,0 @@
-// Copyright 2020 The Gitea Authors. All rights reserved.
-// Use of this source code is governed by a MIT-style
-// license that can be found in the LICENSE file.
-
-package migrations
-
-import (
-	"code.gitea.io/gitea/modules/log"
-
-	"xorm.io/xorm"
-)
-
-func updateCodeCommentReplies(x *xorm.Engine) error {
-	type Comment struct {
-		ID          int64  `xorm:"pk autoincr"`
-		CommitSHA   string `xorm:"VARCHAR(40)"`
-		Patch       string `xorm:"TEXT patch"`
-		Invalidated bool
-
-		// Not extracted but used in the below query
-		Type     int   `xorm:"INDEX"`
-		Line     int64 // - previous line / + proposed line
-		TreePath string
-		ReviewID int64 `xorm:"index"`
-	}
-
-	if err := x.Sync2(new(Comment)); err != nil {
-		return err
-	}
-
-	sess := x.NewSession()
-	defer sess.Close()
-	if err := sess.Begin(); err != nil {
-		return err
-	}
-
-	var start = 0
-	var batchSize = 100
-	for {
-		var comments = make([]*Comment, 0, batchSize)
-		if err := sess.SQL(`SELECT comment.id as id, first.commit_sha as commit_sha, first.patch as patch, first.invalidated as invalidated
-		FROM comment INNER JOIN (
-			SELECT C.id, C.review_id, C.line, C.tree_path, C.patch, C.commit_sha, C.invalidated
-			FROM comment AS C 
-			WHERE C.type = 21
-				AND C.created_unix = 
-					(SELECT MIN(comment.created_unix)
-						FROM comment
-						WHERE comment.review_id = C.review_id
-						AND comment.type = 21
-						AND comment.line = C.line
-						AND comment.tree_path = C.tree_path)
-			) AS first
-			ON comment.review_id = first.review_id
-				AND comment.tree_path = first.tree_path AND comment.line = first.line
-		WHERE comment.type = 21
-			AND comment.id != first.id
-			AND comment.commit_sha != first.commit_sha`).Limit(batchSize, start).Find(&comments); err != nil {
-			log.Error("failed to select: %v", err)
-			return err
-		}
-
-		for _, comment := range comments {
-			if _, err := sess.Table("comment").Cols("commit_sha", "patch", "invalidated").Update(comment); err != nil {
-				log.Error("failed to update comment[%d]: %v %v", comment.ID, comment, err)
-				return err
-			}
-		}
-
-		start += len(comments)
-
-		if len(comments) < batchSize {
-			break
-		}
-	}
-
-	return sess.Commit()
-}

services/pull/review.go

@@ -122,76 +122,41 @@ func createCodeComment(doer *models.User, repo *models.Repository, issue *models
 	}
 	defer gitRepo.Close()
 
-	invalidated := false
+	// FIXME validate treePath
-	head := pr.GetGitRefName()
+	// Get latest commit referencing the commented line
+	// No need for get commit for base branch changes
 	if line > 0 {
-		if reviewID != 0 {
+		commit, err := gitRepo.LineBlame(pr.GetGitRefName(), gitRepo.Path, treePath, uint(line))
-			first, err := models.FindComments(models.FindCommentsOptions{
+		if err == nil {
-				ReviewID: reviewID,
+			commitID = commit.ID.String()
-				Line:     line,
+		} else if !(strings.Contains(err.Error(), "exit status 128 - fatal: no such path") || notEnoughLines.MatchString(err.Error())) {
-				TreePath: treePath,
+			return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err)
-				Type:     models.CommentTypeCode,
-				ListOptions: models.ListOptions{
-					PageSize: 1,
-					Page:     1,
-				},
-			})
-			if err == nil && len(first) > 0 {
-				commitID = first[0].CommitSHA
-				invalidated = first[0].Invalidated
-				patch = first[0].Patch
-			} else if err != nil && !models.IsErrCommentNotExist(err) {
-				return nil, fmt.Errorf("Find first comment for %d line %d path %s. Error: %v", reviewID, line, treePath, err)
-			} else {
-				review, err := models.GetReviewByID(reviewID)
-				if err == nil && len(review.CommitID) > 0 {
-					head = review.CommitID
-				} else if err != nil && !models.IsErrReviewNotExist(err) {
-					return nil, fmt.Errorf("GetReviewByID %d. Error: %v", reviewID, err)
-				}
-			}
-		}
-
-		if len(commitID) == 0 {
-			// FIXME validate treePath
-			// Get latest commit referencing the commented line
-			// No need for get commit for base branch changes
-			commit, err := gitRepo.LineBlame(head, gitRepo.Path, treePath, uint(line))
-			if err == nil {
-				commitID = commit.ID.String()
-			} else if !(strings.Contains(err.Error(), "exit status 128 - fatal: no such path") || notEnoughLines.MatchString(err.Error())) {
-				return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err)
-			}
 		}
 	}
 
 	// Only fetch diff if comment is review comment
-	if len(patch) == 0 && reviewID != 0 {
+	if reviewID != 0 {
-		if len(commitID) == 0 {
+		headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName())
-			commitID, err = gitRepo.GetRefCommitID(pr.GetGitRefName())
+		if err != nil {
-			if err != nil {
+			return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err)
-				return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err)
-			}
 		}
-
 		patchBuf := new(bytes.Buffer)
-		if err := git.GetRepoRawDiffForFile(gitRepo, pr.MergeBase, commitID, git.RawDiffNormal, treePath, patchBuf); err != nil {
+		if err := git.GetRepoRawDiffForFile(gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, treePath, patchBuf); err != nil {
-			return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", gitRepo.Path, pr.MergeBase, commitID, treePath, err)
+			return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", err, gitRepo.Path, pr.MergeBase, headCommitID, treePath)
 		}
 		patch = git.CutDiffAroundLine(patchBuf, int64((&models.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines)
 	}
 	return models.CreateComment(&models.CreateCommentOptions{
-		Type:        models.CommentTypeCode,
+		Type:      models.CommentTypeCode,
-		Doer:        doer,
+		Doer:      doer,
-		Repo:        repo,
+		Repo:      repo,
-		Issue:       issue,
+		Issue:     issue,
-		Content:     content,
+		Content:   content,
-		LineNum:     line,
+		LineNum:   line,
-		TreePath:    treePath,
+		TreePath:  treePath,
-		CommitSHA:   commitID,
+		CommitSHA: commitID,
-		ReviewID:    reviewID,
+		ReviewID:  reviewID,
-		Patch:       patch,
+		Patch:     patch,
-		Invalidated: invalidated,
 	})
 }