feat(drive): add permission get setting shortcut#1775
feat(drive): add permission get setting shortcut#1775zhaojiaxing-coding wants to merge 4 commits into
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:
📝 WalkthroughWalkthroughThis PR adds the ChangesDrive Permission Get Setting
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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@669aac0a53b65a324a6b353b507e9bbbf52bc4d4🧩 Skill updatenpx skills add larksuite/cli#feat/drive-permission-get-setting -y -g |
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 `@shortcuts/drive/drive_permission_get_setting.go`:
- Around line 82-84: The validation wrapping for drivePermissionGetSettingSpec
currently drops the original error from validate.ResourceName, so update both
error-return branches in the drivePermissionGetSetting logic to attach the
underlying err with WithCause(err) alongside the existing WithParam("--token").
Use the existing validate.ResourceName and errs.NewValidationError call sites so
callers can inspect the wrapped cause via errors.Is and errors.Unwrap.
🪄 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: a1cd3cb7-adb2-47f3-9fa3-b2e3475e6eb1
📒 Files selected for processing (9)
shortcuts/drive/drive_permission_get_setting.goshortcuts/drive/drive_permission_get_setting_test.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-workflow-permission-governance-commands.mdskills/lark-drive/references/lark-drive-workflow-permission-governance-outputs.mdskills/lark-drive/references/lark-drive-workflow-permission-governance.mdtests/cli_e2e/drive/drive_permission_get_setting_test.go
| if err := validate.ResourceName(ref.Token, "--token"); err != nil { | ||
| return drivePermissionGetSettingSpec{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--token") | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect validate.ResourceName's implementation and return type.
ast-grep outline internal/validate --items all --match 'ResourceName' --view expanded
rg -nP -A8 'func ResourceName\(' internal/validateRepository: larksuite/cli
Length of output: 976
Preserve the underlying validation error when wrapping validate.ResourceName failures. Add .WithCause(err) in both branches so callers can inspect the original error with errors.Is / errors.Unwrap.
🤖 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/drive/drive_permission_get_setting.go` around lines 82 - 84, The
validation wrapping for drivePermissionGetSettingSpec currently drops the
original error from validate.ResourceName, so update both error-return branches
in the drivePermissionGetSetting logic to attach the underlying err with
WithCause(err) alongside the existing WithParam("--token"). Use the existing
validate.ResourceName and errs.NewValidationError call sites so callers can
inspect the wrapped cause via errors.Is and errors.Unwrap.
Source: Coding guidelines
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 `@skills/lark-drive/references/lark-drive-permission-get-setting.md`:
- Line 75: The markdown table row in the permission setting reference is being
split into extra columns because the type examples use unescaped pipes. Update
the row in the document content so the `docx|folder|file|...` text is escaped or
rewritten as a list, keeping the table structure intact and ensuring the error
message renders correctly.
🪄 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: 39c779c0-4948-409c-9c68-3434f8652e5f
📒 Files selected for processing (1)
skills/lark-drive/references/lark-drive-permission-get-setting.md
| | 症状 | 原因 | 处理 | | ||
| |------|------|------| | ||
| | `--token is required` | 没传目标 | 传目标 URL 或裸 token。 | | ||
| | `--type is required when --token is a bare token` | 裸 token 无法自动推断类型 | 补充 `--type docx|folder|file|...`。 | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Escape the pipes in the error table row.
docx|folder|file|... is being parsed as extra markdown columns, so this row will render incorrectly and drop part of the message. Use escaped pipes or rewrite the value as a list.
Suggested fix
-| `--type ...` | 裸 token 无法自动推断类型 | 补充 `--type docx|folder|file|...`。 |
+| `--type ...` | 裸 token 无法自动推断类型 | 补充 `--type docx&`#124`;folder&`#124`;file&`#124`;...`。 |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | `--type is required when --token is a bare token` | 裸 token 无法自动推断类型 | 补充 `--type docx|folder|file|...`。 | | |
| | `--type is required when --token is a bare token` | 裸 token 无法自动推断类型 | 补充 `--type docx&`#124`;folder&`#124`;file&`#124`;...`。 | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 75-75: Table column count
Expected: 3; Actual: 6; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 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 `@skills/lark-drive/references/lark-drive-permission-get-setting.md` at line
75, The markdown table row in the permission setting reference is being split
into extra columns because the type examples use unescaped pipes. Update the row
in the document content so the `docx|folder|file|...` text is escaped or
rewritten as a list, keeping the table structure intact and ensuring the error
message renders correctly.
Source: Linters/SAST tools
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1775 +/- ##
========================================
Coverage 74.68% 74.69%
========================================
Files 878 879 +1
Lines 91786 92007 +221
========================================
+ Hits 68554 68722 +168
- Misses 17922 17957 +35
- Partials 5310 5328 +18 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
9aaefa5 to
c32fe2d
Compare
Add a read-only Drive shortcut for retrieving a resource's public access, sharing, collaborator management, security, and comment settings. It accepts supported Lark Drive URLs or explicitly typed bare tokens, returning actionable typed validation errors for invalid or conflicting inputs. Key features: - Query the Drive permission public endpoint for documents, files, wiki nodes, and folders - Infer target type from supported URLs and require --type for bare tokens - Register dry-run and validation coverage, including CLI end-to-end tests - Document the command and its non-recursive folder behavior in the Drive skill
c32fe2d to
c484d25
Compare
Summary
Add the
drive +permission-get-settingshortcut to read Drive public permission settings for supported file types.Changes
--type.Related Issues
Summary by CodeRabbit
+permission-get-settingto fetch a node’s ownpermission_publicsettings (token or supported Drive/document URLs; requires--typefor bare tokens; non-recursive/readonly) with streamlined JSON output.DryRunto preview the generatedGET /open-apis/drive/v2/permissions/{token}/publicrequest details.--token/--typeparsing and validation, including clearer typed validation failures and safer handling of mismatched/unsupported URL forms.+permission-get-setting, including folder-specific guidance and wording consistency.