Complete mail rule reorder IDs - #2312
Conversation
Fetch the current mailbox rule list before running the generated reorder command, complete partial rule_id input locally, and reject invalid inputs before making the write call. Co-authored-by: TRAE CLI <noreply@bytedance.com>
github-actions
Bot
added
domain/mail
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe mail rule reorder command now fetches all rule IDs, validates requested IDs, completes partial orders, submits the full order, handles pagination, and adds retry guidance for reorder failures. Tests and documentation cover the new behavior. ChangesMail rule reorder flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MailRuleCommand
participant MailRuleListAPI
participant MailRuleReorderAPI
MailRuleCommand->>MailRuleListAPI: Fetch all current rule IDs
MailRuleListAPI-->>MailRuleCommand: Return paginated rule IDs
MailRuleCommand->>MailRuleCommand: Validate and complete requested order
MailRuleCommand->>MailRuleReorderAPI: Submit completed rule IDs
MailRuleReorderAPI-->>MailRuleCommand: Return response or retry error
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@cmd/service/mail_rules_reorder_test.go`:
- Around line 48-50: Before the cmdutil.TestFactory call in the shared
command-test setup, set LARKSUITE_CLI_CONFIG_DIR to t.TempDir() using t.Setenv
so every test in the file uses an isolated CLI configuration directory.
- Around line 121-125: Update the error assertions in the reorder tests,
including the cases around the shown and additionally referenced blocks, to
inspect errs.ProblemOf(err) metadata instead of matching error strings; for
validation failures, also assert *errs.ValidationError.Param. Track reorder
invocations with an explicit counter rather than relying on “no stub” text, and
verify wrapped causes using errors.Is or errors.As.
In `@cmd/service/service.go`:
- Around line 517-520: Remove the page_token entry from the params map before
the initial list request in the reorder flow, while preserving other
caller-supplied parameters. Add a regression test covering reorder parameters
that include page_token and verify the first CallAPI list request is sent
without that token.
- Around line 469-479: Add command-level regression tests for the local body
validation around serviceStringSlice and rule_ids, covering missing rule_ids, an
empty array, an empty ID, and a non-string ID. Assert each returns the expected
typed validation metadata, and verify neither list nor reorder is invoked for
invalid input.
- Around line 557-565: Update the pagination loop around nextPageToken to track
previously seen non-empty continuation tokens, and return
errs.NewInternalError(errs.SubtypeInvalidResponse, ...) when nextToken has
already been encountered. Preserve the existing missing-token validation and
normal page_token assignment for new tokens.
🪄 Autofix
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 Plus
Run ID: bdce4a15-26b6-4aaf-90d4-52b005049881
📒 Files selected for processing (3)
cmd/service/mail_rules_reorder_test.gocmd/service/service.goskills/lark-mail/references/lark-mail-rules.md
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@0cd80b0f435fb13bd954f33df8c83ba758f14a9d🧩 Skill updatenpx skills add yangr-happy/cli#feat/58b8374 -y -g |
Reset list pagination before completing reorder IDs, reject repeated list page tokens, and cover local validation with typed error assertions. Change-Type: ci-fix Co-authored-by: TRAE CLI <noreply@bytedance.com>
yangr-happy
left a comment
yangr-happy
left a comment
There was a problem hiding this comment.
提交已有草稿回复，释放后续审查评论。
Limit internal mail rules pagination before reorder and treat an empty current rule list as rule_ids validation failure. Change-Type: ci-fix Co-authored-by: TRAE CLI <noreply@bytedance.com>
yangr-happy
left a comment
yangr-happy
left a comment
There was a problem hiding this comment.
🤖 AI Review | CR 汇总 | 有风险（2 个 P2）
增量审查：基于既有已解决评论，本次新增 2 条正确性问题。请优先修复内部 list 参数透传问题，并让 dry-run 不再展示与真实执行不一致的 reorder 请求。
| for k, v := range reorderRequest.Params { | ||
| params[k] = v | ||
| } | ||
| delete(params, "page_token") |
There was a problem hiding this comment.
🤖 AI Review | [P2 正确性] 内部 list 仍会继承外部 page_size
fetchAllMailRuleIDs 只删除了透传进 reorder 的 page_token，但 page_size 仍会从 --params 原样带到内部 list 请求。生成式 service 命令会保留 undeclared params，所以用户如果传入 {"user_mailbox_id":"me","page_size":1}，补齐前的 list 会按 1 条/页拉取；当邮箱规则数超过 100 条时会撞上新增的硬上限并返回 invalid_response，无法完成“拉全并补齐 rule_ids”的目标。
修复建议: 构造内部 list 参数时同时移除外部 page_size，或显式设置一个受控的较大 page size；补充带 page_size:1 的回归测试，验证首个 list 请求不携带该值并仍按返回 token 拉全。
如有疑问或认为判断不准确，欢迎直接回复讨论。
| return err | ||
| } | ||
|
|
||
| if err := prepareMailRuleReorder(opts.Ctx, ac, opts, &request); err != nil { |
There was a problem hiding this comment.
🤖 AI Review | [P2 正确性] dry-run 展示的请求与真实执行不一致
--dry-run 分支在这里之前已经返回，因此 mail rules reorder 的预览仍只显示用户输入的 partial rule_ids，例如 ["C"]；真实执行同一命令会先 GET list，并把 POST body 改写成完整顺序，例如 ["C","A","B"]。这个仓库把 dry-run 作为写操作请求结构验证手段，当前输出会误导用户或自动化审计，以为真实写请求仍是不完整列表。
修复建议: 为该 schema 定制 dry-run：至少在预览中声明真实执行会先 list 并动态补齐 rule_ids，或输出两步计划（GET rules + POST reorder，POST body 标注为运行时补齐）。同时补一个 dry-run 用例，防止预览继续宣称会直接提交 partial body。
如有疑问或认为判断不准确，欢迎直接回复讨论。
1 participant
Complete mail rule reorder IDs
Fetch the current mailbox rule list before running the generated reorder command, complete partial rule_id input locally, and reject invalid inputs before making the write call.
Co-authored-by: TRAE CLI noreply@bytedance.com
Summary by CodeRabbit
New Features
Documentation