feat(base): add workflow enable-all-disabled shortcut#1634
feat(base): add workflow enable-all-disabled shortcut#1634yballul-bytedance wants to merge 1 commit into
Conversation
|
github-pr-syncer 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. |
📝 WalkthroughWalkthroughAdds a new Changesworkflow-enable-all-disabled shortcut
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 docstrings
🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
domain/base
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/workflow_enable_all_disabled.go`:
- Around line 100-132: Add test coverage for the multi-page branch in
listWorkflowsByStatus so the has_more/page_token loop is exercised, not just the
single-page path. Update the relevant tests around workflow_execute_test and the
dry-run CLI e2e flow to mock a first response with has_more true plus a
page_token, then a second response that ends pagination, and assert all returned
items are accumulated from both pages. Reference listWorkflowsByStatus and the
workflow enable-all-disabled path when locating the code.
- Around line 29-35: The new Validate branches in
workflow_enable_all_disabled.go need unit coverage for both blank base-token and
invalid page-size. Add tests around the Validate function to exercise the
baseFlagErrorf path for an empty “base-token” and the
common.ValidatePageSizeTyped path for bad “page-size” values, using the existing
runtime setup helpers. In the assertions, verify typed error metadata with
errs.ProblemOf, including category/subtype/param and cause preservation, rather
than matching only on error text. Keep the tests focused on the Validate
behavior for these specific flags.
🪄 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: 1c8b21a5-a347-4bc9-9752-06f414b49334
📒 Files selected for processing (6)
shortcuts/base/base_shortcuts_test.goshortcuts/base/shortcuts.goshortcuts/base/workflow_enable_all_disabled.goshortcuts/base/workflow_execute_test.goskills/lark-base/SKILL.mdtests/cli_e2e/base/base_workflow_enable_all_disabled_dryrun_test.go
| Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { | ||
| if strings.TrimSpace(runtime.Str("base-token")) == "" { | ||
| return baseFlagErrorf("--base-token must not be blank") | ||
| } | ||
| _, err := common.ValidatePageSizeTyped(runtime, "page-size", 100, 1, 100) | ||
| return err | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add validation-path tests for the new flags.
These new Validate branches are untested. Please add unit coverage for blank --base-token and invalid --page-size, and assert typed error metadata instead of only message text. As per coding guidelines, "Every behavior change needs a test alongside the change" and "**/*_test.go: Error-path tests must assert typed metadata via errs.ProblemOf (category / subtype / param) and cause preservation, not message substrings alone."
🤖 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/workflow_enable_all_disabled.go` around lines 29 - 35, The new
Validate branches in workflow_enable_all_disabled.go need unit coverage for both
blank base-token and invalid page-size. Add tests around the Validate function
to exercise the baseFlagErrorf path for an empty “base-token” and the
common.ValidatePageSizeTyped path for bad “page-size” values, using the existing
runtime setup helpers. In the assertions, verify typed error metadata with
errs.ProblemOf, including category/subtype/param and cause preservation, rather
than matching only on error text. Keep the tests focused on the Validate
behavior for these specific flags.
Source: Coding guidelines
| func listWorkflowsByStatus(runtime *common.RuntimeContext, baseToken string, status string, pageSize int) ([]map[string]interface{}, error) { | ||
| allItems := []map[string]interface{}{} | ||
| pageToken := "" | ||
| for { | ||
| body := map[string]interface{}{ | ||
| "page_size": pageSize, | ||
| "status": status, | ||
| } | ||
| if pageToken != "" { | ||
| body["page_token"] = pageToken | ||
| } | ||
| data, err := baseV3Call(runtime, "POST", baseV3Path("bases", baseToken, "workflows", "list"), nil, body) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if items, _ := data["items"].([]interface{}); len(items) > 0 { | ||
| for _, item := range items { | ||
| if obj, ok := item.(map[string]interface{}); ok { | ||
| allItems = append(allItems, obj) | ||
| } | ||
| } | ||
| } | ||
| hasMore, _ := data["has_more"].(bool) | ||
| if !hasMore { | ||
| break | ||
| } | ||
| nextToken, _ := data["page_token"].(string) | ||
| if nextToken == "" { | ||
| break | ||
| } | ||
| pageToken = nextToken | ||
| } | ||
| return allItems, nil |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Cover the multi-page paginator branch.
listWorkflowsByStatus is the core new behavior here, but the added coverage in shortcuts/base/workflow_execute_test.go Lines 119-174 and tests/cli_e2e/base/base_workflow_enable_all_disabled_dryrun_test.go Lines 16-41 only exercises a single-page flow. A regression in has_more / page_token handling would ship unnoticed. As per coding guidelines, "Every behavior change needs a test alongside the change."
🤖 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/workflow_enable_all_disabled.go` around lines 100 - 132, Add
test coverage for the multi-page branch in listWorkflowsByStatus so the
has_more/page_token loop is exercised, not just the single-page path. Update the
relevant tests around workflow_execute_test and the dry-run CLI e2e flow to mock
a first response with has_more true plus a page_token, then a second response
that ends pagination, and assert all returned items are accumulated from both
pages. Reference listWorkflowsByStatus and the workflow enable-all-disabled path
when locating the code.
Source: Coding guidelines
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@d197af99b3e02a93267da917b4f043d0fc8c4a42🧩 Skill updatenpx skills add larksuite/cli#auto-research-sync/01KW59XW4VYEGQSJS7A0ZX6X0F/mr-697-b83ed3a5 -y -g |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1634 +/- ##
==========================================
- Coverage 74.68% 74.67% -0.01%
==========================================
Files 808 809 +1
Lines 81484 81567 +83
==========================================
+ Hits 60858 60912 +54
- Misses 16091 16114 +23
- Partials 4535 4541 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 participants
Summary
Add a Base workflow shortcut that enables every currently disabled workflow and reports per-workflow results.
Changes
+workflow-enable-all-disabledin the Base shortcut catalog.Test Plan
git diff --checkRelated Issues
Auto research task: 01KW59XW4VYEGQSJS7A0ZX6X0F
Summary by CodeRabbit
New Features
Bug Fixes
Documentation