From d3770c838400e0bb51b09598dec8b00b78d16b6c Mon Sep 17 00:00:00 2001 From: bimakw <51526537+bimakw@users.noreply.github.com> Date: Sun, 11 Jan 2026 11:01:16 +0700 Subject: [PATCH 1/8] refactor: use comment metadata for CODEOWNERS review attribution Revised implementation based on maintainer feedback. Instead of introducing a new system user (CodeOwnersUser), this uses CommentMetaData to mark CODEOWNERS-triggered review requests. Changes: - Removed CodeOwnersUser system user (ID -3) - Added IsCodeOwnersReviewRequest field to CommentMetaData - Created AddCodeOwnersReviewRequest/AddCodeOwnersTeamReviewRequest functions that set the metadata flag - Updated template to check CommentMetaData instead of Poster - Message now shows "X was requested to review due to CODEOWNERS rules" This approach is simpler and doesn't require adding a new system user just for comment attribution. Fixes #36333 --- models/issues/comment.go | 79 +++++------ models/issues/review.go | 124 ++++++++++++++++++ options/locale/locale_en-US.json | 1 + services/issue/pull.go | 4 +- .../repo/issue/view_content/comments.tmpl | 60 +++++---- 5 files changed, 206 insertions(+), 62 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index fd0500833e751..e0075da4a0639 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -235,9 +235,10 @@ func (r RoleInRepo) LocaleHelper(lang translation.Locale) string { // CommentMetaData stores metadata for a comment, these data will not be changed once inserted into database type CommentMetaData struct { - ProjectColumnID int64 `json:"project_column_id,omitempty"` - ProjectColumnTitle string `json:"project_column_title,omitempty"` - ProjectTitle string `json:"project_title,omitempty"` + ProjectColumnID int64 `json:"project_column_id,omitempty"` + ProjectColumnTitle string `json:"project_column_title,omitempty"` + ProjectTitle string `json:"project_title,omitempty"` + IsCodeOwnersReviewRequest bool `json:"is_code_owners_review_request,omitempty"` } // Comment represents a comment in commit and issue page. @@ -773,11 +774,12 @@ func CreateComment(ctx context.Context, opts *CreateCommentOptions) (_ *Comment, } var commentMetaData *CommentMetaData - if opts.ProjectColumnTitle != "" { + if opts.ProjectColumnTitle != "" || opts.IsCodeOwnersReviewRequest { commentMetaData = &CommentMetaData{ - ProjectColumnID: opts.ProjectColumnID, - ProjectColumnTitle: opts.ProjectColumnTitle, - ProjectTitle: opts.ProjectTitle, + ProjectColumnID: opts.ProjectColumnID, + ProjectColumnTitle: opts.ProjectColumnTitle, + ProjectTitle: opts.ProjectTitle, + IsCodeOwnersReviewRequest: opts.IsCodeOwnersReviewRequest, } } @@ -945,37 +947,38 @@ type CreateCommentOptions struct { Issue *Issue Label *Label - DependentIssueID int64 - OldMilestoneID int64 - MilestoneID int64 - OldProjectID int64 - ProjectID int64 - ProjectTitle string - ProjectColumnID int64 - ProjectColumnTitle string - TimeID int64 - AssigneeID int64 - AssigneeTeamID int64 - RemovedAssignee bool - OldTitle string - NewTitle string - OldRef string - NewRef string - CommitID int64 - CommitSHA string - Patch string - LineNum int64 - TreePath string - ReviewID int64 - Content string - Attachments []string // UUIDs of attachments - RefRepoID int64 - RefIssueID int64 - RefCommentID int64 - RefAction references.XRefAction - RefIsPull bool - IsForcePush bool - Invalidated bool + DependentIssueID int64 + OldMilestoneID int64 + MilestoneID int64 + OldProjectID int64 + ProjectID int64 + ProjectTitle string + ProjectColumnID int64 + ProjectColumnTitle string + TimeID int64 + AssigneeID int64 + AssigneeTeamID int64 + RemovedAssignee bool + OldTitle string + NewTitle string + OldRef string + NewRef string + CommitID int64 + CommitSHA string + Patch string + LineNum int64 + TreePath string + ReviewID int64 + Content string + Attachments []string // UUIDs of attachments + RefRepoID int64 + RefIssueID int64 + RefCommentID int64 + RefAction references.XRefAction + RefIsPull bool + IsForcePush bool + Invalidated bool + IsCodeOwnersReviewRequest bool } // GetCommentByID returns the comment by given ID. diff --git a/models/issues/review.go b/models/issues/review.go index b758fa5ffac63..963b8eedf9f8b 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -876,6 +876,130 @@ func RemoveTeamReviewRequest(ctx context.Context, issue *Issue, reviewer *organi }) } +// AddCodeOwnersReviewRequest adds a review request from CODEOWNERS rules. +// It works like AddReviewRequest but marks the comment as CODEOWNERS-triggered. +func AddCodeOwnersReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_model.User) (*Comment, error) { + return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) { + review, err := GetReviewByIssueIDAndUserID(ctx, issue.ID, reviewer.ID) + if err != nil && !IsErrReviewNotExist(err) { + return nil, err + } + + if review != nil { + // skip it when reviewer has been request to review + if review.Type == ReviewTypeRequest { + return nil, nil + } + + if issue.IsClosed { + return nil, ErrReviewRequestOnClosedPR{} + } + + if issue.IsPull { + if err := issue.LoadPullRequest(ctx); err != nil { + return nil, err + } + if issue.PullRequest.HasMerged { + return nil, ErrReviewRequestOnClosedPR{} + } + } + } + + official, err := IsOfficialReviewer(ctx, issue, reviewer) + if err != nil { + return nil, err + } else if official { + if _, err := db.GetEngine(ctx).Exec("UPDATE `review` SET official=? WHERE issue_id=? AND reviewer_id=?", false, issue.ID, reviewer.ID); err != nil { + return nil, err + } + } + + review, err = CreateReview(ctx, CreateReviewOptions{ + Type: ReviewTypeRequest, + Issue: issue, + Reviewer: reviewer, + Official: official, + Stale: false, + }) + if err != nil { + return nil, err + } + + comment, err := CreateComment(ctx, &CreateCommentOptions{ + Type: CommentTypeReviewRequest, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + RemovedAssignee: false, + AssigneeID: reviewer.ID, + ReviewID: review.ID, + IsCodeOwnersReviewRequest: true, + }) + if err != nil { + return nil, err + } + + comment.Review = review + return comment, nil + }) +} + +// AddCodeOwnersTeamReviewRequest adds a team review request from CODEOWNERS rules. +// It works like AddTeamReviewRequest but marks the comment as CODEOWNERS-triggered. +func AddCodeOwnersTeamReviewRequest(ctx context.Context, issue *Issue, reviewer *organization.Team, doer *user_model.User) (*Comment, error) { + return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) { + review, err := GetTeamReviewerByIssueIDAndTeamID(ctx, issue.ID, reviewer.ID) + if err != nil && !IsErrReviewNotExist(err) { + return nil, err + } + + if review != nil { + return nil, nil + } + + official, err := IsOfficialReviewerTeam(ctx, issue, reviewer) + if err != nil { + return nil, fmt.Errorf("isOfficialReviewerTeam(): %w", err) + } else if !official { + if official, err = IsOfficialReviewer(ctx, issue, doer); err != nil { + return nil, fmt.Errorf("isOfficialReviewer(): %w", err) + } + } + + if review, err = CreateReview(ctx, CreateReviewOptions{ + Type: ReviewTypeRequest, + Issue: issue, + ReviewerTeam: reviewer, + Official: official, + Stale: false, + }); err != nil { + return nil, err + } + + if official { + if _, err := db.Exec(ctx, "UPDATE `review` SET official=? WHERE issue_id=? AND reviewer_team_id=?", false, issue.ID, reviewer.ID); err != nil { + return nil, err + } + } + + comment, err := CreateComment(ctx, &CreateCommentOptions{ + Type: CommentTypeReviewRequest, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + RemovedAssignee: false, + AssigneeTeamID: reviewer.ID, + ReviewID: review.ID, + IsCodeOwnersReviewRequest: true, + }) + if err != nil { + return nil, fmt.Errorf("CreateComment(): %w", err) + } + + return comment, nil + }) +} + // MarkConversation Add or remove Conversation mark for a code comment func MarkConversation(ctx context.Context, comment *Comment, doer *user_model.User, isResolve bool) (err error) { if comment.Type != CommentTypeCode { diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 96a541a9476ca..6d3608c1acc36 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -1702,6 +1702,7 @@ "repo.issues.review.reject": "requested changes %s", "repo.issues.review.wait": "was requested for review %s", "repo.issues.review.add_review_request": "requested review from %s %s", + "repo.issues.review.add_review_request_codeowners": "%s was requested to review due to CODEOWNERS rules %s", "repo.issues.review.remove_review_request": "removed review request for %s %s", "repo.issues.review.remove_review_request_self": "declined to review %s", "repo.issues.review.pending": "Pending", diff --git a/services/issue/pull.go b/services/issue/pull.go index 8ee14c0a4b5ae..703a66eea4d69 100644 --- a/services/issue/pull.go +++ b/services/issue/pull.go @@ -149,7 +149,7 @@ func PullRequestCodeOwnersReview(ctx context.Context, pr *issues_model.PullReque for _, u := range uniqUsers { if u.ID != issue.Poster.ID && !contain(latestReivews, u) { - comment, err := issues_model.AddReviewRequest(ctx, issue, u, issue.Poster) + comment, err := issues_model.AddCodeOwnersReviewRequest(ctx, issue, u, issue.Poster) if err != nil { log.Warn("Failed add assignee user: %s to PR review: %s#%d, error: %s", u.Name, pr.BaseRepo.Name, pr.ID, err) return nil, err @@ -166,7 +166,7 @@ func PullRequestCodeOwnersReview(ctx context.Context, pr *issues_model.PullReque } for _, t := range uniqTeams { - comment, err := issues_model.AddTeamReviewRequest(ctx, issue, t, issue.Poster) + comment, err := issues_model.AddCodeOwnersTeamReviewRequest(ctx, issue, t, issue.Poster) if err != nil { log.Warn("Failed add assignee team: %s to PR review: %s#%d, error: %s", t.Name, pr.BaseRepo.Name, pr.ID, err) return nil, err diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 6d23186d08f64..68475a72602b2 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -514,32 +514,48 @@ {{else if eq .Type 27}}
{{svg "octicon-eye"}} - {{template "shared/user/avatarlink" dict "user" .Poster}} - - {{template "shared/user/authorlink" .Poster}} - {{if (gt .AssigneeID 0)}} - {{if .RemovedAssignee}} - {{if eq .PosterID .AssigneeID}} - {{ctx.Locale.Tr "repo.issues.review.remove_review_request_self" $createdStr}} - {{else}} - {{ctx.Locale.Tr "repo.issues.review.remove_review_request" .Assignee.GetDisplayName $createdStr}} - {{end}} + {{if and .CommentMetaData .CommentMetaData.IsCodeOwnersReviewRequest}} + {{/* CODEOWNERS-triggered review request - show different message */}} + {{svg "octicon-file-code" 16}} + + {{if (gt .AssigneeID 0)}} + {{ctx.Locale.Tr "repo.issues.review.add_review_request_codeowners" .Assignee.GetDisplayName $createdStr}} {{else}} - {{ctx.Locale.Tr "repo.issues.review.add_review_request" .Assignee.GetDisplayName $createdStr}} - {{end}} - {{else}} - - {{$teamName := "Ghost Team"}} - {{if .AssigneeTeam}} - {{$teamName = .AssigneeTeam.Name}} + {{$teamName := "Ghost Team"}} + {{if .AssigneeTeam}} + {{$teamName = .AssigneeTeam.Name}} + {{end}} + {{ctx.Locale.Tr "repo.issues.review.add_review_request_codeowners" $teamName $createdStr}} {{end}} - {{if .RemovedAssignee}} - {{ctx.Locale.Tr "repo.issues.review.remove_review_request" $teamName $createdStr}} + + {{else}} + {{template "shared/user/avatarlink" dict "user" .Poster}} + + {{template "shared/user/authorlink" .Poster}} + {{if (gt .AssigneeID 0)}} + {{if .RemovedAssignee}} + {{if eq .PosterID .AssigneeID}} + {{ctx.Locale.Tr "repo.issues.review.remove_review_request_self" $createdStr}} + {{else}} + {{ctx.Locale.Tr "repo.issues.review.remove_review_request" .Assignee.GetDisplayName $createdStr}} + {{end}} + {{else}} + {{ctx.Locale.Tr "repo.issues.review.add_review_request" .Assignee.GetDisplayName $createdStr}} + {{end}} {{else}} - {{ctx.Locale.Tr "repo.issues.review.add_review_request" $teamName $createdStr}} + + {{$teamName := "Ghost Team"}} + {{if .AssigneeTeam}} + {{$teamName = .AssigneeTeam.Name}} + {{end}} + {{if .RemovedAssignee}} + {{ctx.Locale.Tr "repo.issues.review.remove_review_request" $teamName $createdStr}} + {{else}} + {{ctx.Locale.Tr "repo.issues.review.add_review_request" $teamName $createdStr}} + {{end}} {{end}} - {{end}} - + + {{end}}
{{else if and (eq .Type 29) (or (gt .CommitsNum 0) .IsForcePush)}} From ae1e4b6d04803d69f296d2e91c2d5d4793e949df Mon Sep 17 00:00:00 2001 From: Gregorius Bima Kharisma Wicaksana <51526537+bimakw@users.noreply.github.com> Date: Thu, 15 Jan 2026 16:35:19 +0700 Subject: [PATCH 2/8] test: add tests for AddCodeOwnersReviewRequest functions --- models/issues/review_test.go | 165 +++++++++++++++++++++++++++++++++++ 1 file changed, 165 insertions(+) diff --git a/models/issues/review_test.go b/models/issues/review_test.go index 6795ea8e661ce..5361eb69812e9 100644 --- a/models/issues/review_test.go +++ b/models/issues/review_test.go @@ -7,6 +7,7 @@ import ( "testing" issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/models/organization" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" @@ -332,3 +333,167 @@ func TestAddReviewRequest(t *testing.T) { assert.Error(t, err) assert.True(t, issues_model.IsErrReviewRequestOnClosedPR(err)) } + +func TestAddCodeOwnersReviewRequest(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + assert.NoError(t, pull.LoadIssue(t.Context())) + issue := pull.Issue + assert.NoError(t, issue.LoadRepo(t.Context())) + + reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // Test: Add CODEOWNERS review request + comment, err := issues_model.AddCodeOwnersReviewRequest(t.Context(), issue, reviewer, doer) + assert.NoError(t, err) + assert.NotNil(t, comment) + + // Assert: Comment created with correct type + assert.Equal(t, issues_model.CommentTypeReviewRequest, comment.Type) + assert.Equal(t, reviewer.ID, comment.AssigneeID) + assert.Equal(t, doer.ID, comment.PosterID) + + // Assert: Review created + assert.NotNil(t, comment.Review) + assert.Equal(t, issues_model.ReviewTypeRequest, comment.Review.Type) + assert.Equal(t, reviewer.ID, comment.Review.ReviewerID) + + // Assert: Metadata marked as CODEOWNERS request + assert.NotNil(t, comment.CommentMetaData) + assert.True(t, comment.CommentMetaData.IsCodeOwnersReviewRequest) + + // Assert: Verify persisted to database + savedComment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: comment.ID}) + assert.NotNil(t, savedComment.CommentMetaData) + assert.True(t, savedComment.CommentMetaData.IsCodeOwnersReviewRequest) +} + +func TestAddCodeOwnersReviewRequest_SkipExisting(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + assert.NoError(t, pull.LoadIssue(t.Context())) + issue := pull.Issue + assert.NoError(t, issue.LoadRepo(t.Context())) + + reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // Create first review request + comment1, err := issues_model.AddCodeOwnersReviewRequest(t.Context(), issue, reviewer, doer) + assert.NoError(t, err) + assert.NotNil(t, comment1) + + // Try to create duplicate - should skip and return nil + comment2, err := issues_model.AddCodeOwnersReviewRequest(t.Context(), issue, reviewer, doer) + assert.NoError(t, err) + assert.Nil(t, comment2) +} + +func TestAddCodeOwnersReviewRequest_ClosedPR(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + assert.NoError(t, pull.LoadIssue(t.Context())) + issue := pull.Issue + assert.NoError(t, issue.LoadRepo(t.Context())) + + reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // Create existing review (non-request type) so the closed check is triggered + _, err := issues_model.CreateReview(t.Context(), issues_model.CreateReviewOptions{ + Issue: issue, + Reviewer: reviewer, + Type: issues_model.ReviewTypeReject, + }) + assert.NoError(t, err) + + // Close the issue + issue.IsClosed = true + + // Try to add CODEOWNERS request - should error + comment, err := issues_model.AddCodeOwnersReviewRequest(t.Context(), issue, reviewer, doer) + assert.Error(t, err) + assert.True(t, issues_model.IsErrReviewRequestOnClosedPR(err)) + assert.Nil(t, comment) +} + +func TestAddCodeOwnersReviewRequest_MergedPR(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + assert.NoError(t, pull.LoadIssue(t.Context())) + issue := pull.Issue + assert.NoError(t, issue.LoadRepo(t.Context())) + + reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // Create existing review so the merged check is triggered + _, err := issues_model.CreateReview(t.Context(), issues_model.CreateReviewOptions{ + Issue: issue, + Reviewer: reviewer, + Type: issues_model.ReviewTypeApprove, + }) + assert.NoError(t, err) + + // Mark PR as merged + pull.HasMerged = true + assert.NoError(t, pull.UpdateCols(t.Context(), "has_merged")) + + // Try to add CODEOWNERS request - should error + comment, err := issues_model.AddCodeOwnersReviewRequest(t.Context(), issue, reviewer, doer) + assert.Error(t, err) + assert.True(t, issues_model.IsErrReviewRequestOnClosedPR(err)) + assert.Nil(t, comment) +} + +func TestAddCodeOwnersTeamReviewRequest(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + assert.NoError(t, pull.LoadIssue(t.Context())) + issue := pull.Issue + assert.NoError(t, issue.LoadRepo(t.Context())) + + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // Test: Add CODEOWNERS team review request + comment, err := issues_model.AddCodeOwnersTeamReviewRequest(t.Context(), issue, team, doer) + assert.NoError(t, err) + assert.NotNil(t, comment) + + // Assert: Comment created with correct type + assert.Equal(t, issues_model.CommentTypeReviewRequest, comment.Type) + assert.Equal(t, team.ID, comment.AssigneeTeamID) + + // Assert: Metadata marked as CODEOWNERS request + assert.NotNil(t, comment.CommentMetaData) + assert.True(t, comment.CommentMetaData.IsCodeOwnersReviewRequest) +} + +func TestAddCodeOwnersTeamReviewRequest_SkipExisting(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + assert.NoError(t, pull.LoadIssue(t.Context())) + issue := pull.Issue + assert.NoError(t, issue.LoadRepo(t.Context())) + + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 1}) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // Create first team review request + comment1, err := issues_model.AddCodeOwnersTeamReviewRequest(t.Context(), issue, team, doer) + assert.NoError(t, err) + assert.NotNil(t, comment1) + + // Try to create duplicate - should skip and return nil + comment2, err := issues_model.AddCodeOwnersTeamReviewRequest(t.Context(), issue, team, doer) + assert.NoError(t, err) + assert.Nil(t, comment2) +} From 8710fbf380c9655ef3fda73db3cc30729418226c Mon Sep 17 00:00:00 2001 From: Gregorius Bima Kharisma Wicaksana <51526537+bimakw@users.noreply.github.com> Date: Thu, 15 Jan 2026 22:26:32 +0700 Subject: [PATCH 3/8] refactor: reuse AddReviewRequest for CODEOWNERS instead of duplicate functions --- models/issues/comment.go | 14 ++-- models/issues/pull_test.go | 2 +- models/issues/review.go | 158 ++++------------------------------- models/issues/review_test.go | 42 +++++----- services/issue/assignee.go | 4 +- services/issue/pull.go | 4 +- 6 files changed, 53 insertions(+), 171 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index e0075da4a0639..c20074b848b07 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -774,12 +774,16 @@ func CreateComment(ctx context.Context, opts *CreateCommentOptions) (_ *Comment, } var commentMetaData *CommentMetaData - if opts.ProjectColumnTitle != "" || opts.IsCodeOwnersReviewRequest { + if opts.ProjectColumnTitle != "" { commentMetaData = &CommentMetaData{ - ProjectColumnID: opts.ProjectColumnID, - ProjectColumnTitle: opts.ProjectColumnTitle, - ProjectTitle: opts.ProjectTitle, - IsCodeOwnersReviewRequest: opts.IsCodeOwnersReviewRequest, + ProjectColumnID: opts.ProjectColumnID, + ProjectColumnTitle: opts.ProjectColumnTitle, + ProjectTitle: opts.ProjectTitle, + } + } + if opts.IsCodeOwnersReviewRequest { + commentMetaData = &CommentMetaData{ + IsCodeOwnersReviewRequest: true, } } diff --git a/models/issues/pull_test.go b/models/issues/pull_test.go index 7089af253b7a4..25b27cbe9c9fc 100644 --- a/models/issues/pull_test.go +++ b/models/issues/pull_test.go @@ -130,7 +130,7 @@ func TestLoadRequestedReviewers(t *testing.T) { user1, err := user_model.GetUserByID(t.Context(), 1) assert.NoError(t, err) - comment, err := issues_model.AddReviewRequest(t.Context(), issue, user1, &user_model.User{}) + comment, err := issues_model.AddReviewRequest(t.Context(), issue, user1, &user_model.User{}, false) assert.NoError(t, err) assert.NotNil(t, comment) diff --git a/models/issues/review.go b/models/issues/review.go index 963b8eedf9f8b..84cf437f0527d 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -643,7 +643,7 @@ func InsertReviews(ctx context.Context, reviews []*Review) error { } // AddReviewRequest add a review request from one reviewer -func AddReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_model.User) (*Comment, error) { +func AddReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_model.User, isCodeOwners bool) (*Comment, error) { return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) { sess := db.GetEngine(ctx) @@ -695,13 +695,14 @@ func AddReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_mo } comment, err := CreateComment(ctx, &CreateCommentOptions{ - Type: CommentTypeReviewRequest, - Doer: doer, - Repo: issue.Repo, - Issue: issue, - RemovedAssignee: false, // Use RemovedAssignee as !isRequest - AssigneeID: reviewer.ID, // Use AssigneeID as reviewer ID - ReviewID: review.ID, + Type: CommentTypeReviewRequest, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + RemovedAssignee: false, // Use RemovedAssignee as !isRequest + AssigneeID: reviewer.ID, // Use AssigneeID as reviewer ID + ReviewID: review.ID, + IsCodeOwnersReviewRequest: isCodeOwners, }) if err != nil { return nil, err @@ -767,7 +768,7 @@ func restoreLatestOfficialReview(ctx context.Context, issueID, reviewerID int64) } // AddTeamReviewRequest add a review request from one team -func AddTeamReviewRequest(ctx context.Context, issue *Issue, reviewer *organization.Team, doer *user_model.User) (*Comment, error) { +func AddTeamReviewRequest(ctx context.Context, issue *Issue, reviewer *organization.Team, doer *user_model.User, isCodeOwners bool) (*Comment, error) { return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) { review, err := GetTeamReviewerByIssueIDAndTeamID(ctx, issue.ID, reviewer.ID) if err != nil && !IsErrReviewNotExist(err) { @@ -805,13 +806,14 @@ func AddTeamReviewRequest(ctx context.Context, issue *Issue, reviewer *organizat } comment, err := CreateComment(ctx, &CreateCommentOptions{ - Type: CommentTypeReviewRequest, - Doer: doer, - Repo: issue.Repo, - Issue: issue, - RemovedAssignee: false, // Use RemovedAssignee as !isRequest - AssigneeTeamID: reviewer.ID, // Use AssigneeTeamID as reviewer team ID - ReviewID: review.ID, + Type: CommentTypeReviewRequest, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + RemovedAssignee: false, // Use RemovedAssignee as !isRequest + AssigneeTeamID: reviewer.ID, // Use AssigneeTeamID as reviewer team ID + ReviewID: review.ID, + IsCodeOwnersReviewRequest: isCodeOwners, }) if err != nil { return nil, fmt.Errorf("CreateComment(): %w", err) @@ -876,130 +878,6 @@ func RemoveTeamReviewRequest(ctx context.Context, issue *Issue, reviewer *organi }) } -// AddCodeOwnersReviewRequest adds a review request from CODEOWNERS rules. -// It works like AddReviewRequest but marks the comment as CODEOWNERS-triggered. -func AddCodeOwnersReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_model.User) (*Comment, error) { - return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) { - review, err := GetReviewByIssueIDAndUserID(ctx, issue.ID, reviewer.ID) - if err != nil && !IsErrReviewNotExist(err) { - return nil, err - } - - if review != nil { - // skip it when reviewer has been request to review - if review.Type == ReviewTypeRequest { - return nil, nil - } - - if issue.IsClosed { - return nil, ErrReviewRequestOnClosedPR{} - } - - if issue.IsPull { - if err := issue.LoadPullRequest(ctx); err != nil { - return nil, err - } - if issue.PullRequest.HasMerged { - return nil, ErrReviewRequestOnClosedPR{} - } - } - } - - official, err := IsOfficialReviewer(ctx, issue, reviewer) - if err != nil { - return nil, err - } else if official { - if _, err := db.GetEngine(ctx).Exec("UPDATE `review` SET official=? WHERE issue_id=? AND reviewer_id=?", false, issue.ID, reviewer.ID); err != nil { - return nil, err - } - } - - review, err = CreateReview(ctx, CreateReviewOptions{ - Type: ReviewTypeRequest, - Issue: issue, - Reviewer: reviewer, - Official: official, - Stale: false, - }) - if err != nil { - return nil, err - } - - comment, err := CreateComment(ctx, &CreateCommentOptions{ - Type: CommentTypeReviewRequest, - Doer: doer, - Repo: issue.Repo, - Issue: issue, - RemovedAssignee: false, - AssigneeID: reviewer.ID, - ReviewID: review.ID, - IsCodeOwnersReviewRequest: true, - }) - if err != nil { - return nil, err - } - - comment.Review = review - return comment, nil - }) -} - -// AddCodeOwnersTeamReviewRequest adds a team review request from CODEOWNERS rules. -// It works like AddTeamReviewRequest but marks the comment as CODEOWNERS-triggered. -func AddCodeOwnersTeamReviewRequest(ctx context.Context, issue *Issue, reviewer *organization.Team, doer *user_model.User) (*Comment, error) { - return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) { - review, err := GetTeamReviewerByIssueIDAndTeamID(ctx, issue.ID, reviewer.ID) - if err != nil && !IsErrReviewNotExist(err) { - return nil, err - } - - if review != nil { - return nil, nil - } - - official, err := IsOfficialReviewerTeam(ctx, issue, reviewer) - if err != nil { - return nil, fmt.Errorf("isOfficialReviewerTeam(): %w", err) - } else if !official { - if official, err = IsOfficialReviewer(ctx, issue, doer); err != nil { - return nil, fmt.Errorf("isOfficialReviewer(): %w", err) - } - } - - if review, err = CreateReview(ctx, CreateReviewOptions{ - Type: ReviewTypeRequest, - Issue: issue, - ReviewerTeam: reviewer, - Official: official, - Stale: false, - }); err != nil { - return nil, err - } - - if official { - if _, err := db.Exec(ctx, "UPDATE `review` SET official=? WHERE issue_id=? AND reviewer_team_id=?", false, issue.ID, reviewer.ID); err != nil { - return nil, err - } - } - - comment, err := CreateComment(ctx, &CreateCommentOptions{ - Type: CommentTypeReviewRequest, - Doer: doer, - Repo: issue.Repo, - Issue: issue, - RemovedAssignee: false, - AssigneeTeamID: reviewer.ID, - ReviewID: review.ID, - IsCodeOwnersReviewRequest: true, - }) - if err != nil { - return nil, fmt.Errorf("CreateComment(): %w", err) - } - - return comment, nil - }) -} - // MarkConversation Add or remove Conversation mark for a code comment func MarkConversation(ctx context.Context, comment *Comment, doer *user_model.User, isResolve bool) (err error) { if comment.Type != CommentTypeCode { diff --git a/models/issues/review_test.go b/models/issues/review_test.go index 5361eb69812e9..ebb6408da8056 100644 --- a/models/issues/review_test.go +++ b/models/issues/review_test.go @@ -322,19 +322,19 @@ func TestAddReviewRequest(t *testing.T) { pull.HasMerged = false assert.NoError(t, pull.UpdateCols(t.Context(), "has_merged")) issue.IsClosed = true - _, err = issues_model.AddReviewRequest(t.Context(), issue, reviewer, &user_model.User{}) + _, err = issues_model.AddReviewRequest(t.Context(), issue, reviewer, &user_model.User{}, false) assert.Error(t, err) assert.True(t, issues_model.IsErrReviewRequestOnClosedPR(err)) pull.HasMerged = true assert.NoError(t, pull.UpdateCols(t.Context(), "has_merged")) issue.IsClosed = false - _, err = issues_model.AddReviewRequest(t.Context(), issue, reviewer, &user_model.User{}) + _, err = issues_model.AddReviewRequest(t.Context(), issue, reviewer, &user_model.User{}, false) assert.Error(t, err) assert.True(t, issues_model.IsErrReviewRequestOnClosedPR(err)) } -func TestAddCodeOwnersReviewRequest(t *testing.T) { +func TestAddReviewRequest_WithCodeOwners(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) @@ -342,11 +342,11 @@ func TestAddCodeOwnersReviewRequest(t *testing.T) { issue := pull.Issue assert.NoError(t, issue.LoadRepo(t.Context())) - reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 7}) doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - // Test: Add CODEOWNERS review request - comment, err := issues_model.AddCodeOwnersReviewRequest(t.Context(), issue, reviewer, doer) + // Test: Add review request with isCodeOwners=true + comment, err := issues_model.AddReviewRequest(t.Context(), issue, reviewer, doer, true) assert.NoError(t, err) assert.NotNil(t, comment) @@ -370,7 +370,7 @@ func TestAddCodeOwnersReviewRequest(t *testing.T) { assert.True(t, savedComment.CommentMetaData.IsCodeOwnersReviewRequest) } -func TestAddCodeOwnersReviewRequest_SkipExisting(t *testing.T) { +func TestAddReviewRequest_WithCodeOwners_SkipExisting(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) @@ -382,17 +382,17 @@ func TestAddCodeOwnersReviewRequest_SkipExisting(t *testing.T) { doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // Create first review request - comment1, err := issues_model.AddCodeOwnersReviewRequest(t.Context(), issue, reviewer, doer) + comment1, err := issues_model.AddReviewRequest(t.Context(), issue, reviewer, doer, true) assert.NoError(t, err) assert.NotNil(t, comment1) // Try to create duplicate - should skip and return nil - comment2, err := issues_model.AddCodeOwnersReviewRequest(t.Context(), issue, reviewer, doer) + comment2, err := issues_model.AddReviewRequest(t.Context(), issue, reviewer, doer, true) assert.NoError(t, err) assert.Nil(t, comment2) } -func TestAddCodeOwnersReviewRequest_ClosedPR(t *testing.T) { +func TestAddReviewRequest_WithCodeOwners_ClosedPR(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) @@ -414,14 +414,14 @@ func TestAddCodeOwnersReviewRequest_ClosedPR(t *testing.T) { // Close the issue issue.IsClosed = true - // Try to add CODEOWNERS request - should error - comment, err := issues_model.AddCodeOwnersReviewRequest(t.Context(), issue, reviewer, doer) + // Try to add review request - should error + comment, err := issues_model.AddReviewRequest(t.Context(), issue, reviewer, doer, true) assert.Error(t, err) assert.True(t, issues_model.IsErrReviewRequestOnClosedPR(err)) assert.Nil(t, comment) } -func TestAddCodeOwnersReviewRequest_MergedPR(t *testing.T) { +func TestAddReviewRequest_WithCodeOwners_MergedPR(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) @@ -444,14 +444,14 @@ func TestAddCodeOwnersReviewRequest_MergedPR(t *testing.T) { pull.HasMerged = true assert.NoError(t, pull.UpdateCols(t.Context(), "has_merged")) - // Try to add CODEOWNERS request - should error - comment, err := issues_model.AddCodeOwnersReviewRequest(t.Context(), issue, reviewer, doer) + // Try to add review request - should error + comment, err := issues_model.AddReviewRequest(t.Context(), issue, reviewer, doer, true) assert.Error(t, err) assert.True(t, issues_model.IsErrReviewRequestOnClosedPR(err)) assert.Nil(t, comment) } -func TestAddCodeOwnersTeamReviewRequest(t *testing.T) { +func TestAddTeamReviewRequest_WithCodeOwners(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) @@ -462,8 +462,8 @@ func TestAddCodeOwnersTeamReviewRequest(t *testing.T) { team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - // Test: Add CODEOWNERS team review request - comment, err := issues_model.AddCodeOwnersTeamReviewRequest(t.Context(), issue, team, doer) + // Test: Add team review request with isCodeOwners=true + comment, err := issues_model.AddTeamReviewRequest(t.Context(), issue, team, doer, true) assert.NoError(t, err) assert.NotNil(t, comment) @@ -476,7 +476,7 @@ func TestAddCodeOwnersTeamReviewRequest(t *testing.T) { assert.True(t, comment.CommentMetaData.IsCodeOwnersReviewRequest) } -func TestAddCodeOwnersTeamReviewRequest_SkipExisting(t *testing.T) { +func TestAddTeamReviewRequest_WithCodeOwners_SkipExisting(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) @@ -488,12 +488,12 @@ func TestAddCodeOwnersTeamReviewRequest_SkipExisting(t *testing.T) { doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // Create first team review request - comment1, err := issues_model.AddCodeOwnersTeamReviewRequest(t.Context(), issue, team, doer) + comment1, err := issues_model.AddTeamReviewRequest(t.Context(), issue, team, doer, true) assert.NoError(t, err) assert.NotNil(t, comment1) // Try to create duplicate - should skip and return nil - comment2, err := issues_model.AddCodeOwnersTeamReviewRequest(t.Context(), issue, team, doer) + comment2, err := issues_model.AddTeamReviewRequest(t.Context(), issue, team, doer, true) assert.NoError(t, err) assert.Nil(t, comment2) } diff --git a/services/issue/assignee.go b/services/issue/assignee.go index ba9c91e0edc4a..97b32d5865f3b 100644 --- a/services/issue/assignee.go +++ b/services/issue/assignee.go @@ -70,7 +70,7 @@ func ReviewRequest(ctx context.Context, issue *issues_model.Issue, doer *user_mo } if isAdd { - comment, err = issues_model.AddReviewRequest(ctx, issue, reviewer, doer) + comment, err = issues_model.AddReviewRequest(ctx, issue, reviewer, doer, false) } else { comment, err = issues_model.RemoveReviewRequest(ctx, issue, reviewer, doer) } @@ -224,7 +224,7 @@ func TeamReviewRequest(ctx context.Context, issue *issues_model.Issue, doer *use return nil, err } if isAdd { - comment, err = issues_model.AddTeamReviewRequest(ctx, issue, reviewer, doer) + comment, err = issues_model.AddTeamReviewRequest(ctx, issue, reviewer, doer, false) } else { comment, err = issues_model.RemoveTeamReviewRequest(ctx, issue, reviewer, doer) } diff --git a/services/issue/pull.go b/services/issue/pull.go index 703a66eea4d69..43d5aa645d71e 100644 --- a/services/issue/pull.go +++ b/services/issue/pull.go @@ -149,7 +149,7 @@ func PullRequestCodeOwnersReview(ctx context.Context, pr *issues_model.PullReque for _, u := range uniqUsers { if u.ID != issue.Poster.ID && !contain(latestReivews, u) { - comment, err := issues_model.AddCodeOwnersReviewRequest(ctx, issue, u, issue.Poster) + comment, err := issues_model.AddReviewRequest(ctx, issue, u, issue.Poster, true) if err != nil { log.Warn("Failed add assignee user: %s to PR review: %s#%d, error: %s", u.Name, pr.BaseRepo.Name, pr.ID, err) return nil, err @@ -166,7 +166,7 @@ func PullRequestCodeOwnersReview(ctx context.Context, pr *issues_model.PullReque } for _, t := range uniqTeams { - comment, err := issues_model.AddCodeOwnersTeamReviewRequest(ctx, issue, t, issue.Poster) + comment, err := issues_model.AddTeamReviewRequest(ctx, issue, t, issue.Poster, true) if err != nil { log.Warn("Failed add assignee team: %s to PR review: %s#%d, error: %s", t.Name, pr.BaseRepo.Name, pr.ID, err) return nil, err From 377fa3012285d8b05d800a0b55756e6cb234f290 Mon Sep 17 00:00:00 2001 From: Gregorius Bima Kharisma Wicaksana <51526537+bimakw@users.noreply.github.com> Date: Fri, 16 Jan 2026 07:59:03 +0700 Subject: [PATCH 4/8] refactor: use SpecialDoerName instead of IsCodeOwnersReviewRequest Address reviewer feedback: - Rename IsCodeOwnersReviewRequest to SpecialDoerName for more general usage - Simplify tests by adding one case to existing TestAddReviewRequest - Remove separate test functions for CODEOWNERS scenarios --- models/issues/comment.go | 76 ++++++++-------- models/issues/review.go | 40 +++++---- models/issues/review_test.go | 167 ++--------------------------------- 3 files changed, 70 insertions(+), 213 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index c20074b848b07..8c79a5d0a5282 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -235,10 +235,10 @@ func (r RoleInRepo) LocaleHelper(lang translation.Locale) string { // CommentMetaData stores metadata for a comment, these data will not be changed once inserted into database type CommentMetaData struct { - ProjectColumnID int64 `json:"project_column_id,omitempty"` - ProjectColumnTitle string `json:"project_column_title,omitempty"` - ProjectTitle string `json:"project_title,omitempty"` - IsCodeOwnersReviewRequest bool `json:"is_code_owners_review_request,omitempty"` + ProjectColumnID int64 `json:"project_column_id,omitempty"` + ProjectColumnTitle string `json:"project_column_title,omitempty"` + ProjectTitle string `json:"project_title,omitempty"` + SpecialDoerName string `json:"special_doer_name,omitempty"` // e.g. "CODEOWNERS" for CODEOWNERS-triggered review requests } // Comment represents a comment in commit and issue page. @@ -781,9 +781,9 @@ func CreateComment(ctx context.Context, opts *CreateCommentOptions) (_ *Comment, ProjectTitle: opts.ProjectTitle, } } - if opts.IsCodeOwnersReviewRequest { + if opts.SpecialDoerName != "" { commentMetaData = &CommentMetaData{ - IsCodeOwnersReviewRequest: true, + SpecialDoerName: opts.SpecialDoerName, } } @@ -951,38 +951,38 @@ type CreateCommentOptions struct { Issue *Issue Label *Label - DependentIssueID int64 - OldMilestoneID int64 - MilestoneID int64 - OldProjectID int64 - ProjectID int64 - ProjectTitle string - ProjectColumnID int64 - ProjectColumnTitle string - TimeID int64 - AssigneeID int64 - AssigneeTeamID int64 - RemovedAssignee bool - OldTitle string - NewTitle string - OldRef string - NewRef string - CommitID int64 - CommitSHA string - Patch string - LineNum int64 - TreePath string - ReviewID int64 - Content string - Attachments []string // UUIDs of attachments - RefRepoID int64 - RefIssueID int64 - RefCommentID int64 - RefAction references.XRefAction - RefIsPull bool - IsForcePush bool - Invalidated bool - IsCodeOwnersReviewRequest bool + DependentIssueID int64 + OldMilestoneID int64 + MilestoneID int64 + OldProjectID int64 + ProjectID int64 + ProjectTitle string + ProjectColumnID int64 + ProjectColumnTitle string + TimeID int64 + AssigneeID int64 + AssigneeTeamID int64 + RemovedAssignee bool + OldTitle string + NewTitle string + OldRef string + NewRef string + CommitID int64 + CommitSHA string + Patch string + LineNum int64 + TreePath string + ReviewID int64 + Content string + Attachments []string // UUIDs of attachments + RefRepoID int64 + RefIssueID int64 + RefCommentID int64 + RefAction references.XRefAction + RefIsPull bool + IsForcePush bool + Invalidated bool + SpecialDoerName string // e.g. "CODEOWNERS" for CODEOWNERS-triggered review requests } // GetCommentByID returns the comment by given ID. diff --git a/models/issues/review.go b/models/issues/review.go index 84cf437f0527d..fc1bba6440c9a 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -694,15 +694,19 @@ func AddReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_mo return nil, err } + var specialDoerName string + if isCodeOwners { + specialDoerName = "CODEOWNERS" + } comment, err := CreateComment(ctx, &CreateCommentOptions{ - Type: CommentTypeReviewRequest, - Doer: doer, - Repo: issue.Repo, - Issue: issue, - RemovedAssignee: false, // Use RemovedAssignee as !isRequest - AssigneeID: reviewer.ID, // Use AssigneeID as reviewer ID - ReviewID: review.ID, - IsCodeOwnersReviewRequest: isCodeOwners, + Type: CommentTypeReviewRequest, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + RemovedAssignee: false, // Use RemovedAssignee as !isRequest + AssigneeID: reviewer.ID, // Use AssigneeID as reviewer ID + ReviewID: review.ID, + SpecialDoerName: specialDoerName, }) if err != nil { return nil, err @@ -805,15 +809,19 @@ func AddTeamReviewRequest(ctx context.Context, issue *Issue, reviewer *organizat } } + var specialDoerName string + if isCodeOwners { + specialDoerName = "CODEOWNERS" + } comment, err := CreateComment(ctx, &CreateCommentOptions{ - Type: CommentTypeReviewRequest, - Doer: doer, - Repo: issue.Repo, - Issue: issue, - RemovedAssignee: false, // Use RemovedAssignee as !isRequest - AssigneeTeamID: reviewer.ID, // Use AssigneeTeamID as reviewer team ID - ReviewID: review.ID, - IsCodeOwnersReviewRequest: isCodeOwners, + Type: CommentTypeReviewRequest, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + RemovedAssignee: false, // Use RemovedAssignee as !isRequest + AssigneeTeamID: reviewer.ID, // Use AssigneeTeamID as reviewer team ID + ReviewID: review.ID, + SpecialDoerName: specialDoerName, }) if err != nil { return nil, fmt.Errorf("CreateComment(): %w", err) diff --git a/models/issues/review_test.go b/models/issues/review_test.go index ebb6408da8056..661b9b161cf4c 100644 --- a/models/issues/review_test.go +++ b/models/issues/review_test.go @@ -7,7 +7,6 @@ import ( "testing" issues_model "code.gitea.io/gitea/models/issues" - "code.gitea.io/gitea/models/organization" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" @@ -332,168 +331,18 @@ func TestAddReviewRequest(t *testing.T) { _, err = issues_model.AddReviewRequest(t.Context(), issue, reviewer, &user_model.User{}, false) assert.Error(t, err) assert.True(t, issues_model.IsErrReviewRequestOnClosedPR(err)) -} - -func TestAddReviewRequest_WithCodeOwners(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) - assert.NoError(t, pull.LoadIssue(t.Context())) - issue := pull.Issue - assert.NoError(t, issue.LoadRepo(t.Context())) - reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 7}) + // Test CODEOWNERS review request stores metadata correctly + pull2 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + assert.NoError(t, pull2.LoadIssue(t.Context())) + issue2 := pull2.Issue + assert.NoError(t, issue2.LoadRepo(t.Context())) + reviewer2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 7}) doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - // Test: Add review request with isCodeOwners=true - comment, err := issues_model.AddReviewRequest(t.Context(), issue, reviewer, doer, true) + comment, err := issues_model.AddReviewRequest(t.Context(), issue2, reviewer2, doer, true) assert.NoError(t, err) assert.NotNil(t, comment) - - // Assert: Comment created with correct type - assert.Equal(t, issues_model.CommentTypeReviewRequest, comment.Type) - assert.Equal(t, reviewer.ID, comment.AssigneeID) - assert.Equal(t, doer.ID, comment.PosterID) - - // Assert: Review created - assert.NotNil(t, comment.Review) - assert.Equal(t, issues_model.ReviewTypeRequest, comment.Review.Type) - assert.Equal(t, reviewer.ID, comment.Review.ReviewerID) - - // Assert: Metadata marked as CODEOWNERS request assert.NotNil(t, comment.CommentMetaData) - assert.True(t, comment.CommentMetaData.IsCodeOwnersReviewRequest) - - // Assert: Verify persisted to database - savedComment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: comment.ID}) - assert.NotNil(t, savedComment.CommentMetaData) - assert.True(t, savedComment.CommentMetaData.IsCodeOwnersReviewRequest) -} - -func TestAddReviewRequest_WithCodeOwners_SkipExisting(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) - assert.NoError(t, pull.LoadIssue(t.Context())) - issue := pull.Issue - assert.NoError(t, issue.LoadRepo(t.Context())) - - reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) - doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - - // Create first review request - comment1, err := issues_model.AddReviewRequest(t.Context(), issue, reviewer, doer, true) - assert.NoError(t, err) - assert.NotNil(t, comment1) - - // Try to create duplicate - should skip and return nil - comment2, err := issues_model.AddReviewRequest(t.Context(), issue, reviewer, doer, true) - assert.NoError(t, err) - assert.Nil(t, comment2) -} - -func TestAddReviewRequest_WithCodeOwners_ClosedPR(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) - assert.NoError(t, pull.LoadIssue(t.Context())) - issue := pull.Issue - assert.NoError(t, issue.LoadRepo(t.Context())) - - reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - - // Create existing review (non-request type) so the closed check is triggered - _, err := issues_model.CreateReview(t.Context(), issues_model.CreateReviewOptions{ - Issue: issue, - Reviewer: reviewer, - Type: issues_model.ReviewTypeReject, - }) - assert.NoError(t, err) - - // Close the issue - issue.IsClosed = true - - // Try to add review request - should error - comment, err := issues_model.AddReviewRequest(t.Context(), issue, reviewer, doer, true) - assert.Error(t, err) - assert.True(t, issues_model.IsErrReviewRequestOnClosedPR(err)) - assert.Nil(t, comment) -} - -func TestAddReviewRequest_WithCodeOwners_MergedPR(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) - assert.NoError(t, pull.LoadIssue(t.Context())) - issue := pull.Issue - assert.NoError(t, issue.LoadRepo(t.Context())) - - reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - - // Create existing review so the merged check is triggered - _, err := issues_model.CreateReview(t.Context(), issues_model.CreateReviewOptions{ - Issue: issue, - Reviewer: reviewer, - Type: issues_model.ReviewTypeApprove, - }) - assert.NoError(t, err) - - // Mark PR as merged - pull.HasMerged = true - assert.NoError(t, pull.UpdateCols(t.Context(), "has_merged")) - - // Try to add review request - should error - comment, err := issues_model.AddReviewRequest(t.Context(), issue, reviewer, doer, true) - assert.Error(t, err) - assert.True(t, issues_model.IsErrReviewRequestOnClosedPR(err)) - assert.Nil(t, comment) -} - -func TestAddTeamReviewRequest_WithCodeOwners(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) - assert.NoError(t, pull.LoadIssue(t.Context())) - issue := pull.Issue - assert.NoError(t, issue.LoadRepo(t.Context())) - - team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) - doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - - // Test: Add team review request with isCodeOwners=true - comment, err := issues_model.AddTeamReviewRequest(t.Context(), issue, team, doer, true) - assert.NoError(t, err) - assert.NotNil(t, comment) - - // Assert: Comment created with correct type - assert.Equal(t, issues_model.CommentTypeReviewRequest, comment.Type) - assert.Equal(t, team.ID, comment.AssigneeTeamID) - - // Assert: Metadata marked as CODEOWNERS request - assert.NotNil(t, comment.CommentMetaData) - assert.True(t, comment.CommentMetaData.IsCodeOwnersReviewRequest) -} - -func TestAddTeamReviewRequest_WithCodeOwners_SkipExisting(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) - assert.NoError(t, pull.LoadIssue(t.Context())) - issue := pull.Issue - assert.NoError(t, issue.LoadRepo(t.Context())) - - team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 1}) - doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - - // Create first team review request - comment1, err := issues_model.AddTeamReviewRequest(t.Context(), issue, team, doer, true) - assert.NoError(t, err) - assert.NotNil(t, comment1) - - // Try to create duplicate - should skip and return nil - comment2, err := issues_model.AddTeamReviewRequest(t.Context(), issue, team, doer, true) - assert.NoError(t, err) - assert.Nil(t, comment2) + assert.Equal(t, "CODEOWNERS", comment.CommentMetaData.SpecialDoerName) } From 92895d0c8509dcee015ad5bec556a6dc83828f6a Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 16 Jan 2026 10:22:57 +0800 Subject: [PATCH 5/8] refactor --- models/issues/comment.go | 40 ++++++++++++++++++- models/issues/review.go | 12 +----- models/issues/review_test.go | 2 +- options/locale/locale_en-US.json | 2 +- templates/repo/graph/commits.tmpl | 4 +- .../repo/issue/view_content/comments.tmpl | 40 +++---------------- templates/shared/user/authorlink.tmpl | 2 +- web_src/css/base.css | 14 +------ web_src/css/features/gitgraph.css | 4 -- web_src/css/modules/comment.css | 9 ----- 10 files changed, 52 insertions(+), 77 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index 8c79a5d0a5282..01722fe1fb84b 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -20,6 +20,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/container" + "code.gitea.io/gitea/modules/htmlutil" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/references" @@ -233,12 +234,17 @@ func (r RoleInRepo) LocaleHelper(lang translation.Locale) string { return lang.TrString("repo.issues.role." + string(r) + "_helper") } +type SpecialDoerNameType string + +const SpecialDoerNameCodeOwners SpecialDoerNameType = "CODEOWNERS" + // CommentMetaData stores metadata for a comment, these data will not be changed once inserted into database type CommentMetaData struct { ProjectColumnID int64 `json:"project_column_id,omitempty"` ProjectColumnTitle string `json:"project_column_title,omitempty"` ProjectTitle string `json:"project_title,omitempty"` - SpecialDoerName string `json:"special_doer_name,omitempty"` // e.g. "CODEOWNERS" for CODEOWNERS-triggered review requests + + SpecialDoerName SpecialDoerNameType `json:"special_doer_name,omitempty"` // e.g. "CODEOWNERS" for CODEOWNERS-triggered review requests } // Comment represents a comment in commit and issue page. @@ -765,6 +771,36 @@ func (c *Comment) CodeCommentLink(ctx context.Context) string { return fmt.Sprintf("%s/files#%s", c.Issue.Link(), c.HashTag()) } +func (c *Comment) MetaSpecialDoerTr(locale translation.Locale) template.HTML { + if c.CommentMetaData == nil { + return "" + } + if c.CommentMetaData.SpecialDoerName == SpecialDoerNameCodeOwners { + return locale.Tr("repo.issues.review.codeowners_rules") + } + return htmlutil.HTMLFormat("%s", c.CommentMetaData.SpecialDoerName) +} + +func (c *Comment) TimelineRequestedReviewTr(locale translation.Locale, createdStr template.HTML) template.HTML { + if c.AssigneeID > 0 { + if c.RemovedAssignee { + if c.PosterID == c.AssigneeID { + return locale.Tr("repo.issues.review.remove_review_request_self", createdStr) + } + return locale.Tr("repo.issues.review.remove_review_request", c.Assignee.GetDisplayName(), createdStr) + } + return locale.Tr("repo.issues.review.add_review_request", c.Assignee.GetDisplayName(), createdStr) + } + teamName := locale.Tr("repo.issues.guest_team") + if c.AssigneeTeam != nil { + teamName = htmlutil.HTMLFormat("%s", c.AssigneeTeam.Name) + } + if c.RemovedAssignee { + return locale.Tr("repo.issues.review.remove_review_request", teamName, createdStr) + } + return locale.Tr("repo.issues.review.add_review_request", teamName, createdStr) +} + // CreateComment creates comment with context func CreateComment(ctx context.Context, opts *CreateCommentOptions) (_ *Comment, err error) { return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) { @@ -982,7 +1018,7 @@ type CreateCommentOptions struct { RefIsPull bool IsForcePush bool Invalidated bool - SpecialDoerName string // e.g. "CODEOWNERS" for CODEOWNERS-triggered review requests + SpecialDoerName SpecialDoerNameType // e.g. "CODEOWNERS" for CODEOWNERS-triggered review requests } // GetCommentByID returns the comment by given ID. diff --git a/models/issues/review.go b/models/issues/review.go index fc1bba6440c9a..d8caa4d13a87c 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -694,10 +694,6 @@ func AddReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_mo return nil, err } - var specialDoerName string - if isCodeOwners { - specialDoerName = "CODEOWNERS" - } comment, err := CreateComment(ctx, &CreateCommentOptions{ Type: CommentTypeReviewRequest, Doer: doer, @@ -706,7 +702,7 @@ func AddReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_mo RemovedAssignee: false, // Use RemovedAssignee as !isRequest AssigneeID: reviewer.ID, // Use AssigneeID as reviewer ID ReviewID: review.ID, - SpecialDoerName: specialDoerName, + SpecialDoerName: util.Iif(isCodeOwners, SpecialDoerNameCodeOwners, ""), }) if err != nil { return nil, err @@ -809,10 +805,6 @@ func AddTeamReviewRequest(ctx context.Context, issue *Issue, reviewer *organizat } } - var specialDoerName string - if isCodeOwners { - specialDoerName = "CODEOWNERS" - } comment, err := CreateComment(ctx, &CreateCommentOptions{ Type: CommentTypeReviewRequest, Doer: doer, @@ -821,7 +813,7 @@ func AddTeamReviewRequest(ctx context.Context, issue *Issue, reviewer *organizat RemovedAssignee: false, // Use RemovedAssignee as !isRequest AssigneeTeamID: reviewer.ID, // Use AssigneeTeamID as reviewer team ID ReviewID: review.ID, - SpecialDoerName: specialDoerName, + SpecialDoerName: util.Iif(isCodeOwners, SpecialDoerNameCodeOwners, ""), }) if err != nil { return nil, fmt.Errorf("CreateComment(): %w", err) diff --git a/models/issues/review_test.go b/models/issues/review_test.go index 661b9b161cf4c..092d88d1749ea 100644 --- a/models/issues/review_test.go +++ b/models/issues/review_test.go @@ -344,5 +344,5 @@ func TestAddReviewRequest(t *testing.T) { assert.NoError(t, err) assert.NotNil(t, comment) assert.NotNil(t, comment.CommentMetaData) - assert.Equal(t, "CODEOWNERS", comment.CommentMetaData.SpecialDoerName) + assert.Equal(t, issues_model.SpecialDoerNameCodeOwners, comment.CommentMetaData.SpecialDoerName) } diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 6d3608c1acc36..5cd2e8677731a 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -1701,8 +1701,8 @@ "repo.issues.review.content.empty": "You need to leave a comment indicating the requested change(s).", "repo.issues.review.reject": "requested changes %s", "repo.issues.review.wait": "was requested for review %s", + "repo.issues.review.codeowners_rules": "CODEOWNERS rules", "repo.issues.review.add_review_request": "requested review from %s %s", - "repo.issues.review.add_review_request_codeowners": "%s was requested to review due to CODEOWNERS rules %s", "repo.issues.review.remove_review_request": "removed review request for %s %s", "repo.issues.review.remove_review_request_self": "declined to review %s", "repo.issues.review.pending": "Pending", diff --git a/templates/repo/graph/commits.tmpl b/templates/repo/graph/commits.tmpl index 07ec076697175..d92be9c5ed1ca 100644 --- a/templates/repo/graph/commits.tmpl +++ b/templates/repo/graph/commits.tmpl @@ -40,14 +40,14 @@ {{end}} - + {{$userName := $commit.Commit.Author.Name}} {{if $commit.User}} {{if and $commit.User.FullName DefaultShowFullName}} {{$userName = $commit.User.FullName}} {{end}} {{ctx.AvatarUtils.Avatar $commit.User 18}} - {{$userName}} + {{$userName}} {{else}} {{ctx.AvatarUtils.AvatarByEmail $commit.Commit.Author.Email $userName 18}} {{$userName}} diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 68475a72602b2..0eeb10cba7dfd 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -514,46 +514,18 @@ {{else if eq .Type 27}}
{{svg "octicon-eye"}} - {{if and .CommentMetaData .CommentMetaData.IsCodeOwnersReviewRequest}} - {{/* CODEOWNERS-triggered review request - show different message */}} - {{svg "octicon-file-code" 16}} + {{$specialDoerHtml := .MetaSpecialDoerTr ctx.Locale}} + {{$timelineRequestedReviewHtml := .TimelineRequestedReviewTr ctx.Locale $createdStr}} + {{if $specialDoerHtml}} - {{if (gt .AssigneeID 0)}} - {{ctx.Locale.Tr "repo.issues.review.add_review_request_codeowners" .Assignee.GetDisplayName $createdStr}} - {{else}} - {{$teamName := "Ghost Team"}} - {{if .AssigneeTeam}} - {{$teamName = .AssigneeTeam.Name}} - {{end}} - {{ctx.Locale.Tr "repo.issues.review.add_review_request_codeowners" $teamName $createdStr}} - {{end}} + {{$specialDoerHtml}} + {{$timelineRequestedReviewHtml}} {{else}} {{template "shared/user/avatarlink" dict "user" .Poster}} {{template "shared/user/authorlink" .Poster}} - {{if (gt .AssigneeID 0)}} - {{if .RemovedAssignee}} - {{if eq .PosterID .AssigneeID}} - {{ctx.Locale.Tr "repo.issues.review.remove_review_request_self" $createdStr}} - {{else}} - {{ctx.Locale.Tr "repo.issues.review.remove_review_request" .Assignee.GetDisplayName $createdStr}} - {{end}} - {{else}} - {{ctx.Locale.Tr "repo.issues.review.add_review_request" .Assignee.GetDisplayName $createdStr}} - {{end}} - {{else}} - - {{$teamName := "Ghost Team"}} - {{if .AssigneeTeam}} - {{$teamName = .AssigneeTeam.Name}} - {{end}} - {{if .RemovedAssignee}} - {{ctx.Locale.Tr "repo.issues.review.remove_review_request" $teamName $createdStr}} - {{else}} - {{ctx.Locale.Tr "repo.issues.review.add_review_request" $teamName $createdStr}} - {{end}} - {{end}} + {{$timelineRequestedReviewHtml}} {{end}}
diff --git a/templates/shared/user/authorlink.tmpl b/templates/shared/user/authorlink.tmpl index abfee6aae3624..af6624c5c3de4 100644 --- a/templates/shared/user/authorlink.tmpl +++ b/templates/shared/user/authorlink.tmpl @@ -1 +1 @@ -{{.GetDisplayName}}{{if .IsTypeBot}}bot{{end}} +{{.GetDisplayName}}{{if .IsTypeBot}}bot{{end}} diff --git a/web_src/css/base.css b/web_src/css/base.css index 36b3d118ae5a8..ef55be9f2d86d 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -469,19 +469,6 @@ a.label, box-shadow: 1px 1px 0 0 var(--color-secondary); } -.ui.comments .comment .text { - margin: 0; -} - -.ui.comments .comment .text, -.ui.comments .comment .author { - color: var(--color-text); -} - -.ui.comments .comment a.author:hover { - color: var(--color-primary); -} - .ui.comments .comment .metadata { color: var(--color-text-light-2); } @@ -562,6 +549,7 @@ img.ui.avatar, color: var(--color-purple) !important; } +/* it is different from tw-text-black: this one changes in dark theme */ .text.black { color: var(--color-text) !important; } diff --git a/web_src/css/features/gitgraph.css b/web_src/css/features/gitgraph.css index 8bdafc3c99a5a..1e19c826563c1 100644 --- a/web_src/css/features/gitgraph.css +++ b/web_src/css/features/gitgraph.css @@ -46,10 +46,6 @@ height: 20px; } -#git-graph-container li .author { - color: var(--color-text-light); -} - #git-graph-container li .time { color: var(--color-text-light-3); font-size: 80%; diff --git a/web_src/css/modules/comment.css b/web_src/css/modules/comment.css index f0c721eed2016..2783328a6a56b 100644 --- a/web_src/css/modules/comment.css +++ b/web_src/css/modules/comment.css @@ -56,15 +56,6 @@ min-width: 0; } -.ui.comments .comment .author { - font-size: 1em; - font-weight: var(--font-weight-medium); -} - -.ui.comments .comment a.author { - cursor: pointer; -} - .ui.comments .comment .metadata { display: inline-block; margin-left: 0.5em; From 32148c6fa18494b09bd74ebede863d1a53a1391b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 16 Jan 2026 10:41:30 +0800 Subject: [PATCH 6/8] reduce avatar size for consistency, fine tune --- models/issues/comment.go | 5 +++-- templates/shared/user/avatarlink.tmpl | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index 01722fe1fb84b..1ee059cb37ad9 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -783,6 +783,7 @@ func (c *Comment) MetaSpecialDoerTr(locale translation.Locale) template.HTML { func (c *Comment) TimelineRequestedReviewTr(locale translation.Locale, createdStr template.HTML) template.HTML { if c.AssigneeID > 0 { + // it guarantees LoadAssigneeUserAndTeam has been called, and c.Assignee is Ghost user but not nil if c.RemovedAssignee { if c.PosterID == c.AssigneeID { return locale.Tr("repo.issues.review.remove_review_request_self", createdStr) @@ -791,9 +792,9 @@ func (c *Comment) TimelineRequestedReviewTr(locale translation.Locale, createdSt } return locale.Tr("repo.issues.review.add_review_request", c.Assignee.GetDisplayName(), createdStr) } - teamName := locale.Tr("repo.issues.guest_team") + teamName := locale.TrString("repo.issues.guest_team") if c.AssigneeTeam != nil { - teamName = htmlutil.HTMLFormat("%s", c.AssigneeTeam.Name) + teamName = c.AssigneeTeam.Name } if c.RemovedAssignee { return locale.Tr("repo.issues.review.remove_review_request", teamName, createdStr) diff --git a/templates/shared/user/avatarlink.tmpl b/templates/shared/user/avatarlink.tmpl index 5d56fef430d3f..eb6fdf8509861 100644 --- a/templates/shared/user/avatarlink.tmpl +++ b/templates/shared/user/avatarlink.tmpl @@ -1 +1 @@ -{{ctx.AvatarUtils.Avatar .user}} +{{ctx.AvatarUtils.Avatar .user (or .size 20)}} From db931809c4dd449d10a293af2bfb165a75e72e13 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 16 Jan 2026 18:04:29 +0800 Subject: [PATCH 7/8] fix tr key --- models/issues/comment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index 1ee059cb37ad9..c6bf61690a7f5 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -792,7 +792,7 @@ func (c *Comment) TimelineRequestedReviewTr(locale translation.Locale, createdSt } return locale.Tr("repo.issues.review.add_review_request", c.Assignee.GetDisplayName(), createdStr) } - teamName := locale.TrString("repo.issues.guest_team") + teamName := "Ghost Team" if c.AssigneeTeam != nil { teamName = c.AssigneeTeam.Name } From dab00f70d195e31cb7fedeabb0867cd9cb167e75 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 16 Jan 2026 22:22:20 +0800 Subject: [PATCH 8/8] Update models/issues/comment.go Signed-off-by: wxiaoguang --- models/issues/comment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index c6bf61690a7f5..9c249d2c05071 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -783,7 +783,7 @@ func (c *Comment) MetaSpecialDoerTr(locale translation.Locale) template.HTML { func (c *Comment) TimelineRequestedReviewTr(locale translation.Locale, createdStr template.HTML) template.HTML { if c.AssigneeID > 0 { - // it guarantees LoadAssigneeUserAndTeam has been called, and c.Assignee is Ghost user but not nil + // it guarantees LoadAssigneeUserAndTeam has been called, and c.Assignee is Ghost user but not nil if the user doesn't exist if c.RemovedAssignee { if c.PosterID == c.AssigneeID { return locale.Tr("repo.issues.review.remove_review_request_self", createdStr)