From 43390e802c83c5991d86289e08e08cf05e377059 Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Wed, 29 Jul 2026 14:39:35 +0200 Subject: [PATCH] fix(ui): too many participants shown in commit avatar stacks (#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 --- Before before After after --------- Co-authored-by: bircni Co-authored-by: wxiaoguang --- models/gituser/avatar_stack.go | 8 +++ models/gituser/gituser.go | 5 +- modules/git/commit_message.go | 73 ++++++++------------ modules/git/commit_message_test.go | 35 +++++----- modules/templates/util_render.go | 2 +- routers/web/repo/blame.go | 2 +- routers/web/repo/view.go | 2 +- services/repository/gitgraph/graph_models.go | 4 +- 8 files changed, 63 insertions(+), 68 deletions(-) diff --git a/models/gituser/avatar_stack.go b/models/gituser/avatar_stack.go index d38c94bc7b..844bedca55 100644 --- a/models/gituser/avatar_stack.go +++ b/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 diff --git a/models/gituser/gituser.go b/models/gituser/gituser.go index 81a94a3a85..cad8b7fec8 100644 --- a/models/gituser/gituser.go +++ b/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) diff --git a/modules/git/commit_message.go b/modules/git/commit_message.go index 14c7b8f7e2..ce82a88ea0 100644 --- a/modules/git/commit_message.go +++ b/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 } diff --git a/modules/git/commit_message_test.go b/modules/git/commit_message_test.go index a602185d56..94c0317efc 100644 --- a/modules/git/commit_message_test.go +++ b/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 "}, }, - []*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 "}, }, []*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 "}, + Author: sig("", ""), Committer: sig("", ""), + CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: c "}, }, - []*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 \nCo-authored-by: c-other \nCo-authored-by: y "}, + 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 { diff --git a/modules/templates/util_render.go b/modules/templates/util_render.go index 43833034c2..bb241790d4 100644 --- a/modules/templates/util_render.go +++ b/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) } diff --git a/routers/web/repo/blame.go b/routers/web/repo/blame.go index b3c2246ee1..764e01841e 100644 --- a/routers/web/repo/blame.go +++ b/routers/web/repo/blame.go @@ -225,7 +225,7 @@ func processBlameParts(ctx *context.Context, blameParts []*git.BlamePart) map[st } func renderBlameFillFirstBlameRow(ctx *context.Context, repoLink string, part *git.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)) diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index cc10322101..0391732dfc 100644 --- a/routers/web/repo/view.go +++ b/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 diff --git a/services/repository/gitgraph/graph_models.go b/services/repository/gitgraph/graph_models.go index 5ba47c5122..7e246acd74 100644 --- a/services/repository/gitgraph/graph_models.go +++ b/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)