Dont overwrite err with nil & rename PullCheckingFuncs to reflect there usage

routers/private/hook_pre_receive.go

@@ -343,7 +343,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN
 		}
 
 		// Check all status checks and reviews are ok
-		if err := pull_service.CheckPRReadyToMerge(ctx, pr, true); err != nil {
+		if err := pull_service.CheckPullBranchProtections(ctx, pr, true); err != nil {
 			if models.IsErrDisallowedToMerge(err) {
 				log.Warn("Forbidden: User %d is not allowed push to protected branch %s in %-v and pr #%d is not ready to be merged: %s", ctx.opts.UserID, branchName, repo, pr.Index, err.Error())
 				ctx.JSON(http.StatusForbidden, private.Response{

services/pull/check.go

@@ -33,7 +33,7 @@ var prPatchCheckerQueue queue.UniqueQueue
 
 var (
 	ErrIsClosed              = errors.New("pull is cosed")
-	ErrUserNotAllowedToMerge = errors.New("user not allowed to merge")
+	ErrUserNotAllowedToMerge = models.ErrDisallowedToMerge{}
 	ErrHasMerged             = errors.New("has already been merged")
 	ErrIsWorkInProgress      = errors.New("work in progress PRs cannot be merged")
 	ErrIsChecking            = errors.New("cannot merge while conflict checking is in progress")
@@ -81,13 +81,6 @@ func CheckPullMergable(ctx context.Context, doer *user_model.User, perm *models.
 		return nil
 	}
 
-	if pr.ProtectedBranch != nil && (!pr.ProtectedBranch.HasEnoughApprovals(pr) ||
-		pr.ProtectedBranch.MergeBlockedByRejectedReview(pr) ||
-		pr.ProtectedBranch.MergeBlockedByOfficialReviewRequests(pr) ||
-		pr.ProtectedBranch.MergeBlockedByOutdatedBranch(pr)) {
-		return ErrUserNotAllowedToMerge
-	}
-
 	if pr.IsWorkInProgress() {
 		return ErrIsWorkInProgress
 	}
@@ -100,13 +93,13 @@ func CheckPullMergable(ctx context.Context, doer *user_model.User, perm *models.
 		return ErrIsChecking
 	}
 
-	if err := CheckPRReadyToMerge(ctx, pr, false); err != nil {
+	if err := CheckPullBranchProtections(ctx, pr, false); err != nil {
 		if models.IsErrDisallowedToMerge(err) {
 			if force {
-				if isRepoAdmin, err := models.IsUserRepoAdmin(pr.BaseRepo, doer); err != nil {
-					return err
+				if isRepoAdmin, err2 := models.IsUserRepoAdmin(pr.BaseRepo, doer); err2 != nil {
+					return err2
 				} else if !isRepoAdmin {
-					return ErrUserNotAllowedToMerge
+					return err
 				}
 			}
 		} else {

services/pull/merge.go

@@ -662,8 +662,8 @@ func IsUserAllowedToMerge(pr *models.PullRequest, p models.Permission, user *use
 	return false, nil
 }
 
-// CheckPRReadyToMerge checks whether the PR is ready to be merged (reviews and status checks)
-func CheckPRReadyToMerge(ctx context.Context, pr *models.PullRequest, skipProtectedFilesCheck bool) (err error) {
+// CheckPullBranchProtections checks whether the PR is ready to be merged (reviews and status checks)
+func CheckPullBranchProtections(ctx context.Context, pr *models.PullRequest, skipProtectedFilesCheck bool) (err error) {
 	if err = pr.LoadBaseRepoCtx(ctx); err != nil {
 		return fmt.Errorf("LoadBaseRepo: %v", err)
 	}