thepaperpilot/forgejo
1
0
Fork 0
Code Issues Pull requests Releases Activity Actions

Fix misleading comparisons when comparing branches

Browse source 
When comparing branches, only offer those branches to use as a base
where the repository allows pull requests. Those that do not allow pull
request would result in a 404, so offering them as an option would be
misleading.

Signed-off-by: Gergely Nagy <forgejo@gergo.csillger.hu>
This commit is contained in:
Gergely Nagy 2024-01-21 13:22:07 +01:00
parent cf93a4a6fc
commit 022d0e0d71
No known key found for this signature in database
2 changed files with 69 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
templates/repo/diff/compare.tmpl
View file

@ -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}}

64
tests/integration/compare_test.go
View file

@ -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)
})
})
}