From 93ec8d4fd10eee354785939e01dfbe0231785b49 Mon Sep 17 00:00:00 2001 From: william-allspice Date: Fri, 16 Aug 2024 13:17:58 -0500 Subject: [PATCH 1/4] Modify official reviewers to be any reviewer with code access --- models/git/protected_branch.go | 6 +++--- models/issues/review.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index bde6057375e55..ef82f4cb04535 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -206,12 +206,12 @@ func IsUserOfficialReviewer(ctx context.Context, protectBranch *ProtectedBranch, } if !protectBranch.EnableApprovalsWhitelist { - // Anyone with write access is considered official reviewer - writeAccess, err := access_model.HasAccessUnit(ctx, user, repo, unit.TypeCode, perm.AccessModeWrite) + // Anyone with code access is considered official reviewer + access, err := access_model.HasAccessUnit(ctx, user, repo, unit.TypeCode, perm.AccessModeRead) if err != nil { return false, err } - return writeAccess, nil + return access, nil } if slices.Contains(protectBranch.ApprovalsWhitelistUserIDs, user.ID) { diff --git a/models/issues/review.go b/models/issues/review.go index ca6fd6035b130..11ea820aaccfc 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -277,7 +277,6 @@ func IsOfficialReviewer(ctx context.Context, issue *Issue, reviewer *user_model. } return writeAccess, nil } - official, err := git_model.IsUserOfficialReviewer(ctx, rule, reviewer) if official || err != nil { return official, err @@ -300,7 +299,8 @@ func IsOfficialReviewerTeam(ctx context.Context, issue *Issue, team *organizatio } if !pb.EnableApprovalsWhitelist { - return team.UnitAccessMode(ctx, unit.TypeCode) >= perm.AccessModeWrite, nil + // Any team with code access is considered official reviewer + return team.UnitAccessMode(ctx, unit.TypeCode) >= perm.AccessModeRead, nil } return slices.Contains(pb.ApprovalsWhitelistTeamIDs, team.ID), nil From c6d4e19ca541ac4903e3181f3bfbc22c490f99ff Mon Sep 17 00:00:00 2001 From: william-allspice Date: Fri, 16 Aug 2024 15:25:57 -0500 Subject: [PATCH 2/4] Add unit test case --- models/git/protected_branch_test.go | 45 +++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/models/git/protected_branch_test.go b/models/git/protected_branch_test.go index 1962859a8c4e3..ce80856f3977e 100644 --- a/models/git/protected_branch_test.go +++ b/models/git/protected_branch_test.go @@ -1,12 +1,21 @@ // Copyright 2022 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -package git +package git_test import ( "fmt" "testing" + access_model "code.gitea.io/gitea/models/perm/access" + + "code.gitea.io/gitea/models/db" + git_model "code.gitea.io/gitea/models/git" + perm_model "code.gitea.io/gitea/models/perm" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/log" "github.com/stretchr/testify/assert" ) @@ -64,7 +73,7 @@ func TestBranchRuleMatch(t *testing.T) { } for _, kase := range kases { - pb := ProtectedBranch{RuleName: kase.Rule} + pb := git_model.ProtectedBranch{RuleName: kase.Rule} var should, infact string if !kase.ExpectedMatch { should = " not" @@ -76,3 +85,35 @@ func TestBranchRuleMatch(t *testing.T) { ) } } + +func TestIsUserOfficialReviewer(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + log.Info(fmt.Sprintf("repo.IsPrivate %v", repo.IsPrivate)) + protectedBranch := &git_model.ProtectedBranch{ + RepoID: repo.ID, + EnableApprovalsWhitelist: false, + } + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) + + access := &access_model.Access{ + UserID: user.ID, + RepoID: repo.ID, + Mode: perm_model.AccessModeNone, + } + assert.NoError(t, db.Insert(db.DefaultContext, access)) + + official, err := git_model.IsUserOfficialReviewer(db.DefaultContext, protectedBranch, user) + assert.NoError(t, err) + assert.False(t, official) + + access.Mode = perm_model.AccessModeRead + _, err = db.GetEngine(db.DefaultContext).ID(access.ID).Update(access) + assert.NoError(t, err) + + official, err = git_model.IsUserOfficialReviewer(db.DefaultContext, protectedBranch, user) + assert.NoError(t, err) + assert.True(t, official) +} From a91cc98e7f8f706bb5a96dcb0a7d0e22b0a94cd0 Mon Sep 17 00:00:00 2001 From: william-allspice Date: Fri, 16 Aug 2024 15:41:25 -0500 Subject: [PATCH 3/4] clean up --- models/git/protected_branch_test.go | 3 --- models/issues/review.go | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/models/git/protected_branch_test.go b/models/git/protected_branch_test.go index ce80856f3977e..c339999e76ef0 100644 --- a/models/git/protected_branch_test.go +++ b/models/git/protected_branch_test.go @@ -15,7 +15,6 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/log" "github.com/stretchr/testify/assert" ) @@ -90,12 +89,10 @@ func TestIsUserOfficialReviewer(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - log.Info(fmt.Sprintf("repo.IsPrivate %v", repo.IsPrivate)) protectedBranch := &git_model.ProtectedBranch{ RepoID: repo.ID, EnableApprovalsWhitelist: false, } - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) access := &access_model.Access{ diff --git a/models/issues/review.go b/models/issues/review.go index 11ea820aaccfc..ef5fc0b194060 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -277,6 +277,7 @@ func IsOfficialReviewer(ctx context.Context, issue *Issue, reviewer *user_model. } return writeAccess, nil } + official, err := git_model.IsUserOfficialReviewer(ctx, rule, reviewer) if official || err != nil { return official, err From cb37298ff078016f220c2f04e1528fe4062bba11 Mon Sep 17 00:00:00 2001 From: william-allspice Date: Mon, 19 Aug 2024 09:42:38 -0500 Subject: [PATCH 4/4] Format --- models/git/protected_branch_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/git/protected_branch_test.go b/models/git/protected_branch_test.go index c339999e76ef0..29a9f31258768 100644 --- a/models/git/protected_branch_test.go +++ b/models/git/protected_branch_test.go @@ -7,14 +7,14 @@ import ( "fmt" "testing" - access_model "code.gitea.io/gitea/models/perm/access" - "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" perm_model "code.gitea.io/gitea/models/perm" + access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "github.com/stretchr/testify/assert" )