From 35cc82abbf2315f05a5132115bc6e53786df6a3c Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 6 Nov 2020 15:04:21 +0800 Subject: [PATCH] Revert "Replies to outdated code comments should also be outdated (#13217)" (#13439) This reverts commit 3cab3bee5750a12da9ef8a9ba5cbe3da00594921. --- models/issue_comment.go | 12 ----- models/migrations/migrations.go | 2 - models/migrations/v158.go | 78 ------------------------------- services/pull/review.go | 83 ++++++++++----------------------- 4 files changed, 24 insertions(+), 151 deletions(-) delete mode 100644 models/migrations/v158.go diff --git a/models/issue_comment.go b/models/issue_comment.go index 7bcea40b93..a7e9c049bf 100644 --- a/models/issue_comment.go +++ b/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"). diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 4715f192c1..cbf8ae8732 100644 --- a/models/migrations/migrations.go +++ b/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 diff --git a/models/migrations/v158.go b/models/migrations/v158.go deleted file mode 100644 index d056ffe6dc..0000000000 --- a/models/migrations/v158.go +++ /dev/null @@ -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() -} diff --git a/services/pull/review.go b/services/pull/review.go index f0ee234a42..99afdd73c2 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -122,76 +122,41 @@ func createCodeComment(doer *models.User, repo *models.Repository, issue *models } defer gitRepo.Close() - invalidated := false - head := pr.GetGitRefName() + // FIXME validate treePath + // Get latest commit referencing the commented line + // No need for get commit for base branch changes if line > 0 { - if reviewID != 0 { - first, err := models.FindComments(models.FindCommentsOptions{ - ReviewID: reviewID, - Line: line, - TreePath: treePath, - 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) - } + commit, err := gitRepo.LineBlame(pr.GetGitRefName(), 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 len(commitID) == 0 { - commitID, err = gitRepo.GetRefCommitID(pr.GetGitRefName()) - if err != nil { - return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err) - } + if reviewID != 0 { + headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName()) + if err != nil { + 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 { - return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", gitRepo.Path, pr.MergeBase, commitID, treePath, err) + if err := git.GetRepoRawDiffForFile(gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, treePath, patchBuf); err != nil { + 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, - Doer: doer, - Repo: repo, - Issue: issue, - Content: content, - LineNum: line, - TreePath: treePath, - CommitSHA: commitID, - ReviewID: reviewID, - Patch: patch, - Invalidated: invalidated, + Type: models.CommentTypeCode, + Doer: doer, + Repo: repo, + Issue: issue, + Content: content, + LineNum: line, + TreePath: treePath, + CommitSHA: commitID, + ReviewID: reviewID, + Patch: patch, }) }