From 34df4e5df558e7ec648efe083696687be6f8c8a8 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Mon, 21 Dec 2020 23:39:28 +0800 Subject: [PATCH] Add mentionable teams to tributeValues and change team mention rules to gh's style (#13198) * Add mentionable teams to tributeValues Signed-off-by: a1012112796 <1012112796@qq.com> * Apply suggestions from code review Co-authored-by: silverwind * Change team mention rules to gh's style * use org's avator as team avator in ui Signed-off-by: a1012112796 <1012112796@qq.com> * Update modules/markup/html.go * Update models/issue.go Co-authored-by: Lauris BH * Update models/issue.go * fix a small nit and update test code Co-authored-by: silverwind Co-authored-by: Lauris BH Co-authored-by: 6543 <6543@obermui.de> --- models/issue.go | 73 +++++++++++++++++---------- models/issue_test.go | 2 +- modules/markup/html.go | 12 +++-- modules/references/references.go | 4 +- modules/references/references_test.go | 2 + routers/repo/issue.go | 47 +++++++++++++++++ routers/repo/pull.go | 4 ++ templates/base/head.tmpl | 6 ++- 8 files changed, 114 insertions(+), 36 deletions(-) diff --git a/models/issue.go b/models/issue.go index 8c135faa8..787d873f2 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1847,30 +1847,43 @@ func (issue *Issue) ResolveMentionsByVisibility(ctx DBContext, doer *User, menti if err = issue.loadRepo(ctx.e); err != nil { return } - resolved := make(map[string]bool, 20) - names := make([]string, 0, 20) + + resolved := make(map[string]bool, 10) + var mentionTeams []string + + if err := issue.Repo.getOwner(ctx.e); err != nil { + return nil, err + } + + repoOwnerIsOrg := issue.Repo.Owner.IsOrganization() + if repoOwnerIsOrg { + mentionTeams = make([]string, 0, 5) + } + resolved[doer.LowerName] = true for _, name := range mentions { name := strings.ToLower(name) if _, ok := resolved[name]; ok { continue } - resolved[name] = false - names = append(names, name) + if repoOwnerIsOrg && strings.Contains(name, "/") { + names := strings.Split(name, "/") + if len(names) < 2 || names[0] != issue.Repo.Owner.LowerName { + continue + } + mentionTeams = append(mentionTeams, names[1]) + resolved[name] = true + } else { + resolved[name] = false + } } - if err := issue.Repo.getOwner(ctx.e); err != nil { - return nil, err - } - - if issue.Repo.Owner.IsOrganization() { - // Since there can be users with names that match the name of a team, - // if the team exists and can read the issue, the team takes precedence. - teams := make([]*Team, 0, len(names)) + if issue.Repo.Owner.IsOrganization() && len(mentionTeams) > 0 { + teams := make([]*Team, 0, len(mentionTeams)) if err := ctx.e. Join("INNER", "team_repo", "team_repo.team_id = team.id"). Where("team_repo.repo_id=?", issue.Repo.ID). - In("team.lower_name", names). + In("team.lower_name", mentionTeams). Find(&teams); err != nil { return nil, fmt.Errorf("find mentioned teams: %v", err) } @@ -1883,7 +1896,7 @@ func (issue *Issue) ResolveMentionsByVisibility(ctx DBContext, doer *User, menti for _, team := range teams { if team.Authorize >= AccessModeOwner { checked = append(checked, team.ID) - resolved[team.LowerName] = true + resolved[issue.Repo.Owner.LowerName+"/"+team.LowerName] = true continue } has, err := ctx.e.Get(&TeamUnit{OrgID: issue.Repo.Owner.ID, TeamID: team.ID, Type: unittype}) @@ -1892,7 +1905,7 @@ func (issue *Issue) ResolveMentionsByVisibility(ctx DBContext, doer *User, menti } if has { checked = append(checked, team.ID) - resolved[team.LowerName] = true + resolved[issue.Repo.Owner.LowerName+"/"+team.LowerName] = true } } if len(checked) != 0 { @@ -1916,24 +1929,28 @@ func (issue *Issue) ResolveMentionsByVisibility(ctx DBContext, doer *User, menti } } } - - // Remove names already in the list to avoid querying the database if pending names remain - names = make([]string, 0, len(resolved)) - for name, already := range resolved { - if !already { - names = append(names, name) - } - } - if len(names) == 0 { - return - } } - unchecked := make([]*User, 0, len(names)) + // Remove names already in the list to avoid querying the database if pending names remain + mentionUsers := make([]string, 0, len(resolved)) + for name, already := range resolved { + if !already { + mentionUsers = append(mentionUsers, name) + } + } + if len(mentionUsers) == 0 { + return + } + + if users == nil { + users = make([]*User, 0, len(mentionUsers)) + } + + unchecked := make([]*User, 0, len(mentionUsers)) if err := ctx.e. Where("`user`.is_active = ?", true). And("`user`.prohibit_login = ?", false). - In("`user`.lower_name", names). + In("`user`.lower_name", mentionUsers). Find(&unchecked); err != nil { return nil, fmt.Errorf("find mentioned users: %v", err) } diff --git a/models/issue_test.go b/models/issue_test.go index e1995fc5d..1f65cb433 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -400,5 +400,5 @@ func TestIssue_ResolveMentions(t *testing.T) { // Private repo, not a team member testSuccess("user17", "big_test_private_4", "user20", []string{"user5"}, []int64{}) // Private repo, whole team - testSuccess("user17", "big_test_private_4", "user15", []string{"owners"}, []int64{18}) + testSuccess("user17", "big_test_private_4", "user15", []string{"user17/owners"}, []int64{18}) } diff --git a/modules/markup/html.go b/modules/markup/html.go index 586343fae..7ac23d050 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -596,11 +596,15 @@ func mentionProcessor(ctx *postProcessCtx, node *html.Node) { mention := node.Data[loc.Start:loc.End] var teams string teams, ok := ctx.metas["teams"] - if ok && strings.Contains(teams, ","+strings.ToLower(mention[1:])+",") { - replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(setting.AppURL, "org", ctx.metas["org"], "teams", mention[1:]), mention, "mention")) - } else { - replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(setting.AppURL, mention[1:]), mention, "mention")) + // team mention should follow @orgName/teamName style + if ok && strings.Contains(mention, "/") { + mentionOrgAndTeam := strings.Split(mention, "/") + if mentionOrgAndTeam[0][1:] == ctx.metas["org"] && strings.Contains(teams, ","+strings.ToLower(mentionOrgAndTeam[1])+",") { + replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(setting.AppURL, "org", ctx.metas["org"], "teams", mentionOrgAndTeam[1]), mention, "mention")) + } + return } + replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(setting.AppURL, mention[1:]), mention, "mention")) } func shortLinkProcessor(ctx *postProcessCtx, node *html.Node) { diff --git a/modules/references/references.go b/modules/references/references.go index 6e0baefc6..c243f25f5 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -26,8 +26,8 @@ var ( // While fast, this is also incorrect and lead to false positives. // TODO: fix invalid linking issue - // mentionPattern matches all mentions in the form of "@user" - mentionPattern = regexp.MustCompile(`(?:\s|^|\(|\[)(@[0-9a-zA-Z-_]+|@[0-9a-zA-Z-_][0-9a-zA-Z-_.]+[0-9a-zA-Z-_])(?:\s|[:,;.?!]\s|[:,;.?!]?$|\)|\])`) + // mentionPattern matches all mentions in the form of "@user" or "@org/team" + mentionPattern = regexp.MustCompile(`(?:\s|^|\(|\[)(@[0-9a-zA-Z-_]+|@[0-9a-zA-Z-_]+\/?[0-9a-zA-Z-_]+|@[0-9a-zA-Z-_][0-9a-zA-Z-_.]+\/?[0-9a-zA-Z-_.]+[0-9a-zA-Z-_])(?:\s|[:,;.?!]\s|[:,;.?!]?$|\)|\])`) // issueNumericPattern matches string that references to a numeric issue, e.g. #1287 issueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([#!][0-9]+)(?:\s|$|\)|\]|[:;,.?!]\s|[:;,.?!]$)`) // issueAlphanumericPattern matches string that references to an alphanumeric issue, e.g. ABC-1234 diff --git a/modules/references/references_test.go b/modules/references/references_test.go index f51379e4c..d4f080490 100644 --- a/modules/references/references_test.go +++ b/modules/references/references_test.go @@ -325,6 +325,7 @@ func TestRegExp_mentionPattern(t *testing.T) { {"@gitea.", "@gitea"}, {"@gitea,", "@gitea"}, {"@gitea;", "@gitea"}, + {"@gitea/team1;", "@gitea/team1"}, } falseTestCases := []string{ "@ 0", @@ -340,6 +341,7 @@ func TestRegExp_mentionPattern(t *testing.T) { "@gitea?this", "@gitea,this", "@gitea;this", + "@gitea/team1/more", } for _, testCase := range trueTestCases { diff --git a/routers/repo/issue.go b/routers/repo/issue.go index eec0b7fd9..b999f2f51 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -274,6 +274,11 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti return } + handleTeamMentions(ctx) + if ctx.Written() { + return + } + labels, err := models.GetLabelsByRepoID(repo.ID, "", models.ListOptions{}) if err != nil { ctx.ServerError("GetLabelsByRepoID", err) @@ -410,6 +415,11 @@ func RetrieveRepoMilestonesAndAssignees(ctx *context.Context, repo *models.Repos ctx.ServerError("GetAssignees", err) return } + + handleTeamMentions(ctx) + if ctx.Written() { + return + } } func retrieveProjects(ctx *context.Context, repo *models.Repository) { @@ -2445,3 +2455,40 @@ func combineLabelComments(issue *models.Issue) { i-- } } + +// get all teams that current user can mention +func handleTeamMentions(ctx *context.Context) { + if ctx.User == nil || !ctx.Repo.Owner.IsOrganization() { + return + } + + isAdmin := false + var err error + // Admin has super access. + if ctx.User.IsAdmin { + isAdmin = true + } else { + isAdmin, err = ctx.Repo.Owner.IsOwnedBy(ctx.User.ID) + if err != nil { + ctx.ServerError("IsOwnedBy", err) + return + } + } + + if isAdmin { + if err := ctx.Repo.Owner.GetTeams(&models.SearchTeamOptions{}); err != nil { + ctx.ServerError("GetTeams", err) + return + } + } else { + ctx.Repo.Owner.Teams, err = ctx.Repo.Owner.GetUserTeams(ctx.User.ID) + if err != nil { + ctx.ServerError("GetUserTeams", err) + return + } + } + + ctx.Data["MentionableTeams"] = ctx.Repo.Owner.Teams + ctx.Data["MentionableTeamsOrg"] = ctx.Repo.Owner.Name + ctx.Data["MentionableTeamsOrgAvator"] = ctx.Repo.Owner.RelAvatarLink() +} diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 901a66863..442379c43 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -684,6 +684,10 @@ func ViewPullFiles(ctx *context.Context) { ctx.ServerError("GetAssignees", err) return } + handleTeamMentions(ctx) + if ctx.Written() { + return + } ctx.Data["CurrentReview"], err = models.GetCurrentReview(ctx.User, issue) if err != nil && !models.IsErrReviewNotExist(err) { ctx.ServerError("GetCurrentReview", err) diff --git a/templates/base/head.tmpl b/templates/base/head.tmpl index 7d35664c7..51d60c46f 100644 --- a/templates/base/head.tmpl +++ b/templates/base/head.tmpl @@ -44,7 +44,7 @@ EventSourceUpdateTime: {{NotificationSettings.EventSourceUpdateTime}}, }, PageIsProjects: {{if .PageIsProjects }}true{{else}}false{{end}}, - {{if .RequireTribute}} + {{if .RequireTribute}} tributeValues: Array.from(new Map([ {{ range .Participants }} ['{{.Name}}', {key: '{{.Name}} {{.FullName}}', value: '{{.Name}}', @@ -54,6 +54,10 @@ ['{{.Name}}', {key: '{{.Name}} {{.FullName}}', value: '{{.Name}}', name: '{{.Name}}', fullname: '{{.FullName}}', avatar: '{{.RelAvatarLink}}'}], {{ end }} + {{ range .MentionableTeams }} + ['{{$.MentionableTeamsOrg}}/{{.Name}}', {key: '{{$.MentionableTeamsOrg}}/{{.Name}}', value: '{{$.MentionableTeamsOrg}}/{{.Name}}', + name: '{{$.MentionableTeamsOrg}}/{{.Name}}', avatar: '{{$.MentionableTeamsOrgAvator}}'}], + {{ end }} ]).values()), {{end}} };