diff --git a/models/fixtures/comment.yml b/models/fixtures/comment.yml
index d2a1de6559..f4121284a6 100644
--- a/models/fixtures/comment.yml
+++ b/models/fixtures/comment.yml
@@ -94,3 +94,22 @@
content: "test markup light/dark-mode-only "
created_unix: 946684813
updated_unix: 946684813
+
+-
+ id: 11
+ type: 22 # review
+ poster_id: 5
+ issue_id: 3 # in repo_id 1
+ content: "reviewed by user5"
+ review_id: 21
+ created_unix: 946684816
+
+-
+ id: 12
+ type: 27 # review request
+ poster_id: 2
+ issue_id: 3 # in repo_id 1
+ content: "review request for user5"
+ review_id: 22
+ assignee_id: 5
+ created_unix: 946684817
diff --git a/models/fixtures/review.yml b/models/fixtures/review.yml
index ac97e24c2b..0438ceadae 100644
--- a/models/fixtures/review.yml
+++ b/models/fixtures/review.yml
@@ -179,3 +179,22 @@
content: "Review Comment"
updated_unix: 946684810
created_unix: 946684810
+
+-
+ id: 21
+ type: 2
+ reviewer_id: 5
+ issue_id: 3
+ content: "reviewed by user5"
+ commit_id: 4a357436d925b5c974181ff12a994538ddc5a269
+ updated_unix: 946684816
+ created_unix: 946684816
+
+-
+ id: 22
+ type: 4
+ reviewer_id: 5
+ issue_id: 3
+ content: "review request for user5"
+ updated_unix: 946684817
+ created_unix: 946684817
diff --git a/models/issues/pull.go b/models/issues/pull.go
index a035cad649..45e2e19434 100644
--- a/models/issues/pull.go
+++ b/models/issues/pull.go
@@ -408,7 +408,7 @@ func (pr *PullRequest) getReviewedByLines(ctx context.Context, writer io.Writer)
// Note: This doesn't page as we only expect a very limited number of reviews
reviews, err := FindLatestReviews(ctx, FindReviewOptions{
- Type: ReviewTypeApprove,
+ Types: []ReviewType{ReviewTypeApprove},
IssueID: pr.IssueID,
OfficialOnly: setting.Repository.PullRequest.DefaultMergeMessageOfficialApproversOnly,
})
diff --git a/models/issues/review.go b/models/issues/review.go
index ca6fd6035b..a39c12069b 100644
--- a/models/issues/review.go
+++ b/models/issues/review.go
@@ -364,7 +364,7 @@ func GetCurrentReview(ctx context.Context, reviewer *user_model.User, issue *Iss
return nil, nil
}
reviews, err := FindReviews(ctx, FindReviewOptions{
- Type: ReviewTypePending,
+ Types: []ReviewType{ReviewTypePending},
IssueID: issue.ID,
ReviewerID: reviewer.ID,
})
diff --git a/models/issues/review_list.go b/models/issues/review_list.go
index 0ee28874ec..a5ceb21791 100644
--- a/models/issues/review_list.go
+++ b/models/issues/review_list.go
@@ -92,7 +92,7 @@ func (reviews ReviewList) LoadIssues(ctx context.Context) error {
// FindReviewOptions represent possible filters to find reviews
type FindReviewOptions struct {
db.ListOptions
- Type ReviewType
+ Types []ReviewType
IssueID int64
ReviewerID int64
OfficialOnly bool
@@ -107,8 +107,8 @@ func (opts *FindReviewOptions) toCond() builder.Cond {
if opts.ReviewerID > 0 {
cond = cond.And(builder.Eq{"reviewer_id": opts.ReviewerID})
}
- if opts.Type != ReviewTypeUnknown {
- cond = cond.And(builder.Eq{"type": opts.Type})
+ if len(opts.Types) > 0 {
+ cond = cond.And(builder.In("type", opts.Types))
}
if opts.OfficialOnly {
cond = cond.And(builder.Eq{"official": true})
diff --git a/models/issues/review_test.go b/models/issues/review_test.go
index 43dc9ed2c1..51cb940579 100644
--- a/models/issues/review_test.go
+++ b/models/issues/review_test.go
@@ -64,7 +64,7 @@ func TestReviewType_Icon(t *testing.T) {
func TestFindReviews(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
reviews, err := issues_model.FindReviews(db.DefaultContext, issues_model.FindReviewOptions{
- Type: issues_model.ReviewTypeApprove,
+ Types: []issues_model.ReviewType{issues_model.ReviewTypeApprove},
IssueID: 2,
ReviewerID: 1,
})
@@ -76,7 +76,7 @@ func TestFindReviews(t *testing.T) {
func TestFindLatestReviews(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
reviews, err := issues_model.FindLatestReviews(db.DefaultContext, issues_model.FindReviewOptions{
- Type: issues_model.ReviewTypeApprove,
+ Types: []issues_model.ReviewType{issues_model.ReviewTypeApprove},
IssueID: 11,
})
require.NoError(t, err)
diff --git a/modules/indexer/code/elasticsearch/elasticsearch.go b/modules/indexer/code/elasticsearch/elasticsearch.go
index 0bda180fac..aee56684e1 100644
--- a/modules/indexer/code/elasticsearch/elasticsearch.go
+++ b/modules/indexer/code/elasticsearch/elasticsearch.go
@@ -20,6 +20,7 @@ import (
indexer_internal "code.gitea.io/gitea/modules/indexer/internal"
inner_elasticsearch "code.gitea.io/gitea/modules/indexer/internal/elasticsearch"
"code.gitea.io/gitea/modules/json"
+ "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/typesniffer"
@@ -197,8 +198,33 @@ func (b *Indexer) Index(ctx context.Context, repo *repo_model.Repository, sha st
return nil
}
-// Delete deletes indexes by ids
+// Delete entries by repoId
func (b *Indexer) Delete(ctx context.Context, repoID int64) error {
+ if err := b.doDelete(ctx, repoID); err != nil {
+ // Maybe there is a conflict during the delete operation, so we should retry after a refresh
+ log.Warn("Deletion of entries of repo %v within index %v was erroneus. Trying to refresh index before trying again", repoID, b.inner.VersionedIndexName(), err)
+ if err := b.refreshIndex(ctx); err != nil {
+ return err
+ }
+ if err := b.doDelete(ctx, repoID); err != nil {
+ log.Error("Could not delete entries of repo %v within index %v", repoID, b.inner.VersionedIndexName())
+ return err
+ }
+ }
+ return nil
+}
+
+func (b *Indexer) refreshIndex(ctx context.Context) error {
+ if _, err := b.inner.Client.Refresh(b.inner.VersionedIndexName()).Do(ctx); err != nil {
+ log.Error("Error while trying to refresh index %v", b.inner.VersionedIndexName(), err)
+ return err
+ }
+
+ return nil
+}
+
+// Delete entries by repoId
+func (b *Indexer) doDelete(ctx context.Context, repoID int64) error {
_, err := b.inner.Client.DeleteByQuery(b.inner.VersionedIndexName()).
Query(elastic.NewTermsQuery("repo_id", repoID)).
Do(ctx)
diff --git a/release-notes/5480.md b/release-notes/5480.md
new file mode 100644
index 0000000000..5623c0ddc7
--- /dev/null
+++ b/release-notes/5480.md
@@ -0,0 +1,2 @@
+fix: [commit](https://codeberg.org/forgejo/forgejo/commit/7d3a013e5e81bbc054f4a730923e08f61814bf66) PR creation on forked repositories.
+fix: [commit](https://codeberg.org/forgejo/forgejo/commit/700e9f027bce8c783b74de07b3f29e09be045fa7) the logic of finding the latest pull review commit ID is incorrect.
diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go
index 97937fa937..bcb6ef2581 100644
--- a/routers/api/v1/repo/pull.go
+++ b/routers/api/v1/repo/pull.go
@@ -1112,9 +1112,20 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption)
// Check if current user has fork of repository or in the same repository.
headRepo := repo_model.GetForkedRepo(ctx, headUser.ID, baseRepo.ID)
if headRepo == nil && !isSameRepo {
- log.Trace("parseCompareInfo[%d]: does not have fork or in same repository", baseRepo.ID)
- ctx.NotFound("GetForkedRepo")
- return nil, nil, nil, "", ""
+ err := baseRepo.GetBaseRepo(ctx)
+ if err != nil {
+ ctx.Error(http.StatusInternalServerError, "GetBaseRepo", err)
+ return nil, nil, nil, "", ""
+ }
+
+ // Check if baseRepo's base repository is the same as headUser's repository.
+ if baseRepo.BaseRepo == nil || baseRepo.BaseRepo.OwnerID != headUser.ID {
+ log.Trace("parseCompareInfo[%d]: does not have fork or in same repository", baseRepo.ID)
+ ctx.NotFound("GetBaseRepo")
+ return nil, nil, nil, "", ""
+ }
+ // Assign headRepo so it can be used below.
+ headRepo = baseRepo.BaseRepo
}
var headGitRepo *git.Repository
diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go
index 39e1d487fa..8fba085ff7 100644
--- a/routers/api/v1/repo/pull_review.go
+++ b/routers/api/v1/repo/pull_review.go
@@ -82,7 +82,6 @@ func ListPullReviews(ctx *context.APIContext) {
opts := issues_model.FindReviewOptions{
ListOptions: utils.GetListOptions(ctx),
- Type: issues_model.ReviewTypeUnknown,
IssueID: pr.IssueID,
}
diff --git a/services/pull/pull.go b/services/pull/pull.go
index 82ca0d7047..6af7d8ba0c 100644
--- a/services/pull/pull.go
+++ b/services/pull/pull.go
@@ -966,6 +966,8 @@ type CommitInfo struct {
}
// GetPullCommits returns all commits on given pull request and the last review commit sha
+// Attention: The last review commit sha must be from the latest review whose commit id is not empty.
+// So the type of the latest review cannot be "ReviewTypeRequest".
func GetPullCommits(ctx *gitea_context.Context, issue *issues_model.Issue) ([]CommitInfo, string, error) {
pull := issue.PullRequest
@@ -1011,7 +1013,11 @@ func GetPullCommits(ctx *gitea_context.Context, issue *issues_model.Issue) ([]Co
lastreview, err := issues_model.FindLatestReviews(ctx, issues_model.FindReviewOptions{
IssueID: issue.ID,
ReviewerID: ctx.Doer.ID,
- Type: issues_model.ReviewTypeUnknown,
+ Types: []issues_model.ReviewType{
+ issues_model.ReviewTypeApprove,
+ issues_model.ReviewTypeComment,
+ issues_model.ReviewTypeReject,
+ },
})
if err != nil && !issues_model.IsErrReviewNotExist(err) {
diff --git a/services/pull/review.go b/services/pull/review.go
index 011c2a3058..927c43150b 100644
--- a/services/pull/review.go
+++ b/services/pull/review.go
@@ -340,7 +340,7 @@ func DismissApprovalReviews(ctx context.Context, doer *user_model.User, pull *is
reviews, err := issues_model.FindReviews(ctx, issues_model.FindReviewOptions{
ListOptions: db.ListOptionsAll,
IssueID: pull.IssueID,
- Type: issues_model.ReviewTypeApprove,
+ Types: []issues_model.ReviewType{issues_model.ReviewTypeApprove},
Dismissed: optional.Some(false),
})
if err != nil {
diff --git a/tests/integration/api_pull_review_test.go b/tests/integration/api_pull_review_test.go
index b7e5130cb8..b66e65ee41 100644
--- a/tests/integration/api_pull_review_test.go
+++ b/tests/integration/api_pull_review_test.go
@@ -60,6 +60,9 @@ func TestAPIPullReviewCreateDeleteComment(t *testing.T) {
var reviews []*api.PullReview
DecodeJSON(t, resp, &reviews)
for _, review := range reviews {
+ if review.State == api.ReviewStateRequestReview {
+ continue
+ }
req := NewRequestf(t, http.MethodDelete, "/api/v1/repos/%s/pulls/%d/reviews/%d", repo.FullName(), pullIssue.Index, review.ID).
AddTokenAuth(token)
MakeRequest(t, req, http.StatusNoContent)
@@ -93,7 +96,7 @@ func TestAPIPullReviewCreateDeleteComment(t *testing.T) {
DecodeJSON(t, resp, &getReview)
require.EqualValues(t, getReview, review)
}
- requireReviewCount(1)
+ requireReviewCount(2)
newCommentBody := "first new line"
var reviewComment api.PullReviewComment
@@ -140,7 +143,7 @@ func TestAPIPullReviewCreateDeleteComment(t *testing.T) {
AddTokenAuth(token)
MakeRequest(t, req, http.StatusNoContent)
}
- requireReviewCount(0)
+ requireReviewCount(1)
})
}
}
@@ -160,7 +163,7 @@ func TestAPIPullReview(t *testing.T) {
var reviews []*api.PullReview
DecodeJSON(t, resp, &reviews)
- if !assert.Len(t, reviews, 6) {
+ if !assert.Len(t, reviews, 8) {
return
}
for _, r := range reviews {
diff --git a/tests/integration/pull_commit_test.go b/tests/integration/pull_commit_test.go
new file mode 100644
index 0000000000..477f01725d
--- /dev/null
+++ b/tests/integration/pull_commit_test.go
@@ -0,0 +1,34 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package integration
+
+import (
+ "net/http"
+ "net/url"
+ "testing"
+
+ pull_service "code.gitea.io/gitea/services/pull"
+
+ "github.com/stretchr/testify/assert"
+)
+
+func TestListPullCommits(t *testing.T) {
+ onGiteaRun(t, func(t *testing.T, u *url.URL) {
+ session := loginUser(t, "user5")
+ req := NewRequest(t, "GET", "/user2/repo1/pulls/3/commits/list")
+ resp := session.MakeRequest(t, req, http.StatusOK)
+
+ var pullCommitList struct {
+ Commits []pull_service.CommitInfo `json:"commits"`
+ LastReviewCommitSha string `json:"last_review_commit_sha"`
+ }
+ DecodeJSON(t, resp, &pullCommitList)
+
+ if assert.Len(t, pullCommitList.Commits, 2) {
+ assert.Equal(t, "5f22f7d0d95d614d25a5b68592adb345a4b5c7fd", pullCommitList.Commits[0].ID)
+ assert.Equal(t, "4a357436d925b5c974181ff12a994538ddc5a269", pullCommitList.Commits[1].ID)
+ }
+ assert.Equal(t, "4a357436d925b5c974181ff12a994538ddc5a269", pullCommitList.LastReviewCommitSha)
+ })
+}
diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go
index 8bf48f4f6e..b814642bc7 100644
--- a/tests/integration/pull_create_test.go
+++ b/tests/integration/pull_create_test.go
@@ -72,6 +72,30 @@ func testPullCreate(t *testing.T, session *TestSession, user, repo string, toSel
return resp
}
+func testPullCreateDirectly(t *testing.T, session *TestSession, baseRepoOwner, baseRepoName, baseBranch, headRepoOwner, headRepoName, headBranch, title string) *httptest.ResponseRecorder {
+ headCompare := headBranch
+ if headRepoOwner != "" {
+ if headRepoName != "" {
+ headCompare = fmt.Sprintf("%s/%s:%s", headRepoOwner, headRepoName, headBranch)
+ } else {
+ headCompare = fmt.Sprintf("%s:%s", headRepoOwner, headBranch)
+ }
+ }
+ req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/compare/%s...%s", baseRepoOwner, baseRepoName, baseBranch, headCompare))
+ resp := session.MakeRequest(t, req, http.StatusOK)
+
+ // Submit the form for creating the pull
+ htmlDoc := NewHTMLParser(t, resp.Body)
+ link, exists := htmlDoc.doc.Find("form.ui.form").Attr("action")
+ assert.True(t, exists, "The template has changed")
+ req = NewRequestWithValues(t, "POST", link, map[string]string{
+ "_csrf": htmlDoc.GetCSRF(),
+ "title": title,
+ })
+ resp = session.MakeRequest(t, req, http.StatusOK)
+ return resp
+}
+
func TestPullCreate(t *testing.T) {
onGiteaRun(t, func(t *testing.T, u *url.URL) {
session := loginUser(t, "user1")
@@ -505,3 +529,30 @@ func TestRecentlyPushed(t *testing.T) {
})
})
}
+
+/*
+Setup:
+The base repository is: user2/repo1
+Fork repository to: user1/repo1
+Push extra commit to: user2/repo1, which changes README.md
+Create a PR on user1/repo1
+
+Test checks:
+Check if pull request can be created from base to the fork repository.
+*/
+func TestPullCreatePrFromBaseToFork(t *testing.T) {
+ onGiteaRun(t, func(t *testing.T, u *url.URL) {
+ sessionFork := loginUser(t, "user1")
+ testRepoFork(t, sessionFork, "user2", "repo1", "user1", "repo1")
+
+ // Edit base repository
+ sessionBase := loginUser(t, "user2")
+ testEditFile(t, sessionBase, "user2", "repo1", "master", "README.md", "Hello, World (Edited)\n")
+
+ // Create a PR
+ resp := testPullCreateDirectly(t, sessionFork, "user1", "repo1", "master", "user2", "repo1", "master", "This is a pull title")
+ // check the redirected URL
+ url := test.RedirectURL(resp)
+ assert.Regexp(t, "^/user1/repo1/pulls/[0-9]*$", url)
+ })
+}