[GITEA] Fix misleading comparisons when comparing branches
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. Refs: https://codeberg.org/forgejo/forgejo/pulls/2194 Signed-off-by: Gergely Nagy <forgejo@gergo.csillger.hu> (cherry picked from commit 022d0e0d71a92c31302176c5c8ba1e7169bbbf3e) (cherry picked from commit 957990b36a25d0e51d9b75432a577dd63fb6dad2) (cherry picked from commit 6d2df728257922cc716fed8a172ed69adc8d46d3)
This commit is contained in:
parent
bc7e448d49
commit
69688628f5
|
@ -67,12 +67,12 @@
|
||||||
{{range .Branches}}
|
{{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>
|
<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}}
|
{{end}}
|
||||||
{{if not .PullRequestCtx.SameRepo}}
|
{{if and (not .PullRequestCtx.SameRepo) ($.HeadRepo.AllowsPulls ctx)}}
|
||||||
{{range .HeadBranches}}
|
{{range .HeadBranches}}
|
||||||
<div class="item" data-url="{{$.HeadRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$HeadCompareName}}:{{.}}</div>
|
<div class="item" data-url="{{$.HeadRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$HeadCompareName}}:{{.}}</div>
|
||||||
{{end}}
|
{{end}}
|
||||||
{{end}}
|
{{end}}
|
||||||
{{if .OwnForkRepo}}
|
{{if and .OwnForkRepo (.OwnForkRepo.AllowsPulls ctx)}}
|
||||||
{{range .OwnForkRepoBranches}}
|
{{range .OwnForkRepoBranches}}
|
||||||
<div class="item" data-url="{{$.OwnForkRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$OwnForkCompareName}}:{{.}}</div>
|
<div class="item" data-url="{{$.OwnForkRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$OwnForkCompareName}}:{{.}}</div>
|
||||||
{{end}}
|
{{end}}
|
||||||
|
@ -87,17 +87,17 @@
|
||||||
{{range .Tags}}
|
{{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>
|
<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}}
|
{{end}}
|
||||||
{{if not .PullRequestCtx.SameRepo}}
|
{{if and (not .PullRequestCtx.SameRepo) ($.HeadRepo.AllowsPulls ctx)}}
|
||||||
{{range .HeadTags}}
|
{{range .HeadTags}}
|
||||||
<div class="item" data-url="{{$.HeadRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$HeadCompareName}}:{{.}}</div>
|
<div class="item" data-url="{{$.HeadRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$HeadCompareName}}:{{.}}</div>
|
||||||
{{end}}
|
{{end}}
|
||||||
{{end}}
|
{{end}}
|
||||||
{{if .OwnForkRepo}}
|
{{if and .OwnForkRepo (.OwnForkRepo.AllowsPulls ctx)}}
|
||||||
{{range .OwnForkRepoTags}}
|
{{range .OwnForkRepoTags}}
|
||||||
<div class="item" data-url="{{$.OwnForkRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$OwnForkCompareName}}:{{.}}</div>
|
<div class="item" data-url="{{$.OwnForkRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$OwnForkCompareName}}:{{.}}</div>
|
||||||
{{end}}
|
{{end}}
|
||||||
{{end}}
|
{{end}}
|
||||||
{{if .RootRepo}}
|
{{if and .RootRepo (.RootRepo.AllowsPulls ctx)}}
|
||||||
{{range .RootRepoTags}}
|
{{range .RootRepoTags}}
|
||||||
<div class="item" data-url="{{$.RootRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$RootRepoCompareName}}:{{.}}</div>
|
<div class="item" data-url="{{$.RootRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$RootRepoCompareName}}:{{.}}</div>
|
||||||
{{end}}
|
{{end}}
|
||||||
|
|
|
@ -1,4 +1,5 @@
|
||||||
// Copyright 2021 The Gitea Authors. All rights reserved.
|
// 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
|
// SPDX-License-Identifier: MIT
|
||||||
|
|
||||||
package integration
|
package integration
|
||||||
|
@ -6,9 +7,14 @@ package integration
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"net/url"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"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"
|
"code.gitea.io/gitea/tests"
|
||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
|
@ -118,3 +124,61 @@ func TestCompareBranches(t *testing.T) {
|
||||||
|
|
||||||
inspectCompare(t, htmlDoc, diffCount, diffChanges)
|
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)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
Loading…
Reference in a new issue