From c3034c40f19620cba166f9657a07dd512d8bd90d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 5 Sep 2025 16:09:17 -0700 Subject: [PATCH 1/6] Fix bug when issue disabled, pull request number in the commit message cannot be redirected --- routers/web/web.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/web.go b/routers/web/web.go index 6b649dc1f56b2..77d884de16296 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1227,7 +1227,7 @@ func registerWebRoutes(m *web.Router) { m.Group("/{username}/{reponame}/{type:issues}", func() { m.Get("", repo.Issues) m.Get("/{index}", repo.ViewIssue) - }, optSignIn, context.RepoAssignment, context.RequireUnitReader(unit.TypeIssues, unit.TypeExternalTracker)) + }, optSignIn, context.RepoAssignment, context.RequireUnitReader(unit.TypeIssues, unit.TypePullRequests, unit.TypeExternalTracker)) // end "/{username}/{reponame}": issue/pull list, issue/pull view, external tracker m.Group("/{username}/{reponame}", func() { // edit issues, pulls, labels, milestones, etc From 50ee6a9aecc8918dc74158d138ac7aff4e84fda7 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 6 Sep 2025 11:47:47 +0800 Subject: [PATCH 2/6] update --- routers/web/web.go | 5 +++-- tests/integration/issue_test.go | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/routers/web/web.go b/routers/web/web.go index 77d884de16296..ed7339a2291b1 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1225,10 +1225,11 @@ func registerWebRoutes(m *web.Router) { // end "/{username}/{reponame}": view milestone, label, issue, pull, etc m.Group("/{username}/{reponame}/{type:issues}", func() { + // these handlers also check unit permissions internally m.Get("", repo.Issues) - m.Get("/{index}", repo.ViewIssue) + m.Get("/{index}", repo.ViewIssue) // also do pull-request redirection (".../issues/{PR-number}" -> ".../pulls/{PR-number}") }, optSignIn, context.RepoAssignment, context.RequireUnitReader(unit.TypeIssues, unit.TypePullRequests, unit.TypeExternalTracker)) - // end "/{username}/{reponame}": issue/pull list, issue/pull view, external tracker + // end "/{username}/{reponame}": issue list, issue view, external tracker m.Group("/{username}/{reponame}", func() { // edit issues, pulls, labels, milestones, etc m.Group("/issues", func() { diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index 3b5f1534efdb1..551f4a3704809 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -487,6 +487,8 @@ func TestIssueRedirect(t *testing.T) { req = NewRequest(t, "GET", path.Join("org26", "repo_external_tracker_alpha", "issues", "1")) resp = session.MakeRequest(t, req, http.StatusSeeOther) assert.Equal(t, "/"+path.Join("org26", "repo_external_tracker_alpha", "pulls", "1"), test.RedirectURL(resp)) + + // FIXME: add a test to check that the PR redirection works if the issue unit is disabled } func TestSearchIssues(t *testing.T) { From d2d35db9cdf509bef3eec3613b1c777a5b277108 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 6 Sep 2025 11:49:29 +0800 Subject: [PATCH 3/6] update --- routers/web/web.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/web.go b/routers/web/web.go index ed7339a2291b1..98cdf8b964128 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1229,7 +1229,7 @@ func registerWebRoutes(m *web.Router) { m.Get("", repo.Issues) m.Get("/{index}", repo.ViewIssue) // also do pull-request redirection (".../issues/{PR-number}" -> ".../pulls/{PR-number}") }, optSignIn, context.RepoAssignment, context.RequireUnitReader(unit.TypeIssues, unit.TypePullRequests, unit.TypeExternalTracker)) - // end "/{username}/{reponame}": issue list, issue view, external tracker + // end "/{username}/{reponame}": issue list, issue view (pull-request redirection), external tracker m.Group("/{username}/{reponame}", func() { // edit issues, pulls, labels, milestones, etc m.Group("/issues", func() { From 3b21acf0b6803340b5ba0cf26d4cd5345161cd5d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 6 Sep 2025 22:25:24 -0700 Subject: [PATCH 4/6] Add test --- tests/integration/issue_test.go | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index 551f4a3704809..2f412db1d3ef2 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -14,8 +14,10 @@ import ( "testing" "time" + "code.gitea.io/gitea/models/db" issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/indexer/issues" @@ -24,6 +26,7 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/tests" + "xorm.io/builder" "github.com/PuerkitoBio/goquery" "github.com/stretchr/testify/assert" @@ -488,7 +491,35 @@ func TestIssueRedirect(t *testing.T) { resp = session.MakeRequest(t, req, http.StatusSeeOther) assert.Equal(t, "/"+path.Join("org26", "repo_external_tracker_alpha", "pulls", "1"), test.RedirectURL(resp)) - // FIXME: add a test to check that the PR redirection works if the issue unit is disabled + // test to check that the PR redirection works if the issue unit is disabled + // repo1 is a normal repository with issue unit enabled, visit issue 2(which is a pull request) + // will redirect to pulls + req = NewRequest(t, "GET", path.Join("user2", "repo1", "issues", "2")) + resp = session.MakeRequest(t, req, http.StatusSeeOther) + assert.Equal(t, "/"+path.Join("user2", "repo1", "pulls", "2"), test.RedirectURL(resp)) + + // disable issue unit + repoUnit, exist, err := db.Get[repo_model.RepoUnit](t.Context(), builder.Eq{ + "repo_id": 1, + "type": unit.TypeIssues, + }) + assert.NoError(t, err) + assert.True(t, exist) + assert.NotNil(t, repoUnit) + + _, err = db.DeleteByID[repo_model.RepoUnit](t.Context(), repoUnit.ID) + assert.NoError(t, err) + + defer func() { + repoUnit.ID = 0 + assert.NoError(t, db.Insert(t.Context(), repoUnit)) + }() + + // even if the issue unit is disabled, visiting an issue which is a pull request + // will still redirect to pull request + req = NewRequest(t, "GET", path.Join("user2", "repo1", "issues", "2")) + resp = session.MakeRequest(t, req, http.StatusSeeOther) + assert.Equal(t, "/"+path.Join("user2", "repo1", "pulls", "2"), test.RedirectURL(resp)) } func TestSearchIssues(t *testing.T) { From 1f0e11b22543c25fed69e014d44116787ba33e6b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 7 Sep 2025 11:00:33 -0700 Subject: [PATCH 5/6] Fix test --- tests/integration/issue_test.go | 36 +++++++++++++-------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index 2f412db1d3ef2..4a6d1c1acd247 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -26,7 +26,6 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/tests" - "xorm.io/builder" "github.com/PuerkitoBio/goquery" "github.com/stretchr/testify/assert" @@ -477,49 +476,42 @@ func TestIssueRedirect(t *testing.T) { session := loginUser(t, "user2") // Test external tracker where style not set (shall default numeric) - req := NewRequest(t, "GET", path.Join("org26", "repo_external_tracker", "issues", "1")) + req := NewRequest(t, "GET", "/org26/repo_external_tracker/issues/1") resp := session.MakeRequest(t, req, http.StatusSeeOther) assert.Equal(t, "https://tracker.com/org26/repo_external_tracker/issues/1", test.RedirectURL(resp)) // Test external tracker with numeric style - req = NewRequest(t, "GET", path.Join("org26", "repo_external_tracker_numeric", "issues", "1")) + req = NewRequest(t, "GET", "/org26/repo_external_tracker_numeric/issues/1") resp = session.MakeRequest(t, req, http.StatusSeeOther) assert.Equal(t, "https://tracker.com/org26/repo_external_tracker_numeric/issues/1", test.RedirectURL(resp)) // Test external tracker with alphanumeric style (for a pull request) - req = NewRequest(t, "GET", path.Join("org26", "repo_external_tracker_alpha", "issues", "1")) + req = NewRequest(t, "GET", "/org26/repo_external_tracker_alpha/issues/1") resp = session.MakeRequest(t, req, http.StatusSeeOther) - assert.Equal(t, "/"+path.Join("org26", "repo_external_tracker_alpha", "pulls", "1"), test.RedirectURL(resp)) + assert.Equal(t, "/org26/repo_external_tracker_alpha/pulls/1", test.RedirectURL(resp)) // test to check that the PR redirection works if the issue unit is disabled // repo1 is a normal repository with issue unit enabled, visit issue 2(which is a pull request) // will redirect to pulls - req = NewRequest(t, "GET", path.Join("user2", "repo1", "issues", "2")) + req = NewRequest(t, "GET", "/user2/repo1/issues/2") resp = session.MakeRequest(t, req, http.StatusSeeOther) - assert.Equal(t, "/"+path.Join("user2", "repo1", "pulls", "2"), test.RedirectURL(resp)) + assert.Equal(t, "/user2/repo1/pulls/2", test.RedirectURL(resp)) // disable issue unit - repoUnit, exist, err := db.Get[repo_model.RepoUnit](t.Context(), builder.Eq{ - "repo_id": 1, - "type": unit.TypeIssues, - }) - assert.NoError(t, err) - assert.True(t, exist) - assert.NotNil(t, repoUnit) + repoUnit := unittest.AssertExistsAndLoadBean(t, &repo_model.RepoUnit{RepoID: 1, Type: unit.TypeIssues}) - _, err = db.DeleteByID[repo_model.RepoUnit](t.Context(), repoUnit.ID) + _, err := db.DeleteByID[repo_model.RepoUnit](t.Context(), repoUnit.ID) assert.NoError(t, err) - defer func() { - repoUnit.ID = 0 - assert.NoError(t, db.Insert(t.Context(), repoUnit)) - }() - // even if the issue unit is disabled, visiting an issue which is a pull request // will still redirect to pull request - req = NewRequest(t, "GET", path.Join("user2", "repo1", "issues", "2")) + req = NewRequest(t, "GET", "/user2/repo1/issues/2") resp = session.MakeRequest(t, req, http.StatusSeeOther) - assert.Equal(t, "/"+path.Join("user2", "repo1", "pulls", "2"), test.RedirectURL(resp)) + assert.Equal(t, "/user2/repo1/pulls/2", test.RedirectURL(resp)) + + // cleanup, re-enable issue unit + repoUnit.ID = 0 + assert.NoError(t, db.Insert(t.Context(), repoUnit)) } func TestSearchIssues(t *testing.T) { From beb7e50f80ee874e3c02a441c5e669a598b5d3cf Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 7 Sep 2025 14:56:15 -0700 Subject: [PATCH 6/6] Remove unused code --- tests/integration/issue_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index 4a6d1c1acd247..613353e55cbb9 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -497,9 +497,9 @@ func TestIssueRedirect(t *testing.T) { resp = session.MakeRequest(t, req, http.StatusSeeOther) assert.Equal(t, "/user2/repo1/pulls/2", test.RedirectURL(resp)) - // disable issue unit repoUnit := unittest.AssertExistsAndLoadBean(t, &repo_model.RepoUnit{RepoID: 1, Type: unit.TypeIssues}) + // disable issue unit, it will be reset _, err := db.DeleteByID[repo_model.RepoUnit](t.Context(), repoUnit.ID) assert.NoError(t, err) @@ -508,10 +508,6 @@ func TestIssueRedirect(t *testing.T) { req = NewRequest(t, "GET", "/user2/repo1/issues/2") resp = session.MakeRequest(t, req, http.StatusSeeOther) assert.Equal(t, "/user2/repo1/pulls/2", test.RedirectURL(resp)) - - // cleanup, re-enable issue unit - repoUnit.ID = 0 - assert.NoError(t, db.Insert(t.Context(), repoUnit)) } func TestSearchIssues(t *testing.T) {