diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index 55bbe6938cdc4..b4d556a5d0159 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -58,6 +58,7 @@ type ProtectedBranch struct { RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"` BlockOnRejectedReviews bool `xorm:"NOT NULL DEFAULT false"` BlockOnOfficialReviewRequests bool `xorm:"NOT NULL DEFAULT false"` + BlockOnCodeownerReviews bool `xorm:"NOT NULL DEFAULT false"` BlockOnOutdatedBranch bool `xorm:"NOT NULL DEFAULT false"` DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false"` IgnoreStaleApprovals bool `xorm:"NOT NULL DEFAULT false"` diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 176372486e8f6..f43ea4497fc40 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -24,6 +24,7 @@ import ( "code.gitea.io/gitea/models/migrations/v1_22" "code.gitea.io/gitea/models/migrations/v1_23" "code.gitea.io/gitea/models/migrations/v1_24" + "code.gitea.io/gitea/models/migrations/v1_25" "code.gitea.io/gitea/models/migrations/v1_6" "code.gitea.io/gitea/models/migrations/v1_7" "code.gitea.io/gitea/models/migrations/v1_8" @@ -382,6 +383,10 @@ func prepareMigrationTasks() []*migration { newMigration(318, "Add anonymous_access_mode for repo_unit", v1_24.AddRepoUnitAnonymousAccessMode), newMigration(319, "Add ExclusiveOrder to Label table", v1_24.AddExclusiveOrderColumnToLabelTable), newMigration(320, "Migrate two_factor_policy to login_source table", v1_24.MigrateSkipTwoFactor), + + // Gitea 1.24.0 ends at migration ID number 320 (database version 321) + + newMigration(321, "Add block on codeowner reviews branch protection", v1_25.AddBlockOnCodeownerReviews), } return preparedMigrations } diff --git a/models/migrations/v1_25/v321.go b/models/migrations/v1_25/v321.go new file mode 100644 index 0000000000000..ebe28d2af480e --- /dev/null +++ b/models/migrations/v1_25/v321.go @@ -0,0 +1,16 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_25 + +import ( + "xorm.io/xorm" +) + +func AddBlockOnCodeownerReviews(x *xorm.Engine) error { + type ProtectedBranch struct { + BlockOnCodeownerReviews bool `xorm:"NOT NULL DEFAULT false"` + } + + return x.Sync(new(ProtectedBranch)) +} diff --git a/modules/structs/repo_branch.go b/modules/structs/repo_branch.go index 5416f43b0d1a0..f9d7e7e1558f1 100644 --- a/modules/structs/repo_branch.go +++ b/modules/structs/repo_branch.go @@ -47,6 +47,7 @@ type BranchProtection struct { ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"` BlockOnOfficialReviewRequests bool `json:"block_on_official_review_requests"` + BlockOnCodeownerReviews bool `json:"block_on_codeowner_reviews"` BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"` DismissStaleApprovals bool `json:"dismiss_stale_approvals"` IgnoreStaleApprovals bool `json:"ignore_stale_approvals"` @@ -87,6 +88,7 @@ type CreateBranchProtectionOption struct { ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"` BlockOnOfficialReviewRequests bool `json:"block_on_official_review_requests"` + BlockOnCodeownerReviews bool `json:"block_on_codeowner_reviews"` BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"` DismissStaleApprovals bool `json:"dismiss_stale_approvals"` IgnoreStaleApprovals bool `json:"ignore_stale_approvals"` @@ -120,6 +122,7 @@ type EditBranchProtectionOption struct { ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` BlockOnRejectedReviews *bool `json:"block_on_rejected_reviews"` BlockOnOfficialReviewRequests *bool `json:"block_on_official_review_requests"` + BlockOnCodeownerReviews *bool `json:"block_on_codeowner_reviews"` BlockOnOutdatedBranch *bool `json:"block_on_outdated_branch"` DismissStaleApprovals *bool `json:"dismiss_stale_approvals"` IgnoreStaleApprovals *bool `json:"ignore_stale_approvals"` diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index f319b1de3fd2a..919c9a7c839aa 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1903,6 +1903,7 @@ pulls.required_status_check_administrator = As an administrator, you may still m pulls.blocked_by_approvals = "This pull request doesn't have enough required approvals yet. %d of %d official approvals granted." pulls.blocked_by_approvals_whitelisted = "This pull request doesn't have enough required approvals yet. %d of %d approvals granted from users or teams on the allowlist." pulls.blocked_by_rejection = "This pull request has changes requested by an official reviewer." +pulls.blocked_by_codeowners = "This pull request is missing approval from one or more code owners." pulls.blocked_by_official_review_requests = "This pull request has official review requests." pulls.blocked_by_outdated_branch = "This pull request is blocked because it's outdated." pulls.blocked_by_changed_protected_files_1= "This pull request is blocked because it changes a protected file:" @@ -2538,6 +2539,8 @@ settings.block_rejected_reviews = Block merge on rejected reviews settings.block_rejected_reviews_desc = Merging will not be possible when changes are requested by official reviewers, even if there are enough approvals. settings.block_on_official_review_requests = Block merge on official review requests settings.block_on_official_review_requests_desc = Merging will not be possible when it has official review requests, even if there are enough approvals. +settings.block_on_codeowner_reviews = Require approval from code owners +settings.block_on_codeowner_reviews_desc = Merging will only be possible if at least one code owner per code owner rule has given an approving review. settings.block_outdated_branch = Block merge if pull request is outdated settings.block_outdated_branch_desc = Merging will not be possible when head branch is behind base branch. settings.block_admin_merge_override = Administrators must follow branch protection rules diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 9af958a5b7bd8..8fad5a78ef2f8 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -855,6 +855,10 @@ func EditBranchProtection(ctx *context.APIContext) { protectBranch.BlockOnOfficialReviewRequests = *form.BlockOnOfficialReviewRequests } + if form.BlockOnCodeownerReviews != nil { + protectBranch.BlockOnCodeownerReviews = *form.BlockOnCodeownerReviews + } + if form.DismissStaleApprovals != nil { protectBranch.DismissStaleApprovals = *form.DismissStaleApprovals } diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index d4458ed19ece1..bbee70b6af414 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -947,6 +947,7 @@ func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Iss ctx.Data["ProtectedBranch"] = pb ctx.Data["IsBlockedByApprovals"] = !issues_model.HasEnoughApprovals(ctx, pb, pull) ctx.Data["IsBlockedByRejection"] = issues_model.MergeBlockedByRejectedReview(ctx, pb, pull) + ctx.Data["IsBlockedByCodeowners"] = !issue_service.HasAllRequiredCodeownerReviews(ctx, pb, pull) ctx.Data["IsBlockedByOfficialReviewRequests"] = issues_model.MergeBlockedByOfficialReviewRequests(ctx, pb, pull) ctx.Data["IsBlockedByOutdatedBranch"] = issues_model.MergeBlockedByOutdatedBranch(pb, pull) ctx.Data["GrantedApprovals"] = issues_model.GetGrantedApprovalsCount(ctx, pb, pull) diff --git a/routers/web/repo/setting/protected_branch.go b/routers/web/repo/setting/protected_branch.go index 0eea5e3f34062..d6b2f356378fa 100644 --- a/routers/web/repo/setting/protected_branch.go +++ b/routers/web/repo/setting/protected_branch.go @@ -255,6 +255,7 @@ func SettingsProtectedBranchPost(ctx *context.Context) { } protectBranch.BlockOnRejectedReviews = f.BlockOnRejectedReviews protectBranch.BlockOnOfficialReviewRequests = f.BlockOnOfficialReviewRequests + protectBranch.BlockOnCodeownerReviews = f.BlockOnCodeownerReviews protectBranch.DismissStaleApprovals = f.DismissStaleApprovals protectBranch.IgnoreStaleApprovals = f.IgnoreStaleApprovals protectBranch.RequireSignedCommits = f.RequireSignedCommits diff --git a/services/convert/convert.go b/services/convert/convert.go index 0de38221409bb..caa932c33ff52 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -189,6 +189,7 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo ApprovalsWhitelistTeams: approvalsWhitelistTeams, BlockOnRejectedReviews: bp.BlockOnRejectedReviews, BlockOnOfficialReviewRequests: bp.BlockOnOfficialReviewRequests, + BlockOnCodeownerReviews: bp.BlockOnCodeownerReviews, BlockOnOutdatedBranch: bp.BlockOnOutdatedBranch, DismissStaleApprovals: bp.DismissStaleApprovals, IgnoreStaleApprovals: bp.IgnoreStaleApprovals, diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index cb267f891ccb7..e634733afe89f 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -189,6 +189,7 @@ type ProtectBranchForm struct { ApprovalsWhitelistTeams string BlockOnRejectedReviews bool BlockOnOfficialReviewRequests bool + BlockOnCodeownerReviews bool BlockOnOutdatedBranch bool DismissStaleApprovals bool IgnoreStaleApprovals bool diff --git a/services/issue/pull.go b/services/issue/pull.go index 3543b05b18f78..d3136bac7d342 100644 --- a/services/issue/pull.go +++ b/services/issue/pull.go @@ -9,6 +9,7 @@ import ( "slices" "time" + git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" org_model "code.gitea.io/gitea/models/organization" user_model "code.gitea.io/gitea/models/user" @@ -47,6 +48,129 @@ func IsCodeOwnerFile(f string) bool { return slices.Contains(codeOwnerFiles, f) } +func HasAllRequiredCodeownerReviews(ctx context.Context, pb *git_model.ProtectedBranch, pr *issues_model.PullRequest) bool { + if !pb.BlockOnCodeownerReviews { + return true + } + + if err := pr.LoadHeadRepo(ctx); err != nil { + return false + } + + if err := pr.LoadBaseRepo(ctx); err != nil { + return false + } + + if err := pr.LoadIssue(ctx); err != nil { + return false + } + + pr.Issue.Repo = pr.BaseRepo + + if pr.BaseRepo.IsFork { + return true + } + + repo, err := gitrepo.OpenRepository(ctx, pr.BaseRepo) + if err != nil { + return false + } + + defer repo.Close() + + commit, err := repo.GetBranchCommit(pr.BaseRepo.DefaultBranch) + if err != nil { + return false + } + + var data string + + for _, file := range codeOwnerFiles { + if blob, err := commit.GetBlobByPath(file); err == nil { + data, err = blob.GetBlobContent(setting.UI.MaxDisplayFileSize) + if err == nil { + break + } + } + } + + // no code owner file = no one to approve + if data == "" { + return true + } + + rules, _ := issues_model.GetCodeOwnersFromContent(ctx, data) + if len(rules) == 0 { + return true + } + + // get the mergebase + mergeBase, err := getMergeBase(repo, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName()) + if err != nil { + return false + } + + // https://github.com/go-gitea/gitea/issues/29763, we need to get the files changed + // between the merge base and the head commit but not the base branch and the head commit + changedFiles, err := repo.GetFilesChangedBetween(mergeBase, pr.GetGitRefName()) + if err != nil { + return false + } + + approvingReviews, err := issues_model.FindLatestReviews(ctx, issues_model.FindReviewOptions{ + Types: []issues_model.ReviewType{issues_model.ReviewTypeApprove}, + IssueID: pr.IssueID, + }) + if err != nil { + return false + } + + hasApprovals := true + + matchingRules := make([]*issues_model.CodeOwnerRule, 0) + + for _, rule := range rules { + for _, f := range changedFiles { + if (rule.Rule.MatchString(f) && !rule.Negative) || (!rule.Rule.MatchString(f) && rule.Negative) { + matchingRules = append(matchingRules, rule) + break + } + } + } + + for _, rule := range matchingRules { + ruleReviewers := slices.Clone(rule.Users) + for _, t := range rule.Teams { + if err := t.LoadMembers(ctx); err != nil { + return false + } + + ruleReviewers = slices.AppendSeq(ruleReviewers, slices.Values(t.Members)) + } + + // we need at least 1 code owner that isn't the PR author + hasPotentialReviewers := slices.ContainsFunc(ruleReviewers, func(elem *user_model.User) bool { return elem.ID != pr.Issue.OriginalAuthorID }) + + if !hasPotentialReviewers { + break + } + + // then we need at least 1 approving review from any valid code owner for this rule + hasRuleApproval := slices.ContainsFunc(ruleReviewers, func(elem *user_model.User) bool { + return slices.ContainsFunc(approvingReviews, func(review *issues_model.Review) bool { + return review.ReviewerID == elem.ID + }) + }) + + if !hasRuleApproval { + hasApprovals = false + break + } + } + + return hasApprovals +} + func PullRequestCodeOwnersReview(ctx context.Context, pr *issues_model.PullRequest) ([]*ReviewRequestNotifier, error) { if err := pr.LoadIssue(ctx); err != nil { return nil, err diff --git a/services/pull/merge.go b/services/pull/merge.go index cd9aeb2ad1ed5..6553aa23083bb 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -592,6 +592,10 @@ func CheckPullBranchProtections(ctx context.Context, pr *issues_model.PullReques return util.ErrorWrap(ErrNotReadyToMerge, "The head branch is behind the base branch") } + if !issue_service.HasAllRequiredCodeownerReviews(ctx, pb, pr) { + return util.ErrorWrap(ErrNotReadyToMerge, "There are missing code owner reviews.") + } + if skipProtectedFilesCheck { return nil } diff --git a/templates/repo/issue/view_content/pull_merge_box.tmpl b/templates/repo/issue/view_content/pull_merge_box.tmpl index 113bfb732ecee..9cef1a7cedf3d 100644 --- a/templates/repo/issue/view_content/pull_merge_box.tmpl +++ b/templates/repo/issue/view_content/pull_merge_box.tmpl @@ -16,6 +16,7 @@ {{- else if .IsBlockedByApprovals}}red {{- else if .IsBlockedByRejection}}red {{- else if .IsBlockedByOfficialReviewRequests}}red + {{- else if .IsBlockedByCodeowners}}red {{- else if .IsBlockedByOutdatedBranch}}red {{- else if .IsBlockedByChangedProtectedFiles}}red {{- else if and .EnableStatusCheck (or .RequiredStatusCheckState.IsFailure .RequiredStatusCheckState.IsError)}}red @@ -131,6 +132,11 @@ {{svg "octicon-x"}} {{ctx.Locale.Tr "repo.pulls.blocked_by_official_review_requests"}} + {{else if .IsBlockedByCodeowners}} +
+ {{svg "octicon-x"}} + {{ctx.Locale.Tr "repo.pulls.blocked_by_codeowners"}} +
{{else if .IsBlockedByOutdatedBranch}}
{{svg "octicon-x"}} @@ -167,7 +173,7 @@
{{end}} - {{$notAllOverridableChecksOk := or .IsBlockedByApprovals .IsBlockedByRejection .IsBlockedByOfficialReviewRequests .IsBlockedByOutdatedBranch .IsBlockedByChangedProtectedFiles (and .EnableStatusCheck (not .RequiredStatusCheckState.IsSuccess))}} + {{$notAllOverridableChecksOk := or .IsBlockedByApprovals .IsBlockedByRejection .IsBlockedByOfficialReviewRequests .IsBlockedByCodeowners .IsBlockedByOutdatedBranch .IsBlockedByChangedProtectedFiles (and .EnableStatusCheck (not .RequiredStatusCheckState.IsSuccess))}} {{/* admin can merge without checks, writer can merge when checks succeed */}} {{$canMergeNow := and (or (and (not $.ProtectedBranch.BlockAdminMergeOverride) $.IsRepoAdmin) (not $notAllOverridableChecksOk)) (or (not .AllowMerge) (not .RequireSigned) .WillSign)}} @@ -337,6 +343,11 @@ {{svg "octicon-x"}} {{ctx.Locale.Tr "repo.pulls.blocked_by_official_review_requests"}} + {{else if .IsBlockedByCodeowners}} +
+ {{svg "octicon-x"}} + {{ctx.Locale.Tr "repo.pulls.blocked_by_codeowners"}} +
{{else if .IsBlockedByOutdatedBranch}}
{{svg "octicon-x"}} diff --git a/templates/repo/settings/protected_branch.tmpl b/templates/repo/settings/protected_branch.tmpl index 3c311c18c39ee..a7bc591b512c1 100644 --- a/templates/repo/settings/protected_branch.tmpl +++ b/templates/repo/settings/protected_branch.tmpl @@ -320,6 +320,13 @@

{{ctx.Locale.Tr "repo.settings.block_on_official_review_requests_desc"}}

+
+
+ + +

{{ctx.Locale.Tr "repo.settings.block_on_codeowner_reviews_desc"}}

+
+
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 323e0d64ac567..87218105c445a 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -21731,6 +21731,10 @@ "type": "boolean", "x-go-name": "BlockAdminMergeOverride" }, + "block_on_codeowner_reviews": { + "type": "boolean", + "x-go-name": "BlockOnCodeownerReviews" + }, "block_on_official_review_requests": { "type": "boolean", "x-go-name": "BlockOnOfficialReviewRequests" @@ -22524,6 +22528,10 @@ "type": "boolean", "x-go-name": "BlockAdminMergeOverride" }, + "block_on_codeowner_reviews": { + "type": "boolean", + "x-go-name": "BlockOnCodeownerReviews" + }, "block_on_official_review_requests": { "type": "boolean", "x-go-name": "BlockOnOfficialReviewRequests" @@ -23794,6 +23802,10 @@ "type": "boolean", "x-go-name": "BlockAdminMergeOverride" }, + "block_on_codeowner_reviews": { + "type": "boolean", + "x-go-name": "BlockOnCodeownerReviews" + }, "block_on_official_review_requests": { "type": "boolean", "x-go-name": "BlockOnOfficialReviewRequests" diff --git a/tests/integration/pull_review_test.go b/tests/integration/pull_review_test.go index 1121751c88bc7..fdbcf0daa811d 100644 --- a/tests/integration/pull_review_test.go +++ b/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) }) }) }