From c18a62279a54f1ba0dc0293dddbe197f527fbf00 Mon Sep 17 00:00:00 2001
From: Jason Song <i@wolfogre.com>
Date: Sun, 5 Feb 2023 19:57:38 +0800
Subject: [PATCH] Fix time to NotifyPullRequestSynchronized (#22650)

Should call `PushToBaseRepo` before
`notification.NotifyPullRequestSynchronized`.

Or the notifier will get an old commit when reading branch
`pull/xxx/head`.

Found by ~#21937~ #22679.

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
---
 models/issues/pull_list.go | 13 ++++++++++++-
 services/pull/pull.go      | 36 ++++++++++++++++++------------------
 2 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/models/issues/pull_list.go b/models/issues/pull_list.go
index 007c2fd903..68f62ffc43 100644
--- a/models/issues/pull_list.go
+++ b/models/issues/pull_list.go
@@ -13,6 +13,7 @@ import (
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/util"
 
 	"xorm.io/xorm"
 )
@@ -175,7 +176,17 @@ func (prs PullRequestList) loadAttributes(ctx context.Context) error {
 	}
 	for _, pr := range prs {
 		pr.Issue = set[pr.IssueID]
-		pr.Issue.PullRequest = pr // panic here means issueIDs and prs are not in sync
+		/*
+			Old code:
+			pr.Issue.PullRequest = pr // panic here means issueIDs and prs are not in sync
+
+			It's worth panic because it's almost impossible to happen under normal use.
+			But in integration testing, an asynchronous task could read a database that has been reset.
+			So returning an error would make more sense, let the caller has a choice to ignore it.
+		*/
+		if pr.Issue == nil {
+			return fmt.Errorf("issues and prs may be not in sync: cannot find issue %v for pr %v: %w", pr.IssueID, pr.ID, util.ErrNotExist)
+		}
 	}
 	return nil
 }
diff --git a/services/pull/pull.go b/services/pull/pull.go
index 317875d211..0d260c93b1 100644
--- a/services/pull/pull.go
+++ b/services/pull/pull.go
@@ -263,6 +263,24 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string,
 			return
 		}
 
+		for _, pr := range prs {
+			log.Trace("Updating PR[%d]: composing new test task", pr.ID)
+			if pr.Flow == issues_model.PullRequestFlowGithub {
+				if err := PushToBaseRepo(ctx, pr); err != nil {
+					log.Error("PushToBaseRepo: %v", err)
+					continue
+				}
+			} else {
+				continue
+			}
+
+			AddToTaskQueue(pr)
+			comment, err := CreatePushPullComment(ctx, doer, pr, oldCommitID, newCommitID)
+			if err == nil && comment != nil {
+				notification.NotifyPullRequestPushCommits(ctx, doer, pr, comment)
+			}
+		}
+
 		if isSync {
 			requests := issues_model.PullRequestList(prs)
 			if err = requests.LoadAttributes(); err != nil {
@@ -303,24 +321,6 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string,
 			}
 		}
 
-		for _, pr := range prs {
-			log.Trace("Updating PR[%d]: composing new test task", pr.ID)
-			if pr.Flow == issues_model.PullRequestFlowGithub {
-				if err := PushToBaseRepo(ctx, pr); err != nil {
-					log.Error("PushToBaseRepo: %v", err)
-					continue
-				}
-			} else {
-				continue
-			}
-
-			AddToTaskQueue(pr)
-			comment, err := CreatePushPullComment(ctx, doer, pr, oldCommitID, newCommitID)
-			if err == nil && comment != nil {
-				notification.NotifyPullRequestPushCommits(ctx, doer, pr, comment)
-			}
-		}
-
 		log.Trace("AddTestPullRequestTask [base_repo_id: %d, base_branch: %s]: finding pull requests", repoID, branch)
 		prs, err = issues_model.GetUnmergedPullRequestsByBaseInfo(repoID, branch)
 		if err != nil {