Skip to content

Add mentionable teams to tributeValues and change team mention rules to gh's style #13198

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Dec 21, 2020
Merged
69 changes: 43 additions & 26 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 err := issue.Repo.getOwner(ctx.e); err != nil {
return nil, err
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 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)
}
Expand All @@ -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})
Expand All @@ -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 {
Expand All @@ -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
// 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(names))
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)
}
Expand Down
2 changes: 1 addition & 1 deletion models/issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
}
12 changes: 8 additions & 4 deletions modules/markup/html.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions modules/references/references.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions modules/references/references_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ func TestRegExp_mentionPattern(t *testing.T) {
{"@gitea.", "@gitea"},
{"@gitea,", "@gitea"},
{"@gitea;", "@gitea"},
{"@gitea/team1;", "@gitea/team1"},
}
falseTestCases := []string{
"@ 0",
Expand All @@ -340,6 +341,7 @@ func TestRegExp_mentionPattern(t *testing.T) {
"@gitea?this",
"@gitea,this",
"@gitea;this",
"@gitea/team1/more",
}

for _, testCase := range trueTestCases {
Expand Down
47 changes: 47 additions & 0 deletions routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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()
}
4 changes: 4 additions & 0 deletions routers/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion templates/base/head.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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}}',
Expand All @@ -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}}
};
Expand Down