Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
410 changes: 410 additions & 0 deletions shortcuts/base/base_dashboard_layout_precision_test.go

Large diffs are not rendered by default.

54 changes: 54 additions & 0 deletions shortcuts/base/base_dryrun_ops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package base

import (
"context"
"reflect"
"strings"
"testing"
)
Expand Down Expand Up @@ -402,6 +403,59 @@ func TestDryRunDashboardOps(t *testing.T) {
assertDryRunContains(t, dryRunDashboardBlockDelete(ctx, rt), "DELETE /open-apis/base/v3/bases/app_x/dashboards/dash_1/blocks/blk_1")
}

// TestDryRunDashboardBlockPositionAndNumberFormat asserts the shared body
// builder surfaces the optional top-level position and data_config.number_format
// in the dry-run request body, and that create DryRun matches the create Execute
// body shape (isomorphism) for the same inputs.
func TestDryRunDashboardBlockPositionAndNumberFormat(t *testing.T) {
ctx := context.Background()

rt := newBaseTestRuntime(
map[string]string{
"base-token": "app_x",
"dashboard-id": "dash_1",
"block-id": "blk_1",
"name": "Revenue",
"type": "statistics",
"data-config": `{"table_name":"Orders","count_all":true,"number_format":{"formatName":"dollar_rounded","precision":2}}`,
"position": `{"x":0,"y":0,"w":6,"h":4}`,
},
nil,
nil,
)

assertDryRunContains(t, dryRunDashboardBlockCreate(ctx, rt),
"POST /open-apis/base/v3/bases/app_x/dashboards/dash_1/blocks",
`"position"`, `"w":6`, `"number_format"`, `"dollar_rounded"`)
assertDryRunContains(t, dryRunDashboardBlockUpdate(ctx, rt),
"PATCH /open-apis/base/v3/bases/app_x/dashboards/dash_1/blocks/blk_1",
`"position"`, `"w":6`)

// Isomorphism: DryRun body and Execute body must be built identically.
pc := newParseCtx(rt)
dryBody, _ := buildDashboardBlockBody(pc, rt, true, false)
execBody, err := buildDashboardBlockBody(pc, rt, true, true)
if err != nil {
t.Fatalf("execute body build err=%v", err)
}
if !reflect.DeepEqual(dryBody, execBody) {
t.Fatalf("dry-run and execute bodies diverge:\n dry=%v\nexec=%v", dryBody, execBody)
}

// Same isomorphism must hold on the update path (includeType=false).
dryUpdate, _ := buildDashboardBlockBody(pc, rt, false, false)
execUpdate, err := buildDashboardBlockBody(pc, rt, false, true)
if err != nil {
t.Fatalf("update execute body build err=%v", err)
}
if !reflect.DeepEqual(dryUpdate, execUpdate) {
t.Fatalf("update dry-run and execute bodies diverge:\n dry=%v\nexec=%v", dryUpdate, execUpdate)
}
if _, hasType := dryUpdate["type"]; hasType {
t.Fatalf("update body must not carry type: %v", dryUpdate)
}
}

