feat(vc): add +recording shortcut for meeting_id to minute_token conversion#246
feat(vc): add +recording shortcut for meeting_id to minute_token conversion#246
Conversation
github-actions
bot
added
domain/vc
|
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:
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as Client/CLI
participant Validator as Validator
participant Calendar as Calendar API
participant VCApi as VC Recording API
CLI->>Validator: vc +recording (--meeting-ids | --calendar-event-ids)
Validator->>Validator: validate flags (mutual exclusivity, ≤50) and scopes
alt using --meeting-ids
loop per meeting_id
Validator->>VCApi: GET /vc/meeting/recording?meeting_id={id}
VCApi-->>Validator: recording metadata + URL / error
Validator->>Validator: extract minute_token from /minutes/{token}
end
else using --calendar-event-ids
loop per calendar_event_id
Validator->>Calendar: resolve event -> meeting_instance_ids
Calendar-->>Validator: list of meeting_ids (or error)
loop per resolved meeting_id
Validator->>VCApi: GET recording by meeting_id
VCApi-->>Validator: recording metadata + URL / error
Validator->>Validator: extract minute_token if present
end
end
end
Validator-->>CLI: aggregated results (per-item status, minute_token/duration or error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Ren1104
added
size/M
github-actions
bot
added
size/L
Greptile SummaryThis PR adds a Confidence Score: 5/5Safe to merge — all prior P0/P1 findings have been resolved and no new defects were found All previously identified issues (domain flags, empty table column, Chinese comment, nil-IDs edge case) were addressed before this review round. The only remaining finding is two Chinese inline comments in the new test file — a P2 style nit that does not affect correctness or reliability. No files require special attention; vc_recording_test.go has two minor Chinese comments Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CLI as vc +recording
participant RecAPI as vc/v1/meetings/{id}/recording
participant CalAPI as calendar/v4/calendars
participant Minutes as minutes URL
alt --meeting-ids path
User->>CLI: --meeting-ids m1,m2,...
loop each meeting_id
CLI->>RecAPI: GET /recording
RecAPI-->>CLI: recording.url
CLI->>Minutes: extractMinuteToken(url)
Minutes-->>CLI: minute_token
end
else --calendar-event-ids path
User->>CLI: --calendar-event-ids e1,e2,...
CLI->>CalAPI: POST /primary
CalAPI-->>CLI: calendar_id
loop each calendar_event_id
CLI->>CalAPI: POST mget_instance_relation_info
CalAPI-->>CLI: meeting_instance_ids[]
loop each meeting_id (fallback)
CLI->>RecAPI: GET /recording
RecAPI-->>CLI: recording.url
CLI->>Minutes: extractMinuteToken(url)
Minutes-->>CLI: minute_token
note over CLI: break on first success
end
end
end
CLI-->>User: table(meeting_id, minute_token, duration, status)
Reviews (6): Last reviewed commit: "fix(vc): address review findings for +re..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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/vc/vc_recording.go (1)
218-241: Consider includingcalendar_event_idin table output for the calendar-event-ids path.When using
--calendar-event-ids, failed results (Line 182, 198) includecalendar_event_idbut the table rendering (Line 223-224) only extractsmeeting_id, which will be empty for resolution failures. This could make it harder to identify which calendar event failed.♻️ Optional enhancement to show calendar_event_id in table
for _, r := range results { m, _ := r.(map[string]any) meetingID, _ := m["meeting_id"].(string) - row := map[string]interface{}{"meeting_id": meetingID} + row := map[string]interface{}{} + if meetingID != "" { + row["meeting_id"] = meetingID + } + if calEventID, _ := m["calendar_event_id"].(string); calEventID != "" { + row["calendar_event_id"] = calEventID + } if errMsg, _ := m["error"].(string); errMsg != "" {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/vc/vc_recording.go` around lines 218 - 241, The table output omits calendar_event_id so failures from the calendar-event-ids path are hard to trace; update the rows construction inside the runtime.OutFormat lambda (where results are iterated, and rows are built for output.PrintTable) to extract "calendar_event_id" from each result (like you already extract "meeting_id") and include it in the row map for both error and success branches so the printed table shows calendar_event_id when present; ensure you reference the same variables (results, row, successCount) so counts remain correct and output.PrintTable prints the new field.
🤖 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/vc/vc_recording.go`:
- Around line 218-241: The table output omits calendar_event_id so failures from
the calendar-event-ids path are hard to trace; update the rows construction
inside the runtime.OutFormat lambda (where results are iterated, and rows are
built for output.PrintTable) to extract "calendar_event_id" from each result
(like you already extract "meeting_id") and include it in the row map for both
error and success branches so the printed table shows calendar_event_id when
present; ensure you reference the same variables (results, row, successCount) so
counts remain correct and output.PrintTable prints the new field.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ffb61914-5f3c-4f2b-a112-bc596a968a06
📒 Files selected for processing (6)
shortcuts/vc/shortcuts.goshortcuts/vc/vc_notes.goshortcuts/vc/vc_recording.goshortcuts/vc/vc_recording_test.goskills/lark-vc/SKILL.mdskills/lark-vc/references/lark-vc-recording.md
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@43f8f8ed54ec13dbfcd81c06176c8e6bc881b71a🧩 Skill updatenpx skills add larksuite/cli#feat/meeting-id-to-minute-token -y -g |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/vc/vc_recording_test.go (1)
59-69: Use English comments for consistency.Lines 59 and 65 contain Chinese comments. For an open-source project, consider using English comments for better accessibility and consistency with the rest of the codebase.
📝 Suggested changes
- // 没传任何 flag + // No flags provided err := mountAndRun(t, VCRecording, []string{"+recording", "--as", "user"}, f, nil) if err == nil { t.Fatal("expected validation error for no flags") } - // 两个 flag 都传了 + // Both flags provided err = mountAndRun(t, VCRecording, []string{"+recording", "--meeting-ids", "m1", "--calendar-event-ids", "e1", "--as", "user"}, f, nil)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/vc/vc_recording_test.go` around lines 59 - 69, Replace the Chinese inline comments in vc_recording_test.go with English equivalents to match project conventions: update the two comment lines above the mountAndRun calls (the ones describing "no flags passed" and "both flags passed") to clear English phrases (e.g., "no flags provided" and "both flags provided") while leaving the test logic and references to mountAndRun and VCRecording unchanged.
🤖 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/vc/vc_recording_test.go`:
- Around line 59-69: Replace the Chinese inline comments in vc_recording_test.go
with English equivalents to match project conventions: update the two comment
lines above the mountAndRun calls (the ones describing "no flags passed" and
"both flags passed") to clear English phrases (e.g., "no flags provided" and
"both flags provided") while leaving the test logic and references to
mountAndRun and VCRecording unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fe4ae91e-bb6f-4085-8822-9d55c1dcacd4
📒 Files selected for processing (1)
shortcuts/vc/vc_recording_test.go
2 participants
Summary
Add
vc +recordingshortcut that bridges VC and Minutes domains by convertingmeeting_idtominute_tokenvia the recording API.vc +recordingshortcut with--meeting-idsand--calendar-event-idspathsresolveMeetingIDsFromCalendarEventshared function fromvc_notes.gofor reuseChanges
New files
shortcuts/vc/vc_recording.go— +recording shortcut implementation (~170 lines)shortcuts/vc/vc_recording_test.go— 25 test cases (unit + integration)skills/lark-vc/references/lark-vc-recording.md— skill reference docModified files
shortcuts/vc/vc_notes.go— extractresolveMeetingIDsFromCalendarEventshared function (pure refactor, +notes behavior unchanged)shortcuts/vc/shortcuts.go— registerVCRecordingskills/lark-vc/SKILL.md— add +recording to shortcuts table, permissions table, resource diagramAPI
GET /open-apis/vc/v1/meetings/{meeting_id}/recording→ extractminute_tokenfrom recording URLTest plan
extractMinuteToken(12 cases),resolveMeetingIDsFromCalendarEvent(3 cases)fetchRecordingByMeetingID(4 cases), validation (3 cases), dry-run (3 cases)go test ./... -racefull project passgolangci-lintincremental: 0 issues7624382238545480669→ minute_tokenobcns5675246n77gy7lw37o3Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests