Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make distinction between DisplayName and Username in email templates #6495

Merged
merged 4 commits into from
Apr 4, 2019

Conversation

mrsdizzie
Copy link
Member

Store the actual username in the variable named Username and store the separate DisplayName in another variable. This allows us to access the actual username when we need, which currently fails if a user has set a full name.

Fixes #6161

Store the actual username in the variable named Username and store the
separate DisplayName in another variable. This allows us to access the
actual username when we need, which currently fails if a user has set a
full name.

Fixes go-gitea#6161
@codecov-io
Copy link

codecov-io commented Apr 2, 2019

Codecov Report

Merging #6495 into master will increase coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6495      +/-   ##
==========================================
+ Coverage   40.27%   40.28%   +0.01%     
==========================================
  Files         403      403              
  Lines       54068    54069       +1     
==========================================
+ Hits        21775    21781       +6     
+ Misses      29276    29270       -6     
- Partials     3017     3018       +1
Impacted Files Coverage Δ
models/mail.go 23.93% <0%> (-0.21%) ⬇️
routers/repo/view.go 42.07% <0%> (+0.99%) ⬆️
models/unit.go 14.28% <0%> (+14.28%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f4e2d9...2d1a3a3. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 2, 2019
models/mail.go Outdated
@@ -47,7 +47,8 @@ func SendTestMail(email string) error {
// SendUserMail sends a mail to the user
func SendUserMail(c *macaron.Context, u *User, tpl base.TplName, code, subject, info string) {
data := map[string]interface{}{
"Username": u.DisplayName(),
"DisplayName": u.DisplayName(),
"Username": u.DisplayUsername(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can just be u.Name, no need for a func.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yes duh 🙈 fixed

models/user.go Outdated
@@ -650,6 +650,11 @@ func (u *User) DisplayName() string {
return u.Name
}

// DisplayUsername returns username
func (u *User) DisplayUsername() string {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See other comment.

No need for extra function, also change use in all mail sending
functions here
@jolheiser
Copy link
Member

jolheiser commented Apr 2, 2019

Do all the maps actually need both DisplayName and Username?
It looks like only the register_notify.tmpl would need the both whereas the rest use the DisplayName exclusively.

@mrsdizzie
Copy link
Member Author

True, I guess that is preemptive and if people do want to include the username in the future they can also add it to the map at that time. Appreciate the feedback!

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 3, 2019
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 4, 2019
@lafriks lafriks added the type/enhancement An improvement of existing functionality label Apr 4, 2019
@lafriks lafriks added this to the 1.9.0 milestone Apr 4, 2019
@lafriks lafriks merged commit 04003d9 into go-gitea:master Apr 4, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong username in registration notification mail
5 participants