feat(apps): add +user-id-convert shortcut for Miaoda↔Feishu ID conversion - #2270
Conversation
…sion Wrap the platform id_convert OpenAPI as a read-only shortcut that maps Miaoda user_id ↔ Feishu open platform IDs (open_id / union_id / Feishu user_id). It does one thing — conversion — with no local mapping table, caching, permission pre-check, or direction guessing. - --convert-type enum → server id_convert_type (10/11/20/21/40) - --ids: csv / @file / stdin, 1-100 per call, not de-duped, input order - reconstructs data.missed by diffing input positions against returned source_ids (server silently drops unresolved IDs), keyed by 0-based index - meta counters (total/hit_count/missed_count) via pointer fields on output.Meta so an explicit missed_count: 0 survives omitempty
|
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:
📝 WalkthroughWalkthroughThe PR adds the ChangesUser ID conversion
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant AppsUserIDConvert
participant PlatformAPI
participant Output
User->>AppsUserIDConvert: Provide conversion type and IDs
AppsUserIDConvert->>PlatformAPI: Submit conversion request
PlatformAPI-->>AppsUserIDConvert: Return resolved IDs
AppsUserIDConvert->>Output: Emit aligned results and metadata
Possibly related PRs
Suggested labels: 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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_user_id_convert_test.go`:
- Around line 211-220: Expand TestResolveConvertType into table-driven cases
covering every entry in idConvertDirections, including mappings 10, 11, 20, 21,
and 40. Assert each conversion type resolves to its expected ID and preserve the
existing empty-input validation assertions.
- Around line 199-205: Update the validation-error assertions in the affected
conversion tests to call errs.ProblemOf and assert CategoryValidation and
SubtypeInvalidArgument. Retain requireConvertValidation for checking
ValidationError.Param, while preserving the existing hint and message
assertions.
In `@shortcuts/apps/apps_user_id_convert.go`:
- Around line 208-213: Replace the untyped idConvertBody payload with a typed
request struct, and define typed response and item structs for
buildConvertResult. Parse the map[string]interface{} response into the typed
response at the API boundary, rejecting malformed shapes with a typed decoding
error before constructing data.missed. Use a single projection function to
convert each typed item into the existing result shape without coercing invalid
values to empty strings or false not_found results.
- Around line 194-200: Update the CSV parsing loop around strings.Split in the
relevant conversion function to reject any empty trimmed element instead of
continuing past it. Return the existing typed validation error with the element
context, while preserving valid IDs and their original order; add coverage for
an interior empty value such as “id-a,,id-b”.
- Line 101: Add direct coverage for parseConvertIDs via the convert command's
`@file` and stdin inputs, using newline-delimited IDs without commas. Assert that
data.ids contains each expected ID separately, ensuring the input content is
split into individual request IDs rather than treated as one block.
🪄 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: a10a2807-e81b-4dad-8762-69b825922f8d
📒 Files selected for processing (7)
internal/output/envelope.goshortcuts/apps/apps_user_id_convert.goshortcuts/apps/apps_user_id_convert_test.goshortcuts/apps/shortcuts.goshortcuts/apps/shortcuts_test.goskills/lark-apps/SKILL.mdskills/lark-apps/references/lark-apps-user-id-convert.md
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@aa784c3cd57bb5d8b8d4179d839a7d3302db08a4🧩 Skill updatenpx skills add larksuite/cli#feat/apps-user-id-convert -y -g |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2270 +/- ##
========================================
Coverage 76.51% 76.52%
========================================
Files 1019 1020 +1
Lines 112614 112736 +122
========================================
+ Hits 86172 86273 +101
- Misses 19873 19885 +12
- Partials 6569 6578 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- reject empty --ids CSV entries (e.g. "a,,b") with a typed validation error instead of silently dropping them, since a dropped entry shifts every later result's 0-based index and breaks the position-keyed items/missed contract; add an interior-empty-element test - reuse common.GetSlice / common.GetString for response projection (house convention) instead of local asSlice/asString helpers - requireConvertValidation now asserts CategoryValidation + SubtypeInvalidArgument via errs.ProblemOf, keeping ValidationError.Param - table-drive TestResolveConvertType over all five directions so every --convert-type → id_convert_type mapping (10/11/20/21/40) is protected
…tdin @file and - (stdin) input arrives verbatim from the framework as one-ID-per-line text, but parseConvertIDs only split on commas, so such a block was sent as a single malformed request ID. Treat a newline as equivalent to a comma, tolerating a file's trailing newline while still rejecting interior empty entries so position-keyed result indices stay aligned. Add @file and stdin tests asserting the request body's ids are split into discrete IDs.
Responses decode with json.Number (client.ParseJSONResponse uses dec.UseNumber()), so a server that emits source_id/target_id as bare numbers — plausible for the numeric Miaoda user_id form — was silently coerced to "" by buildConvertResult's strict string assertion: the source_id got dropped (false not_found) and the target_id blanked (false success). Add common.GetStringLoose, which stringifies string/json.Number/int64/ float64 via literal text (large integer IDs keep full precision, never routed through a lossy float64), and use it for both id reads. Cover it with a package-level table test plus an end-to-end regression asserting a numeric-JSON response yields intact, non-blank ids and no false miss. Also exercise resolveConvertType's non-empty "not a valid direction" branch directly, since the runner's enum gate preempts it in normal flow.
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/common/extract_test.go`:
- Around line 46-77: Add regression coverage in TestGetStringLoose for the int
conversion branch and replace the float64(42) case with a value that would
expose exponent notation under an alternate formatter, while expecting
fixed-point output. Keep the existing table structure and other numeric cases
unchanged.
🪄 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: c6731ddd-44e3-4a05-9966-f45639ea4b8a
📒 Files selected for processing (4)
shortcuts/apps/apps_user_id_convert.goshortcuts/apps/apps_user_id_convert_test.goshortcuts/common/extract.goshortcuts/common/extract_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- shortcuts/apps/apps_user_id_convert.go
- shortcuts/apps/apps_user_id_convert_test.go
Add an int-branch case (was only covering int64) and swap the float64 fixture from 42 — which no formatter would render in exponent form — to 1e-7, whose fixed-point rendering "0.0000001" fails under the 'g' verb. This turns the "no scientific notation" case into a real guard for the 'f' verb choice, per CodeRabbit review on c64cca3.
…vert # Conflicts: # shortcuts/apps/shortcuts_test.go
Summary
Adds a read-only
apps +user-id-convertshortcut that wraps the existing platform OpenAPIPOST /open-apis/spark/v1/directory/user/id_convert, mapping Miaodauser_id↔ Feishu open-platform IDs (open_id/union_id/ Feishuuser_id). This unblocks sandbox Code Agents that resolve a person to anopen_idviacontact/imbut need a Miaoda/Feishuuser_idfor downstream calls (e.g.feishu-approvalcreateApprovalInstance). lark-cli single-repo change, no internal-repo edits.Changes
shortcuts/apps/apps_user_id_convert.go— one job, conversion only: no local mapping table, no caching, no permission pre-check, no direction guessing.--convert-typeenum → serverid_convert_type:miaoda-to-open-id=10,miaoda-to-union-id=11,open-id-to-miaoda=20,union-id-to-miaoda=21,miaoda-to-feishu-user-id=40. Missing/invalid → typed validation error listing allowed directions.--ids— csv /@file/ stdin; 1–100 per call (CLI tightens the OpenAPI cap to reject no-op empty batches); not de-duplicated, returned in input order.--dry-runprints the assembled request body without calling;--as user|bot.data.missedby diffing input positions against returnedsource_ids (reason: not_found), keeping the 0-based inputindexso callers can back-fill by position even with duplicate IDs.internal/output/envelope.go— addsTotal/HitCount/MissedCountas*intonMeta. Pointers so an unset counter is dropped byomitemptywhile an explicitmissed_count: 0(full hit) is still emitted; non-batch commands leave themnil.shortcuts/apps/shortcuts.go, updated the count guard + intent index inshortcuts_test.go/skills/lark-apps/SKILL.md.skills/lark-apps/references/lark-apps-user-id-convert.md.Required scope:
spark:directory.user.id_convert:read. Rate limit 50 req/s (CLI does not auto-retry).Test Plan
go build ./...,go vet ./shortcuts/apps/... ./internal/output/...,gofmt— all cleango test ./shortcuts/apps/... ./internal/output/...— pass (incl. skill-consistency guard)log_id, no retry), and registration--dry-runverified:miaoda-to-feishu-user-id→id_convert_type: 40, endpoint + ids order correctspark:directory.user.id_convert:read). Please confirmid_convert_type: 40is the active value for the Feishu-user-id direction on the live OpenAPI during review.Note: the pre-existing
cmdtestTestFrameworkOwnedRootHelpTargetsExistInDefaultTree(mail/user_mailbox…) fails on cleanorigin/maintoo — unrelated to this change.Related Issues
Summary by CodeRabbit
New Features
Bug Fixes