fix: add atomic overwrite for whiteboard +update#483
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe update replaces explicit node-read/delete steps with overwrite flags in create requests, restructures Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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)
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@ea4fe46171f258bec6960a5be8389ba65e77cfeb🧩 Skill updatenpx skills add larksuite/cli#feat/whiteboard-dev -y -g |
There was a problem hiding this comment.
Actionable comments posted: 1
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_update.go (1)
193-205:⚠️ Potential issue | 🟠 MajorFail fast when
result.nodesis missing.With the new typed
Result.Nodesshape, a payload like{"code":0,"data":{"to":"openapi"}}now unmarshals successfully and returnsnilhere. Thatnilslice is then forwarded as the live request body inrawNodesCreateReq, so malformed openapi JSON is no longer rejected before the update call. Please validate presence ofresult.nodesexplicitly; you’ll likely need a pointer orjson.RawMessageinWbCliOutputDatato distinguish “missing” from[].Also applies to: 267-275
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/whiteboard/whiteboard_update.go` around lines 193 - 205, parseWBcliNodes currently treats a missing result.nodes the same as an empty slice because WbCliOutputData.Result.Nodes is a concrete slice; change WbCliOutput/WbCliOutputData so Result.Nodes is a pointer ([]Node) or json.RawMessage to distinguish "missing" vs present-but-empty, then in parseWBcliNodes check explicitly that wbOutput.Data.Result.Nodes != nil (or that RawMessage is non-empty) before assigning wbNodes; if missing, return an output.Errorf with ExitValidation and the "whiteboard-cli" context (same pattern used above) to fail fast instead of forwarding a nil slice to rawNodesCreateReq.
🧹 Nitpick comments (1)
shortcuts/whiteboard/whiteboard_update_test.go (1)
480-502: Assertoverwrite: truein these requests.These tests only prove the POST endpoints are hit. If
Overwritestops being serialized inplantumlCreateReqorrawNodesCreateReq, both cases still pass, so the PR’s main behavior change is still untested. Please inspect or match the outbound body here.Also applies to: 504-525
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/whiteboard/whiteboard_update_test.go` around lines 480 - 502, The test must assert that the outbound request includes overwrite: true so the serialization of Overwrite in plantumlCreateReq and rawNodesCreateReq is validated; modify the httpmock.Stub registration in TestWhiteboardUpdateExecute_WithOverwrite (and the other test at 504-525) to inspect the incoming request body (or use a stub BodyMatcher) and assert that the JSON contains "overwrite": true (or return an error when it's absent), referencing runUpdateShortcut, WhiteboardUpdate, plantumlCreateReq and rawNodesCreateReq to locate the requests to validate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/whiteboard/whiteboard_update.go`:
- Line 33: The wbUpdateScopes slice currently includes an unnecessary
permission; remove the "board:whiteboard:node:delete" entry from the
wbUpdateScopes variable so it contains only "board:whiteboard:node:create" to
follow least-privilege requirements; update any related tests or usages that
expect the deleted scope and run the build to ensure no references to
wbUpdateScopes still assume the delete scope.
---
Outside diff comments:
In `@shortcuts/whiteboard/whiteboard_update.go`:
- Around line 193-205: parseWBcliNodes currently treats a missing result.nodes
the same as an empty slice because WbCliOutputData.Result.Nodes is a concrete
slice; change WbCliOutput/WbCliOutputData so Result.Nodes is a pointer ([]Node)
or json.RawMessage to distinguish "missing" vs present-but-empty, then in
parseWBcliNodes check explicitly that wbOutput.Data.Result.Nodes != nil (or that
RawMessage is non-empty) before assigning wbNodes; if missing, return an
output.Errorf with ExitValidation and the "whiteboard-cli" context (same pattern
used above) to fail fast instead of forwarding a nil slice to rawNodesCreateReq.
---
Nitpick comments:
In `@shortcuts/whiteboard/whiteboard_update_test.go`:
- Around line 480-502: The test must assert that the outbound request includes
overwrite: true so the serialization of Overwrite in plantumlCreateReq and
rawNodesCreateReq is validated; modify the httpmock.Stub registration in
TestWhiteboardUpdateExecute_WithOverwrite (and the other test at 504-525) to
inspect the incoming request body (or use a stub BodyMatcher) and assert that
the JSON contains "overwrite": true (or return an error when it's absent),
referencing runUpdateShortcut, WhiteboardUpdate, plantumlCreateReq and
rawNodesCreateReq to locate the requests to validate.
🪄 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: be7b4bee-686f-41ad-be10-40ee21b58f37
📒 Files selected for processing (3)
shortcuts/whiteboard/shortcuts.goshortcuts/whiteboard/whiteboard_update.goshortcuts/whiteboard/whiteboard_update_test.go
Change-Id: I995e9575e333d4c2925eac75a52e4442dc570c5c
62cbff5 to
ea4fe46
Compare
Summary
This PR updates the whiteboard update functionality to use the new backend API's
overwriteparameter for atomic overwrite operations, replacing the previous approach of first reading and then deleting nodes.Changes
overwritefield to API request bodies for both raw nodes and PlantUML/Mermaid updatesTest Plan
lark whiteboard +updatecommand works as expected with--overwriteflagSummary by CodeRabbit