Skip to content

Commit 8b4e077

Browse files
committed
Fix bug
1 parent 1556911 commit 8b4e077

File tree

9 files changed

+94
-111
lines changed

9 files changed

+94
-111
lines changed

models/issues/comment.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ type CommentMetaData struct {
238238
ProjectColumnID int64 `json:"project_column_id,omitempty"`
239239
ProjectColumnTitle string `json:"project_column_title,omitempty"`
240240
ProjectTitle string `json:"project_title,omitempty"`
241+
BeforeCommitID string `json:"before_commit_id,omitempty"` // commit id before this comment
241242
}
242243

243244
// Comment represents a comment in commit and issue page.
@@ -293,7 +294,8 @@ type Comment struct {
293294
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
294295

295296
// Reference issue in commit message
296-
CommitSHA string `xorm:"VARCHAR(64)"`
297+
BeforeCommitID string `xorm:"VARCHAR(64)"`
298+
CommitSHA string `xorm:"VARCHAR(64)"`
297299

298300
Attachments []*repo_model.Attachment `xorm:"-"`
299301
Reactions ReactionList `xorm:"-"`
@@ -764,7 +766,7 @@ func (c *Comment) CodeCommentLink(ctx context.Context) string {
764766
return fmt.Sprintf("%s/files#%s", c.Issue.Link(), c.HashTag())
765767
}
766768

767-
func GetCodeCommentRefName(prIndex, commentID int64) string {
769+
func GetCodeCommentRefName(prIndex, commentID int64, suffix string) string {
768770
return fmt.Sprintf("refs/pull/%d/code-comment-%d", prIndex, commentID)
769771
}
770772

@@ -806,6 +808,7 @@ func CreateComment(ctx context.Context, opts *CreateCommentOptions) (_ *Comment,
806808
AssigneeID: opts.AssigneeID,
807809
AssigneeTeamID: opts.AssigneeTeamID,
808810
CommitID: opts.CommitID,
811+
BeforeCommitID: opts.BeforeCommitID,
809812
CommitSHA: opts.CommitSHA,
810813
Line: opts.LineNum,
811814
Content: opts.Content,
@@ -977,7 +980,8 @@ type CreateCommentOptions struct {
977980
OldRef string
978981
NewRef string
979982
CommitID int64
980-
CommitSHA string
983+
BeforeCommitID string // before commit id when creating this code comment
984+
CommitSHA string // after commit id when creating this code comment, ref commit id for other comment
981985
Patch string
982986
LineNum int64
983987
TreePath string

models/migrations/migrations.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"code.gitea.io/gitea/models/migrations/v1_22"
2525
"code.gitea.io/gitea/models/migrations/v1_23"
2626
"code.gitea.io/gitea/models/migrations/v1_24"
27+
"code.gitea.io/gitea/models/migrations/v1_25"
2728
"code.gitea.io/gitea/models/migrations/v1_6"
2829
"code.gitea.io/gitea/models/migrations/v1_7"
2930
"code.gitea.io/gitea/models/migrations/v1_8"
@@ -382,6 +383,9 @@ func prepareMigrationTasks() []*migration {
382383
newMigration(318, "Add anonymous_access_mode for repo_unit", v1_24.AddRepoUnitAnonymousAccessMode),
383384
newMigration(319, "Add ExclusiveOrder to Label table", v1_24.AddExclusiveOrderColumnToLabelTable),
384385
newMigration(320, "Migrate two_factor_policy to login_source table", v1_24.MigrateSkipTwoFactor),
386+
387+
// Gitea 1.24.0-rc0 ends at migration ID number 320 (database version 321)
388+
newMigration(321, "Add BeforeCommitID to Comment table", v1_25.AddBeforeCommitIDForComment),
385389
}
386390
return preparedMigrations
387391
}

services/automergequeue/automergequeue.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,11 @@ func StartPRCheckAndAutoMerge(ctx context.Context, pull *issues_model.PullReques
3939
return
4040
}
4141
defer gitRepo.Close()
42-
headCommitID, err := gitRepo.GetRefCommitID(pull.GetGitHeadRefName())
42+
commitID, err := gitRepo.GetRefCommitID(pull.GetGitHeadRefName())
4343
if err != nil {
4444
log.Error("GetRefCommitID: %v", err)
4545
return
4646
}
4747

48-
AddToQueue(pull, headCommitID)
48+
AddToQueue(pull, commitID)
4949
}

services/doctor/review.go

Lines changed: 0 additions & 46 deletions
This file was deleted.

services/issue/comments.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,13 @@ func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_m
153153
}
154154

155155
if comment.ReviewID > 0 {
156-
if err := git.RemoveRef(ctx, comment.Issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(comment.Issue.PullRequest.Index, comment.ID)); err != nil {
156+
if err := git.RemoveRef(ctx, comment.Issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(comment.Issue.PullRequest.Index, comment.ID, "before")); err != nil {
157+
log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", comment.Issue.PullRequest.ID, err)
158+
// We should not return error here, because the comment has been removed from database.
159+
// users have to delete this ref manually or we should have a synchronize between
160+
// database comment table and git refs.
161+
}
162+
if err := git.RemoveRef(ctx, comment.Issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(comment.Issue.PullRequest.Index, comment.ID, "after")); err != nil {
157163
log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", comment.Issue.PullRequest.ID, err)
158164
// We should not return error here, because the comment has been removed from database.
159165
// users have to delete this ref manually or we should have a synchronize between

services/issue/issue.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -202,22 +202,22 @@ func DeleteIssue(ctx context.Context, doer *user_model.User, gitRepo *git.Reposi
202202
// delete pull request related git data
203203
if issue.IsPull && gitRepo != nil {
204204
if err := git.RemoveRef(ctx, gitRepo.Path, issue.PullRequest.GetGitHeadRefName()); err != nil {
205-
return err
205+
log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issue.PullRequest.GetGitHeadRefName(), issue.PullRequest.ID, err)
206206
}
207207
if issue.PullRequest.HasMerged {
208208
if err := git.RemoveRef(ctx, gitRepo.Path, issue.PullRequest.GetGitMergeRefName()); err != nil {
209-
return err
209+
log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issue.PullRequest.GetGitMergeRefName(), issue.PullRequest.ID, err)
210210
}
211211
}
212212
}
213213

214214
for _, comment := range comments {
215-
if comment.ReviewID > 0 {
216-
if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID)); err != nil {
217-
log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", issue.PullRequest.ID, err)
218-
// We should not return error here, because the comment has been removed from database.
219-
// users have to delete this ref manually or we should have a synchronize between
220-
// database comment table and git refs.
215+
if comment.Type == issues_model.CommentTypeCode {
216+
if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "before")); err != nil {
217+
log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "before"), issue.PullRequest.ID, err)
218+
}
219+
if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "after")); err != nil {
220+
log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "after"), issue.PullRequest.ID, err)
221221
}
222222
}
223223
}
@@ -431,12 +431,12 @@ func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths []
431431
}
432432

