Skip to content

Commit 602fe45

Browse files
authored
Fix bug on branch API (#10767) (#10775)
* Fix bug on branch API (#10767) * Fix branch api canPush and canMerge
1 parent e2da9cd commit 602fe45

File tree

4 files changed

+60
-9
lines changed

4 files changed

+60
-9
lines changed

integrations/api_branch_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ func testAPIGetBranch(t *testing.T, branchName string, exists bool) {
2828
var branch api.Branch
2929
DecodeJSON(t, resp, &branch)
3030
assert.EqualValues(t, branchName, branch.Name)
31+
assert.True(t, branch.UserCanPush)
32+
assert.True(t, branch.UserCanMerge)
3133
}
3234

3335
func TestAPIGetBranch(t *testing.T) {

models/branches.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,28 @@ func (protectBranch *ProtectedBranch) CanUserMerge(userID int64) bool {
113113
return in
114114
}
115115

116+
// IsUserMergeWhitelisted checks if some user is whitelisted to merge to this branch
117+
func (protectBranch *ProtectedBranch) IsUserMergeWhitelisted(userID int64) bool {
118+
if !protectBranch.EnableMergeWhitelist {
119+
return true
120+
}
121+
122+
if base.Int64sContains(protectBranch.MergeWhitelistUserIDs, userID) {
123+
return true
124+
}
125+
126+
if len(protectBranch.MergeWhitelistTeamIDs) == 0 {
127+
return false
128+
}
129+
130+
in, err := IsUserInTeams(userID, protectBranch.MergeWhitelistTeamIDs)
131+
if err != nil {
132+
log.Error("IsUserInTeams: %v", err)
133+
return false
134+
}
135+
return in
136+
}
137+
116138
// IsUserOfficialReviewer check if user is official reviewer for the branch (counts towards required approvals)
117139
func (protectBranch *ProtectedBranch) IsUserOfficialReviewer(user *User) (bool, error) {
118140
return protectBranch.isUserOfficialReviewer(x, user)

modules/convert/convert.go

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,29 +30,43 @@ func ToEmail(email *models.EmailAddress) *api.Email {
3030
}
3131

3232
// ToBranch convert a git.Commit and git.Branch to an api.Branch
33-
func ToBranch(repo *models.Repository, b *git.Branch, c *git.Commit, bp *models.ProtectedBranch, user *models.User) *api.Branch {
33+
func ToBranch(repo *models.Repository, b *git.Branch, c *git.Commit, bp *models.ProtectedBranch, user *models.User) (*api.Branch, error) {
3434
if bp == nil {
35+
var hasPerm bool
36+
var err error
37+
if user != nil {
38+
hasPerm, err = models.HasAccessUnit(user, repo, models.UnitTypeCode, models.AccessModeWrite)
39+
if err != nil {
40+
return nil, err
41+
}
42+
}
43+
3544
return &api.Branch{
3645
Name: b.Name,
3746
Commit: ToCommit(repo, c),
3847
Protected: false,
3948
RequiredApprovals: 0,
4049
EnableStatusCheck: false,
4150
StatusCheckContexts: []string{},
42-
UserCanPush: true,
43-
UserCanMerge: true,
44-
}
51+
UserCanPush: hasPerm,
52+
UserCanMerge: hasPerm,
53+
}, nil
4554
}
46-
return &api.Branch{
55+
56+
branch := &api.Branch{
4757
Name: b.Name,
4858
Commit: ToCommit(repo, c),
4959
Protected: true,
5060
RequiredApprovals: bp.RequiredApprovals,
5161
EnableStatusCheck: bp.EnableStatusCheck,
5262
StatusCheckContexts: bp.StatusCheckContexts,
53-
UserCanPush: bp.CanUserPush(user.ID),
54-
UserCanMerge: bp.CanUserMerge(user.ID),
5563
}
64+
65+
if user != nil {
66+
branch.UserCanPush = bp.CanUserPush(user.ID)
67+
branch.UserCanMerge = bp.IsUserMergeWhitelisted(user.ID)
68+
}
69+
return branch, nil
5670
}
5771

5872
// ToTag convert a git.Tag to an api.Tag

routers/api/v1/repo/branch.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,13 @@ func GetBranch(ctx *context.APIContext) {
7070
return
7171
}
7272

73-
ctx.JSON(http.StatusOK, convert.ToBranch(ctx.Repo.Repository, branch, c, branchProtection, ctx.User))
73+
br, err := convert.ToBranch(ctx.Repo.Repository, branch, c, branchProtection, ctx.User)
74+
if err != nil {
75+
ctx.Error(http.StatusInternalServerError, "convert.ToBranch", err)
76+
return
77+
}
78+
79+
ctx.JSON(http.StatusOK, br)
7480
}
7581

7682
// ListBranches list all the branches of a repository
@@ -113,7 +119,14 @@ func ListBranches(ctx *context.APIContext) {
113119
ctx.Error(http.StatusInternalServerError, "GetBranchProtection", err)
114120
return
115121
}
116-
apiBranches[i] = convert.ToBranch(ctx.Repo.Repository, branches[i], c, branchProtection, ctx.User)
122+
123+
br, err := convert.ToBranch(ctx.Repo.Repository, branches[i], c, branchProtection, ctx.User)
124+
if err != nil {
125+
ctx.Error(http.StatusInternalServerError, "convert.ToBranch", err)
126+
return
127+
}
128+
129+
apiBranches[i] = br
117130
}
118131

119132
ctx.JSON(http.StatusOK, &apiBranches)

0 commit comments

Comments
 (0)