fine tune

Author: wxiaoguang

services/pull/merge.go

@@ -547,10 +547,10 @@ var escapedSymbols = regexp.MustCompile(`([*[?! \\])`)
 
 // IsUserAllowedToMerge check if user is allowed to merge PR with given permissions and branch protections
 func IsUserAllowedToMerge(ctx context.Context, pr *issues_model.PullRequest, p access_model.Permission, user *user_model.User) (bool, error) {
-	return isUserAllowedToMergeRepoBranch(ctx, pr.BaseRepoID, pr.BaseBranch, p, user)
+	return isUserAllowedToMergeInRepoBranch(ctx, pr.BaseRepoID, pr.BaseBranch, p, user)
 }
 
-func isUserAllowedToMergeRepoBranch(ctx context.Context, repoID int64, branch string, p access_model.Permission, user *user_model.User) (bool, error) {
+func isUserAllowedToMergeInRepoBranch(ctx context.Context, repoID int64, branch string, p access_model.Permission, user *user_model.User) (bool, error) {
 	if user == nil {
 		return false, nil
 	}

services/pull/update.go

@@ -121,27 +121,28 @@ func IsUserAllowedToUpdate(ctx context.Context, pull *issues_model.PullRequest,
 		return false, false, err
 	}
 
-	// 1. check base repository's AllowRebaseUpdate configuration, this should be
-	// the setting of base repository not head repository
-	prUnit, err := pull.BaseRepo.GetUnit(ctx, unit.TypePullRequests)
-	if err != nil {
-		if repo_model.IsErrUnitTypeNotExist(err) {
-			return false, false, nil
-		}
-		log.Error("pr.BaseRepo.GetUnit(unit.TypePullRequests): %v", err)
-		return false, false, err
+	// 1. check base repository's AllowRebaseUpdate configuration
+	// it is a config in base repo but controls the head (fork) repo's "Update" behavior
+	prBaseUnit, err := pull.BaseRepo.GetUnit(ctx, unit.TypePullRequests)
+	if repo_model.IsErrUnitTypeNotExist(err) {
+		return false, false, nil // the PR unit is disabled in base repo
+	} else if err != nil {
+		return false, false, fmt.Errorf("get base repo unit: %v", err)
+	} else {
+		rebaseAllowed = prBaseUnit.PullRequestsConfig().AllowRebaseUpdate
 	}
-	rebaseAllowed = prUnit.PullRequestsConfig().AllowRebaseUpdate
 
 	// 2. check head branch protection whether rebase is allowed, if pb not found then rebase depends on the above setting
-	pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pull.HeadRepoID, pull.HeadBranch)
-	if err != nil {
-		return false, false, err
-	}
-	// If branch protected, disable rebase unless user is whitelisted to force push (which extends regular push)
-	if pb != nil {
-		pb.Repo = pull.HeadRepo
-		rebaseAllowed = rebaseAllowed && pb.CanUserForcePush(ctx, user)
+	{
+		pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pull.HeadRepoID, pull.HeadBranch)
+		if err != nil {
+			return false, false, err
+		}
+		// If branch protected, disable rebase unless user is whitelisted to force push (which extends regular push)
+		if pb != nil {
+			pb.Repo = pull.HeadRepo
+			rebaseAllowed = rebaseAllowed && pb.CanUserForcePush(ctx, user)
+		}
 	}
 
 	// 3. check whether user has write access to head branch
@@ -150,15 +151,15 @@ func IsUserAllowedToUpdate(ctx context.Context, pull *issues_model.PullRequest,
 		return false, false, err
 	}
 
-	mergeAllowed, err = isUserAllowedToMergeRepoBranch(ctx, pull.HeadRepoID, pull.HeadBranch, headRepoPerm, user)
+	mergeAllowed, err = isUserAllowedToMergeInRepoBranch(ctx, pull.HeadRepoID, pull.HeadBranch, headRepoPerm, user)
 	if err != nil {
 		return false, false, err
 	}
 
 	// 4. if the pull creator allows maintainer to edit, it means the write permissions of the head branch has been
 	// granted to the user with write permission of the base repository
 	if pull.AllowMaintainerEdit {
-		mergeAllowedMaintainer, err := isUserAllowedToMergeRepoBranch(ctx, pull.BaseRepoID, pull.BaseBranch, baseRepoPerm, user)
+		mergeAllowedMaintainer, err := isUserAllowedToMergeInRepoBranch(ctx, pull.BaseRepoID, pull.BaseBranch, baseRepoPerm, user)
 		if err != nil {
 			return false, false, err
 		}