feat(base): support button field workflow binding - #2264
Conversation
Add base shortcuts for button field workflow bind, query, unbind, and reverse lookup by workflow. Document the workflow-first button field flow and cover dry-run/execute behavior. Co-authored-by: TRAE CLI <noreply@bytedance.com>
|
wanghaomin seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
github-actions
Bot
added
domain/base
📝 WalkthroughWalkthroughThis change adds four Base shortcuts for button-field workflow binding operations. It validates ChangesBase button-field workflow operations
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 14 UNAVAILABLE: read ECONNRESET 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with 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.
Inline comments:
In `@shortcuts/base/base_execute_test.go`:
- Around line 4075-4080: The “bind rejects internal numeric workflow id” test
currently validates only an error-message substring. Update it to use
errs.ProblemOf for the expected validation category and subtype, and errors.As
with *errs.ValidationError to verify Param equals "--workflow-id", while
retaining the existing rejection assertion.
In `@shortcuts/base/field_button_ops.go`:
- Around line 13-20: Update validateWorkflowIDFlag to reject, rather than
normalize, workflow-id values with leading or trailing whitespace before prefix
validation succeeds. Return the existing typed baseFlagErrorf validation error
for such input, while preserving the original value for dry-run and execution
paths.
In `@skills/lark-base/SKILL.md`:
- Line 76: Clarify the “按钮字段绑定” documentation entry so the wkf... format applies
only to workflow ID values. Explicitly distinguish workflow IDs from the
required field ID argument for the bind command, and note that
+workflow-button-fields returns button-field information rather than implying
all values are workflow IDs.
In `@tests/cli_e2e/base/base_field_dryrun_test.go`:
- Around line 46-122: Add a separate live E2E test alongside
TestBaseFieldButtonWorkflowDryRun that uses bot credentials to create the
required Base, table, button field, and workflow resources, then exercises bind,
binding retrieval, workflow-button-field listing, and unbind through the CLI.
Assert each operation succeeds and validates its response, and ensure cleanup
removes all created resources even when assertions fail; leave the existing
dry-run table unchanged.
🪄 Autofix
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 Plus
Run ID: 4b596724-eeac-43a6-bf92-8853f7a4803f
📒 Files selected for processing (12)
shortcuts/base/base_execute_test.goshortcuts/base/base_shortcuts_test.goshortcuts/base/field_button_bind.goshortcuts/base/field_button_binding_get.goshortcuts/base/field_button_ops.goshortcuts/base/field_button_unbind.goshortcuts/base/shortcuts.goshortcuts/base/workflow_button_fields.goskills/lark-base/SKILL.mdskills/lark-base/references/lark-base-field-create.mdskills/lark-base/references/lark-base-field-json.mdtests/cli_e2e/base/base_field_dryrun_test.go
| t.Run("bind rejects internal numeric workflow id", func(t *testing.T) { | ||
| factory, stdout, _ := newExecuteFactory(t) | ||
| err := runShortcut(t, BaseFieldButtonBind, []string{"+field-button-bind", "--base-token", "app_x", "--table-id", "tbl_x", "--field-id", "fld_x", "--workflow-id", "123"}, factory, stdout) | ||
| if err == nil || !strings.Contains(err.Error(), "wkf prefix") { | ||
| t.Fatalf("err=%v", err) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Assert the typed validation contract.
This test checks only the "wkf prefix" message. It will accept an untyped error with the same text.
Use errs.ProblemOf to assert the validation category and subtype. Use errors.As with *errs.ValidationError to assert Param == "--workflow-id".
As per coding guidelines, error-path tests must assert typed metadata instead of relying only on message substrings. Based on learnings, use errors.As because errs.ProblemOf does not expose Param.
🤖 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/base_execute_test.go` around lines 4075 - 4080, The “bind
rejects internal numeric workflow id” test currently validates only an
error-message substring. Update it to use errs.ProblemOf for the expected
validation category and subtype, and errors.As with *errs.ValidationError to
verify Param equals "--workflow-id", while retaining the existing rejection
assertion.
Sources: Coding guidelines, Learnings
| func validateWorkflowIDFlag(runtime *common.RuntimeContext) error { | ||
| workflowID := strings.TrimSpace(runtime.Str("workflow-id")) | ||
| if workflowID == "" { | ||
| return baseFlagErrorf("--workflow-id must not be blank") | ||
| } | ||
| if !strings.HasPrefix(workflowID, "wkf") { | ||
| return baseFlagErrorf("--workflow-id must be an OpenAPI workflow ID with wkf prefix; internal numeric workflow IDs are not accepted") | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject surrounding whitespace before API execution.
validateWorkflowIDFlag validates the trimmed value. The dry-run and execute functions use the original value. An input such as " wkf_x " therefore passes validation but sends whitespace in the request path or body.
Reject leading or trailing whitespace so validation and execution use the same value.
Proposed fix
func validateWorkflowIDFlag(runtime *common.RuntimeContext) error {
- workflowID := strings.TrimSpace(runtime.Str("workflow-id"))
+ rawWorkflowID := runtime.Str("workflow-id")
+ workflowID := strings.TrimSpace(rawWorkflowID)
if workflowID == "" {
return baseFlagErrorf("--workflow-id must not be blank")
}
+ if workflowID != rawWorkflowID {
+ return baseFlagErrorf("--workflow-id must not contain leading or trailing whitespace")
+ }
if !strings.HasPrefix(workflowID, "wkf") {As per coding guidelines, preserve input values faithfully and return a typed validation error when the requested input cannot be honored.
📝 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.
| func validateWorkflowIDFlag(runtime *common.RuntimeContext) error { | |
| workflowID := strings.TrimSpace(runtime.Str("workflow-id")) | |
| if workflowID == "" { | |
| return baseFlagErrorf("--workflow-id must not be blank") | |
| } | |
| if !strings.HasPrefix(workflowID, "wkf") { | |
| return baseFlagErrorf("--workflow-id must be an OpenAPI workflow ID with wkf prefix; internal numeric workflow IDs are not accepted") | |
| } | |
| func validateWorkflowIDFlag(runtime *common.RuntimeContext) error { | |
| rawWorkflowID := runtime.Str("workflow-id") | |
| workflowID := strings.TrimSpace(rawWorkflowID) | |
| if workflowID == "" { | |
| return baseFlagErrorf("--workflow-id must not be blank") | |
| } | |
| if workflowID != rawWorkflowID { | |
| return baseFlagErrorf("--workflow-id must not contain leading or trailing whitespace") | |
| } | |
| if !strings.HasPrefix(workflowID, "wkf") { | |
| return baseFlagErrorf("--workflow-id must be an OpenAPI workflow ID with wkf prefix; internal numeric workflow IDs are not accepted") | |
| } |
🤖 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/field_button_ops.go` around lines 13 - 20, Update
validateWorkflowIDFlag to reject, rather than normalize, workflow-id values with
leading or trailing whitespace before prefix validation succeeds. Return the
existing typed baseFlagErrorf validation error for such input, while preserving
the original value for dry-run and execution paths.
Source: Coding guidelines
| | 分享表单详情 | `+form-detail --share-token <share_token>` | 只接受表单分享链接里的 `share_token`，不要传 `--base-token` / `--form-id`；提交前读 [lark-base-form-detail.md](references/lark-base-form-detail.md) | | ||
| | 仪表盘与组件 | `+dashboard-*` / `+dashboard-block-*` | 提到图表/看板/block 时先读 [lark-base-dashboard.md](references/lark-base-dashboard.md)；组件 `data_config` 读 [dashboard-block-data-config.md](references/dashboard-block-data-config.md)；读取一个或多个图表计算结果用 `+dashboard-block-get-data`；读取完整仪表盘时按 block 类型分流，文本和不支持直接取数的图表按 reference 恢复 | | ||
| | Workflow | `+workflow-*` | 创建/更新或理解 steps 时读入口 [lark-base-workflow-guide.md](references/lark-base-workflow-guide.md) 和 steps JSON SSOT [lark-base-workflow-schema.md](references/lark-base-workflow-schema.md)；list/get/enable/disable 只处理 workflow ID 与启停状态 | | ||
| | 按钮字段绑定 | `+field-button-bind` / `+field-button-binding-get` / `+field-button-unbind` / `+workflow-button-fields` | 只接受和展示 `wkf...` workflow ID；绑定事实来自独立按钮绑定接口，不来自字段 `property.trigger` | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify which identifiers use the wkf... format.
This text can be read as saying that every shortcut accepts and displays only workflow IDs. The documented bind command also requires --field-id, and +workflow-button-fields returns button-field information. State that only workflow ID values must use the wkf... format. Keep field IDs and workflow IDs distinct.
🤖 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 `@skills/lark-base/SKILL.md` at line 76, Clarify the “按钮字段绑定” documentation
entry so the wkf... format applies only to workflow ID values. Explicitly
distinguish workflow IDs from the required field ID argument for the bind
command, and note that +workflow-button-fields returns button-field information
rather than implying all values are workflow IDs.
| func TestBaseFieldButtonWorkflowDryRun(t *testing.T) { | ||
| setBaseDryRunConfigEnv(t) | ||
|
|
||
| ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | ||
| t.Cleanup(cancel) | ||
|
|
||
| cases := []struct { | ||
| name string | ||
| args []string | ||
| method string | ||
| url string | ||
| }{ | ||
| { | ||
| name: "bind", | ||
| args: []string{ | ||
| "base", "+field-button-bind", | ||
| "--base-token", "app_x", | ||
| "--table-id", "tbl_x", | ||
| "--field-id", "fld_x", | ||
| "--workflow-id", "wkf_x", | ||
| "--dry-run", | ||
| }, | ||
| method: "POST", | ||
| url: "/open-apis/base/v3/bases/app_x/tables/tbl_x/fields/fld_x/button-workflow:bind", | ||
| }, | ||
| { | ||
| name: "get field binding", | ||
| args: []string{ | ||
| "base", "+field-button-binding-get", | ||
| "--base-token", "app_x", | ||
| "--table-id", "tbl_x", | ||
| "--field-id", "fld_x", | ||
| "--dry-run", | ||
| }, | ||
| method: "GET", | ||
| url: "/open-apis/base/v3/bases/app_x/tables/tbl_x/fields/fld_x/button-workflow", | ||
| }, | ||
| { | ||
| name: "unbind", | ||
| args: []string{ | ||
| "base", "+field-button-unbind", | ||
| "--base-token", "app_x", | ||
| "--table-id", "tbl_x", | ||
| "--field-id", "fld_x", | ||
| "--dry-run", | ||
| }, | ||
| method: "POST", | ||
| url: "/open-apis/base/v3/bases/app_x/tables/tbl_x/fields/fld_x/button-workflow:unbind", | ||
| }, | ||
| { | ||
| name: "workflow button fields", | ||
| args: []string{ | ||
| "base", "+workflow-button-fields", | ||
| "--base-token", "app_x", | ||
| "--workflow-id", "wkf_x", | ||
| "--dry-run", | ||
| }, | ||
| method: "GET", | ||
| url: "/open-apis/base/v3/bases/app_x/workflows/wkf_x/button-fields", | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range cases { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| result, err := clie2e.RunCmd(ctx, clie2e.Request{Args: tt.args, DefaultAs: "bot"}) | ||
| require.NoError(t, err) | ||
| result.AssertExitCode(t, 0) | ||
|
|
||
| out := result.Stdout | ||
| require.Equal(t, tt.url, clie2e.DryRunGet(out, "api.0.url").String(), out) | ||
| require.Equal(t, tt.method, clie2e.DryRunGet(out, "api.0.method").String(), out) | ||
| if tt.name == "bind" { | ||
| require.Equal(t, "wkf_x", clie2e.DryRunGet(out, "api.0.body.workflow_id").String(), out) | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Add live E2E coverage for the new workflow operations.
This test verifies only generated dry-run requests. It does not verify the live Base API contract.
Add a self-contained live test that creates the required Base resources, binds the button field, gets the binding, lists the workflow fields, unbinds the field, and cleans up the resources. Use bot credentials where required.
As per coding guidelines, new flows under tests/cli_e2e/ require live create/use/cleanup coverage.
🤖 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 `@tests/cli_e2e/base/base_field_dryrun_test.go` around lines 46 - 122, Add a
separate live E2E test alongside TestBaseFieldButtonWorkflowDryRun that uses bot
credentials to create the required Base, table, button field, and workflow
resources, then exercises bind, binding retrieval, workflow-button-field
listing, and unbind through the CLI. Assert each operation succeeds and
validates its response, and ensure cleanup removes all created resources even
when assertions fail; leave the existing dry-run table unchanged.
Source: Coding guidelines
2 participants
Summary\n- add base shortcuts to bind, query, unbind, and list button field workflow relations\n- document workflow-first button field creation flow for lark-base skill\n- cover dry-run and execute behavior for the new commands\n\n## Tests\n- GOFLAGS=-buildvcs=false make build\n- go test ./shortcuts/base ./tests/cli_e2e/base
Summary by CodeRabbit
New Features
wkf...format.Documentation
Tests