Merge pull request '[gitea] week 2024-41 cherry pick (gitea/main -> forgejo)' (#5477) from earl-warren/wcp/2024-41 into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5477
Reviewed-by: Otto <otto@codeberg.org>

models/fixtures/comment.yml

@@ -94,3 +94,22 @@
   content: "test markup light/dark-mode-only ![GitHub-Mark-Light](https://user-images.githubusercontent.com/3369400/139447912-e0f43f33-6d9f-45f8-be46-2df5bbc91289.png#gh-dark-mode-only)![GitHub-Mark-Dark](https://user-images.githubusercontent.com/3369400/139448065-39a229ba-4b06-434b-bc67-616e2ed80c8f.png#gh-light-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

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

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,
 	})

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,
 	})

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 {
+	if len(opts.Types) > 0 {
-		cond = cond.And(builder.Eq{"type": opts.Type})
+		cond = cond.And(builder.In("type", opts.Types))
 	}
 	if opts.OfficialOnly {
 		cond = cond.And(builder.Eq{"official": true})

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)

models/user/search.go

@@ -69,7 +69,19 @@ func (opts *SearchUserOptions) toSearchQueryBase(ctx context.Context) *xorm.Sess
 			builder.Like{"LOWER(full_name)", lowerKeyword},
 		)
 		if opts.SearchByEmail {
-			keywordCond = keywordCond.Or(builder.Like{"LOWER(email)", lowerKeyword})
+			var emailCond builder.Cond
+			emailCond = builder.Like{"LOWER(email)", lowerKeyword}
+			if opts.Actor == nil {
+				emailCond = emailCond.And(builder.Eq{"keep_email_private": false})
+			} else if !opts.Actor.IsAdmin {
+				emailCond = emailCond.And(
+					builder.Or(
+						builder.Eq{"keep_email_private": false},
+						builder.Eq{"id": opts.Actor.ID},
+					),
+				)
+			}
+			keywordCond = keywordCond.Or(emailCond)
 		}
 
 		cond = cond.And(keywordCond)

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)

release-notes/5477.md

