Merge branch 'main' into lunny/issue_dev

models/issues/comment.go

@@ -197,6 +197,20 @@ func (t CommentType) HasMailReplySupport() bool {
 	return false
 }
 
+func (t CommentType) CountedAsConversation() bool {
+	for _, ct := range ConversationCountedCommentType() {
+		if t == ct {
+			return true
+		}
+	}
+	return false
+}
+
+// ConversationCountedCommentType returns the comment types that are counted as a conversation
+func ConversationCountedCommentType() []CommentType {
+	return []CommentType{CommentTypeComment, CommentTypeReview}
+}
+
 // RoleInRepo presents the user's participation in the repo
 type RoleInRepo string
 
@@ -592,26 +606,26 @@ func (c *Comment) LoadAttachments(ctx context.Context) error {
 	return nil
 }
 
-// UpdateAttachments update attachments by UUIDs for the comment
-func (c *Comment) UpdateAttachments(ctx context.Context, uuids []string) error {
-	ctx, committer, err := db.TxContext(ctx)
-	if err != nil {
-		return err
+// UpdateCommentAttachments update attachments by UUIDs for the comment
+func UpdateCommentAttachments(ctx context.Context, c *Comment, uuids []string) error {
+	if len(uuids) == 0 {
+		return nil
 	}
-	defer committer.Close()
-
-	attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids)
-	if err != nil {
-		return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err)
-	}
-	for i := 0; i < len(attachments); i++ {
-		attachments[i].IssueID = c.IssueID
-		attachments[i].CommentID = c.ID
-		if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil {
-			return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err)
+	return db.WithTx(ctx, func(ctx context.Context) error {
+		attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids)
+		if err != nil {
+			return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err)
 		}
-	}
-	return committer.Commit()
+		for i := 0; i < len(attachments); i++ {
+			attachments[i].IssueID = c.IssueID
+			attachments[i].CommentID = c.ID
+			if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil {
+				return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err)
+			}
+		}
+		c.Attachments = attachments
+		return nil
+	})
 }
 
 // LoadAssigneeUserAndTeam if comment.Type is CommentTypeAssignees, then load assignees
