fix: allow Actions trust management on conflicted PRs (#10594)
Some checks are pending
/ release (push) Waiting to run
testing-integration / test-unit (push) Waiting to run
testing-integration / test-sqlite (push) Waiting to run
testing-integration / test-mariadb (v10.6) (push) Waiting to run
testing-integration / test-mariadb (v11.8) (push) Waiting to run
testing / backend-checks (push) Waiting to run
testing / frontend-checks (push) Waiting to run
testing / test-unit (push) Blocked by required conditions
testing / test-e2e (push) Blocked by required conditions
testing / test-remote-cacher (redis) (push) Blocked by required conditions
testing / test-remote-cacher (valkey) (push) Blocked by required conditions
testing / test-remote-cacher (garnet) (push) Blocked by required conditions
testing / test-remote-cacher (redict) (push) Blocked by required conditions
testing / test-mysql (push) Blocked by required conditions
testing / test-pgsql (push) Blocked by required conditions
testing / test-sqlite (push) Blocked by required conditions
testing / security-check (push) Blocked by required conditions

Fixes #10589.

## Checklist

The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).

### Tests

- I added test coverage for Go changes...
  - [ ] in their respective `*_test.go` for unit tests.
  - [x] 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

- [ ] I do not want this change to show in the release notes.
- [x] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10594
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
This commit is contained in:
Mathieu Fenniak 2025-12-27 15:57:25 +01:00 committed by Mathieu Fenniak
parent c53ea1ba2f
commit e86f5ef7d4
3 changed files with 96 additions and 4 deletions

View file

@ -77,6 +77,7 @@
<li>{{.}}</li>
{{end}}
</ul>
{{template "repo/pulls/trust" .}}
{{else if .IsPullRequestBroken}}
<div class="item">
{{svg "octicon-x"}}

View file

