feat: add default signatures to mail send#1303
Conversation
Append the sender's default send signature in +send while keeping --signature-id as an explicit override and adding --no-signature for opt-out. Plain-text bodies now receive a rendered text signature without upgrading to HTML. sprint: S1
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a ChangesMail Send Signature Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 #1303 +/- ##
==========================================
+ Coverage 70.97% 71.09% +0.11%
==========================================
Files 680 681 +1
Lines 65318 65498 +180
==========================================
+ Hits 46362 46567 +205
+ Misses 15316 15273 -43
- Partials 3640 3658 +18 ☔ 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@08c31c5bb87d588d675fae6fefaf5aad0bcdaafe🧩 Skill updatenpx skills add bubbmon233/cli#feat/ea59764 -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shortcuts/mail/signature_compose.go (1)
114-136: 💤 Low valueOptional: Redundant case normalization.
Line 118 uses
strings.ToLoweronsenderEmail, but line 126 usesstrings.EqualFoldwhich already performs case-insensitive comparison. TheToLowercall is redundant.♻️ Proposed simplification
- sender := strings.ToLower(strings.TrimSpace(senderEmail)) + sender := strings.TrimSpace(senderEmail)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/mail/signature_compose.go` around lines 114 - 136, In selectDefaultSendSignatureID, the sender variable is unnecessarily normalized with strings.ToLower before using strings.EqualFold; change sender := strings.ToLower(strings.TrimSpace(senderEmail)) to just sender := strings.TrimSpace(senderEmail) (or otherwise ensure you use the same normalization on both sides) so the case normalization is not duplicated, keep the existing strings.EqualFold(strings.TrimSpace(usage.EmailAddress), sender) comparison and leave the remaining logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/mail/mail_send_signature_test.go`:
- Around line 216-244: The test
TestMailSendDryRunShowsSignatureLookupUnlessDisabled reuses the same stdout
buffer between two runMountedMailShortcut calls causing the first run's
"/settings/signatures" output to leak into the second assertion; fix by
resetting or replacing the captured output before the second dry-run (e.g., call
stdout.Reset() or recreate f and stdout via
mailShortcutTestFactoryWithSendScope) so the second invocation of
runMountedMailShortcut and its assertion accurately reflect only that run's
output.
---
Nitpick comments:
In `@shortcuts/mail/signature_compose.go`:
- Around line 114-136: In selectDefaultSendSignatureID, the sender variable is
unnecessarily normalized with strings.ToLower before using strings.EqualFold;
change sender := strings.ToLower(strings.TrimSpace(senderEmail)) to just sender
:= strings.TrimSpace(senderEmail) (or otherwise ensure you use the same
normalization on both sides) so the case normalization is not duplicated, keep
the existing strings.EqualFold(strings.TrimSpace(usage.EmailAddress), sender)
comparison and leave the remaining logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8146579b-e3f5-4a4b-9ee0-40231da5a339
📒 Files selected for processing (10)
shortcuts/mail/draft/htmltext.goshortcuts/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_send_signature_test.goshortcuts/mail/mail_template_shortcut_test.goshortcuts/mail/signature_compose.goshortcuts/mail/signature_compose_test.goskills/lark-mail/references/lark-mail-send.md
Change-Type: ci-fix
| if strings.TrimSpace(runtime.Str("signature-id")) != "" { | ||
| desc = "Resolve explicit signature by ID." | ||
| } | ||
| api = api.GET(mailboxPath(mailboxID, "settings", "signatures")).Desc(desc) |
There was a problem hiding this comment.
🤖 AI Review | [P2 正确性] DryRun 少展示签名插值会调用的 send_as 请求
mail +send --dry-run 在非 --no-signature 分支只展示了 /settings/signatures，但真实执行在选中显式签名或默认签名后还会进入 resolveSenderInfo，调用 GET /settings/send_as 来解析签名模板中的 sender 变量。AI/脚本用 dry-run 预判权限、接口计划或准备 mock 时，会少配这个接口，导致 dry-run 与实际执行不一致。
修复建议: 在这个签名分支中补充 GET /settings/send_as，描述为用于签名模板 sender 变量插值；至少对 --signature-id 场景无条件展示，对默认签名场景说明命中默认签名后会调用。
如有疑问或认为判断不准确，欢迎直接回复讨论。
|
🤖 AI Review | CR 汇总 | 有风险（1 个 P2） 增量审查：已读取既有 CodeRabbit 评论，测试缓冲区问题已由后续提交修复，未重复提出。本次新增 1 条 P2 正确性问题： 验证： |
2 participants
Generated by the harness-coding skill (recovery run - original attempt crashed before PR open).
Commits on branch (ahead of main)
This resume run
Source specs
This PR was created autonomously. Quality gates were enforced by the repository pre-commit hooks.
Summary by CodeRabbit
New Features
--no-signatureCLI flag to skip default signature; improved signature handling for both HTML and plain-text sends; explicit signature selection honored.Documentation
--no-signatureand signature-related options.Tests