diff --git a/models/repo/user_repo.go b/models/repo/user_repo.go index 08cf964bc8..e15a64b01e 100644 --- a/models/repo/user_repo.go +++ b/models/repo/user_repo.go @@ -147,19 +147,21 @@ func GetRepoAssignees(ctx context.Context, repo *Repository) (_ []*user_model.Us } // GetIssuePostersWithSearch returns users with limit of 30 whose username started with prefix that have authored an issue/pull request for the given repository -// If isShowFullName is set to true, also include full name prefix search -func GetIssuePostersWithSearch(ctx context.Context, repo *Repository, isPull bool, search string, isShowFullName bool) ([]*user_model.User, error) { +// It searches with the "user.name" and "user.full_name" fields case-insensitively. +func GetIssuePostersWithSearch(ctx context.Context, repo *Repository, isPull bool, search string) ([]*user_model.User, error) { users := make([]*user_model.User, 0, 30) - var prefixCond builder.Cond = builder.Like{"lower_name", strings.ToLower(search) + "%"} - if search != "" && isShowFullName { - prefixCond = prefixCond.Or(db.BuildCaseInsensitiveLike("full_name", "%"+search+"%")) - } cond := builder.In("`user`.id", builder.Select("poster_id").From("issue").Where( builder.Eq{"repo_id": repo.ID}. And(builder.Eq{"is_pull": isPull}), - ).GroupBy("poster_id")).And(prefixCond) + ).GroupBy("poster_id")) + + if search != "" { + var prefixCond builder.Cond = builder.Like{"lower_name", strings.ToLower(search) + "%"} + prefixCond = prefixCond.Or(db.BuildCaseInsensitiveLike("full_name", "%"+search+"%")) + cond = cond.And(prefixCond) + } return users, db.GetEngine(ctx). Where(cond). diff --git a/models/repo/user_repo_test.go b/models/repo/user_repo_test.go index a53cf39dc4..cd8a0f1a1f 100644 --- a/models/repo/user_repo_test.go +++ b/models/repo/user_repo_test.go @@ -44,12 +44,12 @@ func TestGetIssuePostersWithSearch(t *testing.T) { repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) - users, err := repo_model.GetIssuePostersWithSearch(t.Context(), repo2, false, "USER", false /* full name */) + users, err := repo_model.GetIssuePostersWithSearch(t.Context(), repo2, false, "USER") require.NoError(t, err) require.Len(t, users, 1) assert.Equal(t, "user2", users[0].Name) - users, err = repo_model.GetIssuePostersWithSearch(t.Context(), repo2, false, "TW%O", true /* full name */) + users, err = repo_model.GetIssuePostersWithSearch(t.Context(), repo2, false, "TW%O") require.NoError(t, err) require.Len(t, users, 1) assert.Equal(t, "user2", users[0].Name) diff --git a/models/user/user.go b/models/user/user.go index d8f41b869e..a74662bb12 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -8,6 +8,7 @@ import ( "context" "encoding/hex" "fmt" + "html/template" "mime" "net/mail" "net/url" @@ -28,6 +29,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/htmlutil" "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" @@ -417,16 +419,6 @@ func (u *User) IsTokenAccessAllowed() bool { return u.Type == UserTypeIndividual || u.Type == UserTypeBot } -// DisplayName returns full name if it's not empty, -// returns username otherwise. -func (u *User) DisplayName() string { - trimmed := strings.TrimSpace(u.FullName) - if len(trimmed) > 0 { - return trimmed - } - return u.Name -} - // EmailTo returns a string suitable to be put into a e-mail `To:` header. func (u *User) EmailTo() string { sanitizedDisplayName := globalVars().emailToReplacer.Replace(u.DisplayName()) @@ -445,27 +437,45 @@ func (u *User) EmailTo() string { return fmt.Sprintf("%s <%s>", mime.QEncoding.Encode("utf-8", add.Name), add.Address) } -// GetDisplayName returns full name if it's not empty and DEFAULT_SHOW_FULL_NAME is set, -// returns username otherwise. +// TODO: DefaultShowFullName causes messy logic, there are already too many methods to display a user's "display name", need to refactor them +// * user.Name / user.FullName: directly used in templates +// * user.DisplayName(): always show FullName if it's not empty, otherwise show Name +// * user.GetDisplayName(): show FullName if it's not empty and DefaultShowFullName is set, otherwise show Name +// * user.ShortName(): used a lot in templates, but it should be removed and let frontend use "ellipsis" styles +// * activity action.ShortActUserName/GetActDisplayName/GetActDisplayNameTitle, etc: duplicate and messy + +// DisplayName returns full name if it's not empty, returns username otherwise. +func (u *User) DisplayName() string { + fullName := strings.TrimSpace(u.FullName) + if fullName != "" { + return fullName + } + return u.Name +} + +// GetDisplayName returns full name if it's not empty and DEFAULT_SHOW_FULL_NAME is set, otherwise, username. func (u *User) GetDisplayName() string { if setting.UI.DefaultShowFullName { - trimmed := strings.TrimSpace(u.FullName) - if len(trimmed) > 0 { - return trimmed + fullName := strings.TrimSpace(u.FullName) + if fullName != "" { + return fullName } } return u.Name } -// GetCompleteName returns the full name and username in the form of -// "Full Name (username)" if full name is not empty, otherwise it returns -// "username". -func (u *User) GetCompleteName() string { - trimmedFullName := strings.TrimSpace(u.FullName) - if len(trimmedFullName) > 0 { - return fmt.Sprintf("%s (%s)", trimmedFullName, u.Name) +// ShortName ellipses username to length (still used by many templates), it calls GetDisplayName and respects DEFAULT_SHOW_FULL_NAME +func (u *User) ShortName(length int) string { + return util.EllipsisDisplayString(u.GetDisplayName(), length) +} + +func (u *User) GetShortDisplayNameLinkHTML() template.HTML { + fullName := strings.TrimSpace(u.FullName) + displayName, displayTooltip := u.Name, fullName + if setting.UI.DefaultShowFullName && fullName != "" { + displayName, displayTooltip = fullName, u.Name } - return u.Name + return htmlutil.HTMLFormat(`%s`, u.HomeLink(), displayTooltip, displayName) } func gitSafeName(name string) string { @@ -488,14 +498,6 @@ func (u *User) GitName() string { return fmt.Sprintf("user-%d", u.ID) } -// ShortName ellipses username to length -func (u *User) ShortName(length int) string { - if setting.UI.DefaultShowFullName && len(u.FullName) > 0 { - return util.EllipsisDisplayString(u.FullName, length) - } - return util.EllipsisDisplayString(u.Name, length) -} - // IsMailable checks if a user is eligible to receive emails. // System users like Ghost and Gitea Actions are excluded. func (u *User) IsMailable() bool { diff --git a/modules/setting/ui.go b/modules/setting/ui.go index 77a5b45d0a..722341a71e 100644 --- a/modules/setting/ui.go +++ b/modules/setting/ui.go @@ -25,7 +25,6 @@ var UI = struct { ReactionMaxUserNum int MaxDisplayFileSize int64 ShowUserEmail bool - DefaultShowFullName bool DefaultTheme string Themes []string FileIconTheme string @@ -43,6 +42,15 @@ var UI = struct { AmbiguousUnicodeDetection bool + // TODO: DefaultShowFullName is introduced by https://github.com/go-gitea/gitea/pull/6710 + // But there are still many edge cases: + // * Many places still use "username", not respecting this setting + // * Many places use "Full Name" if it is not empty, cause inconsistent UI for users who have set their full name but some others don't + // * Even if DefaultShowFullName=false, many places still need to show the full name + // For most cases, either "username" or "username (Full Name)" should be used and are good enough. + // Only in very few cases (e.g.: unimportant lists, narrow layout), "username" or "Full Name" can be used. + DefaultShowFullName bool + Notification struct { MinTimeout time.Duration TimeoutStep time.Duration diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 11c52bd5a7..82087568df 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -96,9 +96,6 @@ func NewFuncMap() template.FuncMap { "AssetVersion": func() string { return setting.AssetVersion }, - "DefaultShowFullName": func() bool { - return setting.UI.DefaultShowFullName - }, "ShowFooterTemplateLoadTime": func() bool { return setting.Other.ShowFooterTemplateLoadTime }, diff --git a/modules/templates/util_avatar.go b/modules/templates/util_avatar.go index ee9994ab0b..524c64d0b6 100644 --- a/modules/templates/util_avatar.go +++ b/modules/templates/util_avatar.go @@ -16,6 +16,7 @@ import ( user_model "code.gitea.io/gitea/models/user" gitea_html "code.gitea.io/gitea/modules/htmlutil" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" ) type AvatarUtils struct { @@ -29,13 +30,9 @@ func NewAvatarUtils(ctx context.Context) *AvatarUtils { // AvatarHTML creates the HTML for an avatar func AvatarHTML(src string, size int, class, name string) template.HTML { sizeStr := strconv.Itoa(size) - - if name == "" { - name = "avatar" - } - + name = util.IfZero(name, "avatar") // use empty alt, otherwise if the image fails to load, the width will follow the "alt" text's width - return template.HTML(``) + return template.HTML(``) } // Avatar renders user avatars. args: user, size (int), class (string) diff --git a/routers/web/repo/issue_content_history.go b/routers/web/repo/issue_content_history.go index a56df78163..23cedfcb80 100644 --- a/routers/web/repo/issue_content_history.go +++ b/routers/web/repo/issue_content_history.go @@ -6,13 +6,14 @@ package repo import ( "bytes" "html" + "html/template" "net/http" "strings" "code.gitea.io/gitea/models/avatars" issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/modules/htmlutil" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/services/context" @@ -53,29 +54,25 @@ func GetContentHistoryList(ctx *context.Context) { // value is historyId var results []map[string]any for _, item := range items { - var actionText string + var actionHTML template.HTML if item.IsDeleted { - actionTextDeleted := ctx.Locale.TrString("repo.issues.content_history.deleted") - actionText = "" + actionTextDeleted + "" + actionHTML = htmlutil.HTMLFormat(`%s`, ctx.Locale.TrString("repo.issues.content_history.deleted")) } else if item.IsFirstCreated { - actionText = ctx.Locale.TrString("repo.issues.content_history.created") + actionHTML = ctx.Locale.Tr("repo.issues.content_history.created") } else { - actionText = ctx.Locale.TrString("repo.issues.content_history.edited") + actionHTML = ctx.Locale.Tr("repo.issues.content_history.edited") } - username := item.UserName - if setting.UI.DefaultShowFullName && strings.TrimSpace(item.UserFullName) != "" { - username = strings.TrimSpace(item.UserFullName) + var fullNameHTML template.HTML + userName, fullName := item.UserName, strings.TrimSpace(item.UserFullName) + if fullName != "" { + fullNameHTML = htmlutil.HTMLFormat(` (%s)`, fullName) } - src := html.EscapeString(item.UserAvatarLink) - class := avatars.DefaultAvatarClass + " tw-mr-2" - name := html.EscapeString(username) - avatarHTML := string(templates.AvatarHTML(src, 28, class, username)) - timeSinceHTML := string(templates.TimeSince(item.EditedUnix)) - + avatarHTML := templates.AvatarHTML(item.UserAvatarLink, 24, avatars.DefaultAvatarClass+" tw-mr-2", userName) + timeSinceHTML := templates.TimeSince(item.EditedUnix) results = append(results, map[string]any{ - "name": avatarHTML + "" + name + " " + actionText + " " + timeSinceHTML, + "name": htmlutil.HTMLFormat("%s %s%s %s %s", avatarHTML, userName, fullNameHTML, actionHTML, timeSinceHTML), "value": item.HistoryID, }) } diff --git a/routers/web/repo/issue_poster.go b/routers/web/repo/issue_poster.go index 07059b9b7b..4f00f40a91 100644 --- a/routers/web/repo/issue_poster.go +++ b/routers/web/repo/issue_poster.go @@ -10,7 +10,6 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/setting" shared_user "code.gitea.io/gitea/routers/web/shared/user" "code.gitea.io/gitea/services/context" ) @@ -34,7 +33,7 @@ func IssuePullPosters(ctx *context.Context) { func issuePosters(ctx *context.Context, isPullList bool) { repo := ctx.Repo.Repository search := strings.TrimSpace(ctx.FormString("q")) - posters, err := repo_model.GetIssuePostersWithSearch(ctx, repo, isPullList, search, setting.UI.DefaultShowFullName) + posters, err := repo_model.GetIssuePostersWithSearch(ctx, repo, isPullList, search) if err != nil { ctx.JSON(http.StatusInternalServerError, err) return @@ -54,9 +53,7 @@ func issuePosters(ctx *context.Context, isPullList bool) { resp.Results = make([]*userSearchInfo, len(posters)) for i, user := range posters { resp.Results[i] = &userSearchInfo{UserID: user.ID, UserName: user.Name, AvatarLink: user.AvatarLink(ctx)} - if setting.UI.DefaultShowFullName { - resp.Results[i].FullName = user.FullName - } + resp.Results[i].FullName = user.FullName } ctx.JSON(http.StatusOK, resp) } diff --git a/services/mailer/mail.go b/services/mailer/mail.go index 8f831f89ad..a08ed71480 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -158,18 +158,23 @@ func (b64embedder *mailAttachmentBase64Embedder) AttachmentSrcToBase64DataURI(ct func fromDisplayName(u *user_model.User) string { if setting.MailService.FromDisplayNameFormatTemplate != nil { - var ctx bytes.Buffer - err := setting.MailService.FromDisplayNameFormatTemplate.Execute(&ctx, map[string]any{ + var buf bytes.Buffer + err := setting.MailService.FromDisplayNameFormatTemplate.Execute(&buf, map[string]any{ "DisplayName": u.DisplayName(), "AppName": setting.AppName, "Domain": setting.Domain, }) if err == nil { - return mime.QEncoding.Encode("utf-8", ctx.String()) + return mime.QEncoding.Encode("utf-8", buf.String()) } log.Error("fromDisplayName: %w", err) } - return u.GetCompleteName() + def := u.Name + if fullName := strings.TrimSpace(u.FullName); fullName != "" { + // use "Full Name (username)" for email's sender name if Full Name is not empty + def = fullName + " (" + u.Name + ")" + } + return def } func generateMetadataHeaders(repo *repo_model.Repository) map[string]string { diff --git a/templates/admin/org/list.tmpl b/templates/admin/org/list.tmpl index a4317b8c4e..e6de93c5f8 100644 --- a/templates/admin/org/list.tmpl +++ b/templates/admin/org/list.tmpl @@ -48,11 +48,14 @@ - {{range .Users}} + {{range $org := .Users}} {{.ID}} - {{if and DefaultShowFullName .FullName}}{{.FullName}} ({{.Name}}){{else}}{{.Name}}{{end}} + + {{$org.Name}} + {{if $org.FullName}}({{$org.FullName}}){{end}} + {{if .Visibility.IsPrivate}} {{svg "octicon-lock"}} {{end}} diff --git a/templates/repo/commits_list.tmpl b/templates/repo/commits_list.tmpl index 1a236582a2..a0722307a7 100644 --- a/templates/repo/commits_list.tmpl +++ b/templates/repo/commits_list.tmpl @@ -14,18 +14,15 @@ {{range .Commits}} -
- {{$userName := .Author.Name}} - {{if .User}} - {{if and .User.FullName DefaultShowFullName}} - {{$userName = .User.FullName}} - {{end}} - {{ctx.AvatarUtils.Avatar .User 28 "tw-mr-2"}}{{$userName}} - {{else}} - {{ctx.AvatarUtils.AvatarByEmail .Author.Email .Author.Name 28 "tw-mr-2"}} - {{$userName}} - {{end}} -
+ + {{- if .User -}} + {{- ctx.AvatarUtils.Avatar .User 20 "tw-mr-2" -}} + {{- .User.GetShortDisplayNameLinkHTML -}} + {{- else -}} + {{- ctx.AvatarUtils.AvatarByEmail .Author.Email .Author.Name 20 "tw-mr-2" -}} + {{- .Author.Name -}} + {{- end -}} + {{$commitBaseLink := ""}} diff --git a/templates/repo/graph/commits.tmpl b/templates/repo/graph/commits.tmpl index d92be9c5ed..d86f73fe65 100644 --- a/templates/repo/graph/commits.tmpl +++ b/templates/repo/graph/commits.tmpl @@ -41,16 +41,13 @@ - {{$userName := $commit.Commit.Author.Name}} {{if $commit.User}} - {{if and $commit.User.FullName DefaultShowFullName}} - {{$userName = $commit.User.FullName}} - {{end}} {{ctx.AvatarUtils.Avatar $commit.User 18}} - {{$userName}} + {{$commit.User.GetShortDisplayNameLinkHTML}} {{else}} - {{ctx.AvatarUtils.AvatarByEmail $commit.Commit.Author.Email $userName 18}} - {{$userName}} + {{$gitUserName := $commit.Commit.Author.Name}} + {{ctx.AvatarUtils.AvatarByEmail $commit.Commit.Author.Email $gitUserName 18}} + {{$gitUserName}} {{end}} diff --git a/templates/repo/issue/filter_item_user_assign.tmpl b/templates/repo/issue/filter_item_user_assign.tmpl index 42886edaa0..5ca8a8079c 100644 --- a/templates/repo/issue/filter_item_user_assign.tmpl +++ b/templates/repo/issue/filter_item_user_assign.tmpl @@ -10,7 +10,7 @@ {{$queryLink := .QueryLink}}