func TestDryRunViewOps(t *testing.T) {
ctx := context.Background()

Expand Down
20 changes: 8 additions & 12 deletions shortcuts/base/dashboard_block_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,19 @@ var BaseDashboardBlockCreate = common.Shortcut{
{Name: "name", Desc: "block name", Required: true},
{Name: "type", Desc: "block type: column(柱状图)|bar(条形图)|line(折线图)|pie(饼图)|ring(环形图)|area(面积图)|combo(组合图)|scatter(散点图)|funnel(漏斗图)|wordCloud(词云)|radar(雷达图)|statistics(指标卡)|text(文本). Read dashboard-block-data-config.md before creating.", Required: true},
{Name: "data-config", Desc: "data_config JSON object; read dashboard-block-data-config.md for the SSOT"},
{Name: "position", Desc: `optional. component position+size in 12-col grid, JSON {"x","y","w","h"}; advisory bounds x/y>=0, 1<=w<=12 and x+w<=12, h>=1 (NOT validated locally or server-side, out-of-range passes through); omit for server auto-layout`},
{Name: "user-id-type", Desc: "user ID type for user fields in filters: open_id / union_id / user_id"},
{Name: "no-validate", Type: "bool", Desc: "skip local data_config validation and normalization; send data_config as-is"},
},
Tips: []string{
`lark-cli base +dashboard-block-create --base-token <base_token> --dashboard-id <dashboard_id> --name "Order Count" --type statistics --data-config '{"table_name":"Orders","count_all":true}'`,
`lark-cli base +dashboard-block-create --base-token <base_token> --dashboard-id <dashboard_id> --name "Revenue" --type statistics --data-config '{"table_name":"Orders","series":[{"field_name":"Amount","rollup":"SUM"}],"number_format":{"formatName":"dollar_rounded","precision":2}}'`,
`lark-cli base +dashboard-block-create --base-token <base_token> --dashboard-id <dashboard_id> --name "Dashboard Note" --type text --data-config '{"text":"# Sales Dashboard"}'`,
`lark-cli base +dashboard-block-create --base-token <base_token> --dashboard-id <dashboard_id> --name "Order Count" --type statistics --data-config '{"table_name":"Orders","count_all":true}' --position '{"x":0,"y":0,"w":6,"h":4}'`,
"Before creating data-backed blocks, use +table-list and +field-list to confirm real table and field names.",
"data_config uses table and field names, not table_id or field_id.",
"Read dashboard-block-data-config.md as the SSOT for chart templates, filters, metric rules, and type-specific fields; do not invent data_config from natural language.",
"--position is optional precise layout in a 12-col grid; omit it to let the server auto-layout. Coordinate values are not validated locally and overlaps are not server-checked. To re-tidy an existing dashboard use +dashboard-arrange instead.",
"For funnel/stage charts backed by ordered helper data, set the intended group_by.sort in the initial create request; do not create first and then issue a second update just to fix sorting.",
"Record the returned block_id; block update/delete/get-data commands need it.",
"Create dashboard blocks sequentially; do not parallelize multiple block creates for the same dashboard.",
Expand All @@ -44,6 +48,9 @@ var BaseDashboardBlockCreate = common.Shortcut{
if runtime.Bool("no-validate") {
return nil
}
if err := validateDashboardBlockPosition(pc, runtime); err != nil {
return err
}
raw := runtime.Str("data-config")
if strings.TrimSpace(raw) == "" {
// text 类型必须提供 data-config(含 text 内容)
Expand All @@ -67,18 +74,7 @@ var BaseDashboardBlockCreate = common.Shortcut{
},
DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI {
pc := newParseCtx(runtime)
body := map[string]interface{}{}
if name := runtime.Str("name"); name != "" {
body["name"] = name
}
if t := runtime.Str("type"); t != "" {
body["type"] = t
}
if raw := runtime.Str("data-config"); raw != "" {
if parsed, err := parseJSONObject(pc, raw, "data-config"); err == nil {
body["data_config"] = parsed
}
}
body, _ := buildDashboardBlockBody(pc, runtime, true, false)
params := map[string]interface{}{}
if uid := runtime.Str("user-id-type"); uid != "" {
params["user_id_type"] = uid
Expand Down
26 changes: 16 additions & 10 deletions shortcuts/base/dashboard_block_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,29 @@ var BaseDashboardBlockUpdate = common.Shortcut{
blockIDFlag(true),
{Name: "name", Desc: "new block name"},
{Name: "data-config", Desc: "data_config JSON object; read dashboard-block-data-config.md for the SSOT"},
{Name: "position", Desc: `optional. component position+size in 12-col grid, JSON {"x","y","w","h"}; advisory bounds x/y>=0, 1<=w<=12 and x+w<=12, h>=1 (NOT validated locally or server-side, out-of-range passes through); omit to leave layout unchanged`},
{Name: "user-id-type", Desc: "user ID type for user fields in filters: open_id / union_id / user_id"},
{Name: "no-validate", Type: "bool", Desc: "skip local data_config validation and normalization; send data_config as-is"},
},
Tips: []string{
`lark-cli base +dashboard-block-update --base-token <base_token> --dashboard-id <dashboard_id> --block-id <block_id> --name "Total Sales"`,
`lark-cli base +dashboard-block-update --base-token <base_token> --dashboard-id <dashboard_id> --block-id <block_id> --data-config '{"series":[{"field_name":"Amount","rollup":"SUM"}]}'`,
`lark-cli base +dashboard-block-update --base-token <base_token> --dashboard-id <dashboard_id> --block-id <block_id> --data-config '{"number_format":{"precision":0}}'`,
`lark-cli base +dashboard-block-update --base-token <base_token> --dashboard-id <dashboard_id> --block-id <block_id> --position '{"x":6,"y":0,"w":6,"h":4}'`,
"Read dashboard-block-data-config.md as the SSOT for data_config templates, filters, metric rules, and type-specific fields; do not invent data_config from natural language.",
"Use +dashboard-block-get first to inspect the current data_config before replacing nested values.",
"Block type cannot be changed; delete and recreate the block to change chart type.",
"data_config update merges top-level keys, but each provided key is replaced as a whole.",
"--position is optional precise layout in a 12-col grid; omit it to leave the current layout unchanged. Coordinate values are not validated locally and overlaps are not server-checked. To re-tidy an existing dashboard use +dashboard-arrange instead.",
},
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
pc := newParseCtx(runtime)
if runtime.Bool("no-validate") {
return nil
}
if err := validateDashboardBlockPosition(pc, runtime); err != nil {
return err
}
raw := runtime.Str("data-config")
if strings.TrimSpace(raw) == "" {
return nil
Expand All @@ -50,22 +57,21 @@ var BaseDashboardBlockUpdate = common.Shortcut{
return err
}
norm := normalizeDataConfig(cfg)
// update 时不做强类型校验(不传 type),让后端验证具体字段
// update 时不做强类型校验(不传 type),让后端验证具体字段。
// 但 number_format 必须与 create 对称地本地拦截(backend-design §4.5):
// update 无 type flag,无法调用 validateBlockDataConfig 的完整分支
// (那会误报 table_name/series 缺失,破坏仅改 number_format 的用法),
// 因此直接复用其委托的 number_format 子校验,保持双路径一致。
if problems := validateNumberFormat(norm["number_format"]); len(problems) > 0 {
return formatDataConfigErrors(problems)
}
Comment on lines +60 to +67

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Update validates number_format for every block type; create only for statistics.

validateBlockDataConfig gates the check on blockType == "statistics" (helpers.go Line 1231), so a non-statistics block accepts any number_format on create but is rejected on update. Since the block type isn't known here, this is a defensible tradeoff, but the divergence is user-visible — worth documenting in the tips/reference alongside the --no-validate escape hatch.

🤖 Prompt for 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.

In `@shortcuts/base/dashboard_block_update.go` around lines 60 - 67, The update
path around validateNumberFormat applies validation to all block types, unlike
validateBlockDataConfig during create, which only validates statistics blocks.
Document this intentional create/update behavior difference in the relevant tips
or reference documentation, including the --no-validate escape hatch for update
requests.

b, _ := json.Marshal(norm)
_ = runtime.Cmd.Flags().Set("data-config", string(b))
return nil
},
DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI {
pc := newParseCtx(runtime)
body := map[string]interface{}{}
if name := runtime.Str("name"); name != "" {
body["name"] = name
}
if raw := runtime.Str("data-config"); raw != "" {
if parsed, err := parseJSONObject(pc, raw, "data-config"); err == nil {
body["data_config"] = parsed
}
}
body, _ := buildDashboardBlockBody(pc, runtime, false, false)
params := map[string]interface{}{}
if uid := runtime.Str("user-id-type"); uid != "" {
params["user_id_type"] = uid
Expand Down
118 changes: 74 additions & 44 deletions shortcuts/base/dashboard_ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package base

import (
"context"
"encoding/json"
"strings"

"github.com/larksuite/cli/shortcuts/common"
Expand All @@ -20,6 +21,71 @@ func blockIDFlag(required bool) common.Flag {
return common.Flag{Name: "block-id", Desc: "dashboard block ID", Required: required}
}

// buildDashboardBlockBody assembles the request body shared by the dashboard
// block create and update commands. It is the single source of truth for body
// shape so DryRun and Execute stay isomorphic across all call sites.
//
// - includeType controls whether the block type is emitted (create only).
// - The optional top-level position object is parsed as JSON and passed
// through verbatim as a sibling of name/type/data_config; coordinate values
// are NOT validated (aligns with dws grid semantics).
// - When strict is true (Execute path) a malformed data-config/position fails
// the command; when false (DryRun preview) the offending field is skipped so
// the preview still renders, matching the existing dry-run behaviour.
func buildDashboardBlockBody(pc *parseCtx, runtime *common.RuntimeContext, includeType, strict bool) (map[string]interface{}, error) {
body := map[string]interface{}{}
if name := strings.TrimSpace(runtime.Str("name")); name != "" {
body["name"] = name
}
if includeType {
if blockType := strings.TrimSpace(runtime.Str("type")); blockType != "" {
body["type"] = blockType
}
}
if raw := runtime.Str("data-config"); raw != "" {
parsed, err := parseJSONObject(pc, raw, "data-config")
if err != nil {
if strict {
return nil, err
}
} else {
body["data_config"] = parsed
}
}
if raw := strings.TrimSpace(runtime.Str("position")); raw != "" {
parsed, err := parseJSONObject(pc, raw, "position")
if err != nil {
if strict {
return nil, err
}
} else {
body["position"] = parsed
}
}
Comment on lines +45 to +64

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Whitespace handling is asymmetric between data-config and position.

position is trimmed before the empty check, data-config is not. Validate treats a whitespace-only --data-config as absent (strings.TrimSpace(raw) == "" → return nil), but here the same value reaches parseJSONObject and fails the strict Execute path. Trim both for consistency.

♻️ Proposed fix
-	if raw := runtime.Str("data-config"); raw != "" {
+	if raw := strings.TrimSpace(runtime.Str("data-config")); raw != "" {
 		parsed, err := parseJSONObject(pc, raw, "data-config")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if raw := runtime.Str("data-config"); raw != "" {
parsed, err := parseJSONObject(pc, raw, "data-config")
if err != nil {
if strict {
return nil, err
}
} else {
body["data_config"] = parsed
}
}
if raw := strings.TrimSpace(runtime.Str("position")); raw != "" {
parsed, err := parseJSONObject(pc, raw, "position")
if err != nil {
if strict {
return nil, err
}
} else {
body["position"] = parsed
}
}
if raw := strings.TrimSpace(runtime.Str("data-config")); raw != "" {
parsed, err := parseJSONObject(pc, raw, "data-config")
if err != nil {
if strict {
return nil, err
}
} else {
body["data_config"] = parsed
}
}
if raw := strings.TrimSpace(runtime.Str("position")); raw != "" {
parsed, err := parseJSONObject(pc, raw, "position")
if err != nil {
if strict {
return nil, err
}
} else {
body["position"] = parsed
}
}
🤖 Prompt for 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.

In `@shortcuts/base/dashboard_ops.go` around lines 45 - 64, Update the data-config
handling in the surrounding execution method to trim the raw value before
checking for emptiness, matching the existing position handling and Validate
behavior. Use the trimmed value for parseJSONObject so whitespace-only
data-config inputs are treated as absent and non-empty values are parsed
consistently.

return body, nil
}

// validateDashboardBlockPosition parses the optional --position flag as a JSON
// object to fail fast on malformed input. Coordinate values (x/y/w/h) are NOT
// validated — they pass through verbatim, aligning with dws grid semantics and
// leaving overlap handling to the server. Callers must skip this when
// --no-validate is set so dry-run and execute stay consistent.
func validateDashboardBlockPosition(pc *parseCtx, runtime *common.RuntimeContext) error {
raw := strings.TrimSpace(runtime.Str("position"))
if raw == "" {
return nil
}
pos, err := parseJSONObject(pc, raw, "position")
if err != nil {
return err
}
// Rewrite the flag with canonical JSON so the parsed shape is identical
// across Validate, DryRun and Execute.
b, _ := json.Marshal(pos)
_ = runtime.Cmd.Flags().Set("position", string(b))
return nil
}

// dryRunDashboardBase returns a base DryRunAPI with common dashboard parameters set.
func dryRunDashboardBase(runtime *common.RuntimeContext) *common.DryRunAPI {
return common.NewDryRunAPI().
Expand Down Expand Up @@ -111,18 +177,7 @@ func dryRunDashboardBlockGetData(_ context.Context, runtime *common.RuntimeConte
// dryRunDashboardBlockCreate returns a DryRunAPI for creating a dashboard block.
func dryRunDashboardBlockCreate(_ context.Context, runtime *common.RuntimeContext) *common.DryRunAPI {
pc := newParseCtx(runtime)
body := map[string]interface{}{}
if name := strings.TrimSpace(runtime.Str("name")); name != "" {
body["name"] = name
}
if blockType := strings.TrimSpace(runtime.Str("type")); blockType != "" {
body["type"] = blockType
}
if raw := runtime.Str("data-config"); raw != "" {
if parsed, err := parseJSONObject(pc, raw, "data-config"); err == nil {
body["data_config"] = parsed
}
}
body, _ := buildDashboardBlockBody(pc, runtime, true, false)

params := map[string]interface{}{}
if userIDType := strings.TrimSpace(runtime.Str("user-id-type")); userIDType != "" {
Expand All @@ -137,15 +192,7 @@ func dryRunDashboardBlockCreate(_ context.Context, runtime *common.RuntimeContex
// dryRunDashboardBlockUpdate returns a DryRunAPI for updating a dashboard block.
func dryRunDashboardBlockUpdate(_ context.Context, runtime *common.RuntimeContext) *common.DryRunAPI {
pc := newParseCtx(runtime)
body := map[string]interface{}{}
if name := strings.TrimSpace(runtime.Str("name")); name != "" {
body["name"] = name
}
if raw := runtime.Str("data-config"); raw != "" {
if parsed, err := parseJSONObject(pc, raw, "data-config"); err == nil {
body["data_config"] = parsed
}
}
body, _ := buildDashboardBlockBody(pc, runtime, false, false)
params := map[string]interface{}{}
if userIDType := strings.TrimSpace(runtime.Str("user-id-type")); userIDType != "" {
params["user_id_type"] = userIDType
Expand Down Expand Up @@ -274,19 +321,9 @@ func executeDashboardBlockGetData(runtime *common.RuntimeContext) error {
// executeDashboardBlockCreate creates a new dashboard block.
func executeDashboardBlockCreate(runtime *common.RuntimeContext) error {
pc := newParseCtx(runtime)
body := map[string]interface{}{}
if name := strings.TrimSpace(runtime.Str("name")); name != "" {
body["name"] = name
}
if blockType := strings.TrimSpace(runtime.Str("type")); blockType != "" {
body["type"] = blockType
}
if raw := runtime.Str("data-config"); raw != "" {
parsed, err := parseJSONObject(pc, raw, "data-config")
if err != nil {
return err
}
body["data_config"] = parsed
body, err := buildDashboardBlockBody(pc, runtime, true, true)
if err != nil {
return err
}

params := map[string]interface{}{}
Expand All @@ -305,16 +342,9 @@ func executeDashboardBlockCreate(runtime *common.RuntimeContext) error {
// executeDashboardBlockUpdate updates an existing dashboard block.
func executeDashboardBlockUpdate(runtime *common.RuntimeContext) error {
pc := newParseCtx(runtime)
body := map[string]interface{}{}
if name := strings.TrimSpace(runtime.Str("name")); name != "" {
body["name"] = name
}
if raw := runtime.Str("data-config"); raw != "" {
parsed, err := parseJSONObject(pc, raw, "data-config")
if err != nil {
return err
}
body["data_config"] = parsed
body, err := buildDashboardBlockBody(pc, runtime, false, true)
if err != nil {
return err
}
params := map[string]interface{}{}
if userIDType := strings.TrimSpace(runtime.Str("user-id-type")); userIDType != "" {
Expand Down
Loading
Loading