mirror of
https://github.com/larksuite/cli.git
synced 2026-08-03 08:32:46 +08:00
fix(im): add id-source hint and strengthen example and recovery locks
This commit is contained in:
@@ -410,6 +410,9 @@ func TestShortcutValidateBranches(t *testing.T) {
|
||||
if err == nil || !strings.Contains(err.Error(), "--content is not valid JSON") {
|
||||
t.Fatalf("ImMessagesSend.Validate() error = %v", err)
|
||||
}
|
||||
if !strings.Contains(err.Error(), "--text") {
|
||||
t.Fatalf("ImMessagesSend.Validate() error = %v, want it to mention --text as a recovery alternative", err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("ImMessagesSend media with text", func(t *testing.T) {
|
||||
@@ -651,6 +654,9 @@ func TestShortcutValidateBranches(t *testing.T) {
|
||||
if err == nil || !strings.Contains(err.Error(), "requires user identity") {
|
||||
t.Fatalf("ImChatMessageList.Validate() error = %v, want requires user identity", err)
|
||||
}
|
||||
if !strings.Contains(err.Error(), "--as user") || !strings.Contains(err.Error(), "--chat-id") {
|
||||
t.Fatalf("ImChatMessageList.Validate() error = %v, want it to mention both --as user and --chat-id as recovery actions", err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("ImMessagesMGet empty ids", func(t *testing.T) {
|
||||
|
||||
@@ -1482,7 +1482,7 @@ type shortcutItem struct {
|
||||
func collectChatIDs(rt *common.RuntimeContext) ([]string, error) {
|
||||
raw := rt.StrSlice("chat-id")
|
||||
if len(raw) == 0 {
|
||||
return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--chat-id is required (oc_xxx); repeat the flag or pass comma-separated values").WithParam("--chat-id")
|
||||
return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--chat-id is required (oc_xxx); repeat the flag or pass comma-separated values").WithParam("--chat-id").WithHint("get the open_chat_id from im +chat-search (by name) or im +chat-list (my chats)")
|
||||
}
|
||||
|
||||
seen := make(map[string]struct{}, len(raw))
|
||||
@@ -1494,7 +1494,7 @@ func collectChatIDs(rt *common.RuntimeContext) ([]string, error) {
|
||||
}
|
||||
if !strings.HasPrefix(v, "oc_") {
|
||||
return nil, errs.NewValidationError(errs.SubtypeInvalidArgument,
|
||||
"invalid --chat-id %q: must be an open_chat_id starting with oc_", v).WithParam("--chat-id")
|
||||
"invalid --chat-id %q: must be an open_chat_id starting with oc_", v).WithParam("--chat-id").WithHint("get the open_chat_id from im +chat-search (by name) or im +chat-list (my chats)")
|
||||
}
|
||||
if _, ok := seen[v]; ok {
|
||||
continue
|
||||
@@ -1503,7 +1503,7 @@ func collectChatIDs(rt *common.RuntimeContext) ([]string, error) {
|
||||
out = append(out, v)
|
||||
}
|
||||
if len(out) == 0 {
|
||||
return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--chat-id is required (oc_xxx)").WithParam("--chat-id")
|
||||
return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--chat-id is required (oc_xxx)").WithParam("--chat-id").WithHint("get the open_chat_id from im +chat-search (by name) or im +chat-list (my chats)")
|
||||
}
|
||||
if len(out) > feedShortcutBatchLimit {
|
||||
return nil, errs.NewValidationError(errs.SubtypeInvalidArgument,
|
||||
|
||||
@@ -118,6 +118,48 @@ func TestCollectChatIDs(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestCollectChatIDsHint locks that every collectChatIDs validation error
|
||||
// carries an actionable recovery hint pointing the user at how to discover a
|
||||
// real open_chat_id (im +chat-search / im +chat-list), not just what shape
|
||||
// the flag must take.
|
||||
func TestCollectChatIDsHint(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
input []string
|
||||
}{
|
||||
{name: "missing chat-id", input: nil},
|
||||
{name: "bad prefix", input: []string{"om_abc"}},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
cmd := newFeedShortcutCreateCmd(t)
|
||||
for _, v := range tt.input {
|
||||
if err := cmd.Flags().Set("chat-id", v); err != nil {
|
||||
t.Fatalf("Set chat-id %q error = %v", v, err)
|
||||
}
|
||||
}
|
||||
runtime := &common.RuntimeContext{Cmd: cmd}
|
||||
|
||||
_, err := collectChatIDs(runtime)
|
||||
if err == nil {
|
||||
t.Fatalf("collectChatIDs() expected error, got nil")
|
||||
}
|
||||
|
||||
problem, ok := errs.ProblemOf(err)
|
||||
if !ok {
|
||||
t.Fatalf("collectChatIDs() error is not a typed Problem: %v", err)
|
||||
}
|
||||
if problem.Subtype != errs.SubtypeInvalidArgument {
|
||||
t.Fatalf("collectChatIDs() Subtype = %v, want %v", problem.Subtype, errs.SubtypeInvalidArgument)
|
||||
}
|
||||
if !strings.Contains(problem.Hint, "+chat-search") || !strings.Contains(problem.Hint, "+chat-list") {
|
||||
t.Fatalf("collectChatIDs() Hint = %q, want it to mention both +chat-search and +chat-list", problem.Hint)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildShortcutItems(t *testing.T) {
|
||||
got := buildShortcutItems([]string{"oc_a", "oc_b"})
|
||||
if len(got) != 2 {
|
||||
|
||||
@@ -11,9 +11,12 @@ import (
|
||||
"github.com/larksuite/cli/shortcuts/common"
|
||||
)
|
||||
|
||||
// The 12 high-frequency IM shortcuts covered by the governance closeout.
|
||||
// Every entry must carry at least one copyable "Example:" tip locked by the
|
||||
// tests below; other IM shortcuts (feed/flag series) are intentionally exempt.
|
||||
// 12 high-frequency IM shortcuts covered by the original governance closeout,
|
||||
// plus 6 feed/flag shortcuts that carry a real guessing surface (see the
|
||||
// inline comment below). Every entry must carry at least one copyable
|
||||
// "Example:" tip locked by the tests below. The 3 pagination-only feed/flag
|
||||
// shortcuts (+feed-shortcut-list, +feed-group-list, +flag-list) are
|
||||
// intentionally exempt — see the inline comment further down.
|
||||
var tipsExampleTargets = []string{
|
||||
"+messages-send", "+messages-search", "+chat-messages-list", "+messages-reply",
|
||||
"+chat-search", "+chat-list", "+messages-mget", "+threads-messages-list",
|
||||
|
||||
@@ -40,13 +40,15 @@ next action, (4) know how to verify it. All four → PASS.
|
||||
- lock: TestResolveIsHeaderMutualExclusionHint
|
||||
|
||||
## feed.chat_id.not_oc_prefix
|
||||
- source: shortcuts/im/helpers.go parseFeedChatIDs
|
||||
- source: shortcuts/im/helpers.go collectChatIDs
|
||||
- user_task: pass a message id (om_) or plain id where an open_chat_id is required
|
||||
- command: `lark-cli im +feed-shortcut-create --chat-id om_test000 --dry-run`
|
||||
- observed (replayed): `{"ok":false,"identity":"user","error":{"type":"validation","subtype":"invalid_argument","message":"invalid --chat-id \"om_test000\": must be an open_chat_id starting with oc_","param":"--chat-id"}}`
|
||||
- verdict: PASS
|
||||
- observed (replayed, before fix): `{"ok":false,"identity":"user","error":{"type":"validation","subtype":"invalid_argument","message":"invalid --chat-id \"om_test000\": must be an open_chat_id starting with oc_","param":"--chat-id"}}` — names what is required (an oc_ id) but gives no next action or ID-source hint
|
||||
- observed (after fix): same envelope plus `"hint":"get the open_chat_id from im +chat-search (by name) or im +chat-list (my chats)"`
|
||||
- verdict: FIX_HINT (fixed in this PR)
|
||||
- expected_hint: get the open_chat_id from im +chat-search or im +chat-list
|
||||
- expected_next_action: fetch the oc_ id via +chat-search / +chat-list and retry
|
||||
- lock: shortcuts/im/im_feed_shortcut_test.go::TestCollectChatIDs/rejects_bad_prefix; tests/cli_e2e/im/feed_shortcut_workflow_test.go::TestIM_FeedShortcutDryRun/create_dry-run_rejects_non-oc_chat_ids
|
||||
- lock: shortcuts/im/im_feed_shortcut_test.go::TestCollectChatIDsHint
|
||||
|
||||
## chat-messages-list.bot_identity.user_id
|
||||
- source: shortcuts/im/im_chat_messages_list.go (Validate), shortcuts/im/helpers.go resolveP2PChatID
|
||||
|
||||
@@ -17,16 +17,19 @@ import (
|
||||
// Placeholder substitutions turning copyable help examples into syntactically
|
||||
// valid dry-run invocations. IDs are obvious fakes; --dry-run never hits the API.
|
||||
var tipsPlaceholderValues = map[string]string{
|
||||
"<chat_id>": "oc_e2etest000000000000000000",
|
||||
"<open_id>": "ou_e2etest000000000000000000",
|
||||
"<message_id>": "om_e2etest000000000000000000",
|
||||
"<thread_id>": "omt_e2etest00000000000000000",
|
||||
"<file_key>": "file_v3_e2etest0000000000000",
|
||||
"<image_key>": "img_v3_e2etest00000000000000",
|
||||
"<open_id1>": "ou_e2etest000000000000000001",
|
||||
"<open_id2>": "ou_e2etest000000000000000002",
|
||||
"<message_id1>": "om_e2etest000000000000000001",
|
||||
"<message_id2>": "om_e2etest000000000000000002",
|
||||
"<chat_id>": "oc_e2etest000000000000000000",
|
||||
"<open_id>": "ou_e2etest000000000000000000",
|
||||
"<message_id>": "om_e2etest000000000000000000",
|
||||
"<thread_id>": "omt_e2etest00000000000000000",
|
||||
"<file_key>": "file_v3_e2etest0000000000000",
|
||||
"<image_key>": "img_v3_e2etest00000000000000",
|
||||
"<open_id1>": "ou_e2etest000000000000000001",
|
||||
"<open_id2>": "ou_e2etest000000000000000002",
|
||||
"<message_id1>": "om_e2etest000000000000000001",
|
||||
"<message_id2>": "om_e2etest000000000000000002",
|
||||
"<feed_group_id>": "ofg_e2etest00000000000000000",
|
||||
"<chat_id1>": "oc_e2etest000000000000000001",
|
||||
"<chat_id2>": "oc_e2etest000000000000000002",
|
||||
}
|
||||
|
||||
// firstExampleArgs extracts the first "Example:" tip of the shortcut, replaces
|
||||
@@ -115,3 +118,69 @@ func TestIMTipsFirstExampleDryRunChatMessagesList(t *testing.T) {
|
||||
func TestIMTipsFirstExampleDryRunResourcesDownload(t *testing.T) {
|
||||
runFirstExampleDryRun(t, "+messages-resources-download", "/open-apis/im/v1/messages/")
|
||||
}
|
||||
|
||||
// tipsExampleAllTargets mirrors shortcuts/im/tips_examples_test.go's
|
||||
// tipsExampleTargets: the 12 high-frequency + 6 feed/flag shortcuts whose
|
||||
// help carries a locked copyable "Example:" tip. Kept as a literal copy here
|
||||
// because that list lives in an internal _test.go file not visible outside
|
||||
// the shortcuts/im package.
|
||||
var tipsExampleAllTargets = []string{
|
||||
"+messages-send", "+messages-search", "+chat-messages-list", "+messages-reply",
|
||||
"+chat-search", "+chat-list", "+messages-mget", "+threads-messages-list",
|
||||
"+messages-resources-download", "+chat-create", "+chat-update", "+chat-members-list",
|
||||
"+feed-shortcut-create", "+feed-shortcut-remove",
|
||||
"+feed-group-list-item", "+feed-group-query-item",
|
||||
"+flag-create", "+flag-cancel",
|
||||
}
|
||||
|
||||
// defaultAsForCommand picks the identity to run the dry-run under by reading
|
||||
// the shortcut's own AuthTypes: "bot" when the shortcut supports bot identity
|
||||
// (matching the 3 pre-existing path-assertion tests above), otherwise "user"
|
||||
// for user-only shortcuts (+messages-search and the whole feed/flag series).
|
||||
func defaultAsForCommand(t *testing.T, command string) string {
|
||||
t.Helper()
|
||||
for _, sc := range imshortcuts.Shortcuts() {
|
||||
if sc.Command != command {
|
||||
continue
|
||||
}
|
||||
for _, a := range sc.AuthTypes {
|
||||
if a == "bot" {
|
||||
return "bot"
|
||||
}
|
||||
}
|
||||
return "user"
|
||||
}
|
||||
t.Fatalf("shortcut %s not found", command)
|
||||
return ""
|
||||
}
|
||||
|
||||
// TestIMTipsFirstExampleDryRunAll extends the executability lock from the 3
|
||||
// path-assertion tests above (messages-send, chat-messages-list,
|
||||
// resources-download) to every one of the 18 shortcuts carrying a locked
|
||||
// Example tip: the first example, with placeholders substituted and
|
||||
// --dry-run appended, must exit 0. This only asserts exit code, not the API
|
||||
// path — the 3 tests above keep that stronger assertion for their targets.
|
||||
func TestIMTipsFirstExampleDryRunAll(t *testing.T) {
|
||||
for _, cmd := range tipsExampleAllTargets {
|
||||
cmd := cmd
|
||||
t.Run(cmd, func(t *testing.T) {
|
||||
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())
|
||||
t.Setenv("LARKSUITE_CLI_APP_ID", "im_tips_dryrun_test")
|
||||
t.Setenv("LARKSUITE_CLI_APP_SECRET", "im_tips_dryrun_secret")
|
||||
t.Setenv("LARKSUITE_CLI_BRAND", "feishu")
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
|
||||
defer cancel()
|
||||
|
||||
as := defaultAsForCommand(t, cmd)
|
||||
args := append(firstExampleArgs(t, cmd), "--dry-run")
|
||||
result, err := clie2e.RunCmd(ctx, clie2e.Request{
|
||||
Args: args,
|
||||
DefaultAs: as,
|
||||
WorkDir: t.TempDir(),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
result.AssertExitCode(t, 0)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user