Add mentionable teams to tributeValues and change team mention rules …

…to gh's style (go-gitea#13198) * Add mentionable teams to tributeValues Signed-off-by: a1012112796 <[email protected]> * Apply suggestions from code review Co-authored-by: silverwind <[email protected]> * Change team mention rules to gh's style * use org's avator as team avator in ui Signed-off-by: a1012112796 <[email protected]> * Update modules/markup/html.go * Update models/issue.go Co-authored-by: Lauris BH <[email protected]> * Update models/issue.go * fix a small nit and update test code Co-authored-by: silverwind <[email protected]> Co-authored-by: Lauris BH <[email protected]> Co-authored-by: 6543 <[email protected]>
kitspace · Dec 21, 2020 · 34df4e5 · 34df4e5
1 parent 1b1adab
commit 34df4e5
Show file tree

Hide file tree

Showing 8 changed files with 112 additions and 34 deletions.
diff --git 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 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)
 		}
@@ -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
+	// 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)
 	}

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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}}
 		};