Amend code owner test to include branch protection rule

services/issue/pull.go

@@ -61,6 +61,10 @@ func HasAllRequiredCodeownerReviews(ctx context.Context, pb *git_model.Protected
 		return false
 	}
 
+	if err := pr.LoadIssue(ctx); err != nil {
+		return false
+	}
+
 	pr.Issue.Repo = pr.BaseRepo
 
 	if pr.BaseRepo.IsFork {
@@ -116,7 +120,6 @@ func HasAllRequiredCodeownerReviews(ctx context.Context, pb *git_model.Protected
 	reviews, err := issues_model.FindLatestReviews(ctx, issues_model.FindReviewOptions{
 		Types:   []issues_model.ReviewType{issues_model.ReviewTypeApprove},
 		IssueID: pr.IssueID,
-		OfficialOnly: setting.Repository.PullRequest.DefaultMergeMessageOfficialApproversOnly,
 	})
 	if err != nil {
 		return false

tests/integration/pull_review_test.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 
 	"code.gitea.io/gitea/models/db"
+	git_model "code.gitea.io/gitea/models/git"
 	issues_model "code.gitea.io/gitea/models/issues"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
@@ -19,6 +20,7 @@ import (
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/test"
 	issue_service "code.gitea.io/gitea/services/issue"
+	pull_service "code.gitea.io/gitea/services/pull"
 	repo_service "code.gitea.io/gitea/services/repository"
 	files_service "code.gitea.io/gitea/services/repository/files"
 	"code.gitea.io/gitea/tests"
@@ -49,6 +51,8 @@ func TestPullView_ReviewerMissed(t *testing.T) {
 func TestPullView_CodeOwner(t *testing.T) {
 	onGiteaRun(t, func(t *testing.T, u *url.URL) {
 		user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+		user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
+		user8 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 8})
 
 		// Create the repo.
 		repo, err := repo_service.CreateRepositoryDirectly(db.DefaultContext, user2, user2, repo_service.CreateRepoOptions{
@@ -60,6 +64,17 @@ func TestPullView_CodeOwner(t *testing.T) {
 		}, true)
 		assert.NoError(t, err)
 
+		// create code owner branch protection
+		protectBranch := git_model.ProtectedBranch{
+			BlockOnCodeownerReviews: true,
+			RepoID:                  repo.ID,
+			RuleName:                "master",
+			CanPush:                 true,
+		}
+
+		err = pull_service.CreateOrUpdateProtectedBranch(db.DefaultContext, repo, &protectBranch, git_model.WhitelistOptions{})
+		assert.NoError(t, err)
+
 		// add CODEOWNERS to default branch
 		_, err = files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
 			OldBranch: repo.DefaultBranch,
@@ -96,7 +111,7 @@ func TestPullView_CodeOwner(t *testing.T) {
 			assert.NoError(t, pr.LoadIssue(db.DefaultContext))
 
 			// update the file on the pr branch
-			_, err = files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
+			resp, err := files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
 				OldBranch: "codeowner-basebranch",
 				Files: []*files_service.ChangeRepoFile{
 					{
@@ -124,6 +139,22 @@ func TestPullView_CodeOwner(t *testing.T) {
 			prUpdated2 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID})
 			assert.NoError(t, prUpdated2.LoadIssue(db.DefaultContext))
 			assert.Equal(t, "Test Pull Request2", prUpdated2.Issue.Title)
+
+			// ensure it cannot be merged
+			hasCodeownerReviews := issue_service.HasAllRequiredCodeownerReviews(db.DefaultContext, &protectBranch, pr)
+			assert.False(t, hasCodeownerReviews)
+
+			issues_model.SubmitReview(db.DefaultContext, user5, pr.Issue, issues_model.ReviewTypeApprove, "Very good", resp.Commit.SHA, false, make([]string, 0))
+
+			// should still fail (we also need user8)
+			hasCodeownerReviews = issue_service.HasAllRequiredCodeownerReviews(db.DefaultContext, &protectBranch, pr)
+			assert.False(t, hasCodeownerReviews)
+
+			issues_model.SubmitReview(db.DefaultContext, user8, pr.Issue, issues_model.ReviewTypeApprove, "Very good", resp.Commit.SHA, false, make([]string, 0))
+
+			// now we should be able to merge
+			hasCodeownerReviews = issue_service.HasAllRequiredCodeownerReviews(db.DefaultContext, &protectBranch, pr)
+			assert.True(t, hasCodeownerReviews)
 		})
 
 		// change the default branch CODEOWNERS file to change README.md's codeowner
@@ -140,7 +171,7 @@ func TestPullView_CodeOwner(t *testing.T) {
 
 		t.Run("Second Pull Request", func(t *testing.T) {
 			// create a new branch to prepare for pull request
-			_, err = files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
+			resp, err := files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
 				NewBranch: "codeowner-basebranch2",
 				Files: []*files_service.ChangeRepoFile{
 					{
@@ -158,6 +189,15 @@ func TestPullView_CodeOwner(t *testing.T) {
 
 			pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "codeowner-basebranch2"})
 			unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
+
+			// should need user8 approval only now
+			hasCodeownerReviews := issue_service.HasAllRequiredCodeownerReviews(db.DefaultContext, &protectBranch, pr)
+			assert.False(t, hasCodeownerReviews)
+
+			issues_model.SubmitReview(db.DefaultContext, user8, pr.Issue, issues_model.ReviewTypeApprove, "Very good", resp.Commit.SHA, false, make([]string, 0))
+
+			hasCodeownerReviews = issue_service.HasAllRequiredCodeownerReviews(db.DefaultContext, &protectBranch, pr)
+			assert.True(t, hasCodeownerReviews)
 		})
 
 		t.Run("Forked Repo Pull Request", func(t *testing.T) {
@@ -169,7 +209,7 @@ func TestPullView_CodeOwner(t *testing.T) {
 			assert.NoError(t, err)
 
 			// create a new branch to prepare for pull request
-			_, err = files_service.ChangeRepoFiles(db.DefaultContext, forkedRepo, user5, &files_service.ChangeRepoFilesOptions{
+			resp, err := files_service.ChangeRepoFiles(db.DefaultContext, forkedRepo, user5, &files_service.ChangeRepoFilesOptions{
 				NewBranch: "codeowner-basebranch-forked",
 				Files: []*files_service.ChangeRepoFile{
 					{
@@ -194,6 +234,21 @@ func TestPullView_CodeOwner(t *testing.T) {
 
 			pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadRepoID: forkedRepo.ID, HeadBranch: "codeowner-basebranch-forked"})
 			unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
+
+			// will also need user8 for this
+			hasCodeownerReviews := issue_service.HasAllRequiredCodeownerReviews(db.DefaultContext, &protectBranch, pr)
+			assert.False(t, hasCodeownerReviews)
+
+			issues_model.SubmitReview(db.DefaultContext, user5, pr.Issue, issues_model.ReviewTypeApprove, "Very good", resp.Commit.SHA, false, make([]string, 0))
+
+			// should still fail (user5 is not a code owner for this PR)
+			hasCodeownerReviews = issue_service.HasAllRequiredCodeownerReviews(db.DefaultContext, &protectBranch, pr)
+			assert.False(t, hasCodeownerReviews)
+
+			issues_model.SubmitReview(db.DefaultContext, user8, pr.Issue, issues_model.ReviewTypeApprove, "Very good", resp.Commit.SHA, false, make([]string, 0))
+
+			hasCodeownerReviews = issue_service.HasAllRequiredCodeownerReviews(db.DefaultContext, &protectBranch, pr)
+			assert.True(t, hasCodeownerReviews)
 		})
 	})
 }