From 6d2df728257922cc716fed8a172ed69adc8d46d3 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Sun, 21 Jan 2024 13:22:07 +0100 Subject: [PATCH] [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 (cherry picked from commit 022d0e0d71a92c31302176c5c8ba1e7169bbbf3e) (cherry picked from commit 957990b36a25d0e51d9b75432a577dd63fb6dad2) --- templates/repo/diff/compare.tmpl | 10 ++--- tests/integration/compare_test.go | 64 +++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/templates/repo/diff/compare.tmpl b/templates/repo/diff/compare.tmpl index 15574ad988..c2a114ba4e 100644 --- a/templates/repo/diff/compare.tmpl +++ b/templates/repo/diff/compare.tmpl @@ -67,12 +67,12 @@ {{range .Branches}}
{{$BaseCompareName}}:{{.}}
{{end}} - {{if not .PullRequestCtx.SameRepo}} + {{if and (not .PullRequestCtx.SameRepo) ($.HeadRepo.AllowsPulls ctx)}} {{range .HeadBranches}}
{{$HeadCompareName}}:{{.}}
{{end}} {{end}} - {{if .OwnForkRepo}} + {{if and .OwnForkRepo (.OwnForkRepo.AllowsPulls ctx)}} {{range .OwnForkRepoBranches}}
{{$OwnForkCompareName}}:{{.}}
{{end}} @@ -87,17 +87,17 @@ {{range .Tags}}
{{$BaseCompareName}}:{{.}}
{{end}} - {{if not .PullRequestCtx.SameRepo}} + {{if and (not .PullRequestCtx.SameRepo) ($.HeadRepo.AllowsPulls ctx)}} {{range .HeadTags}}
{{$HeadCompareName}}:{{.}}
{{end}} {{end}} - {{if .OwnForkRepo}} + {{if and .OwnForkRepo (.OwnForkRepo.AllowsPulls ctx)}} {{range .OwnForkRepoTags}}
{{$OwnForkCompareName}}:{{.}}
{{end}} {{end}} - {{if .RootRepo}} + {{if and .RootRepo (.RootRepo.AllowsPulls ctx)}} {{range .RootRepoTags}}
{{$RootRepoCompareName}}:{{.}}
{{end}} diff --git a/tests/integration/compare_test.go b/tests/integration/compare_test.go index 509524ca56..cf0bac4c8a 100644 --- a/tests/integration/compare_test.go +++ b/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) + }) + }) +}