Add +dashboard-arrange command for auto-arranging dashboard blocks …#388
Add +dashboard-arrange command for auto-arranging dashboard blocks …#388zhouyue-bytedance merged 1 commit intomainfrom
+dashboard-arrange command for auto-arranging dashboard blocks …#388Conversation
…layout and introduce `text` block type with Markdown support for dashboard visualization.
- Add `+dashboard-arrange` command that triggers server-side smart layout optimization via POST /open-apis/base/v3/bases/{token}/dashboards/{id}/arrange
- Add `text` block type support for dashboard blocks with Markdown syntax (headers, bold, italic, strikethrough, lists)
- Update `validateBlockDataConfig()` to handle text-specific validation rules
- Update documentation (SKILL.md, lark-base-dashboard.md, dashboard-block-data-config.md, lark-base-dashboard-arrange.md)
- Add comprehensive unit tests for new commands and block type
- [x] Unit tests pass (`go test ./shortcuts/base/...`)
- [x] All dashboard-related tests pass including new `TestBaseDashboardExecuteArrange`
- [x] Text block type validation tests pass
- None
📝 WalkthroughWalkthroughThis PR introduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Comment |
Greptile SummaryThis PR adds a Confidence Score: 5/5Safe to merge; all findings are minor style improvements with no impact on correctness or runtime behaviour. Logic is correct across the new arrange command and text block type. Validation, normalization, and flag-overwrite flows all behave as intended. The two findings are P2 style suggestions only. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User invokes command] --> B{Command?}
B -->|dashboard-arrange| C[BaseDashboardArrange]
C --> D{dry-run?}
D -->|yes| E[dryRunDashboardArrange\nPOST arrange with empty body]
D -->|no| F[executeDashboardArrange\nPOST arrange with empty body]
F --> G[Append arranged=true to response]
G --> H[runtime.Out]
B -->|dashboard-block-create| I[BaseDashboardBlockCreate]
I --> J{no-validate flag?}
J -->|yes| N[Skip validation]
J -->|no| K{data-config empty AND type=text?}
K -->|yes| L[Return error: missing data-config]
K -->|no| M{data-config provided?}
M -->|yes| O[parseJSONObject and normalizeDataConfig]
O --> P[validateBlockDataConfig]
P --> Q{type=text?}
Q -->|yes| R[Check cfg.text is non-empty]
Q -->|no| S[Check table_name, series/count_all, group_by, filter]
R --> T[Overwrite flag with normalised JSON]
S --> T
M -->|no| N
N --> U[executeDashboardBlockCreate]
T --> U
Reviews (1): Last reviewed commit: "Add `+dashboard-arrange` command for aut..." | Re-trigger Greptile |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@d2e38891df43155a5a7a7b4ece927429c9c3c7b8🧩 Skill updatenpx skills add larksuite/cli#base_dashboard -y -g |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
shortcuts/base/dashboard_ops.go (1)
353-357: Keep the arrange response under adashboardenvelope.
+dashboard-get/create/updateall expose dashboard data under a top-leveldashboardkey, but arrange currently flattens the API payload and addsarrangedbeside it. Since this command is new, it would be better to align the output shape now and avoid a one-off contract for automation.♻️ Suggested shape
if data == nil { data = map[string]interface{}{} } - data["arranged"] = true - runtime.Out(data, nil) + runtime.Out(map[string]interface{}{ + "dashboard": data, + "arranged": true, + }, nil) return nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/base/dashboard_ops.go` around lines 353 - 357, The arrange response is currently emitting fields flattened with an added arranged flag; wrap the payload under a top-level "dashboard" envelope instead. Modify the code that builds the response (the variable named data and the runtime.Out call) to create an envelope like envelope := map[string]interface{}{"dashboard": dataOrEmpty} then set envelope["dashboard"].(map[string]interface{})["arranged"] = true and call runtime.Out(envelope, nil) so arrange matches the same top-level dashboard shape as get/create/update.shortcuts/base/base_dashboard_execute_test.go (1)
747-800: Assert the real arrange request body in one execute test.These cases only lock the URL/query and response shape. If
executeDashboardArrange()stops sending the explicit{}body, the execute tests still pass. Adding one request-body assertion here would pin the contract that this endpoint is posted with an empty JSON object.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/base/base_dashboard_execute_test.go` around lines 747 - 800, The tests for dashboard arrange only validate URL/query and response; update the two arrange tests (the ones that call newExecuteFactory, reg.Register and runShortcut with BaseDashboardArrange) to also assert the actual request body sent by executeDashboardArrange is an empty JSON object (e.g. map[string]interface{}{}). After calling runShortcut, inspect the registered mock requests (from the reg returned by newExecuteFactory) or the specific httpmock.Stub that captured the request and add an assertion that the request payload equals an empty map, so the test fails if executeDashboardArrange starts sending a non-empty body.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/lark-base/references/dashboard-block-data-config.md`:
- Around line 49-55: Update the "text 类型特殊结构" section to explicitly state that
`count_all` is not supported in addition to `table_name`, `series`, `group_by`,
and `filter`; i.e., change the sentence that lists unsupported data-source
fields to include `count_all`, and mirror the same explicit exclusion in the
other text-block section that documents the `text` component (the similar block
later in the file that lists unsupported fields). Refer to the `text` field and
the unsupported list (`table_name`, `series`, `group_by`, `filter`, `count_all`)
when making the edit so readers won't accidentally copy-paste a data-source
config into text blocks.
In `@skills/lark-base/references/lark-base-dashboard.md`:
- Around line 63-67: The arrange example uses a block-style placeholder
(--dashboard-id blk_xxx) which is misleading; update the lark-cli base
+dashboard-arrange example and any other arrange examples that use
--dashboard-id blk_xxx to use a dashboard-style placeholder (--dashboard-id
dsh_xxx) so the commands target dashboards (e.g., replace instances of
"--dashboard-id blk_xxx" with "--dashboard-id dsh_xxx" in the arrange examples).
---
Nitpick comments:
In `@shortcuts/base/base_dashboard_execute_test.go`:
- Around line 747-800: The tests for dashboard arrange only validate URL/query
and response; update the two arrange tests (the ones that call
newExecuteFactory, reg.Register and runShortcut with BaseDashboardArrange) to
also assert the actual request body sent by executeDashboardArrange is an empty
JSON object (e.g. map[string]interface{}{}). After calling runShortcut, inspect
the registered mock requests (from the reg returned by newExecuteFactory) or the
specific httpmock.Stub that captured the request and add an assertion that the
request payload equals an empty map, so the test fails if
executeDashboardArrange starts sending a non-empty body.
In `@shortcuts/base/dashboard_ops.go`:
- Around line 353-357: The arrange response is currently emitting fields
flattened with an added arranged flag; wrap the payload under a top-level
"dashboard" envelope instead. Modify the code that builds the response (the
variable named data and the runtime.Out call) to create an envelope like
envelope := map[string]interface{}{"dashboard": dataOrEmpty} then set
envelope["dashboard"].(map[string]interface{})["arranged"] = true and call
runtime.Out(envelope, nil) so arrange matches the same top-level dashboard shape
as get/create/update.
🪄 Autofix (Beta)
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
Run ID: 380f79f6-79b8-4d5f-ac79-f746ca742e27
📒 Files selected for processing (13)
shortcuts/base/base_dashboard_execute_test.goshortcuts/base/base_shortcuts_test.goshortcuts/base/dashboard_arrange.goshortcuts/base/dashboard_block_create.goshortcuts/base/dashboard_block_update.goshortcuts/base/dashboard_ops.goshortcuts/base/helpers.goshortcuts/base/shortcuts.goskills/lark-base/SKILL.mdskills/lark-base/references/dashboard-block-data-config.mdskills/lark-base/references/lark-base-dashboard-arrange.mdskills/lark-base/references/lark-base-dashboard-block-create.mdskills/lark-base/references/lark-base-dashboard.md
…layout and introduce
textblock type with Markdown support for dashboard visualization.+dashboard-arrangecommand that triggers server-side smart layout optimization via POST /open-apis/base/v3/bases/{token}/dashboards/{id}/arrangetextblock type support for dashboard blocks with Markdown syntax (headers, bold, italic, strikethrough, lists)validateBlockDataConfig()to handle text-specific validation rulesgo test ./shortcuts/base/...)TestBaseDashboardExecuteArrangeSummary
Changes
Test Plan
lark xxxcommand works as expectedRelated Issues
Summary by CodeRabbit
New Features
+dashboard-arrangecommand for automatic dashboard block layout arrangement using server-side recommendations with adaptive positioningtextblock type for creating Markdown-formatted rich text blocks supporting headings, formatting, and listsDocumentation