mirror of
				https://github.com/go-gitea/gitea.git
				synced 2025-10-31 03:25:11 +01:00 
			
		
		
		
	Backport #30556 by @lunny Fix #22419 Only comments with types `CommentTypeComment` and `CommentTypeReview` will be counted as conversations of the pull request. `CommentTypeReview` was missed in the previous implementation. Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
This commit is contained in:
		
							parent
							
								
									27de60381d
								
							
						
					
					
						commit
						968c04c7da
					
				| @ -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 | ||||
| 
 | ||||
| @ -893,7 +907,7 @@ 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 | ||||
| @ -1182,8 +1196,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 +1314,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 +1361,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 | ||||
| 		} | ||||
| 	} | ||||
|  | ||||
| @ -97,3 +97,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) | ||||
| } | ||||
|  | ||||
| @ -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() | ||||
|  | ||||
| @ -19,6 +19,8 @@ import ( | ||||
| 	"code.gitea.io/gitea/models/unit" | ||||
| 	user_model "code.gitea.io/gitea/models/user" | ||||
| 	"code.gitea.io/gitea/modules/log" | ||||
| 
 | ||||
| 	"xorm.io/builder" | ||||
| ) | ||||
| 
 | ||||
| // Init initialize model | ||||
| @ -27,7 +29,7 @@ func Init(ctx context.Context) error { | ||||
| } | ||||
| 
 | ||||
