fix(ui): too many participants shown in commit avatar stacks (#38689) (#38700)

Backport #38689

Show only author and co-authors without committer, and deduplicate the
same user with multiple email addresses.

The commit list "Author" column should not show the committer. This is
somewhat misleading, and arguably showing it on the individual commit
page is sufficient and consistent with other forges.

The same user with multiple email addresses often happens when
DEFAULT_KEEP_EMAIL_PRIVATE is enabled and Gitea does not use an actual
email address by default for edits. Showing the same avatar and name
twice is not helpful then.

Ref #37594
Fix #38488

Co-authored-by: bircni <bircni@icloud.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

models/gituser/avatar_stack.go

@@ -7,6 +7,7 @@ import (
 	"context"
 
 	"gitea.dev/models/user"
+	"gitea.dev/modules/container"
 	"gitea.dev/modules/git"
 	"gitea.dev/modules/log"
 )
@@ -33,11 +34,18 @@ func BuildAvatarStackData(ctx context.Context, allParticipants []*git.CommitIden
 	ret := &AvatarStackData{
 		Participants: make([]*CommitParticipant, 0, len(allParticipants)),
 	}
+	uniqueUserIDs := make(container.Set[int64])
 	for _, p := range allParticipants {
 		var giteaUser *user.User
 		if emailUserMap != nil {
 			giteaUser = emailUserMap.GetByEmail(p.Email)
 		}
+		if giteaUser != nil {
+			// identities without a Gitea account can only be compared by their git identity
+			if !uniqueUserIDs.Add(giteaUser.ID) {
+				continue
+			}
+		}
 		ret.Participants = append(ret.Participants, &CommitParticipant{GiteaUser: giteaUser, GitIdentity: p})
 	}
 	return ret

models/gituser/gituser.go

@@ -39,8 +39,7 @@ func GetUserCommitsByGitCommits(ctx context.Context, gitCommits []*git.Commit, r
 	emailSet := make(container.Set[string])
 	for _, c := range gitCommits {
 		emailSet.Add(c.Author.Email)
-		emailSet.Add(c.Committer.Email)
-		for _, p := range c.AllParticipantIdentities() {
+		for _, p := range c.AllAuthorIdentities() {
 			emailSet.Add(p.Email)
 		}
 	}
@@ -55,7 +54,7 @@ func GetUserCommitsByGitCommits(ctx context.Context, gitCommits []*git.Commit, r
 		uc := &UserCommit{
 			AuthorUser:      emailUserMap.GetByEmail(c.Author.Email), // FIXME: why GetUserCommitsByGitCommits uses "Author", but ParseCommitsWithSignature uses "Committer"?
 			GitCommit:       c,
-			AvatarStackData: BuildAvatarStackData(ctx, c.AllParticipantIdentities(), emailUserMap),
+			AvatarStackData: BuildAvatarStackData(ctx, c.AllAuthorIdentities(), emailUserMap),
 		}
 		uc.AvatarStackData.SearchByEmailLink = searchByEmailLink
 		userCommits = append(userCommits, uc)

modules/git/commit_message.go

@@ -39,9 +39,7 @@ type CommitMessage struct {
 
 	trailerValues CommitMessageTrailerValues
 
-	allParticipants      []*CommitIdentity
-	committerCoAuthorIdx int
-	committerCoAuthor    *CommitIdentity
+	allAuthors []*CommitIdentity
 }
 
 func (c *CommitMessage) MessageUTF8() string {
@@ -146,63 +144,50 @@ func CommitMessageParseTrailer(s string) CommitMessageTrailerValues {
 	return ret
 }
 
-// AllParticipantIdentities returns all the participants in the commit, the first one is the commit's author
-func (c *Commit) AllParticipantIdentities() []*CommitIdentity {
-	if c.allParticipants != nil {
-		return c.allParticipants
+// AllAuthorIdentities returns all the author and co-authors in the commit. Committer is not included:
+// * Author & Co-author: they changed the code (attribution)
+// * Committer: they submitted the commit but didn't change the code (e.g.: maintainer signed a commit)
+func (c *Commit) AllAuthorIdentities() []*CommitIdentity {
+	if c.allAuthors != nil {
+		return c.allAuthors
 	}
-
+	trailerCoAuthors := c.MessageTrailer()["co-authored-by"]
+	c.allAuthors = make([]*CommitIdentity, 0, 1+len(trailerCoAuthors))
 	exclude := map[string]int{}
-	addParticipant := func(name, email string, role int) (existingRole int) {
+	addAuthor := func(name, email string, role int) {
 		if name == "" && email == "" {
-			return 0
+			return
 		}
-		emailLower := strings.ToLower(email)
-		if existingRole = exclude[emailLower]; emailLower != "" && existingRole != 0 {
-			return existingRole
+		key := strings.ToLower(email)
+		if key == "" {
+			key = strings.ToLower(name)
 		}
-		c.allParticipants = append(c.allParticipants, &CommitIdentity{Name: name, Email: email, role: role})
-		exclude[emailLower] = role
-		return 0
+		if existingRole := exclude[key]; key != "" && existingRole != 0 {
+			return
+		}
+		c.allAuthors = append(c.allAuthors, &CommitIdentity{Name: name, Email: email, role: role})
+		exclude[key] = role
 	}
 
-	c.committerCoAuthorIdx = -1
-	addParticipant(c.Author.Name, c.Author.Email, commitIdentityRoleAuthor)
-	addParticipant(c.Committer.Name, c.Committer.Email, commitIdentityRoleCommitter)
-	for _, coAuthorValue := range c.MessageTrailer()["co-authored-by"] {
+	addAuthor(c.Author.Name, c.Author.Email, commitIdentityRoleAuthor)
+	for _, coAuthorValue := range trailerCoAuthors {
 		addr, err := mail.ParseAddress(coAuthorValue)
 		coAuthorName, coAuthorEmail := coAuthorValue, ""
 		if err == nil {
 			coAuthorName, coAuthorEmail = addr.Name, addr.Address
 		}
-		existingRole := addParticipant(coAuthorName, coAuthorEmail, commitIdentityRoleCoAuthor)
-		if existingRole == commitIdentityRoleCommitter && c.committerCoAuthorIdx == -1 {
-			c.committerCoAuthorIdx = len(c.allParticipants)
-			c.committerCoAuthor = &CommitIdentity{coAuthorName, coAuthorEmail, commitIdentityRoleCoAuthor}
-		}
+		addAuthor(coAuthorName, coAuthorEmail, commitIdentityRoleCoAuthor)
 	}
-	return c.allParticipants
+	return c.allAuthors
 }
 
-// CoAuthorIdentities returns co-author identities defined by "Co-authored-by:" in the git message trailer
-// Only the commit's author is excluded. If committer is declared as co-author, it will be included in the result.
-// * Author & Co-author: they changed the code (attribution)
-// * Committer: they submitted the commit but didn't change the code (e.g.: maintainer signed a commit)
-// So, a committer can also be a co-author if they changed the code.
 func (c *Commit) CoAuthorIdentities() (coAuthors []*CommitIdentity) {
-	all := c.AllParticipantIdentities()
-	if len(all) <= 1 {
-		return nil // no co-author list
+	all := c.AllAuthorIdentities()
+	if len(all) == 0 {
+		return nil
 	}
-	if all[1].role != commitIdentityRoleCommitter {
-		return all[1:] // no committer, so all after author are co-authors
+	if all[0].role == commitIdentityRoleAuthor {
+		return all[1:]
 	}
-	if c.committerCoAuthorIdx == -1 {
-		return all[2:] // the committer is not in the co-author list, so just return the co-author list
-	}
-	// the committer is in the co-author list but de-duplicated, so include them as co-author again
-	coAuthors = append(coAuthors, all[2:c.committerCoAuthorIdx]...)
-	coAuthors = append(coAuthors, c.committerCoAuthor)
-	coAuthors = append(coAuthors, all[c.committerCoAuthorIdx:]...)
-	return coAuthors
+	return all
 }

modules/git/commit_message_test.go

@@ -52,41 +52,44 @@ func TestCommitMessageTrailer(t *testing.T) {
 func TestCommitMessageParticipants(t *testing.T) {
 	sig := func(n, e string) *Signature { return &Signature{Name: n, Email: e} }
 	idt := func(n, e string, r int) *CommitIdentity { return &CommitIdentity{n, e, r} }
-	roleAuthor, roleCommitter, roleCoAuthor := commitIdentityRoleAuthor, commitIdentityRoleCommitter, commitIdentityRoleCoAuthor
+	roleAuthor, _, roleCoAuthor := commitIdentityRoleAuthor, commitIdentityRoleCommitter, commitIdentityRoleCoAuthor
 	type testCase struct {
 		name       string
 		commit     *Commit
 		identities []*CommitIdentity
 	}
-	t.Run("AllParticipants", func(t *testing.T) {
+
+	t.Run("AllAuthors", func(t *testing.T) {
 		cases := []testCase{
 			{
-				"DifferentUsers",
+				"CommitterExcluded",
 				&Commit{
 					Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"),
-					CommitMessage: CommitMessage{MessageRaw: "CO-Authored-BY: x@m.com"},
+					CommitMessage: CommitMessage{MessageRaw: "CO-Authored-BY: Full Name <x@m.com>"},
 				},
-				[]*CommitIdentity{idt("a", "a@m.com", roleAuthor), idt("c", "c@m.com", roleCommitter), idt("", "x@m.com", roleCoAuthor)},
+				[]*CommitIdentity{idt("a", "a@m.com", roleAuthor), idt("Full Name", "x@m.com", roleCoAuthor)},
 			},
 			{
-				"SameUser",
+				"AuthorIsCoAuthor",
 				&Commit{
-					Author: sig("a", "a@m.com"), Committer: sig("a", "A@M.com"),
-					CommitMessage: CommitMessage{MessageRaw: "CO-Authored-BY: a@m.com"},
+					Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"),
+					CommitMessage: CommitMessage{MessageRaw: "CO-Authored-BY: other-name <a@m.com>"},
 				},
 				[]*CommitIdentity{idt("a", "a@m.com", roleAuthor)},
 			},
 			{
-				"NoCommitter",
+				"EmptyAuthor", // synthesized commits (push feed) may have no author signature at all
 				&Commit{
-					Author: sig("a", "a@m.com"), Committer: sig("", ""),
-					CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: Full Name <X@M.com>"},
+					Author: sig("", ""), Committer: sig("", ""),
+					CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: c <c@m.com>"},
 				},
-				[]*CommitIdentity{idt("a", "a@m.com", roleAuthor), idt("Full Name", "X@M.com", roleCoAuthor)},
+				// but if the commit message contains co-authors, the co-authors are still parsed for "all authors"
+				// if it is a problem, the caller should fix the problem (provide correct "author")
+				[]*CommitIdentity{idt("c", "c@m.com", roleCoAuthor)},
 			},
 		}
 		for _, c := range cases {
-			assert.Equal(t, c.identities, c.commit.AllParticipantIdentities(), "case: %s", c.name)
+			assert.Equal(t, c.identities, c.commit.AllAuthorIdentities(), "case: %s", c.name)
 		}
 	})
 	t.Run("CoAuthors", func(t *testing.T) {
@@ -116,12 +119,12 @@ func TestCommitMessageParticipants(t *testing.T) {
 				[]*CommitIdentity{},
 			},
 			{
-				"CoAuthorCommitterNameWithIndex", // restore the committer co-author to the co-author list by the index with correct name
+				"CoAuthorNameOnlyAndDuplicate",
 				&Commit{
 					Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"),
-					CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: x <x@m.com>\nCo-authored-by: c-other <c@m.com>\nCo-authored-by: y <y@m.com>"},
+					CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: b\nCo-authored-by: b\nCo-authored-by: c"},
 				},
-				[]*CommitIdentity{idt("x", "x@m.com", roleCoAuthor), idt("c-other", "c@m.com", roleCoAuthor), idt("y", "y@m.com", roleCoAuthor)},
+				[]*CommitIdentity{idt("b", "", roleCoAuthor), idt("c", "", roleCoAuthor)},
 			},
 		}
 		for _, c := range cases {

modules/templates/util_render.go

@@ -370,7 +370,7 @@ func (ut *RenderUtils) AvatarStackPushCommit(pushCommit *repository.PushCommit)
 		// there is no way to know the real committer, but the field can't be nil
 		Committer: &git.Signature{Name: pushCommit.AuthorName, Email: pushCommit.AuthorEmail},
 	}
-	data := user_model.BuildAvatarStackData(ut.ctx, fakeGitCommit.AllParticipantIdentities(), nil)
+	data := user_model.BuildAvatarStackData(ut.ctx, fakeGitCommit.AllAuthorIdentities(), nil)
 	return ut.AvatarStack(data)
 }
 

routers/web/repo/blame.go

@@ -223,7 +223,7 @@ func processBlameParts(ctx *context.Context, blameParts []*gitrepo.BlamePart) ma
 }
 
 func renderBlameFillFirstBlameRow(ctx *context.Context, repoLink string, part *gitrepo.BlamePart, commit *gituser.UserCommit, br *blameRow) {
-	br.AvatarStackData = gituser.BuildAvatarStackData(ctx, commit.GitCommit.AllParticipantIdentities(), nil)
+	br.AvatarStackData = gituser.BuildAvatarStackData(ctx, commit.GitCommit.AllAuthorIdentities(), nil)
 	br.PreviousSha = part.PreviousSha
 	br.PreviousShaURL = fmt.Sprintf("%s/blame/commit/%s/%s", repoLink, url.PathEscape(part.PreviousSha), util.PathEscapeSegments(part.PreviousPath))
 	br.CommitURL = fmt.Sprintf("%s/commit/%s", repoLink, url.PathEscape(part.Sha))

routers/web/repo/view.go

@@ -134,7 +134,7 @@ func loadLatestCommitData(ctx *context.Context, latestCommit *git.Commit) bool {
 			return false
 		}
 
-		avatarStackData := gituser.BuildAvatarStackData(ctx, latestCommit.AllParticipantIdentities(), nil)
+		avatarStackData := gituser.BuildAvatarStackData(ctx, latestCommit.AllAuthorIdentities(), nil)
 		avatarStackData.SearchByEmailLink = gituser.RepoCommitSearchByEmailLink(ctx.Repo.RepoLink, ctx.Repo.RefFullName)
 		ctx.Data["LatestCommitAvatarStackData"] = avatarStackData
 		ctx.Data["LatestCommitVerification"] = verification

services/repository/gitgraph/graph_models.go

@@ -109,7 +109,7 @@ func (graph *Graph) LoadAndProcessCommits(ctx context.Context, repository *repo_
 		if c.Commit.Author != nil {
 			emailSet.Add(c.Commit.Author.Email)
 		}
-		for _, sig := range c.Commit.AllParticipantIdentities() {
+		for _, sig := range c.Commit.AllAuthorIdentities() {
 			emailSet.Add(sig.Email)
 		}
 	}
@@ -125,7 +125,7 @@ func (graph *Graph) LoadAndProcessCommits(ctx context.Context, repository *repo_
 		}
 
 		c.User = emailUserMap.GetByEmail(c.Commit.Author.Email)
-		c.AvatarStackData = gituser.BuildAvatarStackData(ctx, c.Commit.AllParticipantIdentities(), emailUserMap)
+		c.AvatarStackData = gituser.BuildAvatarStackData(ctx, c.Commit.AllAuthorIdentities(), emailUserMap)
 
 		c.Verification = asymkey_service.ParseCommitWithSignature(ctx, c.Commit)