Skip to content

Hide command line merge instructions when user can't push #17339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 17, 2021
Merged

Hide command line merge instructions when user can't push #17339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7151349
Hide command line merge instructions when user can't push
qwerty287 Oct 17, 2021
2779b97
Merge branch 'main' into cli-guide
qwerty287 Oct 17, 2021
6e25528
Merge branch 'main' into cli-guide
zeripath Oct 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions routers/web/repo/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1543,6 +1543,7 @@ func ViewIssue(ctx *context.Context) {
ctx.ServerError("LoadProtectedBranch", err)
return
}
ctx.Data["ShowMergeInstructions"] = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think instead of true by default, it would be better to use whether the user has write rights for this repo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or is that intended for local merging? In that case, true is the correct value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have write permissions that doesn't not mean that you can push to the branch

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if there is branch protection, which is the line you added below.
But if you cannot even write to the repo, you most likely won't be able to write to the protected branch as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can't merge the PR these instructions won't be shown (and you can't merge without write permission)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. Then the current true suffices.

if pull.ProtectedBranch != nil {
cnt := pull.ProtectedBranch.GetGrantedApprovalsCount(pull)
ctx.Data["IsBlockedByApprovals"] = !pull.ProtectedBranch.HasEnoughApprovals(pull)
Expand All @@ -1554,6 +1555,7 @@ func ViewIssue(ctx *context.Context) {
ctx.Data["ChangedProtectedFiles"] = pull.ChangedProtectedFiles
ctx.Data["IsBlockedByChangedProtectedFiles"] = len(pull.ChangedProtectedFiles) != 0
ctx.Data["ChangedProtectedFilesNum"] = len(pull.ChangedProtectedFiles)
ctx.Data["ShowMergeInstructions"] = pull.ProtectedBranch.CanUserPush(ctx.User.ID)
}
ctx.Data["WillSign"] = false
if ctx.User != nil {
Expand Down
38 changes: 20 additions & 18 deletions templates/repo/issue/view_content/pull.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,25 +485,27 @@
{{end}}
</div>
</div>
<div class="instruct-toggle ml-3"> {{$.i18n.Tr "repo.pulls.merge_instruction_hint" | Safe}} </div>
<div class="instruct-content" style="display:none">
<div class="ui divider"></div>
<div><h3 class="di">{{$.i18n.Tr "step1"}} </h3>{{$.i18n.Tr "repo.pulls.merge_instruction_step1_desc"}}</div>
<div class="ui secondary segment">
{{if eq .Issue.PullRequest.Flow 0}}
<div>git checkout -b {{if ne .Issue.PullRequest.HeadRepo.ID .Issue.PullRequest.BaseRepo.ID}}{{.Issue.PullRequest.HeadRepo.OwnerName}}-{{end}}{{.Issue.PullRequest.HeadBranch}} {{.Issue.PullRequest.BaseBranch}}</div>
<div>git pull {{if ne .Issue.PullRequest.HeadRepo.ID .Issue.PullRequest.BaseRepo.ID}}{{.Issue.PullRequest.HeadRepo.HTMLURL}}{{else}}origin{{end}} {{.Issue.PullRequest.HeadBranch}}</div>
{{else}}
<div>git fetch origin {{.Issue.PullRequest.GetGitRefName}}:{{.Issue.PullRequest.HeadBranch}}</div>
{{end}}
</div>
<div><h3 class="di">{{$.i18n.Tr "step2"}} </h3>{{$.i18n.Tr "repo.pulls.merge_instruction_step2_desc"}}</div>
<div class="ui secondary segment">
<div>git checkout {{.Issue.PullRequest.BaseBranch}}</div>
<div>git merge --no-ff {{if ne .Issue.PullRequest.HeadRepo.ID .Issue.PullRequest.BaseRepo.ID}}{{.Issue.PullRequest.HeadRepo.OwnerName}}-{{end}}{{.Issue.PullRequest.HeadBranch}}</div>
<div>git push origin {{.Issue.PullRequest.BaseBranch}}</div>
{{if .ShowMergeInstructions}}
<div class="instruct-toggle ml-3"> {{$.i18n.Tr "repo.pulls.merge_instruction_hint" | Safe}} </div>
<div class="instruct-content" style="display:none">
<div class="ui divider"></div>
<div><h3 class="di">{{$.i18n.Tr "step1"}} </h3>{{$.i18n.Tr "repo.pulls.merge_instruction_step1_desc"}}</div>
<div class="ui secondary segment">
{{if eq .Issue.PullRequest.Flow 0}}
<div>git checkout -b {{if ne .Issue.PullRequest.HeadRepo.ID .Issue.PullRequest.BaseRepo.ID}}{{.Issue.PullRequest.HeadRepo.OwnerName}}-{{end}}{{.Issue.PullRequest.HeadBranch}} {{.Issue.PullRequest.BaseBranch}}</div>
<div>git pull {{if ne .Issue.PullRequest.HeadRepo.ID .Issue.PullRequest.BaseRepo.ID}}{{.Issue.PullRequest.HeadRepo.HTMLURL}}{{else}}origin{{end}} {{.Issue.PullRequest.HeadBranch}}</div>
{{else}}
<div>git fetch origin {{.Issue.PullRequest.GetGitRefName}}:{{.Issue.PullRequest.HeadBranch}}</div>
{{end}}
</div>
<div><h3 class="di">{{$.i18n.Tr "step2"}} </h3>{{$.i18n.Tr "repo.pulls.merge_instruction_step2_desc"}}</div>
<div class="ui secondary segment">
<div>git checkout {{.Issue.PullRequest.BaseBranch}}</div>
<div>git merge --no-ff {{if ne .Issue.PullRequest.HeadRepo.ID .Issue.PullRequest.BaseRepo.ID}}{{.Issue.PullRequest.HeadRepo.OwnerName}}-{{end}}{{.Issue.PullRequest.HeadBranch}}</div>
<div>git push origin {{.Issue.PullRequest.BaseBranch}}</div>
</div>
</div>
</div>
{{end}}
{{else}}
<div class="ui divider"></div>
<div class="item text red">
Expand Down