add protected-branch feature on pull-request page

* show commit-status if context is require status checks to pass.
  * disable merge if new commit-id has not `commit-status` ok on `Status-checkes`.
  * if some status includes required is not success, merge button is disabled.
  * if any required status is success, and some status not includes required, merge button is active, but button color is white.
  * if any required status is success, merge button is active, and button color is green.
This commit is contained in:
nazoking
2015-12-07 19:51:35 +09:00
parent 34240d16b5
commit 645af4d2c0
6 changed files with 193 additions and 120 deletions

View File

@@ -12,16 +12,26 @@ object ApiBranchProtection{
/** form for enabling-and-disabling-branch-protection */
case class EnablingAndDisabling(protection: ApiBranchProtection)
def apply(info: Option[ProtectedBrancheService.ProtectedBranchInfo]): ApiBranchProtection = info match {
case None => ApiBranchProtection(false, Some(statusNone))
case Some(info) => ApiBranchProtection(true, Some(Status(if(info.includeAdministrators){ Everyone }else{ NonAdmins }, info.requireStatusChecksToPass)))
}
def apply(info: ProtectedBrancheService.ProtectedBranchInfo): ApiBranchProtection = ApiBranchProtection(
enabled = info.enabled,
required_status_checks = Some(Status(EnforcementLevel(info.enabled, info.includeAdministrators), info.contexts)))
val statusNone = Status(Off, Seq.empty)
case class Status(enforcement_level: EnforcementLevel, contexts: Seq[String])
sealed class EnforcementLevel(val name: String)
case object Off extends EnforcementLevel("off")
case object NonAdmins extends EnforcementLevel("non_admins")
case object Everyone extends EnforcementLevel("everyone")
object EnforcementLevel {
def apply(enabled: Boolean, includeAdministrators: Boolean): EnforcementLevel = if(enabled){
if(includeAdministrators){
Everyone
}else{
NonAdmins
}
}else{
Off
}
}
implicit val enforcementLevelSerializer = new CustomSerializer[EnforcementLevel](format => (
{

View File

@@ -27,13 +27,13 @@ import scala.collection.JavaConverters._
class PullRequestsController extends PullRequestsControllerBase
with RepositoryService with AccountService with IssuesService with PullRequestService with MilestonesService with LabelsService
with CommitsService with ActivityService with WebHookPullRequestService with ReferrerAuthenticator with CollaboratorsAuthenticator
with CommitStatusService with MergeService
with CommitStatusService with MergeService with ProtectedBrancheService
trait PullRequestsControllerBase extends ControllerBase {
self: RepositoryService with AccountService with IssuesService with MilestonesService with LabelsService
with CommitsService with ActivityService with PullRequestService with WebHookPullRequestService with ReferrerAuthenticator with CollaboratorsAuthenticator
with CommitStatusService with MergeService =>
with CommitStatusService with MergeService with ProtectedBrancheService =>
private val logger = LoggerFactory.getLogger(classOf[PullRequestsControllerBase])
@@ -171,17 +171,21 @@ trait PullRequestsControllerBase extends ControllerBase {
val owner = repository.owner
val name = repository.name
getPullRequest(owner, name, issueId) map { case(issue, pullreq) =>
val statuses = getCommitStatues(owner, name, pullreq.commitIdTo)
val branchProtection = getProtectedBranchInfo(owner, name, pullreq.branch)
val statuses = branchProtection.withRequireStatues(getCommitStatues(owner, name, pullreq.commitIdTo))
val hasConfrict = LockUtil.lock(s"${owner}/${name}"){
checkConflict(owner, name, pullreq.branch, issueId)
}
val hasProblem = hasConfrict || (!statuses.isEmpty && CommitState.combine(statuses.map(_.state).toSet) != CommitState.SUCCESS)
val hasRequiredStatusProblem = branchProtection.hasProblem(statuses, pullreq.commitIdTo, context.loginAccount.get.userName)
val hasProblem = hasRequiredStatusProblem || hasConfrict || (!statuses.isEmpty && CommitState.combine(statuses.map(_.state).toSet) != CommitState.SUCCESS)
html.mergeguide(
hasConfrict,
hasProblem,
issue,
pullreq,
statuses,
branchProtection,
hasRequiredStatusProblem,
repository,
getRepository(pullreq.requestUserName, pullreq.requestRepositoryName, context.baseUrl).get)
}

View File

@@ -1,6 +1,6 @@
package gitbucket.core.service
import gitbucket.core.model.{Collaborator, Repository, Account, CommitState}
import gitbucket.core.model.{Collaborator, Repository, Account, CommitState, CommitStatus}
import gitbucket.core.model.Profile._
import gitbucket.core.util.JGitUtil
import profile.simple._
@@ -16,14 +16,17 @@ object MockDB{
trait ProtectedBrancheService {
import ProtectedBrancheService._
def getProtectedBranchInfo(owner: String, repository: String, branch: String)(implicit session: Session): Option[ProtectedBranchInfo] = {
private def getProtectedBranchInfoOpt(owner: String, repository: String, branch: String)(implicit session: Session): Option[ProtectedBranchInfo] = {
// TODO: mock
MockDB.data.get((owner, repository, branch)).map{ case (includeAdministrators, requireStatusChecksToPass) =>
new ProtectedBranchInfo(owner, repository, requireStatusChecksToPass, includeAdministrators)
MockDB.data.get((owner, repository, branch)).map{ case (includeAdministrators, contexts) =>
new ProtectedBranchInfo(owner, repository, true, contexts, includeAdministrators)
}
}
def getProtectedBranchInfo(owner: String, repository: String, branch: String)(implicit session: Session): ProtectedBranchInfo = {
getProtectedBranchInfoOpt(owner, repository, branch).getOrElse(ProtectedBranchInfo.disabled(owner, repository))
}
def isProtectedBranchNeedStatusCheck(owner: String, repository: String, branch: String, user: String)(implicit session: Session): Boolean =
getProtectedBranchInfo(owner, repository, branch).map{a => println(a); a.needStatusCheck(user)}.getOrElse(false)
getProtectedBranchInfo(owner, repository, branch).needStatusCheck(user)
def getProtectedBranchList(owner: String, repository: String)(implicit session: Session): List[String] = {
// TODO: mock
MockDB.data.filter{
@@ -31,9 +34,9 @@ trait ProtectedBrancheService {
case _ => false
}.map{ case ((_, _, branch), _) => branch }.toList
}
def enableBranchProtection(owner: String, repository: String, branch:String, includeAdministrators: Boolean, requireStatusChecksToPass: Seq[String])(implicit session: Session): Unit = {
def enableBranchProtection(owner: String, repository: String, branch:String, includeAdministrators: Boolean, contexts: Seq[String])(implicit session: Session): Unit = {
// TODO: mock
MockDB.data.put((owner, repository, branch), includeAdministrators -> requireStatusChecksToPass)
MockDB.data.put((owner, repository, branch), includeAdministrators -> contexts)
}
def disableBranchProtection(owner: String, repository: String, branch:String)(implicit session: Session): Unit = {
// TODO: mock
@@ -43,7 +46,7 @@ trait ProtectedBrancheService {
def getBranchProtectedReason(owner: String, repository: String, receivePack: ReceivePack, command: ReceiveCommand, pusher: String)(implicit session: Session): Option[String] = {
val branch = command.getRefName.stripPrefix("refs/heads/")
if(branch != command.getRefName){
getProtectedBranchInfo(owner, repository, branch).flatMap(_.getStopReason(receivePack, command, pusher))
getProtectedBranchInfo(owner, repository, branch).getStopReason(receivePack, command, pusher)
}else{
None
}
@@ -53,13 +56,14 @@ object ProtectedBrancheService {
case class ProtectedBranchInfo(
owner: String,
repository: String,
enabled: Boolean,
/**
* Require status checks to pass before merging
* Choose which status checks must pass before branches can be merged into test.
* When enabled, commits must first be pushed to another branch,
* then merged or pushed directly to test after status checks have passed.
*/
requireStatusChecksToPass: Seq[String],
contexts: Seq[String],
/**
* Include administrators
* Enforce required status checks for repository administrators.
@@ -74,32 +78,56 @@ object ProtectedBrancheService {
* Can't have changes merged into them until required status checks pass
*/
def getStopReason(receivePack: ReceivePack, command: ReceiveCommand, pusher: String)(implicit session: Session): Option[String] = {
command.getType() match {
case ReceiveCommand.Type.UPDATE|ReceiveCommand.Type.UPDATE_NONFASTFORWARD if receivePack.isAllowNonFastForwards =>
Some("Cannot force-push to a protected branch")
case ReceiveCommand.Type.UPDATE|ReceiveCommand.Type.UPDATE_NONFASTFORWARD if needStatusCheck(pusher) =>
unSuccessedContexts(command.getNewId.name) match {
case s if s.size == 1 => Some(s"""Required status check "${s.toSeq(0)}" is expected""")
case s if s.size >= 1 => Some(s"${s.size} of ${requireStatusChecksToPass.size} required status checks are expected")
case _ => None
}
case ReceiveCommand.Type.DELETE =>
Some("Cannot delete a protected branch")
case _ => None
if(enabled){
command.getType() match {
case ReceiveCommand.Type.UPDATE|ReceiveCommand.Type.UPDATE_NONFASTFORWARD if receivePack.isAllowNonFastForwards =>
Some("Cannot force-push to a protected branch")
case ReceiveCommand.Type.UPDATE|ReceiveCommand.Type.UPDATE_NONFASTFORWARD if needStatusCheck(pusher) =>
unSuccessedContexts(command.getNewId.name) match {
case s if s.size == 1 => Some(s"""Required status check "${s.toSeq(0)}" is expected""")
case s if s.size >= 1 => Some(s"${s.size} of ${contexts.size} required status checks are expected")
case _ => None
}
case ReceiveCommand.Type.DELETE =>
Some("Cannot delete a protected branch")
case _ => None
}
}else{
None
}
}
def unSuccessedContexts(sha1: String)(implicit session: Session): Set[String] = if(requireStatusChecksToPass.isEmpty){
def unSuccessedContexts(sha1: String)(implicit session: Session): Set[String] = if(contexts.isEmpty){
Set.empty
} else {
requireStatusChecksToPass.toSet -- getCommitStatues(owner, repository, sha1).filter(_.state == CommitState.SUCCESS).map(_.context).toSet
contexts.toSet -- getCommitStatues(owner, repository, sha1).filter(_.state == CommitState.SUCCESS).map(_.context).toSet
}
def needStatusCheck(pusher: String)(implicit session: Session): Boolean =
if(requireStatusChecksToPass.isEmpty){
if(!enabled || contexts.isEmpty){
false
}else if(includeAdministrators){
true
}else{
!isAdministrator(pusher)
}
def withRequireStatues(statuses: List[CommitStatus]): List[CommitStatus] = {
statuses ++ (contexts.toSet -- statuses.map(_.context).toSet).map{ context => CommitStatus(
commitStatusId = 0,
userName = owner,
repositoryName = repository,
commitId = "",
context = context,
state = CommitState.PENDING,
targetUrl = None,
description = Some("Waiting for status to be reported"),
creator = "",
registeredDate = new java.util.Date(),
updatedDate = new java.util.Date())
}
}
def hasProblem(statuses: List[CommitStatus], sha1: String, account: String)(implicit session: Session): Boolean =
needStatusCheck(account) && contexts.exists(context => statuses.find(_.context == context).map(_.state) != Some(CommitState.SUCCESS))
}
object ProtectedBranchInfo{
def disabled(owner: String, repository: String): ProtectedBranchInfo = ProtectedBranchInfo(owner, repository, false, Nil, false)
}
}

View File

@@ -288,10 +288,10 @@ object helpers extends AvatarImageProvider with LinkConverter with RequestCache
}
def commitStateIcon(state: CommitState) = Html(state match {
case CommitState.PENDING => ""
case CommitState.SUCCESS => "✔"
case CommitState.ERROR => "×"
case CommitState.FAILURE => "×"
case CommitState.PENDING => """<i style="color:inherit;width:inherit;height:inherit" class="octicon octicon-primitive-dot"></i>"""
case CommitState.SUCCESS => """<i style="color:inherit;width:inherit;height:inherit" class="octicon octicon-check"></i>"""
case CommitState.ERROR => """<i style="color:inherit;width:inherit;height:inherit" class="octicon octicon-x"></i>"""
case CommitState.FAILURE => """<i style="color:inherit;width:inherit;height:inherit" class="octicon octicon-x"></i>"""
})
def commitStateText(state: CommitState, commitId:String) = state match {

View File

@@ -3,6 +3,8 @@
issue: gitbucket.core.model.Issue,
pullreq: gitbucket.core.model.PullRequest,
statuses: List[model.CommitStatus],
branchProtection: gitbucket.core.service.ProtectedBrancheService.ProtectedBranchInfo,
hasRequiredStatusProblem: Boolean,
originRepository: gitbucket.core.service.RepositoryService.RepositoryInfo,
forkedRepository: gitbucket.core.service.RepositoryService.RepositoryInfo)(implicit context: gitbucket.core.controller.Context)
@import gitbucket.core.service.SystemSettingsService
@@ -14,35 +16,27 @@
<div id="merge-pull-request">
@if(!statuses.isEmpty){
<div class="build-statuses">
@if(statuses.size==1){
@defining(statuses.head){ status =>
<div class="build-status-item">
@status.targetUrl.map{ url => <a class="pull-right" href="@url">Details</a> }
<span class="build-status-icon text-@{status.state.name}">@commitStateIcon(status.state)</span>
<strong class="text-@{status.state.name}">@commitStateText(status.state, pullreq.commitIdTo)</strong>
@status.description.map{ desc => <span class="muted">— @desc</span> }
</div>
}
} else {
@defining(statuses.groupBy(_.state)){ stateMap =>
@defining(CommitState.combine(stateMap.keySet)){ state =>
@defining(statuses.groupBy(_.state)){ stateMap =>
@defining(CommitState.combine(stateMap.keySet)){ state =>
<div class="build-status-item-header">
<a class="pull-right" id="toggle-all-checks"></a>
<span class="build-status-icon text-@{state.name}">@commitStateIcon(state)</span>
<strong class="text-@{state.name}">@commitStateText(state, pullreq.commitIdTo)</strong>
<span class="text-@{state.name}">— @{stateMap.map{ case (keyState, states) => states.size+" "+keyState.name }.mkString(", ")} checks</span>
</div>
<div class="build-statuses-list" style="@if(state==CommitState.SUCCESS){ display:none; }else{ }">
@statuses.map{ status =>
<div class="build-status-item">
<a class="pull-right" id="toggle-all-checks"></a>
<span class="build-status-icon text-@{state.name}">@commitStateIcon(state)</span>
<strong class="text-@{state.name}">@commitStateText(state, pullreq.commitIdTo)</strong>
<span class="text-@{state.name}">— @{stateMap.map{ case (keyState, states) => states.size+" "+keyState.name }.mkString(", ")} checks</span>
</div>
<div class="build-statuses-list" style="@if(state==CommitState.SUCCESS){ display:none; }else{ }">
@statuses.map{ status =>
<div class="build-status-item">
@status.targetUrl.map{ url => <a class="pull-right" href="@url">Details</a> }
<span class="build-status-icon text-@{status.state.name}">@commitStateIcon(status.state)</span>
<span class="text-@{status.state.name}">@status.context</span>
@status.description.map{ desc => <span class="muted">— @desc</span> }
<div class="pull-right">
@branchProtection.contexts.find(_==status.context).map{ url => <span class="label">Required</span> }
@status.targetUrl.map{ url => <a href="@url">Details</a> }
</div>
}
<span class="build-status-icon text-@{status.state.name}">@commitStateIcon(status.state)</span>
<strong>@status.context</strong>
@status.description.map{ desc => <span class="muted">— @desc</span> }
</div>
}
}
</div>
}
}
</div>
@@ -52,67 +46,74 @@
</div>
<div>
@if(hasConflict){
<span class="strong">We cant automatically merge this pull request.</span>
<div class="merge-indicator merge-indicator-alert"><span class="octicon octicon-alert"></span></div>
<span class="strong">This branch has conflicts that must be resolved</span>
<div class="small">
<a href="#" class="show-command-line">Use the command line</a> to resolve conflicts before continuing.
</div>
} else { @if(hasRequiredStatusProblem) {
<div class="merge-indicator merge-indicator-warning"><span class="octicon octicon-primitive-dot"></span></div>
<span class="strong">Required statuses must pass before merging.</span>
<div class="small">
All required status checks on this pull request must run successfully to enable automatic merging.
</div>
} else {
@if(hasProblem){
<span class="strong">Merge with caution!</span>
<div class="merge-indicator merge-indicator-success"><span class="octicon octicon-check"></span></div>
<span class="strong">Merging can be performed automatically.</span>
<div class="small">
Merging can be performed automatically.
</div>
} }
</div>
<div style="padding:15px;border-top:solid 1px #e5e5e5;background:#fafafa">
<input type="button" class="btn @if(!hasProblem){ btn-success }" id="merge-pull-request-button" value="Merge pull request"@if(hasConflict||hasRequiredStatusProblem){ disabled="true"}/>
You can also merge branches on the <a href="#" class="show-command-line">command line</a>.
<div id="command-line" style="display: none;margin-top: 15px;">
<hr>
@if(hasConflict){
<span class="strong">Checkout via command line</span>
<p>
If you cannot merge a pull request automatically here, you have the option of checking
it out via command line to resolve conflicts and perform a manual merge.
</p>
} else {
<span class="strong">This pull request can be automatically merged.</span>
<span class="strong">Merging via command line</span>
<p>
If you do not want to use the merge button or an automatic merge cannot be performed,
you can perform a manual merge on the command line.
</p>
}
}
</div>
<div class="small">
@if(hasConflict){
<a href="#" id="show-command-line">Use the command line</a> to resolve conflicts before continuing.
} else {
You can also merge branches on the <a href="#" id="show-command-line">command line</a>.
}
</div>
<div id="command-line" style="display: none;">
<hr>
@if(hasConflict){
<span class="strong">Checkout via command line</span>
<p>
If you cannot merge a pull request automatically here, you have the option of checking
it out via command line to resolve conflicts and perform a manual merge.
</p>
} else {
<span class="strong">Merging via command line</span>
<p>
If you do not want to use the merge button or an automatic merge cannot be performed,
you can perform a manual merge on the command line.
</p>
}
@helper.html.copy("repository-url-copy", forkedRepository.httpUrl, true){
<div class="btn-group" data-toggle="buttons-radio">
<button class="btn btn-small active" type="button" id="repository-url-http">HTTP</button>
@if(settings.ssh && loginAccount.isDefined){
<button class="btn btn-small" type="button" id="repository-url-ssh" style="border-radius: 0px;">SSH</button>
@helper.html.copy("repository-url-copy", forkedRepository.httpUrl, true){
<div class="btn-group" data-toggle="buttons-radio">
<button class="btn btn-small active" type="button" id="repository-url-http">HTTP</button>
@if(settings.ssh && loginAccount.isDefined){
<button class="btn btn-small" type="button" id="repository-url-ssh" style="border-radius: 0px;">SSH</button>
}
</div>
<input type="text" style="width: 500px;" value="@forkedRepository.httpUrl" id="repository-url" readonly>
}
<div>
<p>
<span class="strong">Step 1:</span> From your project repository, check out a new branch and test the changes.
</p>
@defining(s"git checkout -b ${pullreq.requestUserName}-${pullreq.requestBranch} ${pullreq.branch}\n" +
s"git pull ${forkedRepository.httpUrl} ${pullreq.requestBranch}"){ command =>
@helper.html.copy("merge-command-copy-1", command){
<pre style="width: 600px; float: left; font-size: 12px; border-radius: 3px 0px 3px 3px;" id="merge-command">@Html(command)</pre>
}
}
</div>
<input type="text" style="width: 500px;" value="@forkedRepository.httpUrl" id="repository-url" readonly>
}
<div>
<p>
<span class="strong">Step 1:</span> From your project repository, check out a new branch and test the changes.
</p>
@defining(s"git checkout -b ${pullreq.requestUserName}-${pullreq.requestBranch} ${pullreq.branch}\n" +
s"git pull ${forkedRepository.httpUrl} ${pullreq.requestBranch}"){ command =>
@helper.html.copy("merge-command-copy-1", command){
<pre style="width: 600px; float: left; font-size: 12px; border-radius: 3px 0px 3px 3px;" id="merge-command">@Html(command)</pre>
<div>
<p>
<span class="strong">Step 2:</span> Merge the changes and update on the server.
</p>
@defining(s"git checkout ${pullreq.branch}\ngit merge --no-ff ${pullreq.requestUserName}-${pullreq.requestBranch}\n" +
s"git push origin ${pullreq.branch}"){ command =>
@helper.html.copy("merge-command-copy-2", command){
<pre style="width: 600px; float: left; font-size: 12px; border-radius: 3px 0px 3px 3px;">@command</pre>
}
}
}
</div>
<div>
<p>
<span class="strong">Step 2:</span> Merge the changes and update on the server.
</p>
@defining(s"git checkout ${pullreq.branch}\ngit merge --no-ff ${pullreq.requestUserName}-${pullreq.requestBranch}\n" +
s"git push origin ${pullreq.branch}"){ command =>
@helper.html.copy("merge-command-copy-2", command){
<pre style="width: 600px; float: left; font-size: 12px; border-radius: 3px 0px 3px 3px;">@command</pre>
}
}
</div>
</div>
</div>
</div>
@@ -134,8 +135,8 @@
<script>
$(function(){
$('#show-command-line').click(function(){
$('#command-line').show();
$('.show-command-line').click(function(){
$('#command-line').toggle();
return false;
});
function setToggleAllChecksLabel(){

View File

@@ -1405,13 +1405,43 @@ div.author-info div.committer {
margin: -10px -10px 10px -10px;
}
.build-statuses .build-status-item{
padding: 10px 15px 10px 12px;
padding: 10px 15px 10px 64px;
border-bottom: 1px solid #eee;
}
.build-statuses-list .build-status-item{
background-color: #fafafa;
}
.merge-indicator{
float:left;
border-radius: 50%;
width: 30px;
height: 30px;
line-height: 30px;
text-align: center;
vertical-align: middle;
margin-right: 10px;
}
.merge-indicator-success{
background-color: #6cc644;
}
.merge-indicator-warning{
background-color: #cea61b;
}
.merge-indicator-alert{
background-color: #888;
}
.merge-indicator .octicon{
color: white;
font-size: 16px;
width: 15px;
height: 15px;
}
.merge-indicator-warning .octicon{
color: white;
font-size: 30px;
}
/****************************************************************************/
/* Diff */
/****************************************************************************/