From 4a35b85328f380652c6717095eed07ef055ddfeb Mon Sep 17 00:00:00 2001
From: Masaya Suzuki <draftcode@users.noreply.github.com>
Date: Thu, 2 Jan 2025 17:24:57 -0800
Subject: [PATCH] Use MergedEvent to improve the merge commit detection (#503)

Even though PR is in "MERGED" state, the mergeCommit field can be empty
in the GitHub API response. Use the MergedEvent to get the merge commit
hash to complement the detection.
---
 e2e_tests/mock_ghgql_server.go |  2 +-
 internal/gh/pullrequest.go     | 22 ++++++++++++++++++----
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/e2e_tests/mock_ghgql_server.go b/e2e_tests/mock_ghgql_server.go
index 943bce47..9712bd7e 100644
--- a/e2e_tests/mock_ghgql_server.go
+++ b/e2e_tests/mock_ghgql_server.go
@@ -10,7 +10,7 @@ import (
 
 const (
 	// These are ugly, but this is easy way to tell which query is being used.
-	prQuery = "query($after:String$baseRefName:String$first:Int!$headRefName:String$owner:String!$repo:String!$states:[PullRequestState!]){repository(owner: $owner, name: $repo){pullRequests(states: $states, headRefName: $headRefName, baseRefName: $baseRefName, first: $first, after: $after){nodes{id,number,headRefName,baseRefName,isDraft,permalink,state,title,body,mergeCommit{oid},timelineItems(last: 10, itemTypes: CLOSED_EVENT){nodes{... on ClosedEvent{closer{... on Commit{oid}}}}}},pageInfo{endCursor,hasNextPage,hasPreviousPage,startCursor}}}}"
+	prQuery = "query($after:String$baseRefName:String$first:Int!$headRefName:String$owner:String!$repo:String!$states:[PullRequestState!]){repository(owner: $owner, name: $repo){pullRequests(states: $states, headRefName: $headRefName, baseRefName: $baseRefName, first: $first, after: $after){nodes{id,number,headRefName,baseRefName,isDraft,permalink,state,title,body,mergeCommit{oid},timelineItems(last: 10, itemTypes: [CLOSED_EVENT, MERGED_EVENT]){nodes{... on ClosedEvent{closer{... on Commit{oid}}},... on MergedEvent{commit{oid}}}}},pageInfo{endCursor,hasNextPage,hasPreviousPage,startCursor}}}}"
 )
 
 func RunMockGitHubServer(t *testing.T) *mockGitHubServer {
diff --git a/internal/gh/pullrequest.go b/internal/gh/pullrequest.go
index febf8af8..5db3044c 100644
--- a/internal/gh/pullrequest.go
+++ b/internal/gh/pullrequest.go
@@ -30,8 +30,13 @@ type PullRequest struct {
 					} `graphql:"... on Commit"`
 				}
 			} `graphql:"... on ClosedEvent"`
+			MergedEvent struct {
+				Commit struct {
+					Oid string
+				}
+			} `graphql:"... on MergedEvent"`
 		}
-	} `graphql:"timelineItems(last: 10, itemTypes: CLOSED_EVENT)"`
+	} `graphql:"timelineItems(last: 10, itemTypes: [CLOSED_EVENT, MERGED_EVENT])"`
 }
 
 func (p *PullRequest) HeadBranchName() string {
@@ -49,10 +54,19 @@ func (p *PullRequest) BaseBranchName() string {
 func (p *PullRequest) GetMergeCommit() string {
 	if p.State == githubv4.PullRequestStateOpen {
 		return ""
-	} else if p.State == githubv4.PullRequestStateMerged {
+	} else if p.State == githubv4.PullRequestStateMerged && p.PRIVATE_MergeCommit.Oid != "" {
 		return p.PRIVATE_MergeCommit.Oid
-	} else if p.State == githubv4.PullRequestStateClosed && len(p.PRIVATE_TimelineItems.Nodes) != 0 {
-		return p.PRIVATE_TimelineItems.Nodes[0].ClosedEvent.Closer.Commit.Oid
+	}
+	// The timeline is in chronological order, so we can iterate backwards to find the latest
+	// one.
+	for i := len(p.PRIVATE_TimelineItems.Nodes) - 1; i >= 0; i-- {
+		item := p.PRIVATE_TimelineItems.Nodes[i]
+		if item.ClosedEvent.Closer.Commit.Oid != "" {
+			return item.ClosedEvent.Closer.Commit.Oid
+		}
+		if item.MergedEvent.Commit.Oid != "" {
+			return item.MergedEvent.Commit.Oid
+		}
 	}
 	return ""
 }