Skip to content

Commit 69a0ecf

Browse files
Copilotsilverwind
andcommitted
Add nil check and test coverage for self-reference fix
- Add nil check for PullRequest to prevent potential panic - Add comprehensive test case TestUpdateIssuesCommit_SelfReference - Test verifies self-references are skipped while other references work - All tests passing Co-authored-by: silverwind <115237+silverwind@users.noreply.github.com>
1 parent 32c9faf commit 69a0ecf

2 files changed

Lines changed: 61 additions & 1 deletion

File tree

services/issue/commit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ func UpdateIssuesCommit(ctx context.Context, doer *user_model.User, repo *repo_m
162162
if refIssue.IsPull {
163163
if err := refIssue.LoadPullRequest(ctx); err != nil {
164164
log.Error("LoadPullRequest: %v", err)
165-
} else if refIssue.PullRequest.MergedCommitID == c.Sha1 {
165+
} else if refIssue.PullRequest != nil && refIssue.PullRequest.MergedCommitID == c.Sha1 {
166166
// This is a self-reference (PR merge commit referencing its own PR), skip it
167167
continue
168168
}

services/issue/commit_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,3 +298,63 @@ func TestUpdateIssuesCommit_AnotherRepoNoPermission(t *testing.T) {
298298
unittest.AssertNotExistsBean(t, issueBean, "is_closed=1")
299299
unittest.CheckConsistencyFor(t, &activities_model.Action{})
300300
}
301+
302+
func TestUpdateIssuesCommit_SelfReference(t *testing.T) {
303+
assert.NoError(t, unittest.PrepareTestDatabase())
304+
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
305+
306+
// Test that a PR merge commit that references its own PR does not create a self-reference comment
307+
// PR #2 (issue_id=2) has merged_commit_id: 1a8823cd1a9549fde083f992f6b9b87a7ab74fb3
308+
pushCommits := []*repository.PushCommit{
309+
{
310+
Sha1: "1a8823cd1a9549fde083f992f6b9b87a7ab74fb3", // This is the merge commit for PR #2
311+
CommitterEmail: "user2@example.com",
312+
CommitterName: "User Two",
313+
AuthorEmail: "user2@example.com",
314+
AuthorName: "User Two",
315+
Message: "Merge pull request (#2) from branch1", // References its own PR
316+
},
317+
}
318+
319+
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
320+
321+
// This comment should NOT be created (self-reference)
322+
selfRefCommentBean := &issues_model.Comment{
323+
Type: issues_model.CommentTypeCommitRef,
324+
CommitSHA: "1a8823cd1a9549fde083f992f6b9b87a7ab74fb3",
325+
PosterID: user.ID,
326+
IssueID: 2, // PR #2 references itself
327+
}
328+
329+
unittest.AssertNotExistsBean(t, selfRefCommentBean)
330+
assert.NoError(t, UpdateIssuesCommit(t.Context(), user, repo, pushCommits, repo.DefaultBranch))
331+
// The self-reference comment should still not exist
332+
unittest.AssertNotExistsBean(t, selfRefCommentBean)
333+
unittest.CheckConsistencyFor(t, &activities_model.Action{})
334+
335+
// Test that the same merge commit can still create references to other issues
336+
pushCommits2 := []*repository.PushCommit{
337+
{
338+
Sha1: "1a8823cd1a9549fde083f992f6b9b87a7ab74fb3",
339+
CommitterEmail: "user2@example.com",
340+
CommitterName: "User Two",
341+
AuthorEmail: "user2@example.com",
342+
AuthorName: "User Two",
343+
Message: "Merge pull request (#2) - also fixes #1", // References a different issue
344+
},
345+
}
346+
347+
// This comment SHOULD be created (reference to a different issue)
348+
otherRefCommentBean := &issues_model.Comment{
349+
Type: issues_model.CommentTypeCommitRef,
350+
CommitSHA: "1a8823cd1a9549fde083f992f6b9b87a7ab74fb3",
351+
PosterID: user.ID,
352+
IssueID: 1, // References issue #1
353+
}
354+
355+
unittest.AssertNotExistsBean(t, otherRefCommentBean)
356+
assert.NoError(t, UpdateIssuesCommit(t.Context(), user, repo, pushCommits2, repo.DefaultBranch))
357+
// The reference to issue #1 should exist
358+
unittest.AssertExistsAndLoadBean(t, otherRefCommentBean)
359+
unittest.CheckConsistencyFor(t, &activities_model.Action{})
360+
}

0 commit comments

Comments
 (0)