feat(mail): add +rule-reorder shortcut with auto-fill algorithm#1173
Open
bubbmon233 wants to merge 1 commit into
Open
feat(mail): add +rule-reorder shortcut with auto-fill algorithm#1173bubbmon233 wants to merge 1 commit into
bubbmon233 wants to merge 1 commit into
Conversation
Implement mail rule reorder shortcut that auto-fills missing rule IDs based on current sorting order. User-provided IDs are treated as anchors; unmentioned IDs fill gaps between consecutive anchor pairs that enclose them in current order, preserving their relative sequence. sprint: S2
📝 WalkthroughWalkthroughThis PR introduces the ChangesMail rule reorder shortcut
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 #1173 +/- ##
==========================================
+ Coverage 68.92% 68.97% +0.05%
==========================================
Files 628 630 +2
Lines 58744 58885 +141
==========================================
+ Hits 40487 40614 +127
- Misses 14951 14959 +8
- Partials 3306 3312 +6 ☔ 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@3b2aaaa4f53fcf218459d432aae5529f438c666b🧩 Skill updatenpx skills add bubbmon233/cli#feat/f089f54 -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 (2)
shortcuts/mail/mail_rule_reorder_test.go (2)
428-443: ⚡ Quick winPrefer the shared runtime test helper here.
runtimeForRuleReorderhand-buildsRuntimeContext, which is easy to drift from the repo’s shortcut test setup. Using the standard validate/dry-run helper keeps these tests aligned with future runtime initialization changes.Based on learnings, use
common.TestNewRuntimeContext(t, config)for validate/dry-run unit tests that only cover flag parsing and branch logic and do not perform HTTP requests.🤖 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/mail_rule_reorder_test.go` around lines 428 - 443, The test helper runtimeForRuleReorder manually constructs a RuntimeContext from flags and should be replaced with the shared helper to avoid drift; update the function (or callers) to call common.TestNewRuntimeContext(t, values) instead of hand-building cobra.Command and setting flags, or change runtimeForRuleReorder to simply return common.TestNewRuntimeContext(t, values), ensuring the same map[string]string values are passed through so flag parsing and RuntimeContext creation use the canonical test helper.
335-368: ⚡ Quick winAssert the reorder request body in the success path.
This test only checks the formatted output. If
Executeprints the rightrule_idsbut posts a different payload, the backend contract regresses and this still passes. Please have the POST stub verify therule_idsbody as well.🤖 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/mail_rule_reorder_test.go` around lines 335 - 368, The POST stub for "/user_mailboxes/me/rules/reorder" currently only returns {"code":0} but doesn't assert the request payload; update the httpmock.Stub registered for that URL so its Body (or BodyJSON matcher) verifies the incoming "rule_ids" array equals the expected payload (the reordered IDs from the test input, e.g. ["E","A"]) before returning code 0. Locate the stub registration near the test that calls runMountedMailShortcut(t, MailRuleReorder, ...) and add the verification of the "rule_ids" field to ensure the Execute call posts the correct request body.
🤖 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_rule_reorder.go`:
- Around line 117-137: The extractRuleIDs function must fail closed instead of
silently skipping malformed items: change extractRuleIDs to validate every item
and return an error when an entry is not a map[string]interface{} or when an
"id" is missing/empty (e.g. change signature to return ([]string, error)), and
update the caller Execute to check the error and abort before
constructing/posting rule_ids; ensure the error message identifies the offending
item index or content so the reorder POST is never sent with an incomplete
rule_ids list.
---
Nitpick comments:
In `@shortcuts/mail/mail_rule_reorder_test.go`:
- Around line 428-443: The test helper runtimeForRuleReorder manually constructs
a RuntimeContext from flags and should be replaced with the shared helper to
avoid drift; update the function (or callers) to call
common.TestNewRuntimeContext(t, values) instead of hand-building cobra.Command
and setting flags, or change runtimeForRuleReorder to simply return
common.TestNewRuntimeContext(t, values), ensuring the same map[string]string
values are passed through so flag parsing and RuntimeContext creation use the
canonical test helper.
- Around line 335-368: The POST stub for "/user_mailboxes/me/rules/reorder"
currently only returns {"code":0} but doesn't assert the request payload; update
the httpmock.Stub registered for that URL so its Body (or BodyJSON matcher)
verifies the incoming "rule_ids" array equals the expected payload (the
reordered IDs from the test input, e.g. ["E","A"]) before returning code 0.
Locate the stub registration near the test that calls runMountedMailShortcut(t,
MailRuleReorder, ...) and add the verification of the "rule_ids" field to ensure
the Execute call posts the correct request body.
🪄 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: bf332ee4-2d15-4046-807f-912cf4067e8e
📒 Files selected for processing (5)
shortcuts/mail/mail_rule_reorder.goshortcuts/mail/mail_rule_reorder_test.goshortcuts/mail/shortcuts.goskill-template/domains/mail.mdskills/lark-mail/references/lark-mail-rule-reorder.md
Comment on lines
+117
to
+137
| func extractRuleIDs(data map[string]interface{}) []string { | ||
| if data == nil { | ||
| return nil | ||
| } | ||
| items, ok := data["items"].([]interface{}) | ||
| if !ok { | ||
| return nil | ||
| } | ||
| ids := make([]string, 0, len(items)) | ||
| for _, item := range items { | ||
| m, ok := item.(map[string]interface{}) | ||
| if !ok { | ||
| continue | ||
| } | ||
| id, _ := m["id"].(string) | ||
| if id != "" { | ||
| ids = append(ids, id) | ||
| } | ||
| } | ||
| return ids | ||
| } |
There was a problem hiding this comment.
Fail closed on malformed rule items.
extractRuleIDs silently skips non-object entries and empty/missing id fields. That lets Execute build and POST an incomplete rule_ids list even though this API requires the full ordered set. Please return an error here and abort before the reorder call instead of dropping bad items.
Suggested direction
-func extractRuleIDs(data map[string]interface{}) []string {
+func extractRuleIDs(data map[string]interface{}) ([]string, error) {
if data == nil {
- return nil
+ return nil, output.Errorf(output.ExitAPI, "api_error", "list rules response missing data")
}
items, ok := data["items"].([]interface{})
if !ok {
- return nil
+ return nil, output.Errorf(output.ExitAPI, "api_error", "list rules response missing items")
}
ids := make([]string, 0, len(items))
- for _, item := range items {
+ for i, item := range items {
m, ok := item.(map[string]interface{})
if !ok {
- continue
+ return nil, output.Errorf(output.ExitAPI, "api_error", "rule item %d is malformed", i)
}
- id, _ := m["id"].(string)
- if id != "" {
- ids = append(ids, id)
+ id, ok := m["id"].(string)
+ if !ok || id == "" {
+ return nil, output.Errorf(output.ExitAPI, "api_error", "rule item %d missing id", i)
}
+ ids = append(ids, id)
}
- return ids
+ return ids, nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func extractRuleIDs(data map[string]interface{}) []string { | |
| if data == nil { | |
| return nil | |
| } | |
| items, ok := data["items"].([]interface{}) | |
| if !ok { | |
| return nil | |
| } | |
| ids := make([]string, 0, len(items)) | |
| for _, item := range items { | |
| m, ok := item.(map[string]interface{}) | |
| if !ok { | |
| continue | |
| } | |
| id, _ := m["id"].(string) | |
| if id != "" { | |
| ids = append(ids, id) | |
| } | |
| } | |
| return ids | |
| } | |
| func extractRuleIDs(data map[string]interface{}) ([]string, error) { | |
| if data == nil { | |
| return nil, output.Errorf(output.ExitAPI, "api_error", "list rules response missing data") | |
| } | |
| items, ok := data["items"].([]interface{}) | |
| if !ok { | |
| return nil, output.Errorf(output.ExitAPI, "api_error", "list rules response missing items") | |
| } | |
| ids := make([]string, 0, len(items)) | |
| for i, item := range items { | |
| m, ok := item.(map[string]interface{}) | |
| if !ok { | |
| return nil, output.Errorf(output.ExitAPI, "api_error", "rule item %d is malformed", i) | |
| } | |
| id, ok := m["id"].(string) | |
| if !ok || id == "" { | |
| return nil, output.Errorf(output.ExitAPI, "api_error", "rule item %d missing id", i) | |
| } | |
| ids = append(ids, id) | |
| } | |
| return ids, nil | |
| } |
🤖 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/mail_rule_reorder.go` around lines 117 - 137, The
extractRuleIDs function must fail closed instead of silently skipping malformed
items: change extractRuleIDs to validate every item and return an error when an
entry is not a map[string]interface{} or when an "id" is missing/empty (e.g.
change signature to return ([]string, error)), and update the caller Execute to
check the error and abort before constructing/posting rule_ids; ensure the error
message identifies the offending item index or content so the reorder POST is
never sent with an incomplete rule_ids list.
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
Skipped
Source specs
This MR was created autonomously. Quality gates were enforced by the repo's own pre-commit hooks.