Skip to content

Commit e7cf62f

Browse files
authored
remove context from mail struct (#33811)
it can be passed by argument instead
1 parent b0ee340 commit e7cf62f

File tree

4 files changed

+103
-99
lines changed

4 files changed

+103
-99
lines changed

Diff for: services/mailer/mail_comment.go

+4-6
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,8 @@ func MailParticipantsComment(ctx context.Context, c *issues_model.Comment, opTyp
2525
if c.Type == issues_model.CommentTypePullRequestPush {
2626
content = ""
2727
}
28-
if err := mailIssueCommentToParticipants(
29-
&mailCommentContext{
30-
Context: ctx,
28+
if err := mailIssueCommentToParticipants(ctx,
29+
&mailComment{
3130
Issue: issue,
3231
Doer: c.Poster,
3332
ActionType: opType,
@@ -48,9 +47,8 @@ func MailMentionsComment(ctx context.Context, pr *issues_model.PullRequest, c *i
4847

4948
visited := make(container.Set[int64], len(mentions)+1)
5049
visited.Add(c.Poster.ID)
51-
if err = mailIssueCommentBatch(
52-
&mailCommentContext{
53-
Context: ctx,
50+
if err = mailIssueCommentBatch(ctx,
51+
&mailComment{
5452
Issue: pr.Issue,
5553
Doer: c.Poster,
5654
ActionType: activities_model.ActionCommentPull,

Diff for: services/mailer/mail_issue.go

+27-29
Original file line numberDiff line numberDiff line change
@@ -24,88 +24,88 @@ const MailBatchSize = 100 // batch size used in mailIssueCommentBatch
2424
// This function sends two list of emails:
2525
// 1. Repository watchers (except for WIP pull requests) and users who are participated in comments.
2626
// 2. Users who are not in 1. but get mentioned in current issue/comment.
27-
func mailIssueCommentToParticipants(ctx *mailCommentContext, mentions []*user_model.User) error {
27+
func mailIssueCommentToParticipants(ctx context.Context, comment *mailComment, mentions []*user_model.User) error {
2828
// Required by the mail composer; make sure to load these before calling the async function
29-
if err := ctx.Issue.LoadRepo(ctx); err != nil {
29+
if err := comment.Issue.LoadRepo(ctx); err != nil {
3030
return fmt.Errorf("LoadRepo: %w", err)
3131
}
32-
if err := ctx.Issue.LoadPoster(ctx); err != nil {
32+
if err := comment.Issue.LoadPoster(ctx); err != nil {
3333
return fmt.Errorf("LoadPoster: %w", err)
3434
}
35-
if err := ctx.Issue.LoadPullRequest(ctx); err != nil {
35+
if err := comment.Issue.LoadPullRequest(ctx); err != nil {
3636
return fmt.Errorf("LoadPullRequest: %w", err)
3737
}
3838

3939
// Enough room to avoid reallocations
4040
unfiltered := make([]int64, 1, 64)
4141

4242
// =========== Original poster ===========
43-
unfiltered[0] = ctx.Issue.PosterID
43+
unfiltered[0] = comment.Issue.PosterID
4444

4545
// =========== Assignees ===========
46-
ids, err := issues_model.GetAssigneeIDsByIssue(ctx, ctx.Issue.ID)
46+
ids, err := issues_model.GetAssigneeIDsByIssue(ctx, comment.Issue.ID)
4747
if err != nil {
48-
return fmt.Errorf("GetAssigneeIDsByIssue(%d): %w", ctx.Issue.ID, err)
48+
return fmt.Errorf("GetAssigneeIDsByIssue(%d): %w", comment.Issue.ID, err)
4949
}
5050
unfiltered = append(unfiltered, ids...)
5151

5252
// =========== Participants (i.e. commenters, reviewers) ===========
53-
ids, err = issues_model.GetParticipantsIDsByIssueID(ctx, ctx.Issue.ID)
53+
ids, err = issues_model.GetParticipantsIDsByIssueID(ctx, comment.Issue.ID)
5454
if err != nil {
55-
return fmt.Errorf("GetParticipantsIDsByIssueID(%d): %w", ctx.Issue.ID, err)
55+
return fmt.Errorf("GetParticipantsIDsByIssueID(%d): %w", comment.Issue.ID, err)
5656
}
5757
unfiltered = append(unfiltered, ids...)
5858

5959
// =========== Issue watchers ===========
60-
ids, err = issues_model.GetIssueWatchersIDs(ctx, ctx.Issue.ID, true)
60+
ids, err = issues_model.GetIssueWatchersIDs(ctx, comment.Issue.ID, true)
6161
if err != nil {
62-
return fmt.Errorf("GetIssueWatchersIDs(%d): %w", ctx.Issue.ID, err)
62+
return fmt.Errorf("GetIssueWatchersIDs(%d): %w", comment.Issue.ID, err)
6363
}
6464
unfiltered = append(unfiltered, ids...)
6565

6666
// =========== Repo watchers ===========
6767
// Make repo watchers last, since it's likely the list with the most users
68-
if !(ctx.Issue.IsPull && ctx.Issue.PullRequest.IsWorkInProgress(ctx) && ctx.ActionType != activities_model.ActionCreatePullRequest) {
69-
ids, err = repo_model.GetRepoWatchersIDs(ctx, ctx.Issue.RepoID)
68+
if !(comment.Issue.IsPull && comment.Issue.PullRequest.IsWorkInProgress(ctx) && comment.ActionType != activities_model.ActionCreatePullRequest) {
69+
ids, err = repo_model.GetRepoWatchersIDs(ctx, comment.Issue.RepoID)
7070
if err != nil {
71-
return fmt.Errorf("GetRepoWatchersIDs(%d): %w", ctx.Issue.RepoID, err)
71+
return fmt.Errorf("GetRepoWatchersIDs(%d): %w", comment.Issue.RepoID, err)
7272
}
7373
unfiltered = append(ids, unfiltered...)
7474
}
7575

7676
visited := make(container.Set[int64], len(unfiltered)+len(mentions)+1)
7777

7878
// Avoid mailing the doer
79-
if ctx.Doer.EmailNotificationsPreference != user_model.EmailNotificationsAndYourOwn && !ctx.ForceDoerNotification {
80-
visited.Add(ctx.Doer.ID)
79+
if comment.Doer.EmailNotificationsPreference != user_model.EmailNotificationsAndYourOwn && !comment.ForceDoerNotification {
80+
visited.Add(comment.Doer.ID)
8181
}
8282

8383
// =========== Mentions ===========
84-
if err = mailIssueCommentBatch(ctx, mentions, visited, true); err != nil {
84+
if err = mailIssueCommentBatch(ctx, comment, mentions, visited, true); err != nil {
8585
return fmt.Errorf("mailIssueCommentBatch() mentions: %w", err)
8686
}
8787

8888
// Avoid mailing explicit unwatched
89-
ids, err = issues_model.GetIssueWatchersIDs(ctx, ctx.Issue.ID, false)
89+
ids, err = issues_model.GetIssueWatchersIDs(ctx, comment.Issue.ID, false)
9090
if err != nil {
91-
return fmt.Errorf("GetIssueWatchersIDs(%d): %w", ctx.Issue.ID, err)
91+
return fmt.Errorf("GetIssueWatchersIDs(%d): %w", comment.Issue.ID, err)
9292
}
9393
visited.AddMultiple(ids...)
9494

9595
unfilteredUsers, err := user_model.GetMailableUsersByIDs(ctx, unfiltered, false)
9696
if err != nil {
9797
return err
9898
}
99-
if err = mailIssueCommentBatch(ctx, unfilteredUsers, visited, false); err != nil {
99+
if err = mailIssueCommentBatch(ctx, comment, unfilteredUsers, visited, false); err != nil {
100100
return fmt.Errorf("mailIssueCommentBatch(): %w", err)
101101
}
102102

103103
return nil
104104
}
105105

106-
func mailIssueCommentBatch(ctx *mailCommentContext, users []*user_model.User, visited container.Set[int64], fromMention bool) error {
106+
func mailIssueCommentBatch(ctx context.Context, comment *mailComment, users []*user_model.User, visited container.Set[int64], fromMention bool) error {
107107
checkUnit := unit.TypeIssues
108-
if ctx.Issue.IsPull {
108+
if comment.Issue.IsPull {
109109
checkUnit = unit.TypePullRequests
110110
}
111111

@@ -129,7 +129,7 @@ func mailIssueCommentBatch(ctx *mailCommentContext, users []*user_model.User, vi
129129
}
130130

131131
// test if this user is allowed to see the issue/pull
132-
if !access_model.CheckRepoUnitUser(ctx, ctx.Issue.Repo, user, checkUnit) {
132+
if !access_model.CheckRepoUnitUser(ctx, comment.Issue.Repo, user, checkUnit) {
133133
continue
134134
}
135135

@@ -141,7 +141,7 @@ func mailIssueCommentBatch(ctx *mailCommentContext, users []*user_model.User, vi
141141
// working backwards from the last (possibly) incomplete batch. If len(receivers) can be 0 this
142142
// starting condition will need to be changed slightly
143143
for i := ((len(receivers) - 1) / MailBatchSize) * MailBatchSize; i >= 0; i -= MailBatchSize {
144-
msgs, err := composeIssueCommentMessages(ctx, lang, receivers[i:], fromMention, "issue comments")
144+
msgs, err := composeIssueCommentMessages(ctx, comment, lang, receivers[i:], fromMention, "issue comments")
145145
if err != nil {
146146
return err
147147
}
@@ -168,9 +168,8 @@ func MailParticipants(ctx context.Context, issue *issues_model.Issue, doer *user
168168
content = ""
169169
}
170170
forceDoerNotification := opType == activities_model.ActionAutoMergePullRequest
171-
if err := mailIssueCommentToParticipants(
172-
&mailCommentContext{
173-
Context: ctx,
171+
if err := mailIssueCommentToParticipants(ctx,
172+
&mailComment{
174173
Issue: issue,
175174
Doer: doer,
176175
ActionType: opType,
@@ -205,8 +204,7 @@ func SendIssueAssignedMail(ctx context.Context, issue *issues_model.Issue, doer
205204
}
206205

207206
for lang, tos := range langMap {
208-
msgs, err := composeIssueCommentMessages(&mailCommentContext{
209-
Context: ctx,
207+
msgs, err := composeIssueCommentMessages(ctx, &mailComment{
210208
Issue: issue,
211209
Doer: doer,
212210
ActionType: activities_model.ActionType(0),

Diff for: services/mailer/mail_issue_common.go

+32-33
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,7 @@ func fallbackMailSubject(issue *issues_model.Issue) string {
3333
return fmt.Sprintf("[%s] %s (#%d)", issue.Repo.FullName(), issue.Title, issue.Index)
3434
}
3535

36-
type mailCommentContext struct {
37-
context.Context
36+
type mailComment struct {
3837
Issue *issues_model.Issue
3938
Doer *user_model.User
4039
ActionType activities_model.ActionType
@@ -43,7 +42,7 @@ type mailCommentContext struct {
4342
ForceDoerNotification bool
4443
}
4544

46-
func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipients []*user_model.User, fromMention bool, info string) ([]*sender_service.Message, error) {
45+
func composeIssueCommentMessages(ctx context.Context, comment *mailComment, lang string, recipients []*user_model.User, fromMention bool, info string) ([]*sender_service.Message, error) {
4746
var (
4847
subject string
4948
link string
@@ -54,44 +53,44 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient
5453
)
5554

5655
commentType := issues_model.CommentTypeComment
57-
if ctx.Comment != nil {
58-
commentType = ctx.Comment.Type
59-
link = ctx.Issue.HTMLURL() + "#" + ctx.Comment.HashTag()
56+
if comment.Comment != nil {
57+
commentType = comment.Comment.Type
58+
link = comment.Issue.HTMLURL() + "#" + comment.Comment.HashTag()
6059
} else {
61-
link = ctx.Issue.HTMLURL()
60+
link = comment.Issue.HTMLURL()
6261
}
6362

6463
reviewType := issues_model.ReviewTypeComment
65-
if ctx.Comment != nil && ctx.Comment.Review != nil {
66-
reviewType = ctx.Comment.Review.Type
64+
if comment.Comment != nil && comment.Comment.Review != nil {
65+
reviewType = comment.Comment.Review.Type
6766
}
6867

6968
// This is the body of the new issue or comment, not the mail body
70-
rctx := renderhelper.NewRenderContextRepoComment(ctx.Context, ctx.Issue.Repo).WithUseAbsoluteLink(true)
71-
body, err := markdown.RenderString(rctx, ctx.Content)
69+
rctx := renderhelper.NewRenderContextRepoComment(ctx, comment.Issue.Repo).WithUseAbsoluteLink(true)
70+
body, err := markdown.RenderString(rctx, comment.Content)
7271
if err != nil {
7372
return nil, err
7473
}
7574

7675
if setting.MailService.EmbedAttachmentImages {
77-
attEmbedder := newMailAttachmentBase64Embedder(ctx.Doer, ctx.Issue.Repo, maxEmailBodySize)
76+
attEmbedder := newMailAttachmentBase64Embedder(comment.Doer, comment.Issue.Repo, maxEmailBodySize)
7877
bodyAfterEmbedding, err := attEmbedder.Base64InlineImages(ctx, body)
7978
if err != nil {
8079
log.Error("Failed to embed images in mail body: %v", err)
8180
} else {
8281
body = bodyAfterEmbedding
8382
}
8483
}
85-
actType, actName, tplName := actionToTemplate(ctx.Issue, ctx.ActionType, commentType, reviewType)
84+
actType, actName, tplName := actionToTemplate(comment.Issue, comment.ActionType, commentType, reviewType)
8685

8786
if actName != "new" {
8887
prefix = "Re: "
8988
}
90-
fallback = prefix + fallbackMailSubject(ctx.Issue)
89+
fallback = prefix + fallbackMailSubject(comment.Issue)
9190

92-
if ctx.Comment != nil && ctx.Comment.Review != nil {
91+
if comment.Comment != nil && comment.Comment.Review != nil {
9392
reviewComments = make([]*issues_model.Comment, 0, 10)
94-
for _, lines := range ctx.Comment.Review.CodeComments {
93+
for _, lines := range comment.Comment.Review.CodeComments {
9594
for _, comments := range lines {
9695
reviewComments = append(reviewComments, comments...)
9796
}
@@ -104,12 +103,12 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient
104103
"FallbackSubject": fallback,
105104
"Body": body,
106105
"Link": link,
107-
"Issue": ctx.Issue,
108-
"Comment": ctx.Comment,
109-
"IsPull": ctx.Issue.IsPull,
110-
"User": ctx.Issue.Repo.MustOwner(ctx),
111-
"Repo": ctx.Issue.Repo.FullName(),
112-
"Doer": ctx.Doer,
106+
"Issue": comment.Issue,
107+
"Comment": comment.Comment,
108+
"IsPull": comment.Issue.IsPull,
109+
"User": comment.Issue.Repo.MustOwner(ctx),
110+
"Repo": comment.Issue.Repo.FullName(),
111+
"Doer": comment.Doer,
113112
"IsMention": fromMention,
114113
"SubjectPrefix": prefix,
115114
"ActionType": actType,
@@ -140,22 +139,22 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient
140139
}
141140

142141
// Make sure to compose independent messages to avoid leaking user emails
143-
msgID := generateMessageIDForIssue(ctx.Issue, ctx.Comment, ctx.ActionType)
144-
reference := generateMessageIDForIssue(ctx.Issue, nil, activities_model.ActionType(0))
142+
msgID := generateMessageIDForIssue(comment.Issue, comment.Comment, comment.ActionType)
143+
reference := generateMessageIDForIssue(comment.Issue, nil, activities_model.ActionType(0))
145144

146145
var replyPayload []byte
147-
if ctx.Comment != nil {
148-
if ctx.Comment.Type.HasMailReplySupport() {
149-
replyPayload, err = incoming_payload.CreateReferencePayload(ctx.Comment)
146+
if comment.Comment != nil {
147+
if comment.Comment.Type.HasMailReplySupport() {
148+
replyPayload, err = incoming_payload.CreateReferencePayload(comment.Comment)
150149
}
151150
} else {
152-
replyPayload, err = incoming_payload.CreateReferencePayload(ctx.Issue)
151+
replyPayload, err = incoming_payload.CreateReferencePayload(comment.Issue)
153152
}
154153
if err != nil {
155154
return nil, err
156155
}
157156

158-
unsubscribePayload, err := incoming_payload.CreateReferencePayload(ctx.Issue)
157+
unsubscribePayload, err := incoming_payload.CreateReferencePayload(comment.Issue)
159158
if err != nil {
160159
return nil, err
161160
}
@@ -164,7 +163,7 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient
164163
for _, recipient := range recipients {
165164
msg := sender_service.NewMessageFrom(
166165
recipient.Email,
167-
fromDisplayName(ctx.Doer),
166+
fromDisplayName(comment.Doer),
168167
setting.MailService.FromEmail,
169168
subject,
170169
mailBody.String(),
@@ -175,7 +174,7 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient
175174
msg.SetHeader("In-Reply-To", reference)
176175

177176
references := []string{reference}
178-
listUnsubscribe := []string{"<" + ctx.Issue.HTMLURL() + ">"}
177+
listUnsubscribe := []string{"<" + comment.Issue.HTMLURL() + ">"}
179178

180179
if setting.IncomingEmail.Enabled {
181180
if replyPayload != nil {
@@ -203,7 +202,7 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient
203202
msg.SetHeader("References", references...)
204203
msg.SetHeader("List-Unsubscribe", listUnsubscribe...)
205204

206-
for key, value := range generateAdditionalHeaders(ctx, actType, recipient) {
205+
for key, value := range generateAdditionalHeaders(comment, actType, recipient) {
207206
msg.SetHeader(key, value)
208207
}
209208

@@ -303,7 +302,7 @@ func generateMessageIDForIssue(issue *issues_model.Issue, comment *issues_model.
303302
return fmt.Sprintf("<%s/%s/%d%s@%s>", issue.Repo.FullName(), path, issue.Index, extra, setting.Domain)
304303
}
305304

306-
func generateAdditionalHeaders(ctx *mailCommentContext, reason string, recipient *user_model.User) map[string]string {
305+
func generateAdditionalHeaders(ctx *mailComment, reason string, recipient *user_model.User) map[string]string {
307306
repo := ctx.Issue.Repo
308307

309308
return map[string]string{

0 commit comments

Comments
 (0)