feat: request thread roots for chat message list#635
feat: request thread roots for chat message list#635YangJunzhou-01 wants to merge 1 commit intolarksuite:mainfrom
Conversation
Change-Id: I3901b27e70b0e4db506ff199eb03c96fcf98671d
📝 WalkthroughWalkthroughA new query parameter Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/im/builders_test.go (1)
750-774: Consider removing the duplicate dry-run assertion.The new subtest and standalone test both create the same runtime and assert the same
only_thread_root_messages=truesubstring. Keeping one is enough;coverage_additional_test.goalready covers the lower-level params.♻️ Proposed cleanup
t.Run("ImChatMessageList dry run includes root-only query", func(t *testing.T) { runtime := newTestRuntimeContext(t, map[string]string{ "chat-id": "oc_123", "page-size": "20", "sort": "desc", }, nil) formatted := ImChatMessageList.DryRun(context.Background(), runtime).Format() if !strings.Contains(formatted, "only_thread_root_messages=true") { t.Fatalf("ImChatMessageList.DryRun().Format() = %s, want only_thread_root_messages=true", formatted) } }) } - -func TestChatMessageListOnlyThreadRootMessagesDryRun(t *testing.T) { - runtime := newTestRuntimeContext(t, map[string]string{ - "chat-id": "oc_123", - "page-size": "20", - "sort": "desc", - }, nil) - - formatted := ImChatMessageList.DryRun(context.Background(), runtime).Format() - if !strings.Contains(formatted, "only_thread_root_messages=true") { - t.Fatalf("ImChatMessageList.DryRun().Format() = %s, want only_thread_root_messages=true", formatted) - } -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/im/builders_test.go` around lines 750 - 774, The test duplicates the same assertion on ImChatMessageList.DryRun().Format() — remove one of them to avoid redundancy: delete either the subtest "ImChatMessageList dry run includes root-only query" or the standalone TestChatMessageListOnlyThreadRootMessagesDryRun function so only a single assertion for only_thread_root_messages=true remains (leave the other intact); make sure any helper call newTestRuntimeContext and the runtime setup remain in the kept test.
🤖 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/im/builders_test.go`:
- Around line 750-774: The test duplicates the same assertion on
ImChatMessageList.DryRun().Format() — remove one of them to avoid redundancy:
delete either the subtest "ImChatMessageList dry run includes root-only query"
or the standalone TestChatMessageListOnlyThreadRootMessagesDryRun function so
only a single assertion for only_thread_root_messages=true remains (leave the
other intact); make sure any helper call newTestRuntimeContext and the runtime
setup remain in the kept test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8ed61726-c658-4803-98e3-1fc5232a5d3b
📒 Files selected for processing (4)
.gitignoreshortcuts/im/builders_test.goshortcuts/im/coverage_additional_test.goshortcuts/im/im_chat_messages_list.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #635 +/- ##
==========================================
+ Coverage 60.52% 62.37% +1.85%
==========================================
Files 415 415
Lines 43710 36096 -7614
==========================================
- Hits 26454 22515 -3939
+ Misses 15201 11526 -3675
Partials 2055 2055 ☔ 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@0125519e799dfb2853efe53656ab0a85b1c1bd6d🧩 Skill updatenpx skills add YangJunzhou-01/cli#feat/read-history-thread-root-only -y -g |
Summary
Update
im +chat-messages-listto request only thread root messages from/open-apis/im/v1/messagesby default. This aligns the shortcut request shape with topic-group usage and makes the intended API behavior explicit in both runtime params and dry-run output.Changes
only_thread_root_messages=trueto the shared query params built byshortcuts/im/im_chat_messages_list.goonly_thread_root_messages.hammer/to.gitignoreTest Plan
make unit-testlark-cli im +chat-messages-list --chat-id <oc_xxx> --page-size 5 --sort desc --dry-run --format jsonincludesonly_thread_root_messages=truelark-cli im +chat-messages-list --chat-id <oc_xxx> --page-size 5 --sort desc --format jsonreturns the expected root-level message setRelated Issues
Summary by CodeRabbit
Features
Tests
Chores