Skip to content

Comments

fix: Don’t allow hosts to cancel a booking without providing a reason through v1#23647

Merged
anikdhabal merged 7 commits intocalcom:mainfrom
anikdhabal:cancellation-reason-v1
Sep 9, 2025
Merged

fix: Don’t allow hosts to cancel a booking without providing a reason through v1#23647
anikdhabal merged 7 commits intocalcom:mainfrom
anikdhabal:cancellation-reason-v1

Conversation

@anikdhabal
Copy link
Contributor

What does this PR do?

Don’t allow hosts to cancel a booking without providing a reason through v1

@anikdhabal anikdhabal requested a review from a team September 6, 2025 15:37
@anikdhabal anikdhabal requested a review from a team as a code owner September 6, 2025 15:37
@vercel
Copy link

vercel bot commented Sep 6, 2025

@anikdhabal is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 6, 2025

Walkthrough

The pull request updates the Zod validation in apps/api/v1/lib/validations/booking.ts for the bookingCancelSchema. The cancellationReason field changes from z.string().optional().default("Not Provided") to z.string().min(1).optional(). This removes the automatic default value assignment and adds a minimum length constraint for provided values. As a result, when cancellationReason is omitted, no default is set, and if provided, it must be a non-empty string.


📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f5bb734 and 2c8dc51.

📒 Files selected for processing (1)
  • apps/api/v1/lib/validations/booking.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/api/v1/lib/validations/booking.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • apps/api/v1/lib/validations/booking.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • apps/api/v1/lib/validations/booking.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.

Applied to files:

  • apps/api/v1/lib/validations/booking.ts
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.

@graphite-app graphite-app bot requested a review from a team September 6, 2025 15:37
@keithwillcode keithwillcode added the core area: core, team members only label Sep 6, 2025
@dosubot dosubot bot added bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working labels Sep 6, 2025
id: z.number(),
allRemainingBookings: z.boolean().optional(),
cancelSubsequentBookings: z.boolean().optional(),
cancellationReason: z.string().optional().default("Not Provided"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously it bypassed the reason

@anikdhabal anikdhabal enabled auto-merge (squash) September 6, 2025 15:39
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between de98578 and 53c056b.

📒 Files selected for processing (1)
  • apps/api/v1/lib/validations/booking.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/api/v1/lib/validations/booking.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • apps/api/v1/lib/validations/booking.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • apps/api/v1/lib/validations/booking.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.

Applied to files:

  • apps/api/v1/lib/validations/booking.ts
🔇 Additional comments (1)
apps/api/v1/lib/validations/booking.ts (1)

42-57: cancellationReason change is non-breaking
Searches found no uses of the “Not Provided” sentinel in v1 cancel logic, and all v1 handlers and downstream UI/analytics gracefully handle an undefined reason.

Copy link
Contributor

@volnei volnei left a comment

Choose a reason for hiding this comment

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

small comment

@github-actions github-actions bot marked this pull request as draft September 8, 2025 13:04
auto-merge was automatically disabled September 8, 2025 13:04

Pull request was converted to draft

@anikdhabal anikdhabal marked this pull request as ready for review September 8, 2025 13:13
@anikdhabal anikdhabal requested a review from volnei September 8, 2025 13:13
@dosubot dosubot bot added the platform Anything related to our platform plan label Sep 8, 2025
volnei
volnei previously approved these changes Sep 8, 2025
@anikdhabal anikdhabal enabled auto-merge (squash) September 8, 2025 13:19
@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2025

E2E results are ready!

@anikdhabal anikdhabal requested a review from volnei September 8, 2025 19:22
@anikdhabal anikdhabal merged commit 3c72d7a into calcom:main Sep 9, 2025
33 of 37 checks passed
emrysal added a commit that referenced this pull request Sep 11, 2025
@coderabbitai coderabbitai bot mentioned this pull request Oct 13, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working core area: core, team members only platform Anything related to our platform plan ready-for-e2e size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants