From 779198404020e8f501ef3709d23e5dc8534f0ae0 Mon Sep 17 00:00:00 2001 From: Robert Wolff Date: Wed, 11 Feb 2026 18:12:29 +0100 Subject: [PATCH] fix: correct Reviewed-on URL in merge message for subpath deployments (#11240) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: #11238 ### Tests - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. *The decision if the pull request will be shown in the release notes is up to the mergers / release team.* The content of the `release-notes/.md` file will serve as the basis for the release notes. If the file does not exist, the title of the pull request will be used instead. Co-authored-by: Diego Díez Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11240 Reviewed-by: Mathieu Fenniak Co-authored-by: Robert Wolff Co-committed-by: Robert Wolff --- services/pull/merge.go | 7 +------ services/pull/pull_test.go | 7 +++++++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index 1bcd11406c..1d5e82e969 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -7,7 +7,6 @@ package pull import ( "context" "fmt" - "net/url" "os" "path/filepath" "regexp" @@ -80,11 +79,7 @@ func getMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr *issue issueReference = "!" } - issueURL, err := url.JoinPath(setting.AppURL, pr.Issue.Link()) - if err != nil { - return "", "", err - } - reviewedOn := fmt.Sprintf("Reviewed-on: %s", issueURL) + reviewedOn := fmt.Sprintf("Reviewed-on: %s", pr.Issue.HTMLURL()) reviewedBy := pr.GetApprovers(ctx) body = fmt.Sprintf("%s\n%s", reviewedOn, reviewedBy) diff --git a/services/pull/pull_test.go b/services/pull/pull_test.go index bf5c56eda1..1b8b95c1dc 100644 --- a/services/pull/pull_test.go +++ b/services/pull/pull_test.go @@ -14,6 +14,7 @@ import ( "forgejo.org/models/unittest" "forgejo.org/modules/git" "forgejo.org/modules/gitrepo" + "forgejo.org/modules/setting" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -57,6 +58,12 @@ func TestPullRequest_GetDefaultMergeMessage_InternalTracker(t *testing.T) { mergeMessage, _, err = GetDefaultMergeMessage(db.DefaultContext, gitRepo, pr, "") require.NoError(t, err) assert.Equal(t, "Merge pull request 'issue3' (#3) from user2/repo1:branch2 into master", mergeMessage) + + setting.AppURL = "https://example.org/suburl/" + setting.AppSubURL = "/suburl" + _, body, err = GetDefaultMergeMessage(db.DefaultContext, gitRepo, pr, "") + require.NoError(t, err) + assert.Equal(t, "Reviewed-on: https://example.org/suburl/user2/repo1/pulls/3\n", body) } func TestPullRequest_GetDefaultMergeMessage_ExternalTracker(t *testing.T) {