433433
for _, comment := range comments {
434-
if comment.ReviewID > 0 {
435-
if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID)); err != nil {
436-
log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", issue.PullRequest.ID, err)
437-
// We should not return error here, because the comment has been removed from database.
438-
// users have to delete this ref manually or we should have a synchronize between
439-
// database comment table and git refs.
434+
if comment.Type == issues_model.CommentTypeCode {
435+
if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "before")); err != nil {
436+
log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "before"), issue.PullRequest.ID, err)
437+
}
438+
if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "after")); err != nil {
439+
log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "after"), issue.PullRequest.ID, err)
440440
}
441441
}
442442
}

services/migrations/gitea_uploader.go

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -927,15 +927,16 @@ func (g *GiteaLocalUploader) CreateReviews(ctx context.Context, reviews ...*base
927927
}
928928

929929
c := issues_model.Comment{
930-
Type: issues_model.CommentTypeCode,
931-
IssueID: issue.ID,
932-
Content: comment.Content,
933-
Line: int64(line + comment.Position - 1),
934-
TreePath: comment.TreePath,
935-
CommitSHA: comment.CommitID,
936-
Patch: patch,
937-
CreatedUnix: timeutil.TimeStamp(comment.CreatedAt.Unix()),
938-
UpdatedUnix: timeutil.TimeStamp(comment.UpdatedAt.Unix()),
930+
Type: issues_model.CommentTypeCode,
931+
IssueID: issue.ID,
932+
Content: comment.Content,
933+
Line: int64(line + comment.Position - 1),
934+
TreePath: comment.TreePath,
935+
BeforeCommitID: pr.MergeBase,
936+
CommitSHA: comment.CommitID,
937+
Patch: patch,
938+
CreatedUnix: timeutil.TimeStamp(comment.CreatedAt.Unix()),
939+
UpdatedUnix: timeutil.TimeStamp(comment.UpdatedAt.Unix()),
939940
}
940941

