diff --git a/models/issues/comment.go b/models/issues/comment.go index febc49ca76b..5cd16f5cf1c 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -27,6 +27,7 @@ import ( "gitea.dev/modules/markup" "gitea.dev/modules/optional" "gitea.dev/modules/references" + "gitea.dev/modules/setting" "gitea.dev/modules/structs" "gitea.dev/modules/timeutil" "gitea.dev/modules/translation" @@ -643,36 +644,18 @@ func UpdateCommentAttachments(ctx context.Context, c *Comment, uuids []string) e } // LoadAssigneeUserAndTeam if comment.Type is CommentTypeAssignees, then load assignees -func (c *Comment) LoadAssigneeUserAndTeam(ctx context.Context) error { - var err error - +func (c *Comment) LoadAssigneeUserAndTeam(ctx context.Context) (err error) { if c.AssigneeID > 0 && c.Assignee == nil { - c.Assignee, err = user_model.GetUserByID(ctx, c.AssigneeID) + _, c.Assignee, err = user_model.GetPossibleUserByID(ctx, c.AssigneeID) if err != nil { - if !user_model.IsErrUserNotExist(err) { - return err - } - c.Assignee = user_model.NewGhostUser() - } - } else if c.AssigneeTeamID > 0 && c.AssigneeTeam == nil { - if err = c.LoadIssue(ctx); err != nil { return err } - - if err = c.Issue.LoadRepo(ctx); err != nil { + } + if c.AssigneeTeamID > 0 && c.AssigneeTeam == nil { + _, c.AssigneeTeam, err = organization.GetPossibleTeamByID(ctx, c.AssigneeTeamID) + if err != nil { return err } - - if err = c.Issue.Repo.LoadOwner(ctx); err != nil { - return err - } - - if c.Issue.Repo.Owner.IsOrganization() { - c.AssigneeTeam, err = organization.GetTeamByID(ctx, c.AssigneeTeamID) - if err != nil && !organization.IsErrTeamNotExist(err) { - return err - } - } } return nil } @@ -795,8 +778,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 the user doesn't exist + if c.Assignee != nil { if c.RemovedAssignee { if c.PosterID == c.AssigneeID { return locale.Tr("repo.issues.review.remove_review_request_self", createdStr) @@ -805,14 +787,20 @@ func (c *Comment) TimelineRequestedReviewTr(locale translation.Locale, createdSt } return locale.Tr("repo.issues.review.add_review_request", c.Assignee.GetDisplayName(), createdStr) } - teamName := "Ghost Team" if c.AssigneeTeam != nil { - teamName = c.AssigneeTeam.Name + if c.RemovedAssignee { + return locale.Tr("repo.issues.review.remove_review_request", c.AssigneeTeam.Name, createdStr) + } + return locale.Tr("repo.issues.review.add_review_request", c.AssigneeTeam.Name, createdStr) } + + // impossible fallback + assigneePrompt := fmt.Sprintf("(AssigneeID=%d, AssigneeTeamID=%d)", c.AssigneeID, c.AssigneeTeam.ID) + setting.PanicInDevOrTesting("unknown timeline pull request review event comment: id=%d, %s", c.ID, assigneePrompt) if c.RemovedAssignee { - return locale.Tr("repo.issues.review.remove_review_request", teamName, createdStr) + return locale.Tr("repo.issues.review.remove_review_request", assigneePrompt, createdStr) } - return locale.Tr("repo.issues.review.add_review_request", teamName, createdStr) + return locale.Tr("repo.issues.review.add_review_request", assigneePrompt, createdStr) } // CreateComment creates comment with context diff --git a/models/issues/comment_test.go b/models/issues/comment_test.go index c47a0c7a4a4..ae538422b90 100644 --- a/models/issues/comment_test.go +++ b/models/issues/comment_test.go @@ -45,6 +45,19 @@ func TestCreateComment(t *testing.T) { unittest.AssertInt64InRange(t, now, then, int64(updatedIssue.UpdatedUnix)) } +func TestLoadAssigneeUserAndTeam_DeletedTeamBecomesGhostTeam(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 15}) + comment := &issues_model.Comment{ + Type: issues_model.CommentTypeAssignees, + IssueID: issue.ID, + AssigneeTeamID: 999999, // non-existing team ID + } + assert.NoError(t, comment.LoadAssigneeUserAndTeam(t.Context())) + assert.NotNil(t, comment.AssigneeTeam) + assert.EqualValues(t, -1, comment.AssigneeTeam.ID) +} + func Test_UpdateCommentAttachment(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) diff --git a/models/organization/team.go b/models/organization/team.go index ea2f52a1edb..ff33b799853 100644 --- a/models/organization/team.go +++ b/models/organization/team.go @@ -6,6 +6,7 @@ package organization import ( "context" + "errors" "fmt" "strings" @@ -92,6 +93,15 @@ func (t *Team) IsPublic() bool { return t.Visibility.IsPublic() } func (t *Team) IsLimited() bool { return t.Visibility.IsLimited() } func (t *Team) IsPrivate() bool { return t.Visibility.IsPrivate() } +const ( + ghostTeamID = -1 + ghostTeamName = "(deleted team)" +) + +func newGhostTeam() *Team { + return &Team{ID: ghostTeamID, Name: ghostTeamName, LowerName: ghostTeamName} +} + // CanNonMemberReadMeta reports whether a non-member, non-owner doer may read // the team's metadata, based on the team's visibility tier and the parent org's // visibility. Privileged callers (site admins, org owners, team members) are @@ -270,6 +280,17 @@ func GetTeamByID(ctx context.Context, teamID int64) (*Team, error) { return t, nil } +func GetPossibleTeamByID(ctx context.Context, teamID int64) (int64, *Team, error) { + t, err := GetTeamByID(ctx, teamID) + if errors.Is(err, util.ErrNotExist) { + t = newGhostTeam() + return t.ID, t, nil + } else if err != nil { + return 0, nil, err + } + return t.ID, t, nil +} + // IncrTeamRepoNum increases the number of repos for the given team by 1 func IncrTeamRepoNum(ctx context.Context, teamID int64) error { _, err := db.GetEngine(ctx).Incr("num_repos").ID(teamID).Update(new(Team))