Skip to content

fix(calendar): guide approval room booking fallback - #1637

Merged
zhangjun-bytedance merged 1 commit into
larksuite:mainfrom
xu91102:fix/calendar-approval-room-guidance
Jul 6, 2026
Merged

fix(calendar): guide approval room booking fallback#1637
zhangjun-bytedance merged 1 commit into
larksuite:mainfrom
xu91102:fix/calendar-approval-room-guidance

Conversation

@xu91102

@xu91102 xu91102 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Improve calendar +create diagnostics when adding an approval-required meeting room fails because attendees[].approval_reason is required.

Changes

  • Detect room-attendee failures whose typed API hint mentions approval_reason and append actionable guidance to use calendar event.attendees create --as user.
  • Preserve the original typed API metadata (api / invalid_parameters / code) and existing rollback context.
  • Document the approval-room fallback flow in the calendar skill reference.

Verification

  • GOCACHE=$PWD/.cache/go-build GOPROXY=https://goproxy.cn,direct go test ./shortcuts/calendar -run TestCreate_ApprovalRoomMissingReason_GuidesRawAttendeesAPI -count=1
  • GOCACHE=$PWD/.cache/go-build GOPROXY=https://goproxy.cn,direct go test ./shortcuts/calendar -run 'TestCreate_(ApprovalRoomMissingReason_GuidesRawAttendeesAPI|WithAttendees_InvalidParamsWithDetail_RollsBack|WithAttendees_APIError_RollsBack)' -count=1\n- [x] GOCACHE=$PWD/.cache/go-build GOPROXY=https://goproxy.cn,direct go test ./shortcuts/calendar -count=1\n- [x] git diff --check\n\n## Risk and rollback\n- Risk is limited to extra guidance in an existing error path and docs. Rollback is reverting this commit.\n\n## Related Issues\n- Fixes cli预定会议室,遇到需要审批的会议室预定失败,因为缺少approval_reason #1608

Summary by CodeRabbit

  • Bug Fixes

    • Improved error guidance when creating calendar events with meeting room/resource attendees that require approval.
    • Added clearer recovery details after attendee creation fails, including rollback status and the missing approval_reason field.
  • Documentation

    • Clarified that the basic calendar create flow doesn’t support approval_reason for approved meeting rooms.
    • Added an example showing how to add an approval-required meeting room using the full API flow.

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds two helper functions to calendar_create.go to detect room/resource attendees and wrap attendee-add errors with actionable guidance when approval_reason is missing. The error wrapping is wired into the existing rollback path. A test and documentation update accompany the change.

Changes

Approval Room Missing approval_reason Guidance

Layer / File(s) Summary
Helper functions and error wrapping
shortcuts/calendar/calendar_create.go
Adds attendeesIncludeRoom to detect resource attendees and guideApprovalRoomReasonError to replace matching API errors with step-by-step guidance; wires the wrapper into the attendee-addition failure path before rollback.
Test coverage
shortcuts/calendar/calendar_test.go
Adds TestCreate_ApprovalRoomMissingReason_GuidesRawAttendeesAPI stubbing event creation success, attendee-add failure with code 190014, and rollback DELETE; asserts typed problem shape and hint contents.
Documentation
skills/lark-calendar/references/lark-calendar-create.md
Adds a note about the approval_reason limitation in +create and a dedicated example for adding approval-required meeting rooms via the full API.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • hugang-lark

Poem

🐇 A rabbit once booked a room with great care,
But approval_reason wasn't there!
Now the CLI spots the gap,
Gives a hint on the map—
Roll back and try the raw API with flair! 🗓️

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly reflects the main change: improving guidance for approval-required room booking fallback in calendar create.
Description check ✅ Passed The description matches the template closely, covering summary, changes, verification, and related issues, with only the section label differing from 'Test Plan'.
Linked Issues check ✅ Passed The change addresses #1608 by clarifying approval-room failures and guiding users to the user-identity fallback path.
Out of Scope Changes check ✅ Passed The tests and docs stay within the issue scope; no unrelated code paths or features were introduced.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added domain/calendar PR touches the calendar domain size/M Single-domain feat or fix with limited business impact labels Jun 29, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
shortcuts/calendar/calendar_test.go (1)

726-743: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert cause preservation on the wrapped error.

This path is supposed to keep the original typed API error intact after withStepContext, but the test only checks ProblemOf fields and hint text. Please also assert that the wrapped error still preserves its underlying cause so a future refactor cannot silently downgrade the error chain. As per coding guidelines, **/*_test.go: Error-path tests must assert typed metadata via errs.ProblemOf (category / subtype / param) and cause preservation, not message substrings alone.

🤖 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_test.go` around lines 726 - 743, The test around
withStepContext only verifies ProblemOf metadata and hint text, but it should
also ensure the wrapped error still preserves the original cause. Update the
calendar_test.go case to assert cause preservation on the error returned by
withStepContext, using the existing err/p variables and the typed API error path
already checked with errs.ProblemOf. Keep the current category, subtype, and
code assertions, and add a direct cause-chain assertion so future refactors
cannot break error wrapping while still passing this test.

Source: Coding guidelines

🤖 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_test.go`:
- Around line 726-743: The test around withStepContext only verifies ProblemOf
metadata and hint text, but it should also ensure the wrapped error still
preserves the original cause. Update the calendar_test.go case to assert cause
preservation on the error returned by withStepContext, using the existing err/p
variables and the typed API error path already checked with errs.ProblemOf. Keep
the current category, subtype, and code assertions, and add a direct cause-chain
assertion so future refactors cannot break error wrapping while still passing
this test.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6bd6ab5a-2c65-4a10-9168-e7fd04adbf0d

📥 Commits

Reviewing files that changed from the base of the PR and between 30b28cf and 26a5ada.

📒 Files selected for processing (3)
  • shortcuts/calendar/calendar_create.go
  • shortcuts/calendar/calendar_test.go
  • skills/lark-calendar/references/lark-calendar-create.md

@zhangjun-bytedance

Copy link
Copy Markdown
Collaborator

I believe your feedback is valuable; we will take it into account to optimize the workflow for creating meetings in rooms that require approval.

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@26a5adaf64a99d4aa97dc84f750c94c565422f71

🧩 Skill update

npx skills add xu91102/cli#fix/calendar-approval-room-guidance -y -g

@zhangjun-bytedance
zhangjun-bytedance merged commit b76dc18 into larksuite:main Jul 6, 2026
32 checks passed
This was referenced Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/calendar PR touches the calendar domain size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cli预定会议室,遇到需要审批的会议室预定失败,因为缺少approval_reason

2 participants