feat: precheck room availability before calendar +update#1965
Conversation
📝 WalkthroughWalkthrough
ChangesCalendar room availability precheck
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CalendarUpdate
participant EventSnapshot
participant RoomAvailabilityAPI
participant UpdateAPI
CalendarUpdate->>EventSnapshot: Fetch event snapshot
EventSnapshot-->>CalendarUpdate: Return rooms, times, timezone, recurrence
CalendarUpdate->>RoomAvailabilityAPI: Check affected rooms
RoomAvailabilityAPI-->>CalendarUpdate: Return room availability results
CalendarUpdate->>UpdateAPI: Apply calendar update when allowed
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/calendar/calendar_update.go (1)
295-319: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDry-run preview omits existing rooms that will actually be checked.
room_idsis set tonewRoomsonly, but on a time/rrule change with no newly added rooms,resolveRoomCheckPlanalso checks the event's existing rooms. The preview then shows an empty/partialroom_ids, which can mislead agents parsing the dry-run output. Consider a placeholder mirroring the timezone convention.♻️ Optional: signal inherited existing rooms in the preview
previewBody := map[string]interface{}{ "calendar_id": displayCalendarID, "event_id": eventID, "room_ids": newRooms, "start_timezone": "<inherited from event>", } + if len(newRooms) == 0 && (timeChanged || rruleChanged) { + previewBody["room_ids"] = "<existing event rooms inherited from event>" + }🤖 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/calendar/calendar_update.go` around lines 295 - 319, Update the dry-run preview built in the room-check block so previewBody["room_ids"] also signals the event’s existing rooms when timeChanged or rruleChanged causes them to be checked, while preserving explicit newRooms. Use a clear inherited-existing-rooms placeholder consistent with the existing "<inherited from event>" timezone convention, and keep the actual room-check planning behavior unchanged.
🤖 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.
Nitpick comments:
In `@shortcuts/calendar/calendar_update.go`:
- Around line 295-319: Update the dry-run preview built in the room-check block
so previewBody["room_ids"] also signals the event’s existing rooms when
timeChanged or rruleChanged causes them to be checked, while preserving explicit
newRooms. Use a clear inherited-existing-rooms placeholder consistent with the
existing "<inherited from event>" timezone convention, and keep the actual
room-check planning behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 01c2ab41-39a9-4320-a648-9267ba65a0d3
📒 Files selected for processing (4)
shortcuts/calendar/calendar_room_check.goshortcuts/calendar/calendar_test.goshortcuts/calendar/calendar_update.goskills/lark-calendar/references/lark-calendar-update.md
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@e822dcdb5a4c6cb3f17e9d83857c434689e484a9🧩 Skill updatenpx skills add larksuite/cli#feat/room_available -y -g |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1965 +/- ##
==========================================
+ Coverage 74.98% 75.01% +0.02%
==========================================
Files 896 897 +1
Lines 94403 94841 +438
==========================================
+ Hits 70789 71142 +353
- Misses 18198 18251 +53
- Partials 5416 5448 +32 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
da58256 to
e822dcd
Compare
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 `@tests/cli_e2e/calendar/calendar_update_dryrun_test.go`:
- Line 46: The calendar dry-run E2E test currently validates only room_ids;
extend its assertions to cover calendar_id, event_id, start_time, end_time, and
start_timezone in the full room-check payload. Add a separate end-to-end test in
the calendar CLI E2E tests that invokes the update flow with --skip-room-check
and verifies the bypass behavior and expected output.
🪄 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: 7eadeb5a-375e-4187-8a71-dedfb8b00f3b
📒 Files selected for processing (5)
shortcuts/calendar/calendar_room_check.goshortcuts/calendar/calendar_test.goshortcuts/calendar/calendar_update.goskills/lark-calendar/references/lark-calendar-update.mdtests/cli_e2e/calendar/calendar_update_dryrun_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- skills/lark-calendar/references/lark-calendar-update.md
- shortcuts/calendar/calendar_room_check.go
- shortcuts/calendar/calendar_update.go
- shortcuts/calendar/calendar_test.go
Summary
lark-cli calendar +updatenow runs a room-availability precheck before PATCH so agents don't PATCH an event only to end up with a room that will silently drop off (reserved, past-time, disabled, over duration, needs approval, etc.). Blocking outcomes are surfaced as a typed validation error that describes each room's status and the recovery paths the CLI actually supports.Changes
/open-apis/calendar/v4/freebusy/room_availability_checkbefore PATCH; surface blocked rooms with room ID + name + reason + strategy/requisition/approval detailsneed_approval(all / over_duration): compare current duration against threshold; consolidate recovery guidance into a single hint that requires user confirmation before running any recovery pathTest Plan
lark-cli calendar +update --event-id <id> --start-time <past>returns structuredfailed_preconditionnaming the blocked roomlark-cli calendar +update --event-id <id> --add-room-ids <approval-room>blocks withneed_approvaland points atcalendar event.attendees create --as user--skip-room-checkbypasses the precheck when the user explicitly authorizes itRelated Issues
Summary by CodeRabbit
New Features
calendar +updatewhen room attendees and/or meeting times change, including dry-run previews.Bug Fixes
Documentation