feat: support extra auth param for docs media download#628
Open
feat: support extra auth param for docs media download#628
Conversation
Change-Id: I789664715815ef5a6ae883e024d4b4db558b0afa
📝 WalkthroughWalkthroughAdds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 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/ccm
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/doc/doc_media_test.go (1)
652-679: Tests adequately cover the new--extrapropagation.Dry-run test asserts the param is present on the non-whiteboard path; execute test asserts the fully encoded query string hits the stubbed URL. Good coverage for both flows.
Optional: consider adding a negative case asserting
--extrais NOT forwarded when--type whiteboard(the other branch of the new conditional atdoc_media_download.go:81). This would lock in the scoping guarantee promised by the docs.Also applies to: 848-883
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/doc/doc_media_test.go` around lines 652 - 679, Add a negative dry-run test that verifies the --extra flag is not forwarded when the media type is whiteboard: create a test (e.g., TestDocMediaDownloadDryRunDoesNotIncludeExtraForWhiteboard) that builds a cobra command like the existing TestDocMediaDownloadDryRunIncludesExtraQueryParam, sets flags "token", "output", "type" to "whiteboard", and "extra" to the same JSON string, then call decodeDocDryRun on DocMediaDownload.DryRun(context.Background(), common.TestNewRuntimeContext(cmd, nil)) and assert that dry.API has the expected call but that dry.API[0].Params does not contain an "extra" entry (or that it's nil/empty); use the same helpers (decodeDocDryRun, DocMediaDownload.DryRun, common.TestNewRuntimeContext) and name the test to reflect the whiteboard negative case so it covers the alternate branch introduced around doc_media_download.go:81.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@shortcuts/doc/doc_media_test.go`:
- Around line 652-679: Add a negative dry-run test that verifies the --extra
flag is not forwarded when the media type is whiteboard: create a test (e.g.,
TestDocMediaDownloadDryRunDoesNotIncludeExtraForWhiteboard) that builds a cobra
command like the existing TestDocMediaDownloadDryRunIncludesExtraQueryParam,
sets flags "token", "output", "type" to "whiteboard", and "extra" to the same
JSON string, then call decodeDocDryRun on
DocMediaDownload.DryRun(context.Background(), common.TestNewRuntimeContext(cmd,
nil)) and assert that dry.API has the expected call but that dry.API[0].Params
does not contain an "extra" entry (or that it's nil/empty); use the same helpers
(decodeDocDryRun, DocMediaDownload.DryRun, common.TestNewRuntimeContext) and
name the test to reflect the whiteboard negative case so it covers the alternate
branch introduced around doc_media_download.go:81.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 94daf594-70f1-44a0-9da8-7ca697833fb6
📒 Files selected for processing (4)
shortcuts/doc/doc_media_download.goshortcuts/doc/doc_media_test.goskills/lark-doc/SKILL.mdskills/lark-doc/references/lark-doc-media-download.md
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #628 +/- ##
==========================================
+ Coverage 60.20% 62.09% +1.88%
==========================================
Files 407 407
Lines 43340 35806 -7534
==========================================
- Hits 26091 22232 -3859
+ Misses 15221 11545 -3676
- Partials 2028 2029 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@2ef2b1983e639df7518a7de3dfd21b97356fd5f9🧩 Skill updatenpx skills add larksuite/cli#feat/docs-media-download-extra -y -g |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds
--extrasupport todocs +media-downloadso advanced-permission Base attachments can be downloaded through the Drive media API. It also updates the related skill docs to explain theextrapayload structure, usage rules, and masked examples.Changes
--extratodocs +media-downloadand pass it through as the media download query parameter for--type mediaextrahandling and query encodingdocs +media-download --extralark-docandlark-baseskill docs to document--extra, explain each nested field, and replace example IDs/tokens with masked placeholdersTest Plan
lark xxxcommand works as expectedAdditional verification:
go test ./shortcuts/doc -run 'TestDocMediaDownload|TestDocMediaPreviewDryRunUsesMediaEndpoint'go test ./tests/cli_e2e/dryrun -run TestDocs_MediaDownloadDryRunIncludesExtraQueryParammake buildRelated Issues
Summary by CodeRabbit
New Features
--extraflag to document media download command for handling multi-dimensional table attachments with advanced permissions.Tests
Documentation