Complete mail rule reorder rule ID requests#1656
Open
sysuljx wants to merge 2 commits into
Open
Conversation
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds pre-request preparation for the ChangesMail Rule Reorder Pre-request Preparation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
github-actions
Bot
added
the
size/L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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_rule_reorder_test.go`:
- Around line 73-77: The test helper executeMailRuleReorder currently creates a
real cmdutil.TestFactory without isolating CLI config state, which can leak
between tests and make results order-dependent. Update this helper in
mail_rule_reorder_test.go to sandbox LARKSUITE_CLI_CONFIG_DIR by setting it with
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) before calling
cmdutil.TestFactory, so each test runs with its own config directory.
- Around line 105-116: The helper in requireValidationError currently only
checks err.Error() text, which is too weak for these error-path tests. Update it
to assert typed validation metadata using errs.ProblemOf on the returned
*errs.ValidationError, and keep errors.As only to confirm the error is a
ValidationError when Param also needs to be verified. Replace the substring
match with explicit checks for the expected category, subtype, and param fields
so regressions in the typed error contract are caught.
- Around line 154-168: The dry-run assertion in
TestMailRuleReorderDryRunListsAndPrintsCompletedBody only checks that the
emitted payload contains the expected rule_ids values, not that their order is
preserved. Update the test to inspect the completed output from
executeMailRuleReorder and assert the exact sequence of IDs in order, using the
existing TestMailRuleReorderDryRunListsAndPrintsCompletedBody and
mailRuleListStub helpers so it fails if IDs are reordered or extra IDs are
appended.
In `@cmd/service/mail_rule_reorder.go`:
- Around line 49-62: The validation errors in mail_rule_reorder are tagging the
wrong parameter; update the .WithParam usage in the reorder validation branch so
it reports the nested input as --data.rule_ids instead of rule_ids. Make the
same metadata adjustment for the related error-path assertions in the mail rule
reorder tests, and use the existing mail rule reorder validation logic and error
construction in cmd/service/mail_rule_reorder.go as the reference points.
🪄 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: 2cd7b1a9-7626-4553-b8b8-6785dcbe69d8
📒 Files selected for processing (3)
cmd/service/mail_rule_reorder.gocmd/service/mail_rule_reorder_test.gocmd/service/service.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@ce2f7eaa64dce9ce4f91a30ccfb18c0b76ae62a4🧩 Skill updatenpx skills add sysuljx/cli#feat/7442359 -y -g |
Change-Type: ci-fix
Collaborator
Author
|
🤖 AI Review | CR 汇总 | 可合入（未发现问题） 增量审查：基于已有 4 条行级评论（均已在最新提交中标记解决），本次重点复核 验证： 如有疑问或认为判断不准确，欢迎直接回复讨论。 |
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.
Adds request preparation for mail user mailbox rule reordering so callers can provide a partial
rule_idsordering while the CLI sends the full ordered list required by the API.rule_idsbefore sending the reorder request.Summary by CodeRabbit
rule_idsusing the current mailbox order.rule_idsorder and appends any remaining rules.rule_idsbefore any reorder call.--dry-runoutput to display the completedrule_ids.--dry-runscenarios.