Skip to content

Complete mail rule reorder IDs - #2304

Open
yangr-happy wants to merge 2 commits into
mainfrom
feat/b8c086d
Open

Complete mail rule reorder IDs#2304
yangr-happy wants to merge 2 commits into
mainfrom
feat/b8c086d

Conversation

@yangr-happy

@yangr-happy yangr-happy commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • fetch mailbox rules before reorder requests and complete partial rule_ids with the current server order
  • validate empty, duplicate, and unknown rule IDs locally before calling reorder
  • add dry-run planning output and focused service tests

Tests

  • go test ./cmd/service

Summary by CodeRabbit

  • New Features

    • Added support for reordering mail rules.
    • Automatically completes partial rule lists by appending omitted rules in their existing order.
    • Supports dry-run previews before applying changes.
    • Handles paginated rule lists and validates duplicate, unknown, or malformed rule IDs.
  • Bug Fixes

    • Prevents changes from being submitted when validation or rule-list retrieval fails.
    • Improves handling of pagination errors during reorder operations.

Fetch the current mailbox rule order before reorder requests so partial rule_ids are expanded into the complete list expected by the API. Validate empty, duplicate, and unknown IDs locally before submitting reorder.

Co-authored-by: TRAE CLI <noreply@bytedance.com>
@github-actions github-actions Bot added the size/L Large or sensitive change across domains or core paths label Aug 11, 2026
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6fdb12cd-0c4a-4637-9a05-d2edcfbc0d5d

📥 Commits

Reviewing files that changed from the base of the PR and between 0d32886 and f17a671.

📒 Files selected for processing (2)
  • cmd/service/mail_rules_reorder.go
  • cmd/service/mail_rules_reorder_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • cmd/service/mail_rules_reorder_test.go
  • cmd/service/mail_rules_reorder.go

📝 Walkthrough

Walkthrough

Mail rule reorder support validates requested rule IDs, retrieves all mailbox rules across pages, completes the requested ordering, and routes dry-run and execution flows through the service method.

Changes

Mail rule reorder

Layer / File(s) Summary
Request validation and ordering completion
cmd/service/mail_rules_reorder.go
Validates rule_ids, rejects invalid, duplicate, and unknown IDs, and appends omitted rules in their existing order.
Rule retrieval and service routing
cmd/service/mail_rules_reorder.go, cmd/service/service.go
Fetches paginated rule IDs, supports dry-run handling, and completes reorder requests before execution.
End-to-end behavior coverage
cmd/service/mail_rules_reorder_test.go
Tests validation, pagination, dry-run behavior, API ordering, completed payloads, failure paths, and malformed pagination responses.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant serviceMethodRun
  participant completeMailRulesReorderRequest
  participant fetchAllMailRuleIDs
  participant MailRulesAPI
  participant ReorderAPI
  serviceMethodRun->>completeMailRulesReorderRequest: complete reorder request
  completeMailRulesReorderRequest->>fetchAllMailRuleIDs: fetch current rule IDs
  fetchAllMailRuleIDs->>MailRulesAPI: list mailbox rule pages
  MailRulesAPI-->>fetchAllMailRuleIDs: return paginated rule responses
  fetchAllMailRuleIDs-->>completeMailRulesReorderRequest: return all rule IDs
  completeMailRulesReorderRequest-->>serviceMethodRun: update completed rule_ids
  serviceMethodRun->>ReorderAPI: submit completed reorder request
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: completing mail rule reorder IDs.
Description check ✅ Passed The description summarizes the change and test command, but it omits several template sections and the manual verification checklist.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/b8c086d

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_rules_reorder_test.go`:
- Around line 103-106: Update all four error-path test sites in
cmd/service/mail_rules_reorder_test.go:103-106, 133-135, 377-383, and 473-475.
Replace message-only checks with typed error assertions covering category and
subtype; assert ValidationError.Param for unknown, empty, and invalid inputs,
verify the wrapped stringList cause at 133-135, and assert the internal
invalid-response category and subtype at 473-475.

In `@cmd/service/mail_rules_reorder.go`:
- Around line 183-190: Update the pagination loop in the mail-rules reorder flow
to track every previously returned non-empty page token and reject duplicates
with errs.NewInternalError(errs.SubtypeInvalidResponse, ...) before issuing
another request. Preserve the existing missing-token validation, and add a
regression test covering has_more=true with a repeated token and confirming the
reorder POST is not executed.
- Around line 105-114: Update the string-list parsing logic around the item
conversion loop to return an error immediately when a trimmed rule ID is empty
instead of skipping it; preserve the existing type validation and non-empty
append behavior. Add a regression test covering a list containing both a valid
rule ID and a blank element, asserting the command reports invalid input.
- Around line 27-43: Update serviceDryRunMailRulesReorder to parse and validate
request.Data locally, preserve the supplied rule_ids such as rule_c and rule_a,
and append only the <remaining_rule_ids_in_current_order> marker without
replacing user IDs. Keep the dry-run plan free of API calls, and update the
related test to assert rule_c, rule_a, then the remaining-rules marker in that
order.
🪄 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: dc17f950-6556-44de-a22b-d498db94c999

📥 Commits

Reviewing files that changed from the base of the PR and between e1f9872 and 0d32886.

📒 Files selected for processing (3)
  • cmd/service/mail_rules_reorder.go
  • cmd/service/mail_rules_reorder_test.go
  • cmd/service/service.go

Comment thread cmd/service/mail_rules_reorder_test.go
Comment thread cmd/service/mail_rules_reorder.go
Comment thread cmd/service/mail_rules_reorder.go Outdated
Comment thread cmd/service/mail_rules_reorder.go
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@f17a6716d73ce88ac034cb6a5e003bf9c2584167

🧩 Skill update

npx skills add larksuite/cli#feat/b8c086d -y -g

@codecov

codecov Bot commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.77157% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.45%. Comparing base (defd27b) to head (f17a671).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
cmd/service/mail_rules_reorder.go 84.12% 20 Missing and 10 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2304      +/-   ##
==========================================
- Coverage   76.53%   76.45%   -0.08%     
==========================================
  Files        1020     1026       +6     
  Lines      112736   113857    +1121     
==========================================
+ Hits        86277    87047     +770     
- Misses      19881    20125     +244     
- Partials     6578     6685     +107     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Preserve supplied rule IDs in dry-run output, reject blank rule IDs, guard repeated pagination tokens, and assert typed validation metadata in reorder tests.

Change-Type: ci-fix
Co-authored-by: TRAE CLI <noreply@bytedance.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant