From 42837c15a105301a6484881b187faee93506bbf1 Mon Sep 17 00:00:00 2001 From: sysuljx <12532679+sysuljx@users.noreply.github.com> Date: Mon, 29 Jun 2026 21:17:39 +0800 Subject: [PATCH 1/2] fix(service): complete mail rule reorder ids --- cmd/service/mail_rule_reorder.go | 157 ++++++++++++++++++++++++ cmd/service/mail_rule_reorder_test.go | 170 ++++++++++++++++++++++++++ cmd/service/service.go | 19 ++- 3 files changed, 343 insertions(+), 3 deletions(-) create mode 100644 cmd/service/mail_rule_reorder.go create mode 100644 cmd/service/mail_rule_reorder_test.go diff --git a/cmd/service/mail_rule_reorder.go b/cmd/service/mail_rule_reorder.go new file mode 100644 index 000000000..8166efbd3 --- /dev/null +++ b/cmd/service/mail_rule_reorder.go @@ -0,0 +1,157 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package service + +import ( + "strings" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/client" +) + +const mailRuleReorderSchemaPath = "mail.user_mailbox.rules.reorder" + +func needsServiceRequestPreparation(opts *ServiceMethodOptions) bool { + return opts != nil && opts.SchemaPath == mailRuleReorderSchemaPath +} + +func prepareServiceRequest(opts *ServiceMethodOptions, ac *client.APIClient, request *client.RawApiRequest) error { + if !needsServiceRequestPreparation(opts) { + return nil + } + return prepareMailRuleReorderRequest(opts, ac, request) +} + +func prepareMailRuleReorderRequest(opts *ServiceMethodOptions, ac *client.APIClient, request *client.RawApiRequest) error { + inputIDs, err := mailRuleReorderInputIDs(request.Data) + if err != nil { + return err + } + + listResult, err := ac.CallAPI(opts.Ctx, client.RawApiRequest{ + Method: "GET", + URL: mailRuleListURL(request.URL), + As: request.As, + }) + if err != nil { + return err + } + if err := ac.CheckResponse(listResult, request.As); err != nil { + return err + } + + currentIDs, err := mailRuleListIDs(listResult) + if err != nil { + return err + } + if len(currentIDs) == 0 { + return errs.NewValidationError(errs.SubtypeInvalidArgument, + "mail user mailbox rules reorder requires current mailbox rules, but list returned no rules"). + WithParam("rule_ids") + } + + known := make(map[string]bool, len(currentIDs)) + for _, id := range currentIDs { + known[id] = true + } + for _, id := range inputIDs { + if !known[id] { + return errs.NewValidationError(errs.SubtypeInvalidArgument, + "--data.rule_ids contains unknown rule_id %q", id). + WithParam("rule_ids") + } + } + + selected := make(map[string]bool, len(inputIDs)) + merged := make([]string, 0, len(currentIDs)) + for _, id := range inputIDs { + selected[id] = true + merged = append(merged, id) + } + for _, id := range currentIDs { + if !selected[id] { + merged = append(merged, id) + } + } + + body := request.Data.(map[string]interface{}) + body["rule_ids"] = merged + return nil +} + +func mailRuleReorderInputIDs(data interface{}) ([]string, error) { + body, ok := data.(map[string]interface{}) + if !ok || body == nil { + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, + "--data must be a JSON object containing rule_ids").WithParam("--data") + } + raw, ok := body["rule_ids"] + if !ok { + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, + "--data.rule_ids is required").WithParam("rule_ids") + } + rawIDs, ok := raw.([]interface{}) + if !ok { + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, + "--data.rule_ids must be a non-empty string array").WithParam("rule_ids") + } + if len(rawIDs) == 0 { + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, + "--data.rule_ids must not be empty").WithParam("rule_ids") + } + + ids := make([]string, 0, len(rawIDs)) + seen := make(map[string]bool, len(rawIDs)) + for i, rawID := range rawIDs { + id, ok := rawID.(string) + if !ok || id == "" { + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, + "--data.rule_ids[%d] must be a non-empty string", i).WithParam("rule_ids") + } + if seen[id] { + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, + "--data.rule_ids contains duplicate rule_id %q", id).WithParam("rule_ids") + } + seen[id] = true + ids = append(ids, id) + } + return ids, nil +} + +func mailRuleListIDs(result interface{}) ([]string, error) { + resultMap, ok := result.(map[string]interface{}) + if !ok || resultMap == nil { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, + "mail rules list response must be a JSON object") + } + data, ok := resultMap["data"].(map[string]interface{}) + if !ok || data == nil { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, + "mail rules list response missing data object") + } + items, ok := data["items"].([]interface{}) + if !ok { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, + "mail rules list response missing data.items array") + } + ids := make([]string, 0, len(items)) + for i, item := range items { + itemMap, ok := item.(map[string]interface{}) + if !ok || itemMap == nil { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, + "mail rules list response data.items[%d] must be an object", i) + } + id, ok := itemMap["id"].(string) + if !ok || id == "" { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, + "mail rules list response data.items[%d].id must be a non-empty string", i) + } + ids = append(ids, id) + } + return ids, nil +} + +func mailRuleListURL(reorderURL string) string { + return strings.TrimSuffix(reorderURL, "/reorder") +} diff --git a/cmd/service/mail_rule_reorder_test.go b/cmd/service/mail_rule_reorder_test.go new file mode 100644 index 000000000..8d2efb493 --- /dev/null +++ b/cmd/service/mail_rule_reorder_test.go @@ -0,0 +1,170 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package service + +import ( + "bytes" + "encoding/json" + "errors" + "reflect" + "strings" + "testing" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/meta" + "github.com/spf13/cobra" +) + +func mailRuleServiceSpec() meta.Service { + return meta.ServiceFromMap(map[string]interface{}{ + "name": "mail", + "servicePath": "/open-apis/mail/v1", + }) +} + +func mailRuleReorderMethod() meta.Method { + return meta.FromMap(map[string]interface{}{ + "path": "user_mailboxes/{user_mailbox_id}/rules/reorder", + "httpMethod": "POST", + "parameters": map[string]interface{}{ + "user_mailbox_id": map[string]interface{}{ + "type": "string", "location": "path", "required": true, + }, + }, + }) +} + +func newMailRuleReorderCommand(f *cmdutil.Factory) *cobra.Command { + return NewCmdServiceMethod(f, mailRuleServiceSpec(), mailRuleReorderMethod(), "reorder", "user_mailbox.rules", nil) +} + +func mailRuleListStub(ids ...string) *httpmock.Stub { + items := make([]interface{}, 0, len(ids)) + for _, id := range ids { + items = append(items, map[string]interface{}{"id": id}) + } + return &httpmock.Stub{ + Method: "GET", + URL: "/open-apis/mail/v1/user_mailboxes/me/rules", + Body: map[string]interface{}{ + "code": 0, + "msg": "ok", + "data": map[string]interface{}{"items": items}, + }, + } +} + +func mailRuleReorderStub() *httpmock.Stub { + return &httpmock.Stub{ + Method: "POST", + URL: "/open-apis/mail/v1/user_mailboxes/me/rules/reorder", + Body: map[string]interface{}{ + "code": 0, + "msg": "ok", + "data": map[string]interface{}{"ok": true}, + }, + } +} + +func executeMailRuleReorder(t *testing.T, data string, dryRun bool, stubs ...*httpmock.Stub) (*bytes.Buffer, *httpmock.Stub, error) { + t.Helper() + f, stdout, _, reg := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "test-app-mail-rules", AppSecret: "test-secret", Brand: core.BrandFeishu, + }) + for _, stub := range stubs { + reg.Register(stub) + } + cmd := newMailRuleReorderCommand(f) + args := []string{"--as", "bot", "--params", `{"user_mailbox_id":"me"}`, "--data", data} + if dryRun { + args = append(args, "--dry-run") + } + cmd.SetArgs(args) + var last *httpmock.Stub + if len(stubs) > 0 { + last = stubs[len(stubs)-1] + } + return stdout, last, cmd.Execute() +} + +func capturedRuleIDs(t *testing.T, stub *httpmock.Stub) []string { + t.Helper() + var body struct { + RuleIDs []string `json:"rule_ids"` + } + if err := json.Unmarshal(stub.CapturedBody, &body); err != nil { + t.Fatalf("decode reorder body: %v\nraw=%s", err, string(stub.CapturedBody)) + } + return body.RuleIDs +} + +func requireValidationError(t *testing.T, err error, want string) { + t.Helper() + if err == nil { + t.Fatal("expected validation error") + } + var validationErr *errs.ValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("expected ValidationError, got %T: %v", err, err) + } + if !strings.Contains(err.Error(), want) { + t.Fatalf("expected %q in error, got %v", want, err) + } +} + +func TestMailRuleReorderCompletesPartialRuleIDs(t *testing.T) { + list := mailRuleListStub("r1", "r2", "r3", "r4") + reorder := mailRuleReorderStub() + _, _, err := executeMailRuleReorder(t, `{"rule_ids":["r3","r1"]}`, false, list, reorder) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got, want := capturedRuleIDs(t, reorder), []string{"r3", "r1", "r2", "r4"}; !reflect.DeepEqual(got, want) { + t.Fatalf("rule_ids = %#v, want %#v", got, want) + } +} + +func TestMailRuleReorderKeepsCompleteRuleIDs(t *testing.T) { + list := mailRuleListStub("r1", "r2", "r3") + reorder := mailRuleReorderStub() + _, _, err := executeMailRuleReorder(t, `{"rule_ids":["r3","r2","r1"]}`, false, list, reorder) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got, want := capturedRuleIDs(t, reorder), []string{"r3", "r2", "r1"}; !reflect.DeepEqual(got, want) { + t.Fatalf("rule_ids = %#v, want %#v", got, want) + } +} + +func TestMailRuleReorderUnknownIDDoesNotCallReorder(t *testing.T) { + list := mailRuleListStub("r1", "r2") + _, _, err := executeMailRuleReorder(t, `{"rule_ids":["r3"]}`, false, list) + requireValidationError(t, err, "unknown rule_id") +} + +func TestMailRuleReorderDuplicateIDDoesNotCallListOrReorder(t *testing.T) { + _, _, err := executeMailRuleReorder(t, `{"rule_ids":["r1","r1"]}`, false) + requireValidationError(t, err, "duplicate rule_id") +} + +func TestMailRuleReorderDryRunListsAndPrintsCompletedBody(t *testing.T) { + list := mailRuleListStub("r1", "r2", "r3") + stdout, _, err := executeMailRuleReorder(t, `{"rule_ids":["r3"]}`, true, list) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(list.CapturedBodies) != 1 { + t.Fatalf("list call count = %d, want 1", len(list.CapturedBodies)) + } + out := stdout.String() + if !strings.Contains(out, `"rule_ids"`) || + !strings.Contains(out, `"r3"`) || + !strings.Contains(out, `"r1"`) || + !strings.Contains(out, `"r2"`) { + t.Fatalf("dry-run output missing completed rule_ids, got:\n%s", out) + } +} diff --git a/cmd/service/service.go b/cmd/service/service.go index 3cb6ab5d2..79cca3046 100644 --- a/cmd/service/service.go +++ b/cmd/service/service.go @@ -401,6 +401,17 @@ func serviceMethodRun(opts *ServiceMethodOptions) error { return err } + var ac *client.APIClient + if needsServiceRequestPreparation(opts) { + ac, err = f.NewAPIClientWithConfig(config) + if err != nil { + return err + } + if err := prepareServiceRequest(opts, ac, &request); err != nil { + return err + } + } + if opts.DryRun { if fileMeta != nil { return cmdutil.PrintDryRunWithFile(f.IOStreams.Out, request, config, opts.Format, fileMeta.FieldName, fileMeta.FilePath, fileMeta.FormFields) @@ -414,9 +425,11 @@ func serviceMethodRun(opts *ServiceMethodOptions) error { } } - ac, err := f.NewAPIClientWithConfig(config) - if err != nil { - return err + if ac == nil { + ac, err = f.NewAPIClientWithConfig(config) + if err != nil { + return err + } } out := f.IOStreams.Out From ce2f7eaa64dce9ce4f91a30ccfb18c0b76ae62a4 Mon Sep 17 00:00:00 2001 From: sysuljx <12532679+sysuljx@users.noreply.github.com> Date: Mon, 29 Jun 2026 22:45:42 +0800 Subject: [PATCH 2/2] fix(service): address mail rule reorder review feedback Change-Type: ci-fix --- cmd/service/mail_rule_reorder.go | 21 +++++------ cmd/service/mail_rule_reorder_test.go | 51 ++++++++++++++++++++------- 2 files changed, 50 insertions(+), 22 deletions(-) diff --git a/cmd/service/mail_rule_reorder.go b/cmd/service/mail_rule_reorder.go index 8166efbd3..06b35e950 100644 --- a/cmd/service/mail_rule_reorder.go +++ b/cmd/service/mail_rule_reorder.go @@ -48,7 +48,7 @@ func prepareMailRuleReorderRequest(opts *ServiceMethodOptions, ac *client.APICli if len(currentIDs) == 0 { return errs.NewValidationError(errs.SubtypeInvalidArgument, "mail user mailbox rules reorder requires current mailbox rules, but list returned no rules"). - WithParam("rule_ids") + WithParam("--data.rule_ids") } known := make(map[string]bool, len(currentIDs)) @@ -59,7 +59,7 @@ func prepareMailRuleReorderRequest(opts *ServiceMethodOptions, ac *client.APICli if !known[id] { return errs.NewValidationError(errs.SubtypeInvalidArgument, "--data.rule_ids contains unknown rule_id %q", id). - WithParam("rule_ids") + WithParam("--data.rule_ids") } } @@ -88,17 +88,14 @@ func mailRuleReorderInputIDs(data interface{}) ([]string, error) { } raw, ok := body["rule_ids"] if !ok { - return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, - "--data.rule_ids is required").WithParam("rule_ids") + return nil, mailRuleIDsValidationError("--data.rule_ids is required") } rawIDs, ok := raw.([]interface{}) if !ok { - return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, - "--data.rule_ids must be a non-empty string array").WithParam("rule_ids") + return nil, mailRuleIDsValidationError("--data.rule_ids must be a non-empty string array") } if len(rawIDs) == 0 { - return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, - "--data.rule_ids must not be empty").WithParam("rule_ids") + return nil, mailRuleIDsValidationError("--data.rule_ids must not be empty") } ids := make([]string, 0, len(rawIDs)) @@ -107,11 +104,11 @@ func mailRuleReorderInputIDs(data interface{}) ([]string, error) { id, ok := rawID.(string) if !ok || id == "" { return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, - "--data.rule_ids[%d] must be a non-empty string", i).WithParam("rule_ids") + "--data.rule_ids[%d] must be a non-empty string", i).WithParam("--data.rule_ids") } if seen[id] { return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, - "--data.rule_ids contains duplicate rule_id %q", id).WithParam("rule_ids") + "--data.rule_ids contains duplicate rule_id %q", id).WithParam("--data.rule_ids") } seen[id] = true ids = append(ids, id) @@ -119,6 +116,10 @@ func mailRuleReorderInputIDs(data interface{}) ([]string, error) { return ids, nil } +func mailRuleIDsValidationError(message string) *errs.ValidationError { + return errs.NewValidationError(errs.SubtypeInvalidArgument, message).WithParam("--data.rule_ids") +} + func mailRuleListIDs(result interface{}) ([]string, error) { resultMap, ok := result.(map[string]interface{}) if !ok || resultMap == nil { diff --git a/cmd/service/mail_rule_reorder_test.go b/cmd/service/mail_rule_reorder_test.go index 8d2efb493..aa8ead4c5 100644 --- a/cmd/service/mail_rule_reorder_test.go +++ b/cmd/service/mail_rule_reorder_test.go @@ -8,7 +8,6 @@ import ( "encoding/json" "errors" "reflect" - "strings" "testing" "github.com/larksuite/cli/errs" @@ -72,6 +71,7 @@ func mailRuleReorderStub() *httpmock.Stub { func executeMailRuleReorder(t *testing.T, data string, dryRun bool, stubs ...*httpmock.Stub) (*bytes.Buffer, *httpmock.Stub, error) { t.Helper() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) f, stdout, _, reg := cmdutil.TestFactory(t, &core.CliConfig{ AppID: "test-app-mail-rules", AppSecret: "test-secret", Brand: core.BrandFeishu, }) @@ -102,7 +102,29 @@ func capturedRuleIDs(t *testing.T, stub *httpmock.Stub) []string { return body.RuleIDs } -func requireValidationError(t *testing.T, err error, want string) { +func dryRunRuleIDs(t *testing.T, stdout string) []string { + t.Helper() + const prefix = "=== Dry Run ===\n" + if len(stdout) <= len(prefix) || stdout[:len(prefix)] != prefix { + t.Fatalf("unexpected dry-run output:\n%s", stdout) + } + var out struct { + API []struct { + Body struct { + RuleIDs []string `json:"rule_ids"` + } `json:"body"` + } `json:"api"` + } + if err := json.Unmarshal([]byte(stdout[len(prefix):]), &out); err != nil { + t.Fatalf("decode dry-run JSON: %v\nraw=%s", err, stdout) + } + if len(out.API) != 1 { + t.Fatalf("dry-run api call count = %d, want 1", len(out.API)) + } + return out.API[0].Body.RuleIDs +} + +func requireValidationError(t *testing.T, err error, wantMessage, wantParam string) { t.Helper() if err == nil { t.Fatal("expected validation error") @@ -111,8 +133,17 @@ func requireValidationError(t *testing.T, err error, want string) { if !errors.As(err, &validationErr) { t.Fatalf("expected ValidationError, got %T: %v", err, err) } - if !strings.Contains(err.Error(), want) { - t.Fatalf("expected %q in error, got %v", want, err) + if validationErr.Category != errs.CategoryValidation { + t.Fatalf("validation category = %q, want %q", validationErr.Category, errs.CategoryValidation) + } + if validationErr.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("validation subtype = %q, want %q", validationErr.Subtype, errs.SubtypeInvalidArgument) + } + if validationErr.Message != wantMessage { + t.Fatalf("validation message = %q, want %q", validationErr.Message, wantMessage) + } + if validationErr.Param != wantParam { + t.Fatalf("validation param = %q, want %q", validationErr.Param, wantParam) } } @@ -143,12 +174,12 @@ func TestMailRuleReorderKeepsCompleteRuleIDs(t *testing.T) { func TestMailRuleReorderUnknownIDDoesNotCallReorder(t *testing.T) { list := mailRuleListStub("r1", "r2") _, _, err := executeMailRuleReorder(t, `{"rule_ids":["r3"]}`, false, list) - requireValidationError(t, err, "unknown rule_id") + requireValidationError(t, err, `--data.rule_ids contains unknown rule_id "r3"`, "--data.rule_ids") } func TestMailRuleReorderDuplicateIDDoesNotCallListOrReorder(t *testing.T) { _, _, err := executeMailRuleReorder(t, `{"rule_ids":["r1","r1"]}`, false) - requireValidationError(t, err, "duplicate rule_id") + requireValidationError(t, err, `--data.rule_ids contains duplicate rule_id "r1"`, "--data.rule_ids") } func TestMailRuleReorderDryRunListsAndPrintsCompletedBody(t *testing.T) { @@ -160,11 +191,7 @@ func TestMailRuleReorderDryRunListsAndPrintsCompletedBody(t *testing.T) { if len(list.CapturedBodies) != 1 { t.Fatalf("list call count = %d, want 1", len(list.CapturedBodies)) } - out := stdout.String() - if !strings.Contains(out, `"rule_ids"`) || - !strings.Contains(out, `"r3"`) || - !strings.Contains(out, `"r1"`) || - !strings.Contains(out, `"r2"`) { - t.Fatalf("dry-run output missing completed rule_ids, got:\n%s", out) + if got, want := dryRunRuleIDs(t, stdout.String()), []string{"r3", "r1", "r2"}; !reflect.DeepEqual(got, want) { + t.Fatalf("dry-run rule_ids = %#v, want %#v", got, want) } }