Merge pull request 'Fix misleading comparisons when comparing branches' (#2194) from algernon/forgejo:b/diff-compare/avoid-404s into forgejo-dependency

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2194
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>

templates/repo/diff/compare.tmpl

@@ -67,12 +67,12 @@
 					{{range .Branches}}
 						<div class="item {{if eq $.BaseBranch .}}selected{{end}}" data-url="{{$.RepoLink}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{if not $.PullRequestCtx.SameRepo}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{end}}{{PathEscapeSegments $.HeadBranch}}">{{$BaseCompareName}}:{{.}}</div>
 					{{end}}
-					{{if not .PullRequestCtx.SameRepo}}
+					{{if and (not .PullRequestCtx.SameRepo) ($.HeadRepo.AllowsPulls ctx)}}
 						{{range .HeadBranches}}
 							<div class="item" data-url="{{$.HeadRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$HeadCompareName}}:{{.}}</div>
 						{{end}}
 					{{end}}
-					{{if .OwnForkRepo}}
+					{{if and .OwnForkRepo (.OwnForkRepo.AllowsPulls ctx)}}
 						{{range .OwnForkRepoBranches}}
 							<div class="item" data-url="{{$.OwnForkRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$OwnForkCompareName}}:{{.}}</div>
 						{{end}}
@@ -87,17 +87,17 @@
 					{{range .Tags}}
 						<div class="item {{if eq $.BaseBranch .}}selected{{end}}" data-url="{{$.RepoLink}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{if not $.PullRequestCtx.SameRepo}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{end}}{{PathEscapeSegments $.HeadBranch}}">{{$BaseCompareName}}:{{.}}</div>
 					{{end}}
-					{{if not .PullRequestCtx.SameRepo}}
+					{{if and (not .PullRequestCtx.SameRepo) ($.HeadRepo.AllowsPulls ctx)}}
 						{{range .HeadTags}}
 							<div class="item" data-url="{{$.HeadRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$HeadCompareName}}:{{.}}</div>
 						{{end}}
 					{{end}}
-					{{if .OwnForkRepo}}
+					{{if and .OwnForkRepo (.OwnForkRepo.AllowsPulls ctx)}}
 						{{range .OwnForkRepoTags}}
 							<div class="item" data-url="{{$.OwnForkRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$OwnForkCompareName}}:{{.}}</div>
 						{{end}}
 					{{end}}
-					{{if .RootRepo}}
+					{{if and .RootRepo (.RootRepo.AllowsPulls ctx)}}
 						{{range .RootRepoTags}}
 							<div class="item" data-url="{{$.RootRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$RootRepoCompareName}}:{{.}}</div>
 						{{end}}

tests/integration/compare_test.go

@@ -1,4 +1,5 @@
 // Copyright 2021 The Gitea Authors. All rights reserved.
+// Copyright 2024 The Forgejo Authors c/o Codeberg e.V.. All rights reserved.
 // SPDX-License-Identifier: MIT
 
 package integration
@@ -6,9 +7,14 @@ package integration
 import (
 	"fmt"
 	"net/http"
+	"net/url"
 	"strings"
 	"testing"
 
+	"code.gitea.io/gitea/models/db"
+	repo_model "code.gitea.io/gitea/models/repo"
+	unit_model "code.gitea.io/gitea/models/unit"
+	repo_service "code.gitea.io/gitea/services/repository"
 	"code.gitea.io/gitea/tests"
 
 	"github.com/stretchr/testify/assert"
@@ -118,3 +124,61 @@ func TestCompareBranches(t *testing.T) {
 
 	inspectCompare(t, htmlDoc, diffCount, diffChanges)
 }
+
+func TestCompareWithPRsDisabled(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, u *url.URL) {
+		session := loginUser(t, "user1")
+		testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
+		testCreateBranch(t, session, "user1", "repo1", "branch/master", "recent-push", http.StatusSeeOther)
+		testEditFile(t, session, "user1", "repo1", "recent-push", "README.md", "Hello recently!\n")
+
+		repo, err := repo_model.GetRepositoryByOwnerAndName(db.DefaultContext, "user1", "repo1")
+		assert.NoError(t, err)
+
+		defer func() {
+			// Reenable PRs on the repo
+			err := repo_service.UpdateRepositoryUnits(db.DefaultContext, repo,
+				[]repo_model.RepoUnit{{
+					RepoID: repo.ID,
+					Type:   unit_model.TypePullRequests,
+				}},
+				nil)
+			assert.NoError(t, err)
+		}()
+
+		// Disable PRs on the repo
+		err = repo_service.UpdateRepositoryUnits(db.DefaultContext, repo, nil,
+			[]unit_model.Type{unit_model.TypePullRequests})
+		assert.NoError(t, err)
+
+		t.Run("branch view doesn't offer creating PRs", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			req := NewRequest(t, "GET", "/user1/repo1/branches")
+			resp := session.MakeRequest(t, req, http.StatusOK)
+			htmlDoc := NewHTMLParser(t, resp.Body)
+			htmlDoc.AssertElement(t, "a[href='/user1/repo1/compare/master...recent-push']", false)
+		})
+
+		t.Run("compare doesn't offer local branches", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			req := NewRequest(t, "GET", "/user2/repo1/compare/master...user1/repo1:recent-push")
+			resp := session.MakeRequest(t, req, http.StatusOK)
+			htmlDoc := NewHTMLParser(t, resp.Body)
+			branches := htmlDoc.Find(".choose.branch .menu .reference-list-menu.base-branch-list .item, .choose.branch .menu .reference-list-menu.base-tag-list .item")
+
+			expectedPrefix := "user2:"
+			for i := 0; i < len(branches.Nodes); i++ {
+				assert.True(t, strings.HasPrefix(branches.Eq(i).Text(), expectedPrefix))
+			}
+		})
+
+		t.Run("comparing against a disabled-PR repo is 404", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			req := NewRequest(t, "GET", "/user1/repo1/compare/master...recent-push")
+			session.MakeRequest(t, req, http.StatusNotFound)
+		})
+	})
+}