Skip to content

Commit

Permalink
Fix signing.wont_sign.%!s(<nil>) if Require Signing commits but not s…
Browse files Browse the repository at this point in the history
…igned in (#12581)

signing.wont_sign.%!s(<nil>) will be displayed if the repository needs signed
commits but the user is not logged in.

This is displayed because of complicated logic in the the template repo/issue/view_content/pull.tmpl
and a shortcut in the code of routers/repo/issue.go

This PR adds a default value of notsignedin if users are not signed in, which
although our templates will not show will prevent custom templates from showing
the above.

It also fixes the template to avoid showing signing errors if the user is not
authorized to sign.

Replaces #12564
Close #12564

Signed-off-by: Andrew Thornton <[email protected]>
  • Loading branch information
zeripath authored Aug 23, 2020
1 parent dd8ec12 commit 1bf7b8d
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 4 deletions.
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1276,6 +1276,7 @@ signing.wont_sign.basesigned = The merge will not be signed as the base commit i
signing.wont_sign.headsigned = The merge will not be signed as the head commit is not signed
signing.wont_sign.commitssigned = The merge will not be signed as all the associated commits are not signed
signing.wont_sign.approved = The merge will not be signed as the PR is not approved
signing.wont_sign.not_signed_in = You are not signed in
ext_wiki = Ext. Wiki
ext_wiki.desc = Link to an external wiki.
Expand Down
2 changes: 2 additions & 0 deletions routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -1212,6 +1212,8 @@ func ViewIssue(ctx *context.Context) {
log.Error("Error whilst checking if could sign pr %d in repo %s. Error: %v", pull.ID, pull.BaseRepo.FullName(), err)
}
}
} else {
ctx.Data["WontSignReason"] = "not_signed_in"
}
ctx.Data["IsPullBranchDeletable"] = canDelete &&
pull.HeadRepo != nil &&
Expand Down
8 changes: 4 additions & 4 deletions templates/repo/issue/view_content/pull.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
{{- else if .IsBlockedByOutdatedBranch}}red
{{- else if and .EnableStatusCheck (or .RequiredStatusCheckState.IsFailure .RequiredStatusCheckState.IsError)}}red
{{- else if and .EnableStatusCheck (or (not $.LatestCommitStatus) .RequiredStatusCheckState.IsPending .RequiredStatusCheckState.IsWarning)}}yellow
{{- else if and .RequireSigned (not .WillSign)}}red
{{- else if and .AllowMerge .RequireSigned (not .WillSign)}}red
{{- else if .Issue.PullRequest.IsChecking}}yellow
{{- else if .Issue.PullRequest.CanAutoMerge}}green
{{- else}}red{{end}}">{{svg "octicon-git-merge" 32}}</a>
Expand Down Expand Up @@ -159,7 +159,7 @@
<i class="icon icon-octicon">{{svg "octicon-x" 16}}</i>
{{$.i18n.Tr "repo.pulls.required_status_check_missing"}}
</div>
{{else if and .RequireSigned (not .WillSign)}}
{{else if and .AllowMerge .RequireSigned (not .WillSign)}}
<div class="item text red">
<i class="icon icon-octicon">{{svg "octicon-x" 16}}</i>
{{$.i18n.Tr "repo.pulls.require_signed_wont_sign"}}
Expand All @@ -170,7 +170,7 @@
</div>
{{end}}
{{$notAllOverridableChecksOk := or .IsBlockedByApprovals .IsBlockedByRejection .IsBlockedByOutdatedBranch (and .EnableStatusCheck (not .RequiredStatusCheckState.IsSuccess))}}
{{if and (or $.IsRepoAdmin (not $notAllOverridableChecksOk)) (or (not .RequireSigned) .WillSign)}}
{{if and (or $.IsRepoAdmin (not $notAllOverridableChecksOk)) (or (not .AllowMerge) (not .RequireSigned) .WillSign)}}
{{if $notAllOverridableChecksOk}}
<div class="item text yellow">
<i class="icon icon-octicon">{{svg "octicon-dot-fill" 16}}</i>
Expand Down Expand Up @@ -216,7 +216,7 @@
</div>
{{end}}

{{if and (or $.IsRepoAdmin (not $notAllOverridableChecksOk)) (or (not .RequireSigned) .WillSign)}}
{{if and (or $.IsRepoAdmin (not $notAllOverridableChecksOk)) (or (not .AllowMerge) (not .RequireSigned) .WillSign)}}
{{if .AllowMerge}}
{{$prUnit := .Repository.MustGetUnit $.UnitTypePullRequests}}
{{$approvers := .Issue.PullRequest.GetApprovers}}
Expand Down

0 comments on commit 1bf7b8d

Please sign in to comment.