feat(events): subscribe board.whiteboard.updated_v1 #1265
Conversation
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds support for consuming board.whiteboard.updated_v1: defines payload structs, implements per-whiteboard subscribe/unsubscribe pre-consume lifecycle with tests, registers the event key and integrates it into event registration, and documents the feature in SKILL.md and a reference guide. ChangesWhiteboard Event Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@74aaad87aa9816ccf5373483751b11d57ebfdeaf🧩 Skill updatenpx skills add larksuite/cli#feat/board-whiteboard-updated-preconsume -y -g |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1265 +/- ##
==========================================
+ Coverage 70.18% 70.35% +0.17%
==========================================
Files 671 674 +3
Lines 65240 65368 +128
==========================================
+ Hits 45786 45987 +201
+ Misses 15783 15728 -55
+ Partials 3671 3653 -18 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@events/board/preconsume_test.go`:
- Around line 39-162: Add environment isolation by calling
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) at the start of each
whiteboard preconsume test (e.g.
TestWhiteboardSubscriptionPreConsume_MissingWhiteboardID,
TestWhiteboardSubscriptionPreConsume_NilRuntime,
TestWhiteboardSubscriptionPreConsume_SubscribeError,
TestWhiteboardSubscriptionPreConsume_SubscribeAndCleanup,
TestWhiteboardSubscriptionPreConsume_PathSegmentEncoded, and
TestWhiteboardUpdatedV1HasPreConsume) or in a small shared helper invoked from
each test; place the call as the first statement in each test so each run uses
its own config dir and avoids config-state bleed.
🪄 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: 99037ed2-2e76-4d09-ae77-d7845ad564db
📒 Files selected for processing (8)
events/board/native.goevents/board/preconsume.goevents/board/preconsume_test.goevents/board/register.goevents/register.goskills/lark-board/SKILL.mdskills/lark-event/SKILL.mdskills/lark-event/references/lark-event-board.md
afebae8 to
4db118a
Compare
4db118a to
28b0ac9
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/whiteboard/whiteboard_query_test.go (1)
157-201:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDry-run endpoint expectations are declared but never asserted.
At Lines 171/180/189,
wantMethodandwantPathare set, but the test only checksdryRun != nilat Lines 197-199. This won’t catch endpoint-regression in dry-run output.🤖 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/whiteboard/whiteboard_query_test.go` around lines 157 - 201, The test builds table-driven cases with wantMethod/wantPath but never asserts them; update the subtest body that calls WhiteboardQuery.DryRun(ctx, rt) to first assert dryRun is non-nil and then compare dryRun.Method and dryRun.Path (or the actual fields/nested values returned by WhiteboardQuery.DryRun) against tt.wantMethod and tt.wantPath, failing the test with t.Fatalf or t.Errorf when they differ; use the existing tt, rt, dryRun and newTestRuntime symbols to locate and implement these assertions so endpoint-regression is detected.
🤖 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 `@events/whiteboard/register.go`:
- Around line 14-24: The constant eventTypeWhiteboardUpdated is set to
"whiteboard.whiteboard.updated_v1" which mismatches the expected namespace and
breaks subscriptions; update the value of eventTypeWhiteboardUpdated to
"board.whiteboard.updated_v1" and ensure all uses inside Keys() (the Key and
EventType fields of the event.KeyDefinition, and any RequiredConsoleEvents or
subscribe/event_type references) rely on that corrected constant so the runtime
will subscribe and consume the intended event.
In `@shortcuts/whiteboard/whiteboard_query.go`:
- Around line 172-176: The ApiPath in fetchWhiteboardNodes interpolates wbToken
without path-segment encoding; update the ApiPath construction in
fetchWhiteboardNodes to path-escape wbToken the same way used elsewhere (e.g.,
use url.PathEscape on wbToken) before formatting into fmt.Sprintf so reserved
characters like '/' or '?' cannot alter the request path.
---
Outside diff comments:
In `@shortcuts/whiteboard/whiteboard_query_test.go`:
- Around line 157-201: The test builds table-driven cases with
wantMethod/wantPath but never asserts them; update the subtest body that calls
WhiteboardQuery.DryRun(ctx, rt) to first assert dryRun is non-nil and then
compare dryRun.Method and dryRun.Path (or the actual fields/nested values
returned by WhiteboardQuery.DryRun) against tt.wantMethod and tt.wantPath,
failing the test with t.Fatalf or t.Errorf when they differ; use the existing
tt, rt, dryRun and newTestRuntime symbols to locate and implement these
assertions so endpoint-regression is detected.
🪄 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: afe6ce33-ded8-427f-9d70-2666b944c9de
📒 Files selected for processing (13)
cmd/auth/login_test.goevents/register.goevents/whiteboard/native.goevents/whiteboard/preconsume.goevents/whiteboard/preconsume_test.goevents/whiteboard/register.goshortcuts/doc/doc_media_download.goshortcuts/whiteboard/whiteboard_query.goshortcuts/whiteboard/whiteboard_query_test.goshortcuts/whiteboard/whiteboard_update.goshortcuts/whiteboard/whiteboard_update_test.goskills/lark-event/SKILL.mdskills/lark-event/references/lark-event-whiteboard.md
✅ Files skipped from review due to trivial changes (2)
- skills/lark-event/references/lark-event-whiteboard.md
- skills/lark-event/SKILL.md
28b0ac9 to
e01f470
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/doc/doc_media_download.go (1)
38-38: ⚡ Quick winAdd targeted tests for the migrated whiteboard endpoint branches.
These new whiteboard-path branches are currently uncovered; please add/adjust tests that assert both DryRun and Execute use
/open-apis/whiteboard/v1/whiteboards/{token}/download_as_imagefor--type=whiteboard.Also applies to: 66-66
🤖 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/doc/doc_media_download.go` at line 38, Tests need to be added/updated to cover the new whiteboard endpoint branches so both DryRun and Execute use the migrated path string GET("/open-apis/whiteboard/v1/whiteboards/:token/download_as_image") when invoked with --type=whiteboard; update or add unit tests that call the command handler (invoke the DryRun and Execute methods) with the --type=whiteboard flag and assert the outgoing request path equals /open-apis/whiteboard/v1/whiteboards/{token}/download_as_image (verify both branch behaviors), and include assertions for both DryRun and Execute code paths to ensure the new branch is exercised.
🤖 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.
Nitpick comments:
In `@shortcuts/doc/doc_media_download.go`:
- Line 38: Tests need to be added/updated to cover the new whiteboard endpoint
branches so both DryRun and Execute use the migrated path string
GET("/open-apis/whiteboard/v1/whiteboards/:token/download_as_image") when
invoked with --type=whiteboard; update or add unit tests that call the command
handler (invoke the DryRun and Execute methods) with the --type=whiteboard flag
and assert the outgoing request path equals
/open-apis/whiteboard/v1/whiteboards/{token}/download_as_image (verify both
branch behaviors), and include assertions for both DryRun and Execute code paths
to ensure the new branch is exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 72213397-3725-49d7-b8d3-a965a5540a65
📒 Files selected for processing (13)
cmd/auth/login_test.goevents/register.goevents/whiteboard/native.goevents/whiteboard/preconsume.goevents/whiteboard/preconsume_test.goevents/whiteboard/register.goshortcuts/doc/doc_media_download.goshortcuts/whiteboard/whiteboard_query.goshortcuts/whiteboard/whiteboard_query_test.goshortcuts/whiteboard/whiteboard_update.goshortcuts/whiteboard/whiteboard_update_test.goskills/lark-event/SKILL.mdskills/lark-event/references/lark-event-whiteboard.md
✅ Files skipped from review due to trivial changes (3)
- skills/lark-event/SKILL.md
- shortcuts/whiteboard/whiteboard_update_test.go
- skills/lark-event/references/lark-event-whiteboard.md
🚧 Files skipped from review as they are similar to previous changes (9)
- events/whiteboard/native.go
- events/register.go
- events/whiteboard/register.go
- events/whiteboard/preconsume.go
- shortcuts/whiteboard/whiteboard_query_test.go
- events/whiteboard/preconsume_test.go
- shortcuts/whiteboard/whiteboard_update.go
- shortcuts/whiteboard/whiteboard_query.go
- cmd/auth/login_test.go
4aa2dd5 to
3c951fc
Compare
3c951fc to
f03fa17
Compare
Wire the board.whiteboard.updated_v1 EventKey into the consume pipeline so that lark-cli event consume automatically calls the per-whiteboard subscribe / unsubscribe OAPIs instead of requiring callers to manage server-side subscriptions out-of-band. Change-Id: I94323807e8dc649d3296f6922311d2acaf92284e
f03fa17 to
74aaad8
Compare
…larksuite#1265) Wire the board.whiteboard.updated_v1 EventKey into the consume pipeline so that lark-cli event consume automatically calls the per-whiteboard subscribe / unsubscribe OAPIs instead of requiring callers to manage server-side subscriptions out-of-band. Change-Id: I94323807e8dc649d3296f6922311d2acaf92284e
…#1265) Wire the board.whiteboard.updated_v1 EventKey into the consume pipeline so that lark-cli event consume automatically calls the per-whiteboard subscribe / unsubscribe OAPIs instead of requiring callers to manage server-side subscriptions out-of-band. Change-Id: I94323807e8dc649d3296f6922311d2acaf92284e
Summary
Wire the board.whiteboard.updated_v1 EventKey into the consume pipeline so that
lark-cli event consumeautomatically calls the per-whiteboard subscribe / unsubscribe OAPIs instead of requiring callers to manage server-side subscriptions out-of-band.Changes
Test Plan
Related Issues
Summary by CodeRabbit
New Features
Documentation