feat: auto-append default signatures in mail send#1310
Open
bubbmon233 wants to merge 1 commit into
Open
Conversation
Align mail +send with mailbox defaults so compose flows pick the sender configured send signature unless callers explicitly opt out. Preserve the plain-text path by rendering signatures as text instead of upgrading MIME. sprint: S1
📝 WalkthroughWalkthroughThis PR adds a ChangesMail send signature feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
domain/mail
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1310 +/- ##
==========================================
+ Coverage 71.01% 71.06% +0.04%
==========================================
Files 681 681
Lines 65435 65575 +140
==========================================
+ Hits 46470 46601 +131
+ Misses 15320 15317 -3
- Partials 3645 3657 +12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@06827a7522f25a47f8008d9098b3c305b4b60da6🧩 Skill updatenpx skills add bubbmon233/cli#feat/d6fb95d -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/mail/mail_send.go (1)
208-241:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSkip signature image downloads on the plain-text path.
Line 208 resolves the full signature before the
plainTextbranch, andresolveSignatureeagerly downloads inline images. Lines 239-241 then render the signature as text only, while the plain-text renderer drops<img>nodes and the builder never usessigResult.Images. That makes plain-text sends depend on external image downloads they never emit, so an expired or broken signature image URL can now fail an otherwise valid plain-text send. Please gate image downloads on HTML mode only, and add a regression for a plain-text send whose signature image fetch fails.Possible fix
- sigResult, err := resolveComposeSignature(ctx, runtime, mailboxID, senderEmail) + sigResult, err := resolveComposeSignature(ctx, runtime, mailboxID, senderEmail, !plainText)-func resolveComposeSignature(ctx context.Context, runtime *common.RuntimeContext, mailboxID, senderEmail string) (*signatureResult, error) { +func resolveComposeSignature(ctx context.Context, runtime *common.RuntimeContext, mailboxID, senderEmail string, includeImages bool) (*signatureResult, error) { if runtime.Bool("no-signature") { return nil, nil } if signatureID := runtime.Str("signature-id"); signatureID != "" { - return resolveSignature(ctx, runtime, mailboxID, signatureID, senderEmail) + return resolveSignature(ctx, runtime, mailboxID, signatureID, senderEmail, includeImages) } defaultID, err := resolveDefaultSendSignature(runtime, mailboxID, senderEmail) if err != nil { return nil, err } if defaultID == "" { return nil, nil } - return resolveSignature(ctx, runtime, mailboxID, defaultID, senderEmail) + return resolveSignature(ctx, runtime, mailboxID, defaultID, senderEmail, includeImages) }-func resolveSignature(ctx context.Context, runtime *common.RuntimeContext, mailboxID, signatureID, fromEmail string) (*signatureResult, error) { +func resolveSignature(ctx context.Context, runtime *common.RuntimeContext, mailboxID, signatureID, fromEmail string, includeImages bool) (*signatureResult, error) { ... - var images []draftpkg.SignatureImage - for _, img := range sig.Images { - if img.DownloadURL == "" || img.CID == "" { - continue - } - data, ct, err := downloadSignatureImage(runtime, img.DownloadURL, img.ImageName) - if err != nil { - return nil, mailDecorateProblemMessage(err, "failed to download signature image %s", img.ImageName) - } - images = append(images, draftpkg.SignatureImage{ - CID: img.CID, - ContentType: ct, - FileName: img.ImageName, - Data: data, - }) - } + var images []draftpkg.SignatureImage + if includeImages { + for _, img := range sig.Images { + if img.DownloadURL == "" || img.CID == "" { + continue + } + data, ct, err := downloadSignatureImage(runtime, img.DownloadURL, img.ImageName) + if err != nil { + return nil, mailDecorateProblemMessage(err, "failed to download signature image %s", img.ImageName) + } + images = append(images, draftpkg.SignatureImage{ + CID: img.CID, + ContentType: ct, + FileName: img.ImageName, + Data: data, + }) + } + }
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fb09c5ef-a778-4d45-b626-82e3c6dcc06c
📒 Files selected for processing (10)
shortcuts/mail/mail_lint_writepath_test.goshortcuts/mail/mail_request_receipt_integration_test.goshortcuts/mail/mail_send.goshortcuts/mail/mail_send_confirm_output_test.goshortcuts/mail/mail_shortcut_validation_test.goshortcuts/mail/mail_template_shortcut_test.goshortcuts/mail/signature_compose.goshortcuts/mail/signature_compose_test.goskills/lark-mail/references/lark-mail-send.mdtests/cli_e2e/mail/mail_send_workflow_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Generated by the harness-coding skill.
Sprints
This MR was created autonomously. Quality gates were enforced by the repo's own pre-commit hooks.
Summary by CodeRabbit
Release Notes
New Features
--no-signatureflag to disable automatic signature insertion when sending mailTests