Skip to content

Commit 35cc82a

Browse files
authored
Revert "Replies to outdated code comments should also be outdated (#13217)" (#13439)
This reverts commit 3cab3be.
1 parent 3c7908b commit 35cc82a

File tree

4 files changed

+24
-151
lines changed

4 files changed

+24
-151
lines changed

models/issue_comment.go

-12
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,6 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
712712
RefAction: opts.RefAction,
713713
RefIsPull: opts.RefIsPull,
714714
IsForcePush: opts.IsForcePush,
715-
Invalidated: opts.Invalidated,
716715
}
717716
if _, err = e.Insert(comment); err != nil {
718717
return nil, err
@@ -879,7 +878,6 @@ type CreateCommentOptions struct {
879878
RefAction references.XRefAction
880879
RefIsPull bool
881880
IsForcePush bool
882-
Invalidated bool
883881
}
884882

885883
// CreateComment creates comment of issue or commit.
@@ -955,8 +953,6 @@ type FindCommentsOptions struct {
955953
ReviewID int64
956954
Since int64
957955
Before int64
958-
Line int64
959-
TreePath string
960956
Type CommentType
961957
}
962958

@@ -980,12 +976,6 @@ func (opts *FindCommentsOptions) toConds() builder.Cond {
980976
if opts.Type != CommentTypeUnknown {
981977
cond = cond.And(builder.Eq{"comment.type": opts.Type})
982978
}
983-
if opts.Line > 0 {
984-
cond = cond.And(builder.Eq{"comment.line": opts.Line})
985-
}
986-
if len(opts.TreePath) > 0 {
987-
cond = cond.And(builder.Eq{"comment.tree_path": opts.TreePath})
988-
}
989979
return cond
990980
}
991981

@@ -1000,8 +990,6 @@ func findComments(e Engine, opts FindCommentsOptions) ([]*Comment, error) {
1000990
sess = opts.setSessionPagination(sess)
1001991
}
1002992

1003-
// WARNING: If you change this order you will need to fix createCodeComment
1004-
1005993
return comments, sess.
1006994
Asc("comment.created_unix").
1007995
Asc("comment.id").

models/migrations/migrations.go

-2
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,6 @@ var migrations = []Migration{
250250
NewMigration("fix publisher ID for tag releases", fixPublisherIDforTagReleases),
251251
// v157 -> v158
252252
NewMigration("ensure repo topics are up-to-date", fixRepoTopics),
253-
// v158 -> v159
254-
NewMigration("code comment replies should have the commitID of the review they are replying to", updateCodeCommentReplies),
255253
}
256254

257255
// GetCurrentDBVersion returns the current db version

models/migrations/v158.go

-78
This file was deleted.

services/pull/review.go

+24-59
Original file line numberDiff line numberDiff line change
@@ -122,76 +122,41 @@ func createCodeComment(doer *models.User, repo *models.Repository, issue *models
122122
}
123123
defer gitRepo.Close()
124124

125-
invalidated := false
126-
head := pr.GetGitRefName()
125+
// FIXME validate treePath
126+
// Get latest commit referencing the commented line
127+
// No need for get commit for base branch changes
127128
if line > 0 {
128-
if reviewID != 0 {
129-
first, err := models.FindComments(models.FindCommentsOptions{
130-
ReviewID: reviewID,
131-
Line: line,
132-
TreePath: treePath,
133-
Type: models.CommentTypeCode,
134-
ListOptions: models.ListOptions{
135-
PageSize: 1,
136-
Page: 1,
137-
},
138-
})
139-
if err == nil && len(first) > 0 {
140-
commitID = first[0].CommitSHA
141-
invalidated = first[0].Invalidated
142-
patch = first[0].Patch
143-
} else if err != nil && !models.IsErrCommentNotExist(err) {
144-
return nil, fmt.Errorf("Find first comment for %d line %d path %s. Error: %v", reviewID, line, treePath, err)
145-
} else {
146-
review, err := models.GetReviewByID(reviewID)
147-
if err == nil && len(review.CommitID) > 0 {
148-
head = review.CommitID
149-
} else if err != nil && !models.IsErrReviewNotExist(err) {
150-
return nil, fmt.Errorf("GetReviewByID %d. Error: %v", reviewID, err)
151-
}
152-
}
153-
}
154-
155-
if len(commitID) == 0 {
156-
// FIXME validate treePath
157-
// Get latest commit referencing the commented line
158-
// No need for get commit for base branch changes
159-
commit, err := gitRepo.LineBlame(head, gitRepo.Path, treePath, uint(line))
160-
if err == nil {
161-
commitID = commit.ID.String()
162-
} else if !(strings.Contains(err.Error(), "exit status 128 - fatal: no such path") || notEnoughLines.MatchString(err.Error())) {
163-
return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err)
164-
}
129+
commit, err := gitRepo.LineBlame(pr.GetGitRefName(), gitRepo.Path, treePath, uint(line))
130+
if err == nil {
131+
commitID = commit.ID.String()
132+
} else if !(strings.Contains(err.Error(), "exit status 128 - fatal: no such path") || notEnoughLines.MatchString(err.Error())) {
133+
return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err)
165134
}
166135
}
167136

168137
// Only fetch diff if comment is review comment
169-
if len(patch) == 0 && reviewID != 0 {
170-
if len(commitID) == 0 {
171-
commitID, err = gitRepo.GetRefCommitID(pr.GetGitRefName())
172-
if err != nil {
173-
return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err)
174-
}
138+
if reviewID != 0 {
139+
headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName())
140+
if err != nil {
141+
return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err)
175142
}
176-
177143
patchBuf := new(bytes.Buffer)
178-
if err := git.GetRepoRawDiffForFile(gitRepo, pr.MergeBase, commitID, git.RawDiffNormal, treePath, patchBuf); err != nil {
179-
return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", gitRepo.Path, pr.MergeBase, commitID, treePath, err)
144+
if err := git.GetRepoRawDiffForFile(gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, treePath, patchBuf); err != nil {
145+
return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", err, gitRepo.Path, pr.MergeBase, headCommitID, treePath)
180146
}
181147
patch = git.CutDiffAroundLine(patchBuf, int64((&models.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines)
182148
}
183149
return models.CreateComment(&models.CreateCommentOptions{
184-
Type: models.CommentTypeCode,
185-
Doer: doer,
186-
Repo: repo,
187-
Issue: issue,
188-
Content: content,
189-
LineNum: line,
190-
TreePath: treePath,
191-
CommitSHA: commitID,
192-
ReviewID: reviewID,
193-
Patch: patch,
194-
Invalidated: invalidated,
150+
Type: models.CommentTypeCode,
151+
Doer: doer,
152+
Repo: repo,
153+
Issue: issue,
154+
Content: content,
155+
LineNum: line,
156+
TreePath: treePath,
157+
CommitSHA: commitID,
158+
ReviewID: reviewID,
159+
Patch: patch,
195160
})
196161
}
197162

0 commit comments

Comments
 (0)