feat(mail): add +template-get read-only shortcut#675
Closed
infeng wants to merge 1 commit intolarksuite:mainfrom
Closed
feat(mail): add +template-get read-only shortcut#675infeng wants to merge 1 commit intolarksuite:mainfrom
infeng wants to merge 1 commit intolarksuite:mainfrom
Conversation
Implements `lark-cli mail +template-get` for fetching a personal mail template by ID. Validate stage rejects non-decimal template-id strings (no int64 round-trip); Execute reads via runtime.RawAPI so --format json returns the raw OAPI response, while --format pretty surfaces a compact summary with template_content rune-truncated to 200 chars. sprint: S2
github-actions
Bot
added
domain/mail
📝 WalkthroughWalkthroughA new CLI shortcut Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #675 +/- ##
=======================================
Coverage 62.96% 62.97%
=======================================
Files 436 437 +1
Lines 38605 38664 +59
=======================================
+ Hits 24307 24347 +40
- Misses 12150 12165 +15
- Partials 2148 2152 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@c6513fc175274333411c593acc4c2fbec004f970🧩 Skill updatenpx skills add infeng/cli#harness/smoke-cli-mail-template-mini-20260427-160723 -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
shortcuts/mail/mail_template_get_test.go (2)
146-183: Optional: add a multi-byte rune truncation case.The truncation cap is documented as rune-based (
common.TruncateStroperates on[]rune), but the existing test only uses ASCIIas, so it wouldn’t catch a regression that swapped to byte-based slicing. A small follow-up case with multi-byte characters (e.g.,strings.Repeat("中", 250)) would lock the rune semantics in.♻️ Suggested addition
+ // Rune semantics: 250 multi-byte runes should truncate to 200 runes, + // not to a 200-byte slice that may split a code point. + dataUnicode := map[string]interface{}{ + "template_id": "1", + "name": "n", + "subject": "s", + "template_content": strings.Repeat("中", 250), + } + var bufU strings.Builder + renderTemplatePretty(&bufU, dataUnicode) + if !strings.Contains(bufU.String(), strings.Repeat("中", 200)) { + t.Errorf("expected 200 multi-byte runes in pretty output") + } + if strings.Contains(bufU.String(), strings.Repeat("中", 201)) { + t.Errorf("multi-byte template_content was not truncated to 200 runes") + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/mail_template_get_test.go` around lines 146 - 183, Add a test case to ensure truncation is rune-based by using a multi-byte string (e.g., strings.Repeat("中", 250)) and passing it through renderTemplatePretty (or directly through common.TruncateStr if appropriate) similar to the existing ASCII test; assert the output contains exactly the expected number of runes (200) and does not contain 201 runes, mirroring the existing checks that use strings.Repeat("a", ...), so any byte-based slicing regressions are caught.
97-122: Nit:@is a gen-delim, not a sub-delim.Per RFC 3986,
@is a gen-delim; it’s left unescaped in path segments becausepcharexplicitly permits@. The behavioral assertion (URL keeps the literal@) is correct — only the comment’s classification is off.📝 Suggested wording tweak
- // `@` is a sub-delim that url.PathEscape leaves untouched, so the - // outgoing URL keeps the literal `@`. mailboxPath would still escape - // reserved chars (e.g. `/`); this guards against accidental raw - // fmt.Sprintf regression on the segment. + // `@` is allowed in path segments (RFC 3986 pchar) and url.PathEscape + // leaves it untouched, so the outgoing URL keeps the literal `@`. + // mailboxPath would still escape reserved chars (e.g. `/`); this + // guards against accidental raw fmt.Sprintf regression on the segment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/mail_template_get_test.go` around lines 97 - 122, The test comment in TestMailTemplateGet_PathEscapesMailbox misclassifies the '@' character as a sub-delim; update the comment to state '@' is a gen-delim per RFC 3986 and keep the rest of the explanation (that url.PathEscape leaves it unescaped and the test guards against accidental raw fmt.Sprintf regression on the segment). Locate the comment above the httpmock.Stub in TestMailTemplateGet_PathEscapesMailbox (reference mailboxPath in the comment) and replace "sub-delim" with "gen-delim" and adjust wording accordingly so the comment is factually correct while preserving the intent.shortcuts/mail/mail_template_get.go (1)
93-104: Optional: prefer-one-shape recipient counting.Summing across both
toandto_recipients(and similarly for cc/bcc) would double-count if the OAPI response ever returns both forms. In practice the API uses one shape, so this is fine, but if you want stricter behavior you could short-circuit per recipient class:♻️ Optional refactor
func recipientCount(data map[string]interface{}) int { - total := 0 - for _, key := range []string{"to", "to_recipients", "cc", "cc_recipients", "bcc", "bcc_recipients"} { - if list, ok := data[key].([]interface{}); ok { - total += len(list) - } - } - return total + count := func(primary, fallback string) int { + if list, ok := data[primary].([]interface{}); ok { + return len(list) + } + if list, ok := data[fallback].([]interface{}); ok { + return len(list) + } + return 0 + } + return count("to", "to_recipients") + count("cc", "cc_recipients") + count("bcc", "bcc_recipients") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/mail_template_get.go` around lines 93 - 104, The recipientCount function can double-count when both alternate shapes exist; update recipientCount to prefer one shape per recipient class (for each pair: prefer "to_recipients" if present else "to", prefer "cc_recipients" else "cc", prefer "bcc_recipients" else "bcc") and short-circuit after finding the first present list so you only count one shape per class; locate the recipientCount function and change the loop logic to check each pair in turn (checking existence/type of the preferred key first) and add its length to total without also counting the alternate key.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@shortcuts/mail/mail_template_get_test.go`:
- Around line 146-183: Add a test case to ensure truncation is rune-based by
using a multi-byte string (e.g., strings.Repeat("中", 250)) and passing it
through renderTemplatePretty (or directly through common.TruncateStr if
appropriate) similar to the existing ASCII test; assert the output contains
exactly the expected number of runes (200) and does not contain 201 runes,
mirroring the existing checks that use strings.Repeat("a", ...), so any
byte-based slicing regressions are caught.
- Around line 97-122: The test comment in TestMailTemplateGet_PathEscapesMailbox
misclassifies the '@' character as a sub-delim; update the comment to state '@'
is a gen-delim per RFC 3986 and keep the rest of the explanation (that
url.PathEscape leaves it unescaped and the test guards against accidental raw
fmt.Sprintf regression on the segment). Locate the comment above the
httpmock.Stub in TestMailTemplateGet_PathEscapesMailbox (reference mailboxPath
in the comment) and replace "sub-delim" with "gen-delim" and adjust wording
accordingly so the comment is factually correct while preserving the intent.
In `@shortcuts/mail/mail_template_get.go`:
- Around line 93-104: The recipientCount function can double-count when both
alternate shapes exist; update recipientCount to prefer one shape per recipient
class (for each pair: prefer "to_recipients" if present else "to", prefer
"cc_recipients" else "cc", prefer "bcc_recipients" else "bcc") and short-circuit
after finding the first present list so you only count one shape per class;
locate the recipientCount function and change the loop logic to check each pair
in turn (checking existence/type of the preferred key first) and add its length
to total without also counting the alternate key.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 87574dbb-e06a-4e71-93d6-7afa8714d18b
📒 Files selected for processing (4)
shortcuts/mail/mail_template_get.goshortcuts/mail/mail_template_get_test.goshortcuts/mail/shortcuts.goskills/lark-mail/references/lark-mail-template-get.md
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
1 participant
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
mail +template-getread-only shortcutSource specs
This MR was created autonomously. Quality gates were enforced by the repo's own pre-commit hooks.
Summary by CodeRabbit
New Features
mail +template-getcommand to retrieve personal mail templates by template ID with client-side validation. Supports multiple output formats (JSON, pretty, table, NDJSON, CSV) and dry-run mode. Pretty output displays truncated template content and recipient counts.Documentation