| type repoChecker struct { | ||||
| 	querySQL   func(ctx context.Context) ([]map[string][]byte, error) | ||||
| 	querySQL   func(ctx context.Context) ([]int64, error) | ||||
| 	correctSQL func(ctx context.Context, id int64) error | ||||
| 	desc       string | ||||
| } | ||||
| @ -38,8 +40,7 @@ func repoStatsCheck(ctx context.Context, checker *repoChecker) { | ||||
| 		log.Error("Select %s: %v", checker.desc, err) | ||||
| 		return | ||||
| 	} | ||||
| 	for _, result := range results { | ||||
| 		id, _ := strconv.ParseInt(string(result["id"]), 10, 64) | ||||
| 	for _, id := range results { | ||||
| 		select { | ||||
| 		case <-ctx.Done(): | ||||
| 			log.Warn("CheckRepoStats: Cancelled before checking %s for with id=%d", checker.desc, id) | ||||
| @ -54,21 +55,23 @@ func repoStatsCheck(ctx context.Context, checker *repoChecker) { | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| func StatsCorrectSQL(ctx context.Context, sql string, id int64) error { | ||||
| 	_, err := db.GetEngine(ctx).Exec(sql, id, id) | ||||
| func StatsCorrectSQL(ctx context.Context, sql any, ids ...any) error { | ||||
| 	args := []any{sql} | ||||
| 	args = append(args, ids...) | ||||
| 	_, err := db.GetEngine(ctx).Exec(args...) | ||||
| 	return err | ||||
| } | ||||
| 
 | ||||
| func repoStatsCorrectNumWatches(ctx context.Context, id int64) error { | ||||
| 	return StatsCorrectSQL(ctx, "UPDATE `repository` SET num_watches=(SELECT COUNT(*) FROM `watch` WHERE repo_id=? AND mode<>2) WHERE id=?", id) | ||||
| 	return StatsCorrectSQL(ctx, "UPDATE `repository` SET num_watches=(SELECT COUNT(*) FROM `watch` WHERE repo_id=? AND mode<>2) WHERE id=?", id, id) | ||||
| } | ||||
| 
 | ||||
| func repoStatsCorrectNumStars(ctx context.Context, id int64) error { | ||||
| 	return StatsCorrectSQL(ctx, "UPDATE `repository` SET num_stars=(SELECT COUNT(*) FROM `star` WHERE repo_id=?) WHERE id=?", id) | ||||
| 	return StatsCorrectSQL(ctx, "UPDATE `repository` SET num_stars=(SELECT COUNT(*) FROM `star` WHERE repo_id=?) WHERE id=?", id, id) | ||||
| } | ||||
| 
 | ||||
| func labelStatsCorrectNumIssues(ctx context.Context, id int64) error { | ||||
| 	return StatsCorrectSQL(ctx, "UPDATE `label` SET num_issues=(SELECT COUNT(*) FROM `issue_label` WHERE label_id=?) WHERE id=?", id) | ||||
| 	return StatsCorrectSQL(ctx, "UPDATE `label` SET num_issues=(SELECT COUNT(*) FROM `issue_label` WHERE label_id=?) WHERE id=?", id, id) | ||||
| } | ||||
| 
 | ||||
| func labelStatsCorrectNumIssuesRepo(ctx context.Context, id int64) error { | ||||
| @ -105,11 +108,11 @@ func milestoneStatsCorrectNumIssuesRepo(ctx context.Context, id int64) error { | ||||
| } | ||||
| 
 | ||||
| func userStatsCorrectNumRepos(ctx context.Context, id int64) error { | ||||
| 	return StatsCorrectSQL(ctx, "UPDATE `user` SET num_repos=(SELECT COUNT(*) FROM `repository` WHERE owner_id=?) WHERE id=?", id) | ||||
| 	return StatsCorrectSQL(ctx, "UPDATE `user` SET num_repos=(SELECT COUNT(*) FROM `repository` WHERE owner_id=?) WHERE id=?", id, id) | ||||
| } | ||||
| 
 | ||||
| func repoStatsCorrectIssueNumComments(ctx context.Context, id int64) error { | ||||
| 	return StatsCorrectSQL(ctx, "UPDATE `issue` SET num_comments=(SELECT COUNT(*) FROM `comment` WHERE issue_id=? AND type=0) WHERE id=?", id) | ||||
| 	return StatsCorrectSQL(ctx, issues_model.UpdateIssueNumCommentsBuilder(id)) | ||||
| } | ||||
| 
 | ||||
| func repoStatsCorrectNumIssues(ctx context.Context, id int64) error { | ||||
| @ -128,9 +131,12 @@ func repoStatsCorrectNumClosedPulls(ctx context.Context, id int64) error { | ||||
| 	return repo_model.UpdateRepoIssueNumbers(ctx, id, true, true) | ||||
| } | ||||
| 
 | ||||
| func statsQuery(args ...any) func(context.Context) ([]map[string][]byte, error) { | ||||
| 	return func(ctx context.Context) ([]map[string][]byte, error) { | ||||
| 		return db.GetEngine(ctx).Query(args...) | ||||
| // statsQuery returns a function that queries the database for a list of IDs | ||||
| // sql could be a string or a *builder.Builder | ||||
| func statsQuery(sql any, args ...any) func(context.Context) ([]int64, error) { | ||||
| 	return func(ctx context.Context) ([]int64, error) { | ||||
| 		var ids []int64 | ||||
| 		return ids, db.GetEngine(ctx).SQL(sql, args...).Find(&ids) | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| @ -201,7 +207,16 @@ func CheckRepoStats(ctx context.Context) error { | ||||
| 		}, | ||||
| 		// Issue.NumComments | ||||
| 		{ | ||||
| 			statsQuery("SELECT `issue`.id FROM `issue` WHERE `issue`.num_comments!=(SELECT COUNT(*) FROM `comment` WHERE issue_id=`issue`.id AND type=0)"), | ||||
| 			statsQuery(builder.Select("`issue`.id").From("`issue`").Where( | ||||
| 				builder.Neq{ | ||||
| 					"`issue`.num_comments": builder.Select("COUNT(*)").From("`comment`").Where( | ||||
| 						builder.Expr("issue_id = `issue`.id").And( | ||||
| 							builder.In("type", issues_model.ConversationCountedCommentType()), | ||||
| 						), | ||||
| 					), | ||||
| 				}, | ||||
| 			), | ||||
| 			), | ||||
| 			repoStatsCorrectIssueNumComments, | ||||
| 			"issue count 'num_comments'", | ||||
| 		}, | ||||
|  | ||||
| @ -7,6 +7,7 @@ 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" | ||||
| @ -22,3 +23,16 @@ func TestDoctorUserStarNum(t *testing.T) { | ||||
| 
 | ||||
| 	assert.NoError(t, DoctorUserStarNum(db.DefaultContext)) | ||||
| } | ||||
| 
 | ||||
| func Test_repoStatsCorrectIssueNumComments(t *testing.T) { | ||||
| 	assert.NoError(t, unittest.PrepareTestDatabase()) | ||||
| 
 | ||||
| 	issue2 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2}) | ||||
| 	assert.NotNil(t, issue2) | ||||
| 	assert.EqualValues(t, 0, issue2.NumComments) // the fixture data is wrong, but we don't fix it here | ||||
| 
 | ||||
| 	assert.NoError(t, repoStatsCorrectIssueNumComments(db.DefaultContext, 2)) | ||||
| 	// reload the issue | ||||
| 	issue2 = unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2}) | ||||
| 	assert.EqualValues(t, 1, issue2.NumComments) | ||||
| } | ||||
|  | ||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user