@@ -878,7 +892,7 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment
 	// Check comment type.
 	switch opts.Type {
 	case CommentTypeCode:
-		if err = updateAttachments(ctx, opts, comment); err != nil {
+		if err = UpdateCommentAttachments(ctx, comment, opts.Attachments); err != nil {
 			return err
 		}
 		if comment.ReviewID != 0 {
@@ -893,12 +907,12 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment
 		}
 		fallthrough
 	case CommentTypeComment:
-		if _, err = db.Exec(ctx, "UPDATE `issue` SET num_comments=num_comments+1 WHERE id=?", opts.Issue.ID); err != nil {
+		if err := UpdateIssueNumComments(ctx, opts.Issue.ID); err != nil {
 			return err
 		}
 		fallthrough
 	case CommentTypeReview:
-		if err = updateAttachments(ctx, opts, comment); err != nil {
+		if err = UpdateCommentAttachments(ctx, comment, opts.Attachments); err != nil {
 			return err
 		}
 	case CommentTypeReopen, CommentTypeClose:
@@ -910,23 +924,6 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment
 	return UpdateIssueCols(ctx, opts.Issue, "updated_unix")
 }
 
-func updateAttachments(ctx context.Context, opts *CreateCommentOptions, comment *Comment) error {
-	attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, opts.Attachments)
-	if err != nil {
-		return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", opts.Attachments, err)
-	}
-	for i := range attachments {
-		attachments[i].IssueID = opts.Issue.ID
-		attachments[i].CommentID = comment.ID
-		// No assign value could be 0, so ignore AllCols().
-		if _, err = db.GetEngine(ctx).ID(attachments[i].ID).Update(attachments[i]); err != nil {
-			return fmt.Errorf("update attachment [%d]: %w", attachments[i].ID, err)
-		}
-	}
-	comment.Attachments = attachments
-	return nil
-}
-
 func createDeadlineComment(ctx context.Context, doer *user_model.User, issue *Issue, newDeadlineUnix timeutil.TimeStamp) (*Comment, error) {
 	var content string
 	var commentType CommentType
@@ -1182,8 +1179,8 @@ func DeleteComment(ctx context.Context, comment *Comment) error {
 		return err
 	}
 
-	if comment.Type == CommentTypeComment {
-		if _, err := e.ID(comment.IssueID).Decr("num_comments").Update(new(Issue)); err != nil {
+	if comment.Type.CountedAsConversation() {
+		if err := UpdateIssueNumComments(ctx, comment.IssueID); err != nil {
 			return err
 		}
 	}
@@ -1300,6 +1297,21 @@ func (c *Comment) HasOriginalAuthor() bool {
 	return c.OriginalAuthor != "" && c.OriginalAuthorID != 0
 }
 
+func UpdateIssueNumCommentsBuilder(issueID int64) *builder.Builder {
+	subQuery := builder.Select("COUNT(*)").From("`comment`").Where(
+		builder.Eq{"issue_id": issueID}.And(
+			builder.In("`type`", ConversationCountedCommentType()),
+		))
+
+	return builder.Update(builder.Eq{"num_comments": subQuery}).
+		From("`issue`").Where(builder.Eq{"id": issueID})
+}
+
+func UpdateIssueNumComments(ctx context.Context, issueID int64) error {
+	_, err := db.GetEngine(ctx).Exec(UpdateIssueNumCommentsBuilder(issueID))
+	return err
+}
+
 // InsertIssueComments inserts many comments of issues.
 func InsertIssueComments(ctx context.Context, comments []*Comment) error {
 	if len(comments) == 0 {
@@ -1332,8 +1344,7 @@ func InsertIssueComments(ctx context.Context, comments []*Comment) error {
 	}
 
 	for _, issueID := range issueIDs {
-		if _, err := db.Exec(ctx, "UPDATE issue set num_comments = (SELECT count(*) FROM comment WHERE issue_id = ? AND `type`=?) WHERE id = ?",
-			issueID, CommentTypeComment, issueID); err != nil {
+		if err := UpdateIssueNumComments(ctx, issueID); err != nil {
 			return err
 		}
 	}

models/issues/comment_test.go

@@ -45,6 +45,24 @@ func TestCreateComment(t *testing.T) {
 	unittest.AssertInt64InRange(t, now, then, int64(updatedIssue.UpdatedUnix))
 }
 
+func Test_UpdateCommentAttachment(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 1})
+	attachment := repo_model.Attachment{
+		Name: "test.txt",
+	}
+	assert.NoError(t, db.Insert(db.DefaultContext, &attachment))
+
+	err := issues_model.UpdateCommentAttachments(db.DefaultContext, comment, []string{attachment.UUID})
+	assert.NoError(t, err)
+
+	attachment2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: attachment.ID})
+	assert.EqualValues(t, attachment.Name, attachment2.Name)
+	assert.EqualValues(t, comment.ID, attachment2.CommentID)
+	assert.EqualValues(t, comment.IssueID, attachment2.IssueID)
+}
+
 func TestFetchCodeComments(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 
@@ -64,7 +82,7 @@ func TestFetchCodeComments(t *testing.T) {
 }
 
 func TestAsCommentType(t *testing.T) {
-	assert.Equal(t, issues_model.CommentType(0), issues_model.CommentTypeComment)
+	assert.Equal(t, issues_model.CommentTypeComment, issues_model.CommentType(0))
 	assert.Equal(t, issues_model.CommentTypeUndefined, issues_model.AsCommentType(""))
 	assert.Equal(t, issues_model.CommentTypeUndefined, issues_model.AsCommentType("nonsense"))
 	assert.Equal(t, issues_model.CommentTypeComment, issues_model.AsCommentType("comment"))
@@ -97,3 +115,12 @@ func TestMigrate_InsertIssueComments(t *testing.T) {
 
 	unittest.CheckConsistencyFor(t, &issues_model.Issue{})
 }
+
+func Test_UpdateIssueNumComments(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+	issue2 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
+
+	assert.NoError(t, issues_model.UpdateIssueNumComments(db.DefaultContext, issue2.ID))
+	issue2 = unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
+	assert.EqualValues(t, 1, issue2.NumComments)
+}

models/issues/dependency_test.go

@@ -49,9 +49,13 @@ func TestCreateIssueDependency(t *testing.T) {
 	assert.False(t, left)
 
 	// Close #2 and check again
-	_, err = issues_model.ChangeIssueStatus(db.DefaultContext, issue2, user1, true)
+	_, err = issues_model.CloseIssue(db.DefaultContext, issue2, user1)
 	assert.NoError(t, err)
 
+	issue2Closed, err := issues_model.GetIssueByID(db.DefaultContext, 2)
+	assert.NoError(t, err)
+	assert.True(t, issue2Closed.IsClosed)
+
 	left, err = issues_model.IssueNoDependenciesLeft(db.DefaultContext, issue1)
 	assert.NoError(t, err)
 	assert.True(t, left)
@@ -59,4 +63,11 @@ func TestCreateIssueDependency(t *testing.T) {
 	// Test removing the dependency
 	err = issues_model.RemoveIssueDependency(db.DefaultContext, user1, issue1, issue2, issues_model.DependencyTypeBlockedBy)
 	assert.NoError(t, err)
+
+	_, err = issues_model.ReopenIssue(db.DefaultContext, issue2, user1)
+	assert.NoError(t, err)
+
+	issue2Reopened, err := issues_model.GetIssueByID(db.DefaultContext, 2)
+	assert.NoError(t, err)
+	assert.False(t, issue2Reopened.IsClosed)
 }

models/issues/issue.go

@@ -125,8 +125,11 @@ type Issue struct {
 	IsPull            bool             `xorm:"INDEX"` // Indicates whether is a pull request or not.
 	PullRequest       *PullRequest     `xorm:"-"`
 	NumComments       int
-	Ref               string
-	PinOrder          int `xorm:"DEFAULT 0"`
+
+	// TODO: RemoveIssueRef: see "repo/issue/branch_selector_field.tmpl"
+	Ref string
+
+	PinOrder int `xorm:"DEFAULT 0"`
 
 	DeadlineUnix timeutil.TimeStamp `xorm:"INDEX"`
 

models/issues/issue_index.go

@@ -18,12 +18,12 @@ func RecalculateIssueIndexForRepo(ctx context.Context, repoID int64) error {
 	}
 	defer committer.Close()
 
-	var max int64
-	if _, err = db.GetEngine(ctx).Select(" MAX(`index`)").Table("issue").Where("repo_id=?", repoID).Get(&max); err != nil {
+	var maxIndex int64
+	if _, err = db.GetEngine(ctx).Select(" MAX(`index`)").Table("issue").Where("repo_id=?", repoID).Get(&maxIndex); err != nil {
 		return err
 	}
 
-	if err = db.SyncMaxResourceIndex(ctx, "issue_index", repoID, max); err != nil {
+	if err = db.SyncMaxResourceIndex(ctx, "issue_index", repoID, maxIndex); err != nil {
 		return err
 	}
 

models/issues/issue_search.go

@@ -27,8 +27,8 @@ type IssuesOptions struct { //nolint
 	RepoIDs            []int64 // overwrites RepoCond if the length is not 0
 	AllPublic          bool    // include also all public repositories
 	RepoCond           builder.Cond
-	AssigneeID         int64
-	PosterID           int64
+	AssigneeID         optional.Option[int64]
+	PosterID           optional.Option[int64]
 	MentionedID        int64
 	ReviewRequestedID  int64
 	ReviewedID         int64
@@ -231,15 +231,8 @@ func applyConditions(sess *xorm.Session, opts *IssuesOptions) {
 		sess.And("issue.is_closed=?", opts.IsClosed.Value())
 	}
 
-	if opts.AssigneeID > 0 {
-		applyAssigneeCondition(sess, opts.AssigneeID)
-	} else if opts.AssigneeID == db.NoConditionID {
-		sess.Where("issue.id NOT IN (SELECT issue_id FROM issue_assignees)")
-	}
-
-	if opts.PosterID > 0 {
-		applyPosterCondition(sess, opts.PosterID)
-	}
+	applyAssigneeCondition(sess, opts.AssigneeID)
+	applyPosterCondition(sess, opts.PosterID)
 
 	if opts.MentionedID > 0 {
 		applyMentionedCondition(sess, opts.MentionedID)
@@ -359,13 +352,27 @@ func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *organizati
 	return cond
 }
 
-func applyAssigneeCondition(sess *xorm.Session, assigneeID int64) {
-	sess.Join("INNER", "issue_assignees", "issue.id = issue_assignees.issue_id").
-		And("issue_assignees.assignee_id = ?", assigneeID)
+func applyAssigneeCondition(sess *xorm.Session, assigneeID optional.Option[int64]) {
+	// old logic: 0 is also treated as "not filtering assignee", because the "assignee" was read as FormInt64
+	if !assigneeID.Has() || assigneeID.Value() == 0 {
+		return
+	}
+	if assigneeID.Value() == db.NoConditionID {
+		sess.Where("issue.id NOT IN (SELECT issue_id FROM issue_assignees)")
+	} else {
+		sess.Join("INNER", "issue_assignees", "issue.id = issue_assignees.issue_id").
+			And("issue_assignees.assignee_id = ?", assigneeID.Value())
+	}
 }
 
-func applyPosterCondition(sess *xorm.Session, posterID int64) {
-	sess.And("issue.poster_id=?", posterID)
+func applyPosterCondition(sess *xorm.Session, posterID optional.Option[int64]) {
+	if !posterID.Has() {
+		return
+	}
+	// poster doesn't need to support db.NoConditionID(-1), so just use the value as-is
+	if posterID.Has() {
+		sess.And("issue.poster_id=?", posterID.Value())
+	}
 }
 
 func applyMentionedCondition(sess *xorm.Session, mentionedID int64) {

models/issues/issue_stats.go

@@ -151,15 +151,9 @@ func applyIssuesOptions(sess *xorm.Session, opts *IssuesOptions, issueIDs []int6
 
 	applyProjectCondition(sess, opts)
 
-	if opts.AssigneeID > 0 {
-		applyAssigneeCondition(sess, opts.AssigneeID)
-	} else if opts.AssigneeID == db.NoConditionID {
-		sess.Where("issue.id NOT IN (SELECT issue_id FROM issue_assignees)")
-	}
+	applyAssigneeCondition(sess, opts.AssigneeID)
 
-	if opts.PosterID > 0 {
-		applyPosterCondition(sess, opts.PosterID)
-	}
+	applyPosterCondition(sess, opts.PosterID)
 
 	if opts.MentionedID > 0 {
 		applyMentionedCondition(sess, opts.MentionedID)

models/issues/issue_test.go

@@ -16,6 +16,7 @@ import (
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/optional"
 	"code.gitea.io/gitea/modules/setting"
 
 	"github.com/stretchr/testify/assert"
@@ -155,7 +156,7 @@ func TestIssues(t *testing.T) {
 	}{
 		{
 			issues_model.IssuesOptions{
-				AssigneeID: 1,
+				AssigneeID: optional.Some(int64(1)),
 				SortType:   "oldest",
 			},
 			[]int64{1, 6},
@@ -433,7 +434,7 @@ func assertCreateIssues(t *testing.T, isPull bool) {
 	owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
 	label := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 1})
 	milestone := unittest.AssertExistsAndLoadBean(t, &issues_model.Milestone{ID: 1})
-	assert.EqualValues(t, milestone.ID, 1)
+	assert.EqualValues(t, 1, milestone.ID)
 	reaction := &issues_model.Reaction{
 		Type:   "heart",
 		UserID: owner.ID,

models/issues/issue_update.go

@@ -34,7 +34,7 @@ func UpdateIssueCols(ctx context.Context, issue *Issue, cols ...string) error {
 	return nil
 }
 
-func changeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed, isMergePull bool) (*Comment, error) {
+func ChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed, isMergePull bool) (*Comment, error) {
 	// Reload the issue
 	currentIssue, err := GetIssueByID(ctx, issue.ID)
 	if err != nil {
@@ -119,8 +119,8 @@ func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.Use
 	})
 }
 
-// ChangeIssueStatus changes issue status to open or closed.
-func ChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed bool) (*Comment, error) {
+// CloseIssue changes issue status to closed.
+func CloseIssue(ctx context.Context, issue *Issue, doer *user_model.User) (*Comment, error) {
 	if err := issue.LoadRepo(ctx); err != nil {
 		return nil, err
 	}
@@ -128,7 +128,45 @@ func ChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User,
 		return nil, err
 	}
 
-	return changeIssueStatus(ctx, issue, doer, isClosed, false)
+	ctx, committer, err := db.TxContext(ctx)
+	if err != nil {
+		return nil, err
+	}
+	defer committer.Close()
+
+	comment, err := ChangeIssueStatus(ctx, issue, doer, true, false)
+	if err != nil {
+		return nil, err
+	}
+	if err := committer.Commit(); err != nil {
+		return nil, err
+	}
+	return comment, nil
+}
+
+// ReopenIssue changes issue status to open.
+func ReopenIssue(ctx context.Context, issue *Issue, doer *user_model.User) (*Comment, error) {
+	if err := issue.LoadRepo(ctx); err != nil {
+		return nil, err
+	}
+	if err := issue.LoadPoster(ctx); err != nil {
+		return nil, err
+	}
+
+	ctx, committer, err := db.TxContext(ctx)
+	if err != nil {
+		return nil, err
+	}
+	defer committer.Close()
+
+	comment, err := ChangeIssueStatus(ctx, issue, doer, false, false)
+	if err != nil {
+		return nil, err
+	}
+	if err := committer.Commit(); err != nil {
+		return nil, err
+	}
+	return comment, nil
 }
 
 // ChangeIssueTitle changes the title of this issue, as the given user.
@@ -139,7 +177,7 @@ func ChangeIssueTitle(ctx context.Context, issue *Issue, doer *user_model.User,
 	}
 	defer committer.Close()
 
-	issue.Title, _ = util.SplitStringAtByteN(issue.Title, 255)
+	issue.Title = util.EllipsisDisplayString(issue.Title, 255)
 	if err = UpdateIssueCols(ctx, issue, "name"); err != nil {
 		return fmt.Errorf("updateIssueCols: %w", err)
 	}
@@ -367,19 +405,10 @@ func NewIssueWithIndex(ctx context.Context, doer *user_model.User, opts NewIssue
 		return err
 	}
 
-	if len(opts.Attachments) > 0 {
-		attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, opts.Attachments)
-		if err != nil {
-			return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", opts.Attachments, err)
-		}
-
-		for i := 0; i < len(attachments); i++ {
-			attachments[i].IssueID = opts.Issue.ID
-			if _, err = e.ID(attachments[i].ID).Update(attachments[i]); err != nil {
-				return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err)
-			}
-		}
+	if err := UpdateIssueAttachments(ctx, opts.Issue.ID, opts.Attachments); err != nil {
+		return err
 	}
+
 	if err = opts.Issue.LoadAttributes(ctx); err != nil {
 		return err
 	}
@@ -402,7 +431,7 @@ func NewIssue(ctx context.Context, repo *repo_model.Repository, issue *Issue, la
 	}
 
 	issue.Index = idx
-	issue.Title, _ = util.SplitStringAtByteN(issue.Title, 255)
+	issue.Title = util.EllipsisDisplayString(issue.Title, 255)
 
 	if err = NewIssueWithIndex(ctx, issue.Poster, NewIssueOptions{
 		Repo:        repo,

models/issues/issue_user_test.go

@@ -12,6 +12,7 @@ import (
 	"code.gitea.io/gitea/models/unittest"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func Test_NewIssueUsers(t *testing.T) {
@@ -27,9 +28,8 @@ func Test_NewIssueUsers(t *testing.T) {
 	}
 
 	// artificially insert new issue
-	unittest.AssertSuccessfulInsert(t, newIssue)
-
-	assert.NoError(t, issues_model.NewIssueUsers(db.DefaultContext, repo, newIssue))
+	require.NoError(t, db.Insert(db.DefaultContext, newIssue))
+	require.NoError(t, issues_model.NewIssueUsers(db.DefaultContext, repo, newIssue))
 
 	// issue_user table should now have entries for new issue
 	unittest.AssertExistsAndLoadBean(t, &issues_model.IssueUser{IssueID: newIssue.ID, UID: newIssue.PosterID})

models/issues/issue_watch_test.go

@@ -48,17 +48,17 @@ func TestGetIssueWatchers(t *testing.T) {
 	iws, err := issues_model.GetIssueWatchers(db.DefaultContext, 1, db.ListOptions{})
 	assert.NoError(t, err)
 	// Watcher is inactive, thus 0
-	assert.Len(t, iws, 0)
+	assert.Empty(t, iws)
 
 	iws, err = issues_model.GetIssueWatchers(db.DefaultContext, 2, db.ListOptions{})
 	assert.NoError(t, err)
 	// Watcher is explicit not watching
-	assert.Len(t, iws, 0)
+	assert.Empty(t, iws)
 
 	iws, err = issues_model.GetIssueWatchers(db.DefaultContext, 5, db.ListOptions{})
 	assert.NoError(t, err)
 	// Issue has no Watchers
-	assert.Len(t, iws, 0)
+	assert.Empty(t, iws)
 
 	iws, err = issues_model.GetIssueWatchers(db.DefaultContext, 7, db.ListOptions{})
 	assert.NoError(t, err)

models/issues/issue_xref_test.go

@@ -98,7 +98,7 @@ func TestXRef_ResolveCrossReferences(t *testing.T) {
 	i1 := testCreateIssue(t, 1, 2, "title1", "content1", false)
 	i2 := testCreateIssue(t, 1, 2, "title2", "content2", false)
 	i3 := testCreateIssue(t, 1, 2, "title3", "content3", false)
-	_, err := issues_model.ChangeIssueStatus(db.DefaultContext, i3, d, true)
+	_, err := issues_model.CloseIssue(db.DefaultContext, i3, d)
 	assert.NoError(t, err)
 
 	pr := testCreatePR(t, 1, 2, "titlepr", fmt.Sprintf("closes #%d", i1.Index))

models/issues/label.go

@@ -349,6 +349,17 @@ func GetLabelIDsInRepoByNames(ctx context.Context, repoID int64, labelNames []st
 		Find(&labelIDs)
 }
 
+// GetLabelIDsInOrgByNames returns a list of labelIDs by names in a given org.
+func GetLabelIDsInOrgByNames(ctx context.Context, orgID int64, labelNames []string) ([]int64, error) {
+	labelIDs := make([]int64, 0, len(labelNames))
+	return labelIDs, db.GetEngine(ctx).Table("label").
+		Where("org_id = ?", orgID).
+		In("name", labelNames).
+		Asc("name").
+		Cols("id").
+		Find(&labelIDs)
+}
+
 // BuildLabelNamesIssueIDsCondition returns a builder where get issue ids match label names
 func BuildLabelNamesIssueIDsCondition(labelNames []string) *builder.Builder {
 	return builder.Select("issue_label.issue_id").

models/issues/label_test.go

@@ -31,12 +31,12 @@ func TestLabel_LoadSelectedLabelsAfterClick(t *testing.T) {
 	// First test : with negative and scope
 	label.LoadSelectedLabelsAfterClick([]int64{1, -8}, []string{"", "scope"})
 	assert.Equal(t, "1", label.QueryString)
-	assert.Equal(t, true, label.IsSelected)
+	assert.True(t, label.IsSelected)
 
 	// Second test : with duplicates
 	label.LoadSelectedLabelsAfterClick([]int64{1, 7, 1, 7, 7}, []string{"", "scope", "", "scope", "scope"})
 	assert.Equal(t, "1,8", label.QueryString)
-	assert.Equal(t, false, label.IsSelected)
+	assert.False(t, label.IsSelected)
 
 	// Third test : empty set
 	label.LoadSelectedLabelsAfterClick([]int64{}, []string{})
@@ -248,7 +248,7 @@ func TestGetLabelsByIssueID(t *testing.T) {
 
 	labels, err = issues_model.GetLabelsByIssueID(db.DefaultContext, unittest.NonexistentID)
 	assert.NoError(t, err)
-	assert.Len(t, labels, 0)
+	assert.Empty(t, labels)
 }
 
 func TestUpdateLabel(t *testing.T) {
@@ -271,7 +271,7 @@ func TestUpdateLabel(t *testing.T) {
 	assert.EqualValues(t, label.Color, newLabel.Color)
 	assert.EqualValues(t, label.Name, newLabel.Name)
 	assert.EqualValues(t, label.Description, newLabel.Description)
-	assert.EqualValues(t, newLabel.ArchivedUnix, 0)
+	assert.EqualValues(t, 0, newLabel.ArchivedUnix)
 	unittest.CheckConsistencyFor(t, &issues_model.Label{}, &repo_model.Repository{})
 }
 
@@ -387,7 +387,7 @@ func TestDeleteIssueLabel(t *testing.T) {
 
 		expectedNumIssues := label.NumIssues
 		expectedNumClosedIssues := label.NumClosedIssues
-		if unittest.BeanExists(t, &issues_model.IssueLabel{IssueID: issueID, LabelID: labelID}) {
+		if unittest.GetBean(t, &issues_model.IssueLabel{IssueID: issueID, LabelID: labelID}) != nil {
 			expectedNumIssues--
 			if issue.IsClosed {
 				expectedNumClosedIssues--

models/issues/milestone_test.go

@@ -87,7 +87,7 @@ func TestGetMilestonesByRepoID(t *testing.T) {
 		IsClosed: optional.Some(false),
 	})
 	assert.NoError(t, err)
-	assert.Len(t, milestones, 0)
+	assert.Empty(t, milestones)
 }
 
 func TestGetMilestones(t *testing.T) {

models/issues/pull.go

@@ -499,65 +499,6 @@ func (pr *PullRequest) IsFromFork() bool {
 	return pr.HeadRepoID != pr.BaseRepoID
 }
 
-// SetMerged sets a pull request to merged and closes the corresponding issue
-func (pr *PullRequest) SetMerged(ctx context.Context) (bool, error) {
-	if pr.HasMerged {
-		return false, fmt.Errorf("PullRequest[%d] already merged", pr.Index)
-	}
-	if pr.MergedCommitID == "" || pr.MergedUnix == 0 || pr.Merger == nil {
-		return false, fmt.Errorf("Unable to merge PullRequest[%d], some required fields are empty", pr.Index)
-	}
-
-	pr.HasMerged = true
-	sess := db.GetEngine(ctx)
-
-	if _, err := sess.Exec("UPDATE `issue` SET `repo_id` = `repo_id` WHERE `id` = ?", pr.IssueID); err != nil {
-		return false, err
-	}
-
-	if _, err := sess.Exec("UPDATE `pull_request` SET `issue_id` = `issue_id` WHERE `id` = ?", pr.ID); err != nil {
-		return false, err
-	}
-
-	pr.Issue = nil
-	if err := pr.LoadIssue(ctx); err != nil {
-		return false, err
-	}
-
-	if tmpPr, err := GetPullRequestByID(ctx, pr.ID); err != nil {
-		return false, err
-	} else if tmpPr.HasMerged {
-		if pr.Issue.IsClosed {
-			return false, nil
-		}
-		return false, fmt.Errorf("PullRequest[%d] already merged but it's associated issue [%d] is not closed", pr.Index, pr.IssueID)
-	} else if pr.Issue.IsClosed {
-		return false, fmt.Errorf("PullRequest[%d] already closed", pr.Index)
-	}
-
-	if err := pr.Issue.LoadRepo(ctx); err != nil {
-		return false, err
-	}
-
-	if err := pr.Issue.Repo.LoadOwner(ctx); err != nil {
-		return false, err
-	}
-
-	if _, err := changeIssueStatus(ctx, pr.Issue, pr.Merger, true, true); err != nil {
-		return false, fmt.Errorf("Issue.changeStatus: %w", err)
-	}
-
-	// reset the conflicted files as there cannot be any if we're merged
-	pr.ConflictedFiles = []string{}
-
-	// We need to save all of the data used to compute this merge as it may have already been changed by TestPatch. FIXME: need to set some state to prevent TestPatch from running whilst we are merging.
-	if _, err := sess.Where("id = ?", pr.ID).Cols("has_merged, status, merge_base, merged_commit_id, merger_id, merged_unix, conflicted_files").Update(pr); err != nil {
-		return false, fmt.Errorf("Failed to update pr[%d]: %w", pr.ID, err)
-	}
-
-	return true, nil
-}
-
 // NewPullRequest creates new pull request with labels for repository.
 func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *Issue, labelIDs []int64, uuids []string, pr *PullRequest) (err error) {
 	ctx, committer, err := db.TxContext(ctx)
@@ -572,7 +513,7 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *Iss
 	}
 
 	issue.Index = idx
-	issue.Title, _ = util.SplitStringAtByteN(issue.Title, 255)
+	issue.Title = util.EllipsisDisplayString(issue.Title, 255)
 
 	if err = NewIssueWithIndex(ctx, issue.Poster, NewIssueOptions{
 		Repo:        repo,

models/issues/pull_list.go

@@ -16,6 +16,7 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/util"
 
+	"xorm.io/builder"
 	"xorm.io/xorm"
 )
 
@@ -240,6 +241,64 @@ func (prs PullRequestList) GetIssueIDs() []int64 {
 	})
 }
 
+func (prs PullRequestList) LoadReviewCommentsCounts(ctx context.Context) (map[int64]int, error) {
+	issueIDs := prs.GetIssueIDs()
+	countsMap := make(map[int64]int, len(issueIDs))
+	counts := make([]struct {
+		IssueID int64
+		Count   int
+	}, 0, len(issueIDs))
+	if err := db.GetEngine(ctx).Select("issue_id, count(*) as count").
+		Table("comment").In("issue_id", issueIDs).And("type = ?", CommentTypeReview).
+		GroupBy("issue_id").Find(&counts); err != nil {
+		return nil, err
+	}
+	for _, c := range counts {
+		countsMap[c.IssueID] = c.Count
+	}
+	return countsMap, nil
+}
+
+func (prs PullRequestList) LoadReviews(ctx context.Context) (ReviewList, error) {
+	issueIDs := prs.GetIssueIDs()
+	reviews := make([]*Review, 0, len(issueIDs))
+
+	subQuery := builder.Select("max(id) as id").
+		From("review").
+		Where(builder.In("issue_id", issueIDs)).
+		And(builder.In("`type`", ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest)).
+		And(builder.Eq{
+			"dismissed":          false,
+			"original_author_id": 0,
+			"reviewer_team_id":   0,
+		}).
+		GroupBy("issue_id, reviewer_id")
+	// Get latest review of each reviewer, sorted in order they were made
+	if err := db.GetEngine(ctx).In("id", subQuery).OrderBy("review.updated_unix ASC").Find(&reviews); err != nil {
+		return nil, err
+	}
+
+	teamReviewRequests := make([]*Review, 0, 5)
+	subQueryTeam := builder.Select("max(id) as id").
+		From("review").
+		Where(builder.In("issue_id", issueIDs)).
+		And(builder.Eq{
+			"original_author_id": 0,
+		}).And(builder.Neq{
+		"reviewer_team_id": 0,
+	}).
+		GroupBy("issue_id, reviewer_team_id")
+	if err := db.GetEngine(ctx).In("id", subQueryTeam).OrderBy("review.updated_unix ASC").Find(&teamReviewRequests); err != nil {
+		return nil, err
+	}
+
+	if len(teamReviewRequests) > 0 {
+		reviews = append(reviews, teamReviewRequests...)
+	}
+
+	return reviews, nil
+}
+
 // HasMergedPullRequestInRepo returns whether the user(poster) has merged pull-request in the repo
 func HasMergedPullRequestInRepo(ctx context.Context, repoID, posterID int64) (bool, error) {
 	return db.GetEngine(ctx).

models/issues/pull_list_test.go

@@ -0,0 +1,64 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package issues_test
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/db"
+	issues_model "code.gitea.io/gitea/models/issues"
+	"code.gitea.io/gitea/models/unittest"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPullRequestList_LoadAttributes(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	prs := []*issues_model.PullRequest{
+		unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1}),
+		unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}),
+	}
+	assert.NoError(t, issues_model.PullRequestList(prs).LoadAttributes(db.DefaultContext))
+	for _, pr := range prs {
+		assert.NotNil(t, pr.Issue)
+		assert.Equal(t, pr.IssueID, pr.Issue.ID)
+	}
+
+	assert.NoError(t, issues_model.PullRequestList([]*issues_model.PullRequest{}).LoadAttributes(db.DefaultContext))
+}
+
+func TestPullRequestList_LoadReviewCommentsCounts(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	prs := []*issues_model.PullRequest{
+		unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1}),
+		unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}),
+	}
+	reviewComments, err := issues_model.PullRequestList(prs).LoadReviewCommentsCounts(db.DefaultContext)
+	assert.NoError(t, err)
+	assert.Len(t, reviewComments, 2)
+	for _, pr := range prs {
+		assert.EqualValues(t, 1, reviewComments[pr.IssueID])
+	}
+}
+
+func TestPullRequestList_LoadReviews(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	prs := []*issues_model.PullRequest{
+		unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1}),
+		unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}),
+	}
+	reviewList, err := issues_model.PullRequestList(prs).LoadReviews(db.DefaultContext)
+	assert.NoError(t, err)
+	// 1, 7, 8, 9, 10, 22
+	assert.Len(t, reviewList, 6)
+	assert.EqualValues(t, 1, reviewList[0].ID)
+	assert.EqualValues(t, 7, reviewList[1].ID)
+	assert.EqualValues(t, 8, reviewList[2].ID)
+	assert.EqualValues(t, 9, reviewList[3].ID)
+	assert.EqualValues(t, 10, reviewList[4].ID)
+	assert.EqualValues(t, 22, reviewList[5].ID)
+}

models/issues/pull_test.go

@@ -79,11 +79,11 @@ func TestPullRequestsNewest(t *testing.T) {
 func TestLoadRequestedReviewers(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 
-	pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1})
+	pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
 	assert.NoError(t, pull.LoadIssue(db.DefaultContext))
 	issue := pull.Issue
 	assert.NoError(t, issue.LoadRepo(db.DefaultContext))
-	assert.Len(t, pull.RequestedReviewers, 0)
+	assert.Empty(t, pull.RequestedReviewers)
 
 	user1, err := user_model.GetUserByID(db.DefaultContext, 1)
 	assert.NoError(t, err)
@@ -93,7 +93,7 @@ func TestLoadRequestedReviewers(t *testing.T) {
 	assert.NotNil(t, comment)
 
 	assert.NoError(t, pull.LoadRequestedReviewers(db.DefaultContext))
-	assert.Len(t, pull.RequestedReviewers, 1)
+	assert.Len(t, pull.RequestedReviewers, 6)
 
 	comment, err = issues_model.RemoveReviewRequest(db.DefaultContext, issue, user1, &user_model.User{})
 	assert.NoError(t, err)
@@ -234,22 +234,6 @@ func TestPullRequest_UpdateCols(t *testing.T) {
 	unittest.CheckConsistencyFor(t, pr)
 }
 
-func TestPullRequestList_LoadAttributes(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	prs := []*issues_model.PullRequest{
-		unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1}),
-		unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}),
-	}
-	assert.NoError(t, issues_model.PullRequestList(prs).LoadAttributes(db.DefaultContext))
-	for _, pr := range prs {
-		assert.NotNil(t, pr.Issue)
-		assert.Equal(t, pr.IssueID, pr.Issue.ID)
-	}
-
-	assert.NoError(t, issues_model.PullRequestList([]*issues_model.PullRequest{}).LoadAttributes(db.DefaultContext))
-}
-
 // TODO TestAddTestPullRequestTask
 
 func TestPullRequest_IsWorkInProgress(t *testing.T) {

models/issues/review.go

@@ -639,6 +639,10 @@ func InsertReviews(ctx context.Context, reviews []*Review) error {
 				return err
 			}
 		}
+
+		if err := UpdateIssueNumComments(ctx, review.IssueID); err != nil {
+			return err
+		}
 	}
 
 	return committer.Commit()

models/issues/review_list.go

@@ -47,14 +47,9 @@ func (reviews ReviewList) LoadReviewersTeams(ctx context.Context) error {
 		}
 	}
 
-	teamsMap := make(map[int64]*organization_model.Team, 0)
-	for _, teamID := range reviewersTeamsIDs {
-		team, err := organization_model.GetTeamByID(ctx, teamID)
-		if err != nil {
-			return err
-		}
-
-		teamsMap[teamID] = team
+	teamsMap, err := organization_model.GetTeamsByIDs(ctx, reviewersTeamsIDs)
+	if err != nil {
+		return err
 	}
 
 	for _, review := range reviews {

models/issues/review_test.go

@@ -126,42 +126,48 @@ func TestGetReviewersByIssueID(t *testing.T) {
 	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
 	org3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3})
 	user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
+	user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
 
 	expectedReviews := []*issues_model.Review{}
 	expectedReviews = append(expectedReviews,
 		&issues_model.Review{
+			ID:          7,
 			Reviewer:    org3,
 			Type:        issues_model.ReviewTypeReject,
 			UpdatedUnix: 946684812,
 		},
 		&issues_model.Review{
+			ID:          8,
 			Reviewer:    user4,
 			Type:        issues_model.ReviewTypeApprove,
 			UpdatedUnix: 946684813,
 		},
 		&issues_model.Review{
+			ID:          9,
 			Reviewer:    user2,
 			Type:        issues_model.ReviewTypeReject,
 			UpdatedUnix: 946684814,
-		})
+		},
+		&issues_model.Review{
+			ID:          10,
+			Reviewer:    user_model.NewGhostUser(),
+			Type:        issues_model.ReviewTypeReject,
+			UpdatedUnix: 946684815,
+		},
+		&issues_model.Review{
+			ID:          22,
+			Reviewer:    user5,
+			Type:        issues_model.ReviewTypeRequest,
+			UpdatedUnix: 946684817,
+		},
+	)
 
 	allReviews, err := issues_model.GetReviewsByIssueID(db.DefaultContext, issue.ID)
 	assert.NoError(t, err)
 	for _, review := range allReviews {
 		assert.NoError(t, review.LoadReviewer(db.DefaultContext))
 	}
-	if assert.Len(t, allReviews, 3) {
-		for i, review := range allReviews {
-			assert.Equal(t, expectedReviews[i].Reviewer, review.Reviewer)
-			assert.Equal(t, expectedReviews[i].Type, review.Type)
-			assert.Equal(t, expectedReviews[i].UpdatedUnix, review.UpdatedUnix)
-		}
-	}
-
-	allReviews, err = issues_model.GetReviewsByIssueID(db.DefaultContext, issue.ID)
-	assert.NoError(t, err)
-	assert.NoError(t, allReviews.LoadReviewers(db.DefaultContext))
-	if assert.Len(t, allReviews, 3) {
+	if assert.Len(t, allReviews, 5) {
 		for i, review := range allReviews {
 			assert.Equal(t, expectedReviews[i].Reviewer, review.Reviewer)
 			assert.Equal(t, expectedReviews[i].Type, review.Type)

models/issues/stopwatch_test.go

@@ -32,7 +32,7 @@ func TestCancelStopwatch(t *testing.T) {
 
 	_ = unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{Type: issues_model.CommentTypeCancelTracking, PosterID: user1.ID, IssueID: issue1.ID})
 
-	assert.Nil(t, issues_model.CancelStopwatch(db.DefaultContext, user1, issue2))
+	assert.NoError(t, issues_model.CancelStopwatch(db.DefaultContext, user1, issue2))
 }
 
 func TestStopwatchExists(t *testing.T) {

models/issues/tracked_time_test.go

@@ -50,7 +50,7 @@ func TestGetTrackedTimes(t *testing.T) {
 
 	times, err = issues_model.GetTrackedTimes(db.DefaultContext, &issues_model.FindTrackedTimesOptions{IssueID: -1})
 	assert.NoError(t, err)
-	assert.Len(t, times, 0)
+	assert.Empty(t, times)
 
 	// by User
 	times, err = issues_model.GetTrackedTimes(db.DefaultContext, &issues_model.FindTrackedTimesOptions{UserID: 1})
@@ -60,7 +60,7 @@ func TestGetTrackedTimes(t *testing.T) {
 
 	times, err = issues_model.GetTrackedTimes(db.DefaultContext, &issues_model.FindTrackedTimesOptions{UserID: 3})
 	assert.NoError(t, err)
-	assert.Len(t, times, 0)
+	assert.Empty(t, times)
 
 	// by Repo
 	times, err = issues_model.GetTrackedTimes(db.DefaultContext, &issues_model.FindTrackedTimesOptions{RepositoryID: 2})
@@ -69,7 +69,7 @@ func TestGetTrackedTimes(t *testing.T) {
 	assert.Equal(t, int64(1), times[0].Time)
 	issue, err := issues_model.GetIssueByID(db.DefaultContext, times[0].IssueID)
 	assert.NoError(t, err)
-	assert.Equal(t, issue.RepoID, int64(2))
+	assert.Equal(t, int64(2), issue.RepoID)
 
 	times, err = issues_model.GetTrackedTimes(db.DefaultContext, &issues_model.FindTrackedTimesOptions{RepositoryID: 1})
 	assert.NoError(t, err)
@@ -77,7 +77,7 @@ func TestGetTrackedTimes(t *testing.T) {
 
 	times, err = issues_model.GetTrackedTimes(db.DefaultContext, &issues_model.FindTrackedTimesOptions{RepositoryID: 10})
 	assert.NoError(t, err)
-	assert.Len(t, times, 0)
+	assert.Empty(t, times)
 }
 
 func TestTotalTimesForEachUser(t *testing.T) {