Allow unauthenticated users to compare (#11240)

* routers: make /compare route available to unauthenticated users

Remove some bits of the compare interface if the user isn't signed in.
Notably, they don't need to see the "New Pull Request" button box nor the
hidden form that would fail to submit due to the POST request continuing to
require proper privileges.

Follow-up commits will improve the UI a bit around this, removing some
"Pull Request" verbiage in favor of "Compare."

* ui: home: show "compare" button for unauthenticated users

This change requires pulling in the BaseRepo unconditionally and
recording if the pull request is in-fact not allowed
(.PullRequestCtx.Allowed). If the user isn't allowed to create a pull
request, either because this isn't a fork or same-fork branch PRs aren't
allowed, then we'll name the button "Compare" instead of "Pull Request."

* ui: branch list: use the new Compare language when available

When viewing the branch listing as an unauthenticated user, you'll get
"Pull Request" buttons. use the new "Compare" verbiage instead, which
matches GitHub behavior when you can't issue a pull request from the
branches.

Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com>

7 files changed, 49 insertions, 41 deletions

diff --git a/modules/context/repo.go b/modules/context/repo.go
index 841dcd960e..5ebed0eb7e 100644
--- a/modules/context/repo.go
+++ b/modules/context/repo.go
@@ -547,23 +547,27 @@ func RepoAssignment() macaron.Handler {
 		ctx.Data["CommitID"] = ctx.Repo.CommitID
 
 		// People who have push access or have forked repository can propose a new pull request.
-		if ctx.Repo.CanWrite(models.UnitTypeCode) || (ctx.IsSigned && ctx.User.HasForkedRepo(ctx.Repo.Repository.ID)) {
-			// Pull request is allowed if this is a fork repository
-			// and base repository accepts pull requests.
-			if repo.BaseRepo != nil && repo.BaseRepo.AllowsPulls() {
-				ctx.Data["BaseRepo"] = repo.BaseRepo
-				ctx.Repo.PullRequest.BaseRepo = repo.BaseRepo
-				ctx.Repo.PullRequest.Allowed = true
-				ctx.Repo.PullRequest.HeadInfo = ctx.Repo.Owner.Name + ":" + ctx.Repo.BranchName
-			} else if repo.AllowsPulls() {
-				// Or, this is repository accepts pull requests between branches.
-				ctx.Data["BaseRepo"] = repo
-				ctx.Repo.PullRequest.BaseRepo = repo
-				ctx.Repo.PullRequest.Allowed = true
-				ctx.Repo.PullRequest.SameRepo = true
-				ctx.Repo.PullRequest.HeadInfo = ctx.Repo.BranchName
-			}
-		}
+		canPush := ctx.Repo.CanWrite(models.UnitTypeCode) || (ctx.IsSigned && ctx.User.HasForkedRepo(ctx.Repo.Repository.ID))
+		canCompare := false
+
+		// Pull request is allowed if this is a fork repository
+		// and base repository accepts pull requests.
+		if repo.BaseRepo != nil && repo.BaseRepo.AllowsPulls() {
+			canCompare = true
+			ctx.Data["BaseRepo"] = repo.BaseRepo
+			ctx.Repo.PullRequest.BaseRepo = repo.BaseRepo
+			ctx.Repo.PullRequest.Allowed = canPush
+			ctx.Repo.PullRequest.HeadInfo = ctx.Repo.Owner.Name + ":" + ctx.Repo.BranchName
+		} else if repo.AllowsPulls() {
+			// Or, this is repository accepts pull requests between branches.
+			canCompare = true
+			ctx.Data["BaseRepo"] = repo
+			ctx.Repo.PullRequest.BaseRepo = repo
+			ctx.Repo.PullRequest.Allowed = canPush
+			ctx.Repo.PullRequest.SameRepo = true
+			ctx.Repo.PullRequest.HeadInfo = ctx.Repo.BranchName
+		}
+		ctx.Data["CanCompareOrPull"] = canCompare
 		ctx.Data["PullRequestCtx"] = ctx.Repo.PullRequest
 
 		if ctx.Query("go-get") == "1" {
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index fe68573529..37efecbff5 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -2270,6 +2270,7 @@ transfer_repo = transferred repository <code>%s</code> to <a href="%s">%s</a>
 push_tag = pushed tag <a href="%s/src/tag/%s">%[2]s</a> to <a href="%[1]s">%[3]s</a>
 delete_tag = deleted tag %[2]s from <a href="%[1]s">%[3]s</a>
 delete_branch = deleted branch %[2]s from <a href="%[1]s">%[3]s</a>
+compare_branch = Compare
 compare_commits = Compare %d commits
 compare_commits_general = Compare commits
 mirror_sync_push = synced commits to <a href="%[1]s/src/%[2]s">%[3]s</a> at <a href="%[1]s">%[4]s</a> from mirror
diff --git a/routers/repo/branch.go b/routers/repo/branch.go
index 615450b368..1664f68ec1 100644
--- a/routers/repo/branch.go
+++ b/routers/repo/branch.go
@@ -46,6 +46,7 @@ func Branches(ctx *context.Context) {
 	ctx.Data["AllowsPulls"] = ctx.Repo.Repository.AllowsPulls()
 	ctx.Data["IsWriter"] = ctx.Repo.CanWrite(models.UnitTypeCode)
 	ctx.Data["IsMirror"] = ctx.Repo.Repository.IsMirror
+	ctx.Data["CanPull"] = ctx.Repo.CanWrite(models.UnitTypeCode) || (ctx.IsSigned && ctx.User.HasForkedRepo(ctx.Repo.Repository.ID))
 	ctx.Data["PageIsViewCode"] = true
 	ctx.Data["PageIsBranches"] = true
 
diff --git a/routers/routes/routes.go b/routers/routes/routes.go
index f3bd42f02a..ac0f3f4f14 100644
--- a/routers/routes/routes.go
+++ b/routers/routes/routes.go
@@ -709,6 +709,9 @@ func RegisterRoutes(m *macaron.Macaron) {
 		m.Group("/milestone", func() {
 			m.Get("/:id", repo.MilestoneIssuesAndPulls)
 		}, reqRepoIssuesOrPullsReader, context.RepoRef())
+		m.Combo("/compare/*", repo.MustBeNotEmpty, reqRepoCodeReader, repo.SetEditorconfigIfExists).
+			Get(repo.SetDiffViewStyle, repo.CompareDiff).
+			Post(reqSignIn, context.RepoMustNotBeArchived(), reqRepoPullsReader, repo.MustAllowPulls, bindIgnErr(auth.CreateIssueForm{}), repo.CompareAndPullRequestPost)
 	}, context.RepoAssignment(), context.UnitTypes())
 
 	// Grouping for those endpoints that do require authentication
@@ -769,9 +772,6 @@ func RegisterRoutes(m *macaron.Macaron) {
 			m.Post("/:id/:action", repo.ChangeMilestonStatus)
 			m.Post("/delete", repo.DeleteMilestone)
 		}, context.RepoMustNotBeArchived(), reqRepoIssuesOrPullsWriter, context.RepoRef())
-		m.Combo("/compare/*", repo.MustBeNotEmpty, reqRepoCodeReader, repo.SetEditorconfigIfExists).
-			Get(repo.SetDiffViewStyle, repo.CompareDiff).
-			Post(context.RepoMustNotBeArchived(), reqRepoPullsReader, repo.MustAllowPulls, bindIgnErr(auth.CreateIssueForm{}), repo.CompareAndPullRequestPost)
 		m.Group("/pull", func() {
 			m.Post("/:index/target_branch", repo.UpdatePullRequestTarget)
 		}, context.RepoMustNotBeArchived())
diff --git a/templates/repo/branch/list.tmpl b/templates/repo/branch/list.tmpl
index b310edfbd3..d2eedef49c 100644
--- a/templates/repo/branch/list.tmpl
+++ b/templates/repo/branch/list.tmpl
@@ -81,13 +81,13 @@
 												</a>
 											{{else if and (not .IsDeleted) $.AllowsPulls (gt .CommitsAhead 0)}}
 											<a href="{{$.RepoLink}}/compare/{{$.DefaultBranch | EscapePound}}...{{if ne $.Repository.Owner.Name $.Owner.Name}}{{$.Owner.Name}}:{{end}}{{.Name | EscapePound}}">
-												<button id="new-pull-request" class="ui compact basic button">{{$.i18n.Tr "repo.pulls.compare_changes"}}</button>
+												<button id="new-pull-request" class="ui compact basic button">{{if $.CanPull}}{{$.i18n.Tr "repo.pulls.compare_changes"}}{{else}}{{$.i18n.Tr "action.compare_branch"}}{{end}}</button>
 											</a>
 											{{end}}
 										{{else if and .LatestPullRequest.HasMerged .MergeMovedOn}}
 											{{if and (not .IsDeleted) $.AllowsPulls (gt .CommitsAhead 0)}}
 											<a href="{{$.RepoLink}}/compare/{{$.DefaultBranch | EscapePound}}...{{if ne $.Repository.Owner.Name $.Owner.Name}}{{$.Owner.Name}}:{{end}}{{.Name | EscapePound}}">
-												<button id="new-pull-request" class="ui compact basic button">{{$.i18n.Tr "repo.pulls.compare_changes"}}</button>
+												<button id="new-pull-request" class="ui compact basic button">{{if $.CanPull}}{{$.i18n.Tr "repo.pulls.compare_changes"}}{{else}}{{$.i18n.Tr "action.compare_branch"}}{{end}}</button>
 											</a>
 											{{end}}
 										{{else}}
diff --git a/templates/repo/diff/compare.tmpl b/templates/repo/diff/compare.tmpl
index b088636434..35a37ab761 100644
--- a/templates/repo/diff/compare.tmpl
+++ b/templates/repo/diff/compare.tmpl
@@ -5,7 +5,7 @@
 
 	{{if .PageIsComparePull}}
 		<h2 class="ui header">
-			{{if not .Repository.IsArchived}}
+			{{if and $.IsSigned (not .Repository.IsArchived)}}
 				{{.i18n.Tr "repo.pulls.compare_changes"}}
 				<div class="sub header">{{.i18n.Tr "repo.pulls.compare_changes_desc"}}</div>
 			{{ else }}
@@ -64,22 +64,24 @@
         	<div class="ui segment">
         		{{.i18n.Tr "repo.pulls.has_pull_request" $.RepoLink $.RepoRelPath .PullRequest.Index | Safe}}
         	</div>
-        {{else}}
-        	{{if not .Repository.IsArchived}}
-        	<div class="ui info message show-form-container">
-        		<button class="ui button green show-form">{{.i18n.Tr "repo.pulls.new"}}</button>
-        	</div>
-        	{{ else }}
-        		<div class="ui warning message">
-        			{{.i18n.Tr "repo.archive.title"}}
-        		</div>
-        	{{ end }}
-        	<div class="pullrequest-form" style="display: none">
-        		{{template "repo/issue/new_form" .}}
-        	</div>
-        	{{template "repo/commits_table" .}}
-        	{{template "repo/diff/box" .}}
-        {{end}}
+		{{else}}
+			{{if and $.IsSigned (not .Repository.IsArchived)}}
+				<div class="ui info message show-form-container">
+					<button class="ui button green show-form">{{.i18n.Tr "repo.pulls.new"}}</button>
+				</div>
+			{{else if .Repository.IsArchived}}
+				<div class="ui warning message">
+					{{.i18n.Tr "repo.archive.title"}}
+				</div>
+			{{end}}
+			{{if $.IsSigned}}
+				<div class="pullrequest-form" style="display: none">
+					{{template "repo/issue/new_form" .}}
+				</div>
+			{{end}}
+			{{template "repo/commits_table" .}}
+			{{template "repo/diff/box" .}}
+		{{end}}
 	{{else}}
 		{{template "repo/commits_table" .}}
 		{{template "repo/diff/box" .}}
diff --git a/templates/repo/home.tmpl b/templates/repo/home.tmpl
index 51e924747c..3a0d364f34 100644
--- a/templates/repo/home.tmpl
+++ b/templates/repo/home.tmpl
@@ -62,10 +62,10 @@
 			{{ $l := Subtract $n 1}}
 			<!-- If home page, show new PR. If not, show breadcrumb -->
 			{{if eq $n 0}}
-				{{if and .PullRequestCtx.Allowed .IsViewBranch (not .Repository.IsArchived)}}
+				{{if and .CanCompareOrPull .IsViewBranch (not .Repository.IsArchived)}}
 					<div class="fitted item">
 						<a href="{{.BaseRepo.Link}}/compare/{{.BaseRepo.DefaultBranch | EscapePound}}...{{if ne .Repository.Owner.Name .BaseRepo.Owner.Name}}{{.Repository.Owner.Name}}:{{end}}{{.BranchName | EscapePound}}">
-							<button id="new-pull-request" class="ui compact basic button">{{.i18n.Tr "repo.pulls.compare_changes"}}</button>
+							<button id="new-pull-request" class="ui compact basic button">{{if .PullRequestCtx.Allowed}}{{.i18n.Tr "repo.pulls.compare_changes"}}{{else}}{{.i18n.Tr "action.compare_branch"}}{{end}}</button>
 						</a>
 					</div>
 				{{end}}