941942
if err := g.remapUser(ctx, review, &c); err != nil {

services/pull/review.go

Lines changed: 48 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -268,30 +268,34 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo
268268
return nil, fmt.Errorf("GetPatch failed: %w", err)
269269
}
270270

271-
lineCommitID := util.Iif(line < 0, beforeCommitID, afterCommitID)
272271
return db.WithTx2(ctx, func(ctx context.Context) (*issues_model.Comment, error) {
273272
comment, err := issues_model.CreateComment(ctx, &issues_model.CreateCommentOptions{
274-
Type: issues_model.CommentTypeCode,
275-
Doer: doer,
276-
Repo: repo,
277-
Issue: issue,
278-
Content: content,
279-
LineNum: line,
280-
TreePath: treePath,
281-
CommitSHA: lineCommitID,
282-
ReviewID: reviewID,
283-
Patch: patch,
284-
Invalidated: false,
285-
Attachments: attachments,
273+
Type: issues_model.CommentTypeCode,
274+
Doer: doer,
275+
Repo: repo,
276+
Issue: issue,
277+
Content: content,
278+
LineNum: line,
279+
TreePath: treePath,
280+
BeforeCommitID: beforeCommitID,
281+
CommitSHA: afterCommitID,
282+
ReviewID: reviewID,
283+
Patch: patch,
284+
Invalidated: false,
285+
Attachments: attachments,
286286
})
287287
if err != nil {
288288
return nil, err
289289
}
290290

291291
// The line commit ID Must be referenced in the git repository, because the branch maybe rebased or force-pushed.
292292
// If the review commit is GC, the position can not be calculated dynamically.
293-
if err := git.UpdateRef(ctx, pr.BaseRepo.RepoPath(), issues_model.GetCodeCommentRefName(pr.Index, comment.ID), lineCommitID); err != nil {
294-
log.Error("Unable to update ref in base repository for PR[%d] Error: %v", pr.ID, err)
293+
if err := git.UpdateRef(ctx, pr.BaseRepo.RepoPath(), issues_model.GetCodeCommentRefName(pr.Index, comment.ID, "before"), beforeCommitID); err != nil {
294+
log.Error("Unable to update ref before_commitid in base repository for PR[%d] Error: %v", pr.ID, err)
295+
return nil, err
296+
}
297+
if err := git.UpdateRef(ctx, pr.BaseRepo.RepoPath(), issues_model.GetCodeCommentRefName(pr.Index, comment.ID, "after"), afterCommitID); err != nil {
298+
log.Error("Unable to update ref after_commitid in base repository for PR[%d] Error: %v", pr.ID, err)
295299
return nil, err
296300
}
297301

@@ -484,6 +488,7 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string,
484488
}
485489

486490
// ReCalculateLineNumber recalculates the line number based on the hunks of the diff.
491+
// left side is the commit the comment was created on, right side is the commit the comment is displayed on.
487492
// If the returned line number is zero, it should not be displayed.
488493
func ReCalculateLineNumber(hunks []*git.HunkInfo, leftLine int64) int64 {
489494
if len(hunks) == 0 || leftLine == 0 {
@@ -498,15 +503,16 @@ func ReCalculateLineNumber(hunks []*git.HunkInfo, leftLine int64) int64 {
498503
newLine := absLine
499504

500505
for _, hunk := range hunks {
501-
if hunk.LeftLine+hunk.LeftHunk <= absLine {
502-
newLine += hunk.RightHunk - hunk.LeftHunk
503-
} else if hunk.LeftLine <= absLine && absLine < hunk.LeftLine+hunk.LeftHunk {
504-
// The line has been removed, so it should not be displayed
505-
return 0
506-
} else if absLine < hunk.LeftLine {
506+
if absLine < hunk.LeftLine {
507507
// The line is before the hunk, so we can ignore it
508508
continue
509+
} else if hunk.LeftLine <= absLine && absLine < hunk.LeftLine+hunk.LeftHunk {
510+
// The line is within the hunk, that means the line is deleted from the current commit
511+
// So that we don't need to display this line
512+
return 0
509513
}
514+
// The line is after the hunk, so we can add the right hunk size
515+
newLine += hunk.RightHunk - hunk.LeftHunk
510516
}
511517
return util.Iif(isLeft, -newLine, newLine)
512518
}
@@ -614,27 +620,35 @@ func LoadCodeComments(ctx context.Context, gitRepo *git.Repository, repo *repo_m
614620
}
615621

616622
dstCommitID := beforeCommit.ID.String()
623+
commentCommitID := comment.BeforeCommitID
617624
if comment.Line > 0 {
618625
dstCommitID = afterCommit.ID.String()
626+
commentCommitID = comment.CommitSHA
619627
}
620628

621-
if comment.CommitSHA == dstCommitID {
622-
lineComments[comment.Line] = append(lineComments[comment.Line], comment)
623-
continue
629+
if commentCommitID != dstCommitID {
630+
// If the comment is not for the current commit, we need to recalculate the line number
631+
hunks, ok := hunksCache[commentCommitID+".."+dstCommitID]
632+
if !ok {
633+
hunks, err = git.GetAffectedHunksForTwoCommitsSpecialFile(ctx, repo.RepoPath(), commentCommitID, dstCommitID, file.Name)
634+
if err != nil {
635+
return fmt.Errorf("GetAffectedHunksForTwoCommitsSpecialFile[%s, %s, %s]: %w", repo.FullName(), commentCommitID, dstCommitID, err)
636+
}
637+
hunksCache[commentCommitID+".."+dstCommitID] = hunks
638+
}
639+
comment.Line = ReCalculateLineNumber(hunks, comment.Line)
624640
}
625641

626-
// If the comment is not for the current commit, we need to recalculate the line number
627-
hunks, ok := hunksCache[comment.CommitSHA+".."+dstCommitID]
628-
if !ok {
629-
hunks, err = git.GetAffectedHunksForTwoCommitsSpecialFile(ctx, repo.RepoPath(), comment.CommitSHA, dstCommitID, file.Name)
642+
if comment.Line != 0 {
643+
commentAfterCommit, err := gitRepo.GetCommit(comment.CommitSHA)
630644
if err != nil {
631-
return fmt.Errorf("GetAffectedHunksForTwoCommitsSpecialFile[%s, %s, %s]: %w", repo.FullName(), dstCommitID, comment.CommitSHA, err)
645+
return fmt.Errorf("GetCommit[%s]: %w", comment.CommitSHA, err)
646+
}
647+
// If the comment is not the first one or the comment created before the current commit
648+
if lineComments[comment.Line] != nil || comment.CommitSHA == afterCommit.ID.String() ||
649+
commentAfterCommit.Committer.When.Before(afterCommit.Committer.When) {
650+
lineComments[comment.Line] = append(lineComments[comment.Line], comment)
632651
}
633-
hunksCache[comment.CommitSHA+".."+dstCommitID] = hunks
634-
}
635-
comment.Line = ReCalculateLineNumber(hunks, comment.Line)
636-
if comment.Line != 0 {
637-
lineComments[comment.Line] = append(lineComments[comment.Line], comment)
638652
}
639653
}
640654

web_src/js/components/DiffCommitSelector.vue

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,10 +195,10 @@ export default defineComponent({
195195
start = this.commits[firstSelected - 1].id;
196196
}
197197
const end = this.commits.findLast((x) => x.selected).id;
198-
if (start === end) {
198+
if (firstSelected === end) {
199199
// if the start and end are the same, we show this single commit
200200
window.location.assign(`${this.issueLink}/commits/${start}${this.queryParams}`);
201-
} else if (firstSelected === 0 && end === this.commits.at(-1).id) {
201+
} else if (start === this.merge_base && end === this.commits.at(-1).id) {
202202
// if the first commit is selected and the last commit is selected, we show all commits
203203
window.location.assign(`${this.issueLink}/files${this.queryParams}`);
204204
} else {

0 commit comments

Comments
 (0)