diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json
index b1c98e4551..4f1c3904e2 100644
--- a/options/locale_next/locale_en-US.json
+++ b/options/locale_next/locale_en-US.json
@@ -92,7 +92,7 @@
"mail.actions.not_successful_run": "Workflow %[1]s failed in repository %[2]s",
"mail.actions.run_info_cur_status": "This Run's Status: %[1]s (just updated from %[2]s)",
"mail.actions.run_info_previous_status": "Previous Run's Status: %[1]s",
- "mail.actions.run_info_ref": "Branch: %[1]s (%[2]s)",
+ "mail.actions.run_info_sha": "Commit: %[1]s",
"mail.actions.run_info_trigger": "Triggered because: %[1]s by: %[2]s",
"repo.diff.commit.next-short": "Next",
"repo.diff.commit.previous-short": "Prev",
diff --git a/services/agit/agit.go b/services/agit/agit.go
index 20e87642c3..8ef641629a 100644
--- a/services/agit/agit.go
+++ b/services/agit/agit.go
@@ -221,7 +221,7 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
}
// Validate pull request.
- pull_service.ValidatePullRequest(ctx, pr, oldCommitID, opts.NewCommitIDs[i], pusher)
+ pull_service.ValidatePullRequest(ctx, pr, opts.NewCommitIDs[i], oldCommitID, pusher)
// TODO: call `InvalidateCodeComments`
diff --git a/services/mailer/mail_actions.go b/services/mailer/mail_actions.go
index fa0d2635f1..a99af823b3 100644
--- a/services/mailer/mail_actions.go
+++ b/services/mailer/mail_actions.go
@@ -60,7 +60,6 @@ func sendMailActionRun(to *user_model.User, run *actions_model.ActionRun, priorS
if len(commitSHA) > 7 {
commitSHA = commitSHA[:7]
}
- branch := run.PrettyRef()
data := map[string]any{
"locale": locale,
@@ -73,7 +72,6 @@ func sendMailActionRun(to *user_model.User, run *actions_model.ActionRun, priorS
"LastRun": lastRun,
"PriorStatus": priorStatus,
"CommitSHA": commitSHA,
- "Branch": branch,
"IsSuccess": run.Status.IsSuccess(),
}
diff --git a/services/mailer/mail_actions_now_done_test.go b/services/mailer/mail_actions_now_done_test.go
index f4c597c99c..e84441f460 100644
--- a/services/mailer/mail_actions_now_done_test.go
+++ b/services/mailer/mail_actions_now_done_test.go
@@ -54,7 +54,7 @@ func getActionsNowDoneTestOrg(t *testing.T, name, email string, owner *user_mode
}
func assertTranslatedLocaleMailActionsNowDone(t *testing.T, msgBody string) {
- AssertTranslatedLocale(t, msgBody, "mail.actions.successful_run_after_failure", "mail.actions.not_successful_run", "mail.actions.run_info_cur_status", "mail.actions.run_info_ref", "mail.actions.run_info_previous_status", "mail.actions.run_info_trigger", "mail.view_it_on")
+ AssertTranslatedLocale(t, msgBody, "mail.actions.successful_run_after_failure", "mail.actions.not_successful_run", "mail.actions.run_info_cur_status", "mail.actions.run_info_sha", "mail.actions.run_info_previous_status", "mail.actions.run_info_trigger", "mail.view_it_on")
}
func TestActionRunNowDoneStatusMatrix(t *testing.T) {
diff --git a/templates/mail/actions/now_done.tmpl b/templates/mail/actions/now_done.tmpl
index a890411055..adc990c545 100644
--- a/templates/mail/actions/now_done.tmpl
+++ b/templates/mail/actions/now_done.tmpl
@@ -21,7 +21,7 @@
{{.locale.Tr "mail.actions.run_info_cur_status" .Run.Status .PriorStatus}}
- {{.locale.Tr "mail.actions.run_info_ref" .Branch .CommitSHA}}
+ {{.locale.Tr "mail.actions.run_info_sha" .CommitSHA}}
{{if .LastRun}}
{{.locale.Tr "mail.actions.run_info_previous_status" .LastRun.Status}}
{{end}}
diff --git a/tests/integration/pull_review_test.go b/tests/integration/pull_review_test.go
index 603252f45f..4af0a1100e 100644
--- a/tests/integration/pull_review_test.go
+++ b/tests/integration/pull_review_test.go
@@ -5,19 +5,23 @@
package integration
import (
+ "bytes"
"fmt"
"io"
"net/http"
"net/http/httptest"
"net/url"
+ "os"
"path"
"strconv"
"strings"
"testing"
+ "time"
"forgejo.org/models/db"
issues_model "forgejo.org/models/issues"
repo_model "forgejo.org/models/repo"
+ unit_model "forgejo.org/models/unit"
"forgejo.org/models/unittest"
user_model "forgejo.org/models/user"
"forgejo.org/modules/git"
@@ -761,3 +765,158 @@ func updateFileInBranch(user *user_model.User, repo *repo_model.Repository, tree
_, err = files_service.ChangeRepoFiles(git.DefaultContext, repo, user, opts)
return err
}
+
+func TestPullRequestStaleReview(t *testing.T) {
+ onGiteaRun(t, func(t *testing.T, u *url.URL) {
+ user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+ session := loginUser(t, user2.Name)
+
+ // Create temporary repository.
+ repo, _, f := tests.CreateDeclarativeRepo(t, user2, "",
+ []unit_model.Type{unit_model.TypePullRequests}, nil,
+ []*files_service.ChangeRepoFile{
+ {
+ Operation: "create",
+ TreePath: "FUNFACT",
+ ContentReader: strings.NewReader("Smithy was the runner up to be Forgejo's name"),
+ },
+ },
+ )
+ defer f()
+
+ // Clone it.
+ dstPath := t.TempDir()
+ r := fmt.Sprintf("%suser2/%s.git", u.String(), repo.Name)
+ cloneURL, _ := url.Parse(r)
+ cloneURL.User = url.UserPassword("user2", userPassword)
+ require.NoError(t, git.CloneWithArgs(t.Context(), nil, cloneURL.String(), dstPath, git.CloneRepoOptions{}))
+
+ // Create first commit.
+ require.NoError(t, os.WriteFile(path.Join(dstPath, "README.md"), []byte("## test content"), 0o600))
+ require.NoError(t, git.AddChanges(dstPath, true))
+ require.NoError(t, git.CommitChanges(dstPath, git.CommitChangesOptions{
+ Committer: &git.Signature{
+ Email: "user2@example.com",
+ Name: "user2",
+ When: time.Now(),
+ },
+ Author: &git.Signature{
+ Email: "user2@example.com",
+ Name: "user2",
+ When: time.Now(),
+ },
+ Message: "Add README.",
+ }))
+ stdout := &bytes.Buffer{}
+ require.NoError(t, git.NewCommand(t.Context(), "rev-parse", "HEAD").Run(&git.RunOpts{Dir: dstPath, Stdout: stdout}))
+ firstCommitID := strings.TrimSpace(stdout.String())
+
+ // Create agit PR.
+ require.NoError(t, git.NewCommand(t.Context(), "push", "origin", "HEAD:refs/for/main", "-o", "topic=agit-pr").Run(&git.RunOpts{Dir: dstPath}))
+
+ pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{Index: 1, BaseRepoID: repo.ID})
+
+ req := NewRequest(t, "GET", "/"+repo.FullName()+"/pulls/1/files/reviews/new_comment")
+ resp := session.MakeRequest(t, req, http.StatusOK)
+ doc := NewHTMLParser(t, resp.Body)
+
+ t.Run("Mark review as stale", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+
+ // Create a approved review against against this commit.
+ req = NewRequestWithValues(t, "POST", "/"+repo.FullName()+"/pulls/1/files/reviews/comments", map[string]string{
+ "_csrf": doc.GetCSRF(),
+ "origin": doc.GetInputValueByName("origin"),
+ "latest_commit_id": firstCommitID,
+ "side": "proposed",
+ "line": "1",
+ "path": "FUNFACT",
+ "diff_start_cid": doc.GetInputValueByName("diff_start_cid"),
+ "diff_end_cid": doc.GetInputValueByName("diff_end_cid"),
+ "diff_base_cid": doc.GetInputValueByName("diff_base_cid"),
+ "content": "nitpicking comment",
+ "pending_review": "",
+ })
+ session.MakeRequest(t, req, http.StatusOK)
+
+ req = NewRequestWithValues(t, "POST", "/"+repo.FullName()+"/pulls/1/files/reviews/submit", map[string]string{
+ "_csrf": doc.GetCSRF(),
+ "commit_id": firstCommitID,
+ "content": "looks good",
+ "type": "comment",
+ })
+ session.MakeRequest(t, req, http.StatusOK)
+
+ // Review is not stale.
+ review := unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID})
+ assert.False(t, review.Stale)
+
+ // Create second commit
+ require.NoError(t, os.WriteFile(path.Join(dstPath, "README.md"), []byte("## I prefer this heading"), 0o600))
+ require.NoError(t, git.AddChanges(dstPath, true))
+ require.NoError(t, git.CommitChanges(dstPath, git.CommitChangesOptions{
+ Committer: &git.Signature{
+ Email: "user2@example.com",
+ Name: "user2",
+ When: time.Now(),
+ },
+ Author: &git.Signature{
+ Email: "user2@example.com",
+ Name: "user2",
+ When: time.Now(),
+ },
+ Message: "Add README.",
+ }))
+
+ // Push to agit PR.
+ require.NoError(t, git.NewCommand(t.Context(), "push", "origin", "HEAD:refs/for/main", "-o", "topic=agit-pr").Run(&git.RunOpts{Dir: dstPath}))
+
+ // Review is stale.
+ review = unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID})
+ assert.True(t, review.Stale)
+ })
+
+ t.Run("Create stale review", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+
+ // Review based on the first commit, which is a stale review because the
+ // PR's head is at the seconnd commit.
+ req := NewRequestWithValues(t, "POST", "/"+repo.FullName()+"/pulls/1/files/reviews/submit", map[string]string{
+ "_csrf": doc.GetCSRF(),
+ "commit_id": firstCommitID,
+ "content": "looks good",
+ "type": "approve",
+ })
+ session.MakeRequest(t, req, http.StatusOK)
+
+ // There does not exist a review that is not stale, because all reviews
+ // are based on the first commit and the PR's head is at the second commit.
+ unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID}, "stale = false")
+ })
+
+ t.Run("Mark unstale", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+
+ // Force push the PR to the first commit.
+ require.NoError(t, git.NewCommand(t.Context(), "reset", "--hard", "HEAD~1").Run(&git.RunOpts{Dir: dstPath}))
+ require.NoError(t, git.NewCommand(t.Context(), "push", "origin", "HEAD:refs/for/main", "-o", "topic=agit-pr", "-o", "force-push").Run(&git.RunOpts{Dir: dstPath}))
+
+ // There does not exist a review that is stale, because all reviews
+ // are based on the first commit and thus all reviews are no longer marked
+ // as stale.
+ unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID}, "stale = true")
+ })
+
+ t.Run("Diff did not change", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+
+ // Create a empty commit and push it to the PR.
+ require.NoError(t, git.NewCommand(t.Context(), "commit", "--allow-empty", "-m", "Empty commit").Run(&git.RunOpts{Dir: dstPath}))
+ require.NoError(t, git.NewCommand(t.Context(), "push", "origin", "HEAD:refs/for/main", "-o", "topic=agit-pr").Run(&git.RunOpts{Dir: dstPath}))
+
+ // There does not exist a review that is stale, because the diff did not
+ // change.
+ unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID}, "stale = true")
+ })
+ })
+}