fix(pull): fix code context of review comments on unchanged lines (#39799) (#39805)

Backport #39799

Fixes #39794

Co-authored-by: fixfon <12512757+fixfon@users.noreply.github.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

modules/git/fastimport.go

@@ -27,6 +27,7 @@ type FastImportFile struct {
 
 type FastImportCommit struct {
 	Ref     string
+	Parent  string
 	Message string
 	Files   []FastImportFile
 
@@ -72,6 +73,9 @@ func ForceFastImport(ctx context.Context, repo RepositoryFacade, commits []FastI
 
 		msg := util.IfZero(c.Message, fmt.Sprintf("test commit %d", i+1))
 		_, _ = fmt.Fprintf(buf, "data %d\n%s\n", len(msg), msg)
+		if c.Parent != "" {
+			_, _ = fmt.Fprintf(buf, "from %s\n", c.Parent)
+		}
 
 		for _, f := range c.Files {
 			mode := util.IfZero(f.Mode, EntryModeBlob)

services/pull/review.go

@@ -275,11 +275,12 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo
 		}
 
 		// If patch is still empty (unchanged line), generate code context
-		if patch == "" && commitID != "" {
-			patch, err = gitdiff.GeneratePatchForUnchangedLine(ctx, gitRepo, commitID, treePath, line, setting.UI.CodeCommentLines)
+		contextCommitID := util.Iif(line < 0, pr.MergeBase, headCommitID) // the line number is relative to the diff sides, not to the blamed commitID
+		if patch == "" && contextCommitID != "" {
+			patch, err = gitdiff.GeneratePatchForUnchangedLine(ctx, gitRepo, contextCommitID, treePath, line, setting.UI.CodeCommentLines)
 			if err != nil {
 				// Log the error but don't fail comment creation
-				log.Debug("Unable to generate patch for unchanged line (file=%s, line=%d, commit=%s): %v", treePath, line, commitID, err)
+				log.Debug("Unable to generate patch for unchanged line (file=%s, line=%d, commit=%s): %v", treePath, line, contextCommitID, err)
 			}
 		}
 	}

services/pull/review_test.go

@@ -4,14 +4,18 @@
 package pull_test
 
 import (
+	"fmt"
+	"strings"
 	"testing"
 
 	issues_model "gitea.dev/models/issues"
 	"gitea.dev/models/unittest"
 	user_model "gitea.dev/models/user"
+	"gitea.dev/modules/git"
 	pull_service "gitea.dev/services/pull"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestDismissReview(t *testing.T) {
@@ -45,3 +49,59 @@ func TestDismissReview(t *testing.T) {
 	assert.Error(t, err)
 	assert.True(t, pull_service.IsErrDismissRequestOnClosedPR(err))
 }
+
+func TestCreateCodeCommentContextOfUnchangedLine(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+	ctx := t.Context()
+	doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 6})
+	require.NoError(t, pr.LoadBaseRepo(ctx))
+
+	numberedLines := func(prefix string, count int) string {
+		var sb strings.Builder
+		for i := range count {
+			_, _ = fmt.Fprintf(&sb, "%s %d\n", prefix, i+1)
+		}
+		return sb.String()
+	}
+
+	// "old 10" was last changed by the first commit (line 10), it is line 15 in the merge base and line 17 in the PR head
+	const treePath = "unchanged-line.txt"
+	oldLines := numberedLines("old", 20)
+	baseLines := numberedLines("base", 5) + oldLines
+	headLines := numberedLines("head", 2) + baseLines
+
+	headRef := pr.GetGitHeadRefName()
+	require.NoError(t, git.ForceFastImport(ctx, pr.BaseRepo, []git.FastImportCommit{
+		{Ref: headRef, Files: []git.FastImportFile{{Path: treePath, Content: oldLines}}},
+		{Ref: headRef, Parent: ":1", Files: []git.FastImportFile{{Path: treePath, Content: baseLines}}},
+		{Ref: headRef, Parent: ":2", Files: []git.FastImportFile{{Path: treePath, Content: headLines}}},
+	}))
+
+	gitRepo, err := git.OpenRepository(ctx, pr.BaseRepo)
+	require.NoError(t, err)
+	defer gitRepo.Close()
+
+	headCommit, err := gitRepo.GetCommit(ctx, headRef)
+	require.NoError(t, err)
+	mergeBase, err := headCommit.ParentID(0)
+	require.NoError(t, err)
+	pr.MergeBase = mergeBase.String()
+	require.NoError(t, pr.UpdateCols(ctx, "merge_base"))
+
+	for _, c := range []struct {
+		name string
+		line int64
+	}{
+		{"NewSide", 17},
+		{"OldSide", -15},
+	} {
+		t.Run(c.name, func(t *testing.T) {
+			issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: pr.IssueID})
+			require.NoError(t, issue.LoadRepo(ctx))
+			comment, err := pull_service.CreateCodeComment(ctx, doer, gitRepo, issue, c.line, "test", treePath, true, 0, headCommit.ID.String(), nil)
+			require.NoError(t, err)
+			assert.True(t, strings.HasSuffix(comment.Patch, "\n old 10\n"), "patch: %q", comment.Patch)
+		})
+	}
+}