@@ -0,0 +1,3 @@
+feat: [commit](https://codeberg.org/forgejo/forgejo/commit/af901ac7bb03d27f175f2292581fc67fa9c8d567) Add support for searching users by email.
+fix: [commit](https://codeberg.org/forgejo/forgejo/commit/1dfe58ad11bc6fdc73a2b5ffb3c1481fbddbf46b) PR creation on forked repositories.
+fix: [commit](https://codeberg.org/forgejo/forgejo/commit/b67b7c12385059898fc8cb7997755a88b3afa483) the logic of finding the latest pull review commit ID is incorrect.

routers/api/v1/repo/pull.go

@@ -1112,11 +1112,22 @@ 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)
+		err := baseRepo.GetBaseRepo(ctx)
-		ctx.NotFound("GetForkedRepo")
+		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
 	if isSameRepo {
 		headRepo = ctx.Repo.Repository

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,
 	}
 

routers/api/v1/user/user.go

@@ -73,6 +73,7 @@ func Search(ctx *context.APIContext) {
 			Keyword:       ctx.FormTrim("q"),
 			UID:           uid,
 			Type:          user_model.UserTypeIndividual,
+			SearchByEmail: true,
 			ListOptions:   listOptions,
 		})
 		if err != nil {

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) {

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 {

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 {

tests/integration/api_user_search_test.go

@@ -130,3 +130,39 @@ func TestAPIUserSearchNotLoggedInUserHidden(t *testing.T) {
 	DecodeJSON(t, resp, &results)
 	assert.Empty(t, results.Data)
 }
+
+func TestAPIUserSearchByEmail(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	// admin can search user with private email
+	adminUsername := "user1"
+	session := loginUser(t, adminUsername)
+	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadUser)
+	query := "user2@example.com"
+	req := NewRequestf(t, "GET", "/api/v1/users/search?q=%s", query).
+		AddTokenAuth(token)
+	resp := MakeRequest(t, req, http.StatusOK)
+
+	var results SearchResults
+	DecodeJSON(t, resp, &results)
+	assert.Len(t, results.Data, 1)
+	assert.Equal(t, query, results.Data[0].Email)
+
+	// no login user can not search user with private email
+	req = NewRequestf(t, "GET", "/api/v1/users/search?q=%s", query)
+	resp = MakeRequest(t, req, http.StatusOK)
+	DecodeJSON(t, resp, &results)
+	assert.Empty(t, results.Data)
+
+	// user can search self with private email
+	user2 := "user2"
+	session = loginUser(t, user2)
+	token = getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadUser)
+	req = NewRequestf(t, "GET", "/api/v1/users/search?q=%s", query).
+		AddTokenAuth(token)
+	resp = MakeRequest(t, req, http.StatusOK)
+
+	DecodeJSON(t, resp, &results)
+	assert.Len(t, results.Data, 1)
+	assert.Equal(t, query, results.Data[0].Email)
+}

tests/integration/attachment_test.go

@@ -30,7 +30,7 @@ func generateImg() bytes.Buffer {
 	return buff
 }
 
-func createAttachment(t *testing.T, session *TestSession, repoURL, filename string, buff bytes.Buffer, expectedStatus int) string {
+func createAttachment(t *testing.T, session *TestSession, csrf, repoURL, filename string, buff bytes.Buffer, expectedStatus int) string {
 	body := &bytes.Buffer{}
 
 	// Setup multi-part
@@ -42,8 +42,6 @@ func createAttachment(t *testing.T, session *TestSession, repoURL, filename stri
 	err = writer.Close()
 	require.NoError(t, err)
 
-	csrf := GetCSRF(t, session, repoURL)
-
 	req := NewRequestWithBody(t, "POST", repoURL+"/issues/attachments", body)
 	req.Header.Add("X-Csrf-Token", csrf)
 	req.Header.Add("Content-Type", writer.FormDataContentType())
@@ -60,15 +58,14 @@ func createAttachment(t *testing.T, session *TestSession, repoURL, filename stri
 func TestCreateAnonymousAttachment(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 	session := emptyTestSession(t)
-	// this test is not right because it just doesn't pass the CSRF validation
+	createAttachment(t, session, GetCSRF(t, session, "/user/login"), "user2/repo1", "image.png", generateImg(), http.StatusSeeOther)
-	createAttachment(t, session, "user2/repo1", "image.png", generateImg(), http.StatusBadRequest)
 }
 
 func TestCreateIssueAttachment(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 	const repoURL = "user2/repo1"
 	session := loginUser(t, "user2")
-	uuid := createAttachment(t, session, repoURL, "image.png", generateImg(), http.StatusOK)
+	uuid := createAttachment(t, session, GetCSRF(t, session, repoURL), repoURL, "image.png", generateImg(), http.StatusOK)
 
 	req := NewRequest(t, "GET", repoURL+"/issues/new")
 	resp := session.MakeRequest(t, req, http.StatusOK)

tests/integration/integration_test.go

@@ -663,12 +663,17 @@ func VerifyJSONSchema(t testing.TB, resp *httptest.ResponseRecorder, schemaFile
 	require.NoError(t, schemaValidation)
 }
 
+// GetCSRF returns CSRF token from body
+// If it fails, it means the CSRF token is not found in the response body returned by the url with the given session.
+// In this case, you should find a better url to get it.
 func GetCSRF(t testing.TB, session *TestSession, urlStr string) string {
 	t.Helper()
 	req := NewRequest(t, "GET", urlStr)
 	resp := session.MakeRequest(t, req, http.StatusOK)
 	doc := NewHTMLParser(t, resp.Body)
-	return doc.GetCSRF()
+	csrf := doc.GetCSRF()
+	require.NotEmpty(t, csrf)
+	return csrf
 }
 
 func GetHTMLTitle(t testing.TB, session *TestSession, urlStr string) string {

tests/integration/issue_test.go

@@ -500,7 +500,7 @@ func TestIssueCommentAttachment(t *testing.T) {
 	link, exists := htmlDoc.doc.Find("#comment-form").Attr("action")
 	assert.True(t, exists, "The template has changed")
 
-	uuid := createAttachment(t, session, repoURL, "image.png", generateImg(), http.StatusOK)
+	uuid := createAttachment(t, session, GetCSRF(t, session, repoURL), repoURL, "image.png", generateImg(), http.StatusOK)
 
 	commentCount := htmlDoc.doc.Find(".comment-list .comment .render-content").Length()
 

tests/integration/org_test.go

@@ -204,9 +204,7 @@ func TestTeamSearch(t *testing.T) {
 	var results TeamSearchResults
 
 	session := loginUser(t, user.Name)
-	csrf := GetCSRF(t, session, "/"+org.Name)
 	req := NewRequestf(t, "GET", "/org/%s/teams/-/search?q=%s", org.Name, "_team")
-	req.Header.Add("X-Csrf-Token", csrf)
 	resp := session.MakeRequest(t, req, http.StatusOK)
 	DecodeJSON(t, resp, &results)
 	assert.NotEmpty(t, results.Data)
@@ -217,9 +215,7 @@ func TestTeamSearch(t *testing.T) {
 	// no access if not organization member
 	user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
 	session = loginUser(t, user5.Name)
-	csrf = GetCSRF(t, session, "/"+org.Name)
 	req = NewRequestf(t, "GET", "/org/%s/teams/-/search?q=%s", org.Name, "team")
-	req.Header.Add("X-Csrf-Token", csrf)
 	session.MakeRequest(t, req, http.StatusNotFound)
 }
 

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)
+	})
+}

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)
+	})
+}