feat: add miaoda app collaborator management - #2230
Conversation
|
Supersedes #2191 to align the head branch with the repository naming convention. The commit and diff are unchanged. |
📝 WalkthroughWalkthroughThis change adds six Miaoda app collaborator and permission shortcuts. It adds typed validation, request construction, response projection, error normalization, shortcut registration, documentation, and unit and CLI dry-run coverage. ChangesApp collaborator management
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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@5e973905963bc21e5abd1a369b287501f597c25b🧩 Skill updatenpx skills add larksuite/cli#feat/miaoda-member-permissions -y -g |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
shortcuts/apps/apps_member_common.go (1)
402-423: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the invalid-parameter list from
memberSettingSpecs.Lines 417-420 hardcode the four writable flag names.
memberSettingSpecsalready declares them. If a writable spec is added orwritablechanges, this list drifts and the error omits the new flag. Build the list from the table instead.The early
return errinside the loop is also correct but hard to read.buildMemberSettingsUpdateRequestvalidates every writable spec, so a single post-loop call is clearer.♻️ Proposed refactor
func validateMemberSettingsSet(rctx *common.RuntimeContext) error { if err := validateMemberAppID(rctx); err != nil { return err } + missing := make([]errs.InvalidParam, 0, len(memberSettingSpecs)) for _, spec := range memberSettingSpecs { if !spec.writable { continue } if rctx.Changed(spec.flag) { - _, err := buildMemberSettingsUpdateRequest(rctx) - return err + _, err := buildMemberSettingsUpdateRequest(rctx) + return err } + missing = append(missing, appsInvalidParam("--"+spec.flag, "not provided")) } return appsValidationError("at least one collaborator setting must be provided"). - WithParams( - appsInvalidParam("--external-access", "not provided"), - appsInvalidParam("--link-share", "not provided"), - appsInvalidParam("--manage-collaborators-by", "not provided"), - appsInvalidParam("--comment-by", "not provided"), - ). + WithParams(missing...). WithHint("pass at least one setting flag; omitted settings remain unchanged") }Adjust the
appsInvalidParamreturn type in the slice declaration to match the repository signature.🤖 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/apps/apps_member_common.go` around lines 402 - 423, Update validateMemberSettingsSet to call buildMemberSettingsUpdateRequest once after checking all writable memberSettingSpecs, rather than returning from inside the loop. Derive the appsInvalidParam list in the missing-settings error from memberSettingSpecs, including each writable spec’s flag name, and use the repository’s expected slice type for that list so newly added writable settings remain represented automatically.
🤖 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/apps/apps_member_common_test.go`:
- Around line 84-102: Update requireAppsMemberValidationError in
shortcuts/apps/apps_member_common_test.go lines 84-102 to obtain the problem via
errs.ProblemOf and assert errs.CategoryValidation before retaining the existing
ValidationError.Param checks. Also update the typed-error helper in
shortcuts/apps/apps_member_response_test.go lines 64-76 to use errs.ProblemOf
and require errs.CategoryInternal with errs.SubtypeInvalidResponse.
- Around line 183-185: Update the mocked response and public projection
assertions in the AppsMemberSettingsSet test to include page_token and has_more.
Verify both pagination fields are returned and preserve the page_token value
exactly without transformation.
- Around line 148-162: The test around normalizeMemberAPIError must also verify
the normalized problem retains the API category and preserves the original input
as its cause. Add an assertion that problem.Category equals errs.CategoryAPI and
an errors.Is(got, input) check, while keeping the existing field and hint
validations.
In `@tests/cli_e2e/apps/coverage.md`:
- Line 8: Add an executable, fixture-gated live E2E workflow for collaborator
commands that performs self-contained create, update, settings read/write,
remove, and cleanup operations using bot credentials where required. Update the
coverage documentation to describe this automated coverage while retaining BOE
verification as supplemental evidence; anchor the implementation to the existing
collaborator E2E tests and fixture-gating conventions.
In `@tests/cli_e2e/dryrun/apps_member_dryrun_test.go`:
- Around line 207-212: Update the test around runAppsMemberCLI to assert the
exact error.params values for the “no settings specified” validation failure,
rather than only checking that at least one parameter exists. Preserve the
existing exit-code, stdout, and error.type assertions while ensuring the
reported parameter names match the rejected settings flags exactly.
- Around line 81-153: Add a live collaborator lifecycle E2E test alongside
TestAppsMemberDryRun, using the existing live E2E helpers and bot credentials to
create or select a test app, add and list a collaborator, update its permission,
remove it, and clean up the app. Make the workflow self-contained, and capture
and restore any member settings it modifies before cleanup.
---
Nitpick comments:
In `@shortcuts/apps/apps_member_common.go`:
- Around line 402-423: Update validateMemberSettingsSet to call
buildMemberSettingsUpdateRequest once after checking all writable
memberSettingSpecs, rather than returning from inside the loop. Derive the
appsInvalidParam list in the missing-settings error from memberSettingSpecs,
including each writable spec’s flag name, and use the repository’s expected
slice type for that list so newly added writable settings remain represented
automatically.
🪄 Autofix
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 Plus
Run ID: 98827d42-fada-48ed-b494-6516dff5ff4a
📒 Files selected for processing (10)
errs/subtypes.goshortcuts/apps/apps_member.goshortcuts/apps/apps_member_common.goshortcuts/apps/apps_member_common_test.goshortcuts/apps/apps_member_response_test.goshortcuts/apps/shortcuts.goshortcuts/apps/shortcuts_test.goskills/lark-apps/SKILL.mdtests/cli_e2e/apps/coverage.mdtests/cli_e2e/dryrun/apps_member_dryrun_test.go
Summary
Add AI-friendly collaborator management commands for Miaoda applications. The new shortcuts let agents list, add, update, and remove collaborators, as well as read and update collaborator permission settings, using structured external IDs and typed errors.
Changes
apps +member-list,+member-add,+member-update, and+member-removewith user, chat, and department collaborator support.apps +member-settings-getand+member-settings-setfor the supported sharing and collaborator policy fields.--dry-run.feature_not_availableerror for applications whose collaborators must be managed in the Miaoda console.lark-appsskill guidance and add unit, response-contract, and dry-run E2E coverage.Test Plan
go test ./errs -count=1go test ./shortcuts/apps -count=1go test ./tests/cli_e2e/dryrun -run '^TestAppsMember' -count=1go vet ./errs ./shortcuts/apps ./tests/cli_e2e/dryrunRelated Issues
Summary by CodeRabbit
New Features
Documentation
Tests