feat: add slides replace-pages and xml-get shortcuts - #1585
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:
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 (15)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughThe PR adds ChangesSlides shortcut updates
Sequence Diagram(s)Replace-pages flow sequenceDiagram
participant SlidesReplacePages
participant runtime.CallAPITyped
participant Slides API
SlidesReplacePages->>runtime.CallAPITyped: create replacement slide with before_slide_id
runtime.CallAPITyped->>Slides API: POST replacement content
Slides API-->>runtime.CallAPITyped: slide_id and revision_id
SlidesReplacePages->>runtime.CallAPITyped: delete old slide_id
runtime.CallAPITyped->>Slides API: DELETE old slide
Slides API-->>SlidesReplacePages: per-page result
XML fetch flow sequenceDiagram
participant User
participant SlidesXMLGet
participant Wiki API
participant Slides API
participant FileSystem
User->>SlidesXMLGet: --presentation + --output
alt wiki presentation reference
SlidesXMLGet->>Wiki API: resolve wiki node
Wiki API-->>SlidesXMLGet: slides presentation token
end
SlidesXMLGet->>Slides API: GET xml_presentation with revision_id and remove_attr_id
Slides API-->>SlidesXMLGet: xml_presentation.content
SlidesXMLGet->>FileSystem: write XML to --output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.23.0)shortcuts/slides/shortcuts.go┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.14][ERROR]: unable to find a config; path shortcuts/slides/slides_create.go┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.14][ERROR]: unable to find a config; path shortcuts/slides/slides_create_test.go┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.13][ERROR]: unable to find a config; path
🔧 markdownlint-cli2 (0.22.1)skills/lark-slides/SKILL.mdmarkdownlint-cli2 wrapper config was not available before execution skills/lark-slides/references/lark-slides-edit-workflows.mdmarkdownlint-cli2 wrapper config was not available before execution skills/lark-slides/references/lark-slides-replace-pages.mdmarkdownlint-cli2 wrapper config was not available before execution
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
the
size/L
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@6ddbbafc4f9560eb6bd28caa83be3a8e8858d2ec🧩 Skill updatenpx skills add larksuite/cli#feat/slides_shortcut -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shortcuts/slides/slides_create.go (1)
207-213: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueURL selection logic is correct.
Prefer API-returned
url, fall back to local build.datais guaranteed non-nil by the earlierpresentationIDcheck, socommon.GetString(data, "url")is safe.Optional: the unchanged NB comment at lines 36-38 ("the presentation URL is built locally (see Execute)") is now slightly stale since Execute prefers the API-returned
urland only falls back to a local build. The "no drive scope" justification still holds, but consider tweaking the wording to avoid confusion.🤖 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/slides/slides_create.go` around lines 207 - 213, Update the stale NB comment in the slides creation flow to match the current URL selection behavior. In the slides shortcut’s Execute path, the code now prefers the API-returned url from presentation.create and only falls back to BuildResourceURL when it is missing, so revise the comment to reflect that logic while keeping the existing no-drive-scope justification clear. Use the Execute method and the presentation URL comment near the earlier NB note as the anchor points for the edit.
🤖 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/slides/slides_xml_get_test.go`:
- Around line 150-156: The test in slides_xml_get_test.go is accessing a
non-existent Param field on the result of errs.ProblemOf, which returns
*errs.Problem and cannot satisfy the assertion as written. Update the check to
unwrap the original error with errors.As into *errs.ValidationError, then assert
on ve.Param instead, following the same pattern used in
slides_replace_pages_test.go, and add the errors import if it is not already
present.
---
Nitpick comments:
In `@shortcuts/slides/slides_create.go`:
- Around line 207-213: Update the stale NB comment in the slides creation flow
to match the current URL selection behavior. In the slides shortcut’s Execute
path, the code now prefers the API-returned url from presentation.create and
only falls back to BuildResourceURL when it is missing, so revise the comment to
reflect that logic while keeping the existing no-drive-scope justification
clear. Use the Execute method and the presentation URL comment near the earlier
NB note as the anchor points for the edit.
🪄 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: a2760669-cbca-4234-8f41-85358bacf7da
📒 Files selected for processing (14)
cmd/auth/login_test.goshortcuts/slides/shortcuts.goshortcuts/slides/slides_create.goshortcuts/slides/slides_create_test.goshortcuts/slides/slides_replace_pages.goshortcuts/slides/slides_replace_pages_test.goshortcuts/slides/slides_screenshot.goshortcuts/slides/slides_screenshot_test.goshortcuts/slides/slides_xml_get.goshortcuts/slides/slides_xml_get_test.goskills/lark-slides/SKILL.mdskills/lark-slides/references/lark-slides-edit-workflows.mdskills/lark-slides/references/lark-slides-replace-pages.mdskills/lark-slides/references/lark-slides-screenshot.md
a9ae0d1 to
e1efbb7
Compare
fangshuyu-768
left a comment
fangshuyu-768
left a comment
There was a problem hiding this comment.
I found two issues while reviewing the Slides shortcut changes.
e1efbb7 to
bfdc175
Compare
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/slides/slides_replace_pages_test.go`:
- Around line 296-302: The validation-path tests in the slide page replacement
cases only assert *errs.ValidationError and ve.Param, so they can miss
regressions in typed metadata and wrapped causes. Update the relevant subtests
to also assert errs.ProblemOf(err) for the problem fields, especially Category
and Subtype, while keeping errors.As for ve.Param since errs.ProblemOf does not
expose Param. For the branches that use .WithCause(...), add checks with
errors.Unwrap or equivalent cause assertions to confirm the original cause is
preserved.
- Around line 37-107: The Slides replace-pages test currently only checks the
final response and does not verify the POST→DELETE sequencing or that DELETE
uses the revision returned by the create call. Update the test around
runSlidesShortcut, createStub, and deleteStub to capture/assert request order
and validate the DELETE payload includes the expected slide_id and revision_id
from the POST response. Keep the existing output assertions, but add explicit
checks that the replacement is created before the old slide is deleted and that
the delete request targets the created revision.
🪄 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: fbf72046-03a5-456c-954c-fffb0567b393
📒 Files selected for processing (14)
cmd/auth/login_test.goshortcuts/slides/shortcuts.goshortcuts/slides/slides_create.goshortcuts/slides/slides_create_test.goshortcuts/slides/slides_replace_pages.goshortcuts/slides/slides_replace_pages_test.goshortcuts/slides/slides_screenshot.goshortcuts/slides/slides_screenshot_test.goshortcuts/slides/slides_xml_get.goshortcuts/slides/slides_xml_get_test.goskills/lark-slides/SKILL.mdskills/lark-slides/references/lark-slides-edit-workflows.mdskills/lark-slides/references/lark-slides-replace-pages.mdskills/lark-slides/references/lark-slides-screenshot.md
✅ Files skipped from review due to trivial changes (3)
- skills/lark-slides/references/lark-slides-screenshot.md
- skills/lark-slides/references/lark-slides-edit-workflows.md
- skills/lark-slides/references/lark-slides-replace-pages.md
🚧 Files skipped from review as they are similar to previous changes (10)
- shortcuts/slides/slides_screenshot.go
- shortcuts/slides/slides_xml_get_test.go
- cmd/auth/login_test.go
- shortcuts/slides/slides_screenshot_test.go
- shortcuts/slides/shortcuts.go
- shortcuts/slides/slides_xml_get.go
- shortcuts/slides/slides_create.go
- skills/lark-slides/SKILL.md
- shortcuts/slides/slides_create_test.go
- shortcuts/slides/slides_replace_pages.go
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1585 +/- ##
==========================================
- Coverage 74.74% 74.64% -0.11%
==========================================
Files 799 806 +7
Lines 80380 81386 +1006
==========================================
+ Hits 60084 60752 +668
- Misses 15849 16101 +252
- Partials 4447 4533 +86 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
bfdc175 to
8688d7a
Compare
8688d7a to
6ddbbaf
Compare
2 participants
Summary
Add new Slides shortcuts for replacing pages and fetching presentation XML, and tighten related Slides UX/output behavior.
Changes
Test Plan
Related Issues
Summary by CodeRabbit
+replace-pagesfor multi-page slide rebuilds (supports dry-run, validate-only, and continue-on-error with partial results).+xml-getto download full Slides XML to a local file (supports revision and XML attribute stripping).+replace-pagesand+xml-get, including updated screenshot usage guidance.