@ -20,6 +20,7 @@ import (
actions_module "forgejo.org/modules/actions"
"forgejo.org/modules/git"
"forgejo.org/modules/structs"
"forgejo.org/modules/translation"
actions_service "forgejo.org/services/actions"
pull_service "forgejo.org/services/pull"
repo_service "forgejo.org/services/repository"
@ -68,6 +69,32 @@ func actionsTrustTestAssertNoTrustPanel(t *testing.T, session *TestSession, url
actionsTrustTestAssertTrustPanelPresence(t, session, url, false)
}
func actionsTrustTestAssertPRIsWIP(t *testing.T, session *TestSession, url string) {
t.Helper()
req := NewRequest(t, "GET", url)
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
locale := translation.NewLocale("en-US")
assert.Equal(t, 1, htmlDoc.FindByTextTrim("div", locale.TrString("repo.pulls.cannot_merge_work_in_progress")).Length())
}
func actionsTrustTestAssertPRConflicted(t *testing.T, session *TestSession, url string) {
t.Helper()
req := NewRequest(t, "GET", url)
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
locale := translation.NewLocale("en-US")
// ....Eventually is used because conflict checking is async and may not complete immediately.
require.Eventually(t, func() bool {
return htmlDoc.FindByTextTrim("div", locale.TrString("repo.pulls.files_conflicted")).Length() == 1
}, 5*time.Second, time.Millisecond*100)
}
func actionsTrustTestCreateBaseRepo(t *testing.T, owner *user_model.User) (*repo_model.Repository, func()) {
t.Helper()
@ -127,13 +154,19 @@ func actionsTrustTestRequireRun(t *testing.T, repo *repo_model.Repository, modif
func actionsTrustTestRepoCreateBranch(t *testing.T, doer *user_model.User, repo *repo_model.Repository) *structs.FilesResponse {
t.Helper()
return actionsTrustTestModifyRepo(t, doer, repo, "file_in_fork.txt", "main", "fork-branch-1")
return actionsTrustTestModifyRepo(t, doer, repo, "file_in_fork.txt", "main", "fork-branch-1", "content")
}
func actionsTrustMakePRConflicted(t *testing.T, doer *user_model.User, repo *repo_model.Repository) *structs.FilesResponse {
t.Helper()
return actionsTrustTestModifyRepo(t, doer, repo, "file_in_fork.txt", "main", "main", "conflicting content")
}
func actionsTrustTestRepoModify(t *testing.T, doer *user_model.User, baseRepo, headRepo *repo_model.Repository, filename string) *structs.FilesResponse {
t.Helper()
modified := actionsTrustTestModifyRepo(t, doer, headRepo, filename, "fork-branch-1", "fork-branch-1")
modified := actionsTrustTestModifyRepo(t, doer, headRepo, filename, "fork-branch-1", "fork-branch-1", "content")
// the creation of the run is not synchronous
require.Eventually(t, func() bool {
return unittest.BeanExists(t, &actions_model.ActionRun{RepoID: baseRepo.ID, CommitSHA: modified.Commit.SHA})
@ -141,7 +174,7 @@ func actionsTrustTestRepoModify(t *testing.T, doer *user_model.User, baseRepo, h
return modified
}
func actionsTrustTestModifyRepo(t *testing.T, doer *user_model.User, repo *repo_model.Repository, filename, oldBranch, newBranch string) *structs.FilesResponse {
func actionsTrustTestModifyRepo(t *testing.T, doer *user_model.User, repo *repo_model.Repository, filename, oldBranch, newBranch, content string) *structs.FilesResponse {
t.Helper()
// add a new file to the forked repo
@ -150,7 +183,7 @@ func actionsTrustTestModifyRepo(t *testing.T, doer *user_model.User, repo *repo_
{
Operation: "create",
TreePath: filename,
ContentReader: strings.NewReader("content"),
ContentReader: strings.NewReader(content),
},
},
Message: "add " + filename,
@ -220,6 +253,23 @@ func actionsTrustTestCreatePullRequestFromForkedRepo(t *testing.T, baseUser *use
return forkedRepo, pullRequest, addFileToForkedResp
}
// Mark the PR as a work-in-progress PR
func actionsTrustTestSetPullRequestWIP(t *testing.T, pullRequest *issues_model.PullRequest, wip bool) {
t.Helper()
newTitle := pullRequest.Issue.Title
if wip && !pullRequest.IsWorkInProgress(t.Context()) {
newTitle = fmt.Sprintf("WIP: %s", pullRequest.Issue.Title)
} else if !wip {
prefix := pullRequest.GetWorkInProgressPrefix(t.Context())
newTitle = pullRequest.Issue.Title[len(prefix):]
}
pullRequest.Issue.Title = newTitle
require.NoError(t, issues_model.UpdateIssueCols(t.Context(), pullRequest.Issue, "name"))
pullRequest.Issue = nil
require.NoError(t, pullRequest.LoadIssue(t.Context()))
}
func TestActionsPullRequestTrustPanel(t *testing.T) {
onApplicationRun(t, func(t *testing.T, u *url.URL) {
ownerUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // owner of the repo
@ -353,3 +403,36 @@ func TestActionsPullRequestTrustPanel(t *testing.T) {
})
})
}
func TestActionsPullRequestTrustPanelWIPConflicts(t *testing.T) {
onApplicationRun(t, func(t *testing.T, u *url.URL) {
ownerUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // owner of the repo
regularUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) // a regular user with no specific permission
regularSession := loginUser(t, regularUser.Name)
userAdmin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) // the instance admin
adminSession := loginUser(t, userAdmin.Name)
baseRepo, f := actionsTrustTestCreateBaseRepo(t, ownerUser)
defer f()
_, pullRequest, _ := actionsTrustTestCreatePullRequestFromForkedRepo(t, ownerUser, baseRepo, regularUser)
pullRequestLink := pullRequest.Issue.Link()
actionsTrustTestSetPullRequestWIP(t, pullRequest, true)
actionsTrustTestAssertPRIsWIP(t, adminSession, pullRequestLink)
t.Run("Regular user sees pending approval even though PR is a WIP PR", func(t *testing.T) {
actionsTrustTestAssertTrustPanel(t, regularSession, pullRequestLink)
})
actionsTrustTestSetPullRequestWIP(t, pullRequest, false)
_ = actionsTrustMakePRConflicted(t, userAdmin, baseRepo)
actionsTrustTestAssertPRConflicted(t, adminSession, pullRequestLink)
t.Run("Regular user sees pending approval even though PR is conflicted", func(t *testing.T) {
actionsTrustTestAssertTrustPanel(t, regularSession, pullRequestLink)
})
})
}

View file

@ -6,6 +6,7 @@ package integration
import (
"bytes"
"fmt"
"strings"
"testing"
"github.com/PuerkitoBio/goquery"
@ -108,6 +109,13 @@ func (doc *HTMLDoc) FindByText(selector, text string) *goquery.Selection {
})
}
// FindByText gets all elements by selector that also has the given text, w/ leading & trailing whitespace trimmed
func (doc *HTMLDoc) FindByTextTrim(selector, text string) *goquery.Selection {
return doc.doc.Find(selector).FilterFunction(func(i int, s *goquery.Selection) bool {
return strings.TrimSpace(s.Text()) == text
})
}
// AssertSelection check if selection exists or does not exist depending on checkExists
func (doc *HTMLDoc) AssertSelection(t testing.TB, selection *goquery.Selection, checkExists bool) {
if checkExists {