mirror of
https://github.com/go-gitea/gitea.git
synced 2026-07-31 22:58:18 +00:00
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 <img width="4088" height="2138" alt="before" src="https://github.com/user-attachments/assets/a9e919f4-72d0-495f-a0b5-ec094f148f5a" /> After <img width="4124" height="2142" alt="after" src="https://github.com/user-attachments/assets/7212e527-ab83-440a-a8a4-95b5a79b3580" /> --------- Co-authored-by: bircni <bircni@icloud.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
committed by
GitHub
parent
298d73cde6
commit
43390e802c
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user