Skip to content

Commit 465c9f2

Browse files
salmonumbrellaclaude
salmonumbrella
and
claude
committed
refactor(gmail): improve forward command with tests and shared helpers
- Fix shadow declaration lint error in forward test - Add edge case tests: HTML-only, custom subject, CC/BCC - Extract shared resolveSendFrom() helper to gmail_helpers.go - Remove redundant subject empty check (dead code) - Fix slice preallocation lint warnings - Add defaultAttachmentFilename constant to fix goconst lint Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 1e094dd commit 465c9f2

5 files changed

Lines changed: 334 additions & 61 deletions

File tree

‎internal/cmd/execute_gmail_forward_test.go‎

Lines changed: 278 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ func TestExecute_GmailForward_DefaultSubjectAndAttachments(t *testing.T) {
6161
t.Fatalf("ReadAll: %v", err)
6262
}
6363
var msg gmail.Message
64-
if err := json.Unmarshal(body, &msg); err != nil {
65-
t.Fatalf("unmarshal: %v body=%q", err, string(body))
64+
if unmarshalErr := json.Unmarshal(body, &msg); unmarshalErr != nil {
65+
t.Fatalf("unmarshal: %v body=%q", unmarshalErr, string(body))
6666
}
6767
raw, err := base64.RawURLEncoding.DecodeString(msg.Raw)
6868
if err != nil {
@@ -131,3 +131,279 @@ func TestExecute_GmailForward_DefaultSubjectAndAttachments(t *testing.T) {
131131
})
132132
})
133133
}
134+
135+
func TestExecute_GmailForward_HTMLOnlyMessage(t *testing.T) {
136+
origNew := newGmailService
137+
t.Cleanup(func() { newGmailService = origNew })
138+
139+
htmlBody := "<p>HTML <strong>body</strong></p>"
140+
htmlEncoded := base64.RawURLEncoding.EncodeToString([]byte(htmlBody))
141+
142+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
143+
switch {
144+
case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/gmail/v1/users/me/messages/m2"):
145+
w.Header().Set("Content-Type", "application/json")
146+
_ = json.NewEncoder(w).Encode(map[string]any{
147+
"id": "m2",
148+
"threadId": "t2",
149+
"payload": map[string]any{
150+
"headers": []map[string]any{
151+
{"name": "From", "value": "sender@example.com"},
152+
{"name": "To", "value": "you@example.com"},
153+
{"name": "Subject", "value": "HTML Email"},
154+
{"name": "Date", "value": "Wed, 17 Dec 2025 15:00:00 -0800"},
155+
},
156+
"parts": []map[string]any{
157+
{
158+
"mimeType": "text/html",
159+
"body": map[string]any{"data": htmlEncoded},
160+
},
161+
},
162+
},
163+
})
164+
return
165+
case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/gmail/v1/users/me/messages/send"):
166+
body, err := io.ReadAll(r.Body)
167+
if err != nil {
168+
t.Fatalf("ReadAll: %v", err)
169+
}
170+
var msg gmail.Message
171+
if unmarshalErr := json.Unmarshal(body, &msg); unmarshalErr != nil {
172+
t.Fatalf("unmarshal: %v body=%q", unmarshalErr, string(body))
173+
}
174+
raw, err := base64.RawURLEncoding.DecodeString(msg.Raw)
175+
if err != nil {
176+
t.Fatalf("decode raw: %v", err)
177+
}
178+
s := string(raw)
179+
if !strings.Contains(s, "Subject: Fwd: HTML Email\r\n") {
180+
t.Fatalf("missing forward subject in raw:\n%s", s)
181+
}
182+
if !strings.Contains(s, "-------- Forwarded message --------") {
183+
t.Fatalf("missing forwarded header in raw:\n%s", s)
184+
}
185+
if !strings.Contains(s, "HTML body") {
186+
t.Fatalf("missing stripped HTML content in plain part:\n%s", s)
187+
}
188+
if !strings.Contains(s, htmlBody) {
189+
t.Fatalf("missing original HTML in HTML part:\n%s", s)
190+
}
191+
w.Header().Set("Content-Type", "application/json")
192+
_ = json.NewEncoder(w).Encode(map[string]any{"id": "s2", "threadId": "t2"})
193+
return
194+
default:
195+
http.NotFound(w, r)
196+
return
197+
}
198+
}))
199+
defer srv.Close()
200+
201+
svc, err := gmail.NewService(context.Background(),
202+
option.WithoutAuthentication(),
203+
option.WithHTTPClient(srv.Client()),
204+
option.WithEndpoint(srv.URL+"/"),
205+
)
206+
if err != nil {
207+
t.Fatalf("NewService: %v", err)
208+
}
209+
newGmailService = func(context.Context, string) (*gmail.Service, error) { return svc, nil }
210+
211+
_ = captureStdout(t, func() {
212+
_ = captureStderr(t, func() {
213+
if err := Execute([]string{
214+
"--json",
215+
"--account", "a@b.com",
216+
"gmail", "forward", "m2", "to@example.com",
217+
"--body", "Check this out.",
218+
}); err != nil {
219+
t.Fatalf("Execute: %v", err)
220+
}
221+
})
222+
})
223+
}
224+
225+
func TestExecute_GmailForward_CustomSubject(t *testing.T) {
226+
origNew := newGmailService
227+
t.Cleanup(func() { newGmailService = origNew })
228+
229+
bodyText := "Original body"
230+
bodyEncoded := base64.RawURLEncoding.EncodeToString([]byte(bodyText))
231+
232+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
233+
switch {
234+
case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/gmail/v1/users/me/messages/m3"):
235+
w.Header().Set("Content-Type", "application/json")
236+
_ = json.NewEncoder(w).Encode(map[string]any{
237+
"id": "m3",
238+
"threadId": "t3",
239+
"payload": map[string]any{
240+
"headers": []map[string]any{
241+
{"name": "From", "value": "sender@example.com"},
242+
{"name": "To", "value": "you@example.com"},
243+
{"name": "Subject", "value": "Original Subject"},
244+
{"name": "Date", "value": "Wed, 17 Dec 2025 16:00:00 -0800"},
245+
},
246+
"parts": []map[string]any{
247+
{
248+
"mimeType": "text/plain",
249+
"body": map[string]any{"data": bodyEncoded},
250+
},
251+
},
252+
},
253+
})
254+
return
255+
case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/gmail/v1/users/me/messages/send"):
256+
body, err := io.ReadAll(r.Body)
257+
if err != nil {
258+
t.Fatalf("ReadAll: %v", err)
259+
}
260+
var msg gmail.Message
261+
if unmarshalErr := json.Unmarshal(body, &msg); unmarshalErr != nil {
262+
t.Fatalf("unmarshal: %v body=%q", unmarshalErr, string(body))
263+
}
264+
raw, err := base64.RawURLEncoding.DecodeString(msg.Raw)
265+
if err != nil {
266+
t.Fatalf("decode raw: %v", err)
267+
}
268+
s := string(raw)
269+
if !strings.Contains(s, "Subject: Custom Forward Subject\r\n") {
270+
t.Fatalf("missing custom subject in raw:\n%s", s)
271+
}
272+
if strings.Contains(s, "Subject: Fwd: Original Subject\r\n") {
273+
t.Fatalf("should not contain default Fwd: subject:\n%s", s)
274+
}
275+
if !strings.Contains(s, "-------- Forwarded message --------") {
276+
t.Fatalf("missing forwarded header in raw:\n%s", s)
277+
}
278+
if !strings.Contains(s, bodyText) {
279+
t.Fatalf("missing original body in raw:\n%s", s)
280+
}
281+
w.Header().Set("Content-Type", "application/json")
282+
_ = json.NewEncoder(w).Encode(map[string]any{"id": "s3", "threadId": "t3"})
283+
return
284+
default:
285+
http.NotFound(w, r)
286+
return
287+
}
288+
}))
289+
defer srv.Close()
290+
291+
svc, err := gmail.NewService(context.Background(),
292+
option.WithoutAuthentication(),
293+
option.WithHTTPClient(srv.Client()),
294+
option.WithEndpoint(srv.URL+"/"),
295+
)
296+
if err != nil {
297+
t.Fatalf("NewService: %v", err)
298+
}
299+
newGmailService = func(context.Context, string) (*gmail.Service, error) { return svc, nil }
300+
301+
_ = captureStdout(t, func() {
302+
_ = captureStderr(t, func() {
303+
if err := Execute([]string{
304+
"--json",
305+
"--account", "a@b.com",
306+
"gmail", "forward", "m3", "to@example.com",
307+
"--subject", "Custom Forward Subject",
308+
"--body", "FYI.",
309+
}); err != nil {
310+
t.Fatalf("Execute: %v", err)
311+
}
312+
})
313+
})
314+
}
315+
316+
func TestExecute_GmailForward_CcAndBcc(t *testing.T) {
317+
origNew := newGmailService
318+
t.Cleanup(func() { newGmailService = origNew })
319+
320+
bodyText := "Original body"
321+
bodyEncoded := base64.RawURLEncoding.EncodeToString([]byte(bodyText))
322+
323+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
324+
switch {
325+
case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/gmail/v1/users/me/messages/m4"):
326+
w.Header().Set("Content-Type", "application/json")
327+
_ = json.NewEncoder(w).Encode(map[string]any{
328+
"id": "m4",
329+
"threadId": "t4",
330+
"payload": map[string]any{
331+
"headers": []map[string]any{
332+
{"name": "From", "value": "sender@example.com"},
333+
{"name": "To", "value": "you@example.com"},
334+
{"name": "Subject", "value": "Test Message"},
335+
{"name": "Date", "value": "Wed, 17 Dec 2025 17:00:00 -0800"},
336+
},
337+
"parts": []map[string]any{
338+
{
339+
"mimeType": "text/plain",
340+
"body": map[string]any{"data": bodyEncoded},
341+
},
342+
},
343+
},
344+
})
345+
return
346+
case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/gmail/v1/users/me/messages/send"):
347+
body, err := io.ReadAll(r.Body)
348+
if err != nil {
349+
t.Fatalf("ReadAll: %v", err)
350+
}
351+
var msg gmail.Message
352+
if unmarshalErr := json.Unmarshal(body, &msg); unmarshalErr != nil {
353+
t.Fatalf("unmarshal: %v body=%q", unmarshalErr, string(body))
354+
}
355+
raw, err := base64.RawURLEncoding.DecodeString(msg.Raw)
356+
if err != nil {
357+
t.Fatalf("decode raw: %v", err)
358+
}
359+
s := string(raw)
360+
if !strings.Contains(s, "To: to@example.com\r\n") {
361+
t.Fatalf("missing To header in raw:\n%s", s)
362+
}
363+
if !strings.Contains(s, "Cc: cc1@example.com, cc2@example.com\r\n") {
364+
t.Fatalf("missing Cc header in raw:\n%s", s)
365+
}
366+
if !strings.Contains(s, "Bcc: bcc@example.com\r\n") {
367+
t.Fatalf("missing Bcc header in raw:\n%s", s)
368+
}
369+
if !strings.Contains(s, "-------- Forwarded message --------") {
370+
t.Fatalf("missing forwarded header in raw:\n%s", s)
371+
}
372+
if !strings.Contains(s, bodyText) {
373+
t.Fatalf("missing original body in raw:\n%s", s)
374+
}
375+
w.Header().Set("Content-Type", "application/json")
376+
_ = json.NewEncoder(w).Encode(map[string]any{"id": "s4", "threadId": "t4"})
377+
return
378+
default:
379+
http.NotFound(w, r)
380+
return
381+
}
382+
}))
383+
defer srv.Close()
384+
385+
svc, err := gmail.NewService(context.Background(),
386+
option.WithoutAuthentication(),
387+
option.WithHTTPClient(srv.Client()),
388+
option.WithEndpoint(srv.URL+"/"),
389+
)
390+
if err != nil {
391+
t.Fatalf("NewService: %v", err)
392+
}
393+
newGmailService = func(context.Context, string) (*gmail.Service, error) { return svc, nil }
394+
395+
_ = captureStdout(t, func() {
396+
_ = captureStderr(t, func() {
397+
if err := Execute([]string{
398+
"--json",
399+
"--account", "a@b.com",
400+
"gmail", "forward", "m4", "to@example.com",
401+
"--cc", "cc1@example.com,cc2@example.com",
402+
"--bcc", "bcc@example.com",
403+
"--body", "Important message.",
404+
}); err != nil {
405+
t.Fatalf("Execute: %v", err)
406+
}
407+
})
408+
})
409+
}

‎internal/cmd/gmail_attachments.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ func collectAttachments(p *gmail.MessagePart) []attachmentInfo {
181181
if p.Body != nil && p.Body.AttachmentId != "" {
182182
filename := p.Filename
183183
if strings.TrimSpace(filename) == "" {
184-
filename = "attachment"
184+
filename = defaultAttachmentFilename
185185
}
186186
out = append(out, attachmentInfo{
187187
Filename: filename,

‎internal/cmd/gmail_forward.go‎

Lines changed: 6 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func (c *GmailForwardCmd) Run(ctx context.Context, flags *RootFlags) error {
5555
return fmt.Errorf("message %s has no payload", messageID)
5656
}
5757

58-
fromAddr, err := resolveForwardFrom(ctx, svc, account, c.From)
58+
fromAddr, err := resolveSendFrom(ctx, svc, account, c.From)
5959
if err != nil {
6060
return err
6161
}
@@ -64,9 +64,6 @@ func (c *GmailForwardCmd) Run(ctx context.Context, flags *RootFlags) error {
6464
if subject == "" {
6565
subject = forwardSubject(headerValue(msg.Payload, "Subject"))
6666
}
67-
if subject == "" {
68-
subject = "Fwd: (no subject)"
69-
}
7067

7168
plainBody, htmlBody := buildForwardBodies(msg.Payload, prefacePlain, c.BodyHTML)
7269

@@ -111,33 +108,6 @@ func (c *GmailForwardCmd) Run(ctx context.Context, flags *RootFlags) error {
111108
return writeSendResults(ctx, u, fromAddr, results)
112109
}
113110

114-
func resolveForwardFrom(ctx context.Context, svc *gmail.Service, account, from string) (string, error) {
115-
fromAddr := account
116-
from = strings.TrimSpace(from)
117-
if from != "" {
118-
sa, err := svc.Users.Settings.SendAs.Get("me", from).Context(ctx).Do()
119-
if err != nil {
120-
return "", fmt.Errorf("invalid --from address %q: %w", from, err)
121-
}
122-
if sa.VerificationStatus != gmailVerificationAccepted {
123-
return "", fmt.Errorf("--from address %q is not verified (status: %s)", from, sa.VerificationStatus)
124-
}
125-
fromAddr = from
126-
if sa.DisplayName != "" {
127-
fromAddr = sa.DisplayName + " <" + from + ">"
128-
}
129-
return fromAddr, nil
130-
}
131-
132-
// No --from specified: look up the primary account's send-as settings
133-
// to get the display name
134-
sa, saErr := svc.Users.Settings.SendAs.Get("me", account).Context(ctx).Do()
135-
if saErr == nil && sa.DisplayName != "" {
136-
fromAddr = sa.DisplayName + " <" + account + ">"
137-
}
138-
return fromAddr, nil
139-
}
140-
141111
func forwardSubject(original string) string {
142112
subject := strings.TrimSpace(original)
143113
if subject == "" {
@@ -168,15 +138,17 @@ func forwardHeaderFields(p *gmail.MessagePart) []forwardHeaderField {
168138
}
169139

170140
func forwardHeaderPlain(p *gmail.MessagePart) string {
171-
lines := []string{"-------- Forwarded message --------"}
141+
lines := make([]string, 0, 6)
142+
lines = append(lines, "-------- Forwarded message --------")
172143
for _, field := range forwardHeaderFields(p) {
173144
lines = append(lines, fmt.Sprintf("%s: %s", field.Label, field.Value))
174145
}
175146
return strings.Join(lines, "\n")
176147
}
177148

178149
func forwardHeaderHTML(p *gmail.MessagePart) string {
179-
lines := []string{html.EscapeString("-------- Forwarded message --------")}
150+
lines := make([]string, 0, 6)
151+
lines = append(lines, html.EscapeString("-------- Forwarded message --------"))
180152
for _, field := range forwardHeaderFields(p) {
181153
lines = append(lines, fmt.Sprintf("%s: %s", html.EscapeString(field.Label), html.EscapeString(field.Value)))
182154
}
@@ -247,7 +219,7 @@ func collectForwardAttachments(ctx context.Context, svc *gmail.Service, messageI
247219
hasInlineData := strings.TrimSpace(p.Body.Data) != "" && filename != ""
248220
if hasAttachmentID || hasInlineData {
249221
if filename == "" {
250-
filename = "attachment"
222+
filename = defaultAttachmentFilename
251223
}
252224
att := mailAttachment{Filename: filename, MIMEType: p.MimeType}
253225
var data []byte

0 commit comments

Comments
 (0)