Skip to content

Comments

feat: add no show setting in cal video#24351

Open
dhairyashiil wants to merge 26 commits intocalcom:mainfrom
dhairyashiil:feat/cal-video-no-show-setting-2
Open

feat: add no show setting in cal video#24351
dhairyashiil wants to merge 26 commits intocalcom:mainfrom
dhairyashiil:feat/cal-video-no-show-setting-2

Conversation

@dhairyashiil
Copy link
Member

What does this PR do?

Visual Demo (For contributors especially)

add.no.show.setting.in.cal.video.mov

@dhairyashiil dhairyashiil requested a review from a team as a code owner October 8, 2025 09:44
@vercel
Copy link

vercel bot commented Oct 8, 2025

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

A member of the Team first needs to authorize it.

@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Oct 8, 2025
@graphite-app graphite-app bot requested a review from a team October 8, 2025 09:44
@github-actions github-actions bot added consumer enterprise area: enterprise, audit log, organisation, SAML, SSO Medium priority Created by Linear-GitHub Sync webhooks area: webhooks, callback, webhook payload ✨ feature New feature or request ❗️ migrations contains migration files labels Oct 8, 2025
@keithwillcode keithwillcode added the community-interns The team responsible for reviewing, testing and shipping low/medium community PRs label Oct 8, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 8, 2025

Walkthrough

Adds two Cal Video boolean flags (enableAutomaticNoShowTrackingForHosts, enableAutomaticNoShowTrackingForGuests) across DB, Prisma schema, migrations (including a backfill), types, and UI. getEventTypesFromDB now selects calVideoSettings; booking flows (handleNewBooking, handleConfirmation) normalize and propagate calVideoSettings and pass it into scheduleNoShowTriggers, which gains a calVideoSettings parameter and conditionally schedules host/guest no-show webhooks. CalVideoSettings UI accepts eventTypeId, queries existing webhooks, and conditionally renders the two toggles. Two localization keys were added.

Possibly related PRs

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly captures the primary feature of the changeset by indicating the addition of no-show settings in Cal Video, matching the contents of the pull request without extraneous detail.
Linked Issues Check ✅ Passed The implementation adds the two requested toggles with corresponding locale strings and database fields, updates the UI to conditionally display them based on existing webhooks, propagates calVideoSettings through backend booking flows, and preserves existing webhook-based tracking behavior, fully satisfying the objectives of issues #24319 and CAL-6532.
Out of Scope Changes Check ✅ Passed All changes are focused on introducing and supporting the new no-show tracking toggles, including UI adjustments, type and schema updates, scheduling logic, and migrations, with no unrelated modifications detected.
Description Check ✅ Passed The description references the linked issue fixes and provides a visual demo which directly pertains to the UI enhancements and feature behavior being introduced.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • 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 1d70465 and 56bffab.

📒 Files selected for processing (4)
  • apps/web/public/static/locales/en/common.json (1 hunks)
  • packages/features/bookings/lib/handleConfirmation.ts (2 hunks)
  • packages/features/bookings/lib/handleNewBooking.ts (5 hunks)
  • packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/features/bookings/lib/handleConfirmation.ts
  • packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts
  • apps/web/public/static/locales/en/common.json
🧰 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:

  • packages/features/bookings/lib/handleNewBooking.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:

  • packages/features/bookings/lib/handleNewBooking.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:

  • packages/features/bookings/lib/handleNewBooking.ts
🧠 Learnings (1)
📚 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:

  • packages/features/bookings/lib/handleNewBooking.ts
🧬 Code graph analysis (1)
packages/features/bookings/lib/handleNewBooking.ts (2)
packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts (1)
  • getEventTypeResponse (224-224)
packages/features/bookings/lib/handleNewBooking/getBookingData.ts (1)
  • getBookingData (86-86)
🔇 Additional comments (1)
packages/features/bookings/lib/handleNewBooking.ts (1)

476-476: Consistent use of eventTypeForProcessing throughout.

The normalized event type is correctly propagated to getBookingData, eventTypeWithUsers, createBooking, and scheduleNoShowTriggers. This ensures calVideoSettings is always present (or explicitly null) across all downstream paths.

Also applies to: 481-481, 796-796, 1559-1559, 2418-2418


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.

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: 4

🧹 Nitpick comments (1)
packages/features/bookings/lib/handleNewBooking.ts (1)

2398-2411: Avoid duplicating the normalization logic.

Multiple hasCalVideoSettings checks plus manual casts make this path harder to follow. Once you normalize eventTypeForProcessing, prefer to reuse it (or mutate the original eventType) so you don't need repeated guards and casts; it keeps no‑show scheduling and any future consumers aligned.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • 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 4d51873 and 9fa1359.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (10)
  • apps/web/public/static/locales/en/common.json (1 hunks)
  • packages/features/bookings/lib/handleConfirmation.ts (2 hunks)
  • packages/features/bookings/lib/handleNewBooking.ts (5 hunks)
  • packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts (1 hunks)
  • packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.ts (6 hunks)
  • packages/features/eventtypes/components/locations/CalVideoSettings.tsx (3 hunks)
  • packages/features/eventtypes/components/locations/Locations.tsx (4 hunks)
  • packages/features/eventtypes/lib/types.ts (2 hunks)
  • packages/prisma/migrations/20251008092040_add_no_show_config_in_calvideosettings/migration.sql (1 hunks)
  • packages/prisma/schema.prisma (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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:

  • packages/features/eventtypes/lib/types.ts
  • packages/features/bookings/lib/handleConfirmation.ts
  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.ts
  • packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.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:

  • packages/features/eventtypes/lib/types.ts
  • packages/features/bookings/lib/handleConfirmation.ts
  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.ts
  • packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts
  • packages/features/eventtypes/components/locations/Locations.tsx
  • packages/features/eventtypes/components/locations/CalVideoSettings.tsx
**/*.{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:

  • packages/features/eventtypes/lib/types.ts
  • packages/features/bookings/lib/handleConfirmation.ts
  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.ts
  • packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts
  • packages/features/eventtypes/components/locations/Locations.tsx
  • packages/features/eventtypes/components/locations/CalVideoSettings.tsx
**/*.tsx

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

Always use t() for text localization in frontend code; direct text embedding should trigger a warning

Files:

  • packages/features/eventtypes/components/locations/Locations.tsx
  • packages/features/eventtypes/components/locations/CalVideoSettings.tsx
🧠 Learnings (4)
📚 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:

  • packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-21T13:44:06.805Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.805Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.

Applied to files:

  • packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts
📚 Learning: 2025-09-03T11:54:05.409Z
Learnt from: supalarry
PR: calcom/cal.com#23514
File: apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts:579-582
Timestamp: 2025-09-03T11:54:05.409Z
Learning: In calcom/cal.com bookings repository methods, when Prisma select uses `eventType: true`, all eventType fields including seatsShowAttendees are automatically included in the selection. Explicit field selection is not required when using `true` for nested relations.

Applied to files:

  • packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts
📚 Learning: 2025-09-03T11:54:05.409Z
Learnt from: supalarry
PR: calcom/cal.com#23514
File: apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts:579-582
Timestamp: 2025-09-03T11:54:05.409Z
Learning: In calcom/cal.com bookings repository methods, when Prisma include uses `eventType: true`, all eventType fields including seatsShowAttendees are automatically included in the selection. Explicit field selection is not required when using `true` for nested relations.

Applied to files:

  • packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts
🧬 Code graph analysis (4)
packages/features/bookings/lib/handleNewBooking.ts (1)
packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts (1)
  • getEventTypeResponse (224-224)
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.ts (1)
packages/features/eventtypes/lib/types.ts (1)
  • CalVideoSettings (298-308)
packages/features/eventtypes/components/locations/Locations.tsx (1)
packages/features/eventtypes/lib/types.ts (2)
  • EventTypeSetupProps (30-30)
  • CalVideoSettings (298-308)
packages/features/eventtypes/components/locations/CalVideoSettings.tsx (4)
packages/features/eventtypes/lib/types.ts (2)
  • CalVideoSettings (298-308)
  • FormValues (84-181)
packages/trpc/react/trpc.ts (1)
  • trpc (54-138)
packages/platform/libraries/index.ts (1)
  • WebhookTriggerEvents (36-36)
packages/ui/components/form/switch/SettingsToggle.tsx (1)
  • SettingsToggle (29-125)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Tests / Unit
  • GitHub Check: Linters / lint
  • GitHub Check: Type check / check-types
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (7)
packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts (1)

120-120: LGTM!

The addition of calVideoSettings to the select statement is correct and necessary for downstream processing. It follows the established pattern and coding guidelines for Prisma queries.

packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.ts (2)

7-7: LGTM!

The addition of calVideoSettings parameter and its type import is correct and enables the conditional scheduling logic implemented below.

Also applies to: 23-23, 36-36


55-102: Verify triggerHostNoShowWebhook handles default webhook payload

Ensure that when no subscribers exist but automatic tracking is enabled (default payload at lines 79–101), the triggerHostNoShowWebhook handler can process empty subscriberUrl, id, and other null fields without errors.

packages/features/eventtypes/components/locations/CalVideoSettings.tsx (4)

1-1: LGTM!

The new imports are necessary and correctly used:

  • useAutoAnimate for smooth expansion animation (line 32)
  • useState for managing expanded state (line 31)
  • WebhookTriggerEvents and trpc for webhook checking (lines 34-45)
  • Icon and Tooltip for UI components (lines 50, 61)

Also applies to: 3-3, 10-11, 16-17


21-27: LGTM!

The component signature update correctly accepts eventTypeId to enable webhook fetching for conditional toggle rendering.


34-45: LGTM!

The webhook fetching logic correctly:

  • Queries webhooks only when eventTypeId is present
  • Checks for existing host and guest no-show webhooks
  • Enables conditional rendering of the new toggles

This ensures that toggles are hidden when webhooks are already configured, preserving the existing behavior as specified in the PR objectives.


117-151: LGTM!

The new toggles for automatic no-show tracking are correctly implemented:

  • Properly integrated with react-hook-form using Controller
  • Conditionally rendered only when no webhooks exist and not on platform
  • Use localization with t() as required by coding guidelines
  • Follow the established pattern of other settings in this component
  • Include appropriate upgrade badges

The implementation aligns with the PR objectives to provide explicit toggles while preserving existing webhook-based behavior.

Based on coding guidelines.

Copy link
Contributor

@pallava-joshi pallava-joshi left a comment

Choose a reason for hiding this comment

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

please address code-rabbit suggestion

@pallava-joshi pallava-joshi marked this pull request as draft October 8, 2025 12:15
@dhairyashiil dhairyashiil marked this pull request as ready for review October 14, 2025 08:30
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: 0

♻️ Duplicate comments (2)
packages/features/bookings/lib/handleNewBooking.ts (2)

135-140: Fix the type guard to assert the correct type.

The type guard currently asserts eventType is getEventTypeResponse, but getEventTypeResponse is the array type returned by getEventTypesFromDB, not a single event type object. This widens types downstream and defeats type safety.

Please refer to the previous review comment for details and suggested fixes.


461-463: Use the correct type when creating eventTypeForProcessing.

The type cast to getEventTypeResponse (which is an array type) is incorrect and bypasses TypeScript's type checking with the double cast pattern eventType as unknown as getEventTypeResponse. This creates type confusion downstream.

Please refer to the previous review comment for the recommended fix.

🧹 Nitpick comments (1)
packages/features/bookings/lib/handleNewBooking.ts (1)

27-29: Remove unused imports.

Two of the three newly imported services (getCheckBookingAndDurationLimitsService and getLuckyUserService) don't appear to be used in the visible code changes. While getCacheService is used at line 626, the other two imports seem unnecessary and should be removed to keep the imports clean.

Apply this diff to remove the unused imports:

-import { getCheckBookingAndDurationLimitsService } from "@calcom/features/di/containers/BookingLimits";
 import { getCacheService } from "@calcom/features/di/containers/Cache";
-import { getLuckyUserService } from "@calcom/features/di/containers/LuckyUser";
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • 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 9fa1359 and 1d70465.

📒 Files selected for processing (2)
  • packages/features/bookings/lib/handleNewBooking.ts (7 hunks)
  • packages/prisma/migrations/20251014120000_backfill_cal_video_no_show_flags/migration.sql (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:

  • packages/features/bookings/lib/handleNewBooking.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:

  • packages/features/bookings/lib/handleNewBooking.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:

  • packages/features/bookings/lib/handleNewBooking.ts
🧠 Learnings (2)
📚 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:

  • packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-09-03T09:52:51.182Z
Learnt from: hariombalhara
PR: calcom/cal.com#23541
File: packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts:22-28
Timestamp: 2025-09-03T09:52:51.182Z
Learning: The IBookingServiceDependencies interface in packages/features/bookings/lib/handleNewBooking.ts contains 6 properties: cacheService, checkBookingAndDurationLimitsService, prismaClient, bookingRepository, featuresRepository, and checkBookingLimitsService. This interface is used by RegularBookingService and matches the depsMap structure in RegularBookingServiceModule.

Applied to files:

  • packages/features/bookings/lib/handleNewBooking.ts
🧬 Code graph analysis (1)
packages/features/bookings/lib/handleNewBooking.ts (1)
packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts (1)
  • getEventTypeResponse (224-224)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Tests / Unit
  • GitHub Check: Linters / lint
  • GitHub Check: Type check / check-types
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
packages/prisma/migrations/20251014120000_backfill_cal_video_no_show_flags/migration.sql (2)

3-17: LGTM!

The webhook-based backfill logic correctly enables the flags when an active webhook with the corresponding trigger exists. The use of PostgreSQL's ANY operator for array containment checks is appropriate.


19-33: Workflow model has no active/enabled field. No filter is possible; current backfill queries are correct.

packages/features/bookings/lib/handleNewBooking.ts (1)

467-467: Consistent propagation of calVideoSettings looks good.

The changes consistently propagate calVideoSettings through the booking flow:

  • Using eventTypeForProcessing in schema creation (line 467) and data retrieval (line 472)
  • Explicitly spreading into eventTypeWithUsers (line 791)
  • Passing to createBooking (line 1549) and scheduleNoShowTriggers (line 2408)

Once the type issues with hasCalVideoSettings and eventTypeForProcessing are resolved (as noted in previous comments), this propagation logic will be sound.

Also applies to: 472-472, 791-791, 1549-1549, 2408-2408

@pallava-joshi
Copy link
Contributor

shouldn't we change these unused import statements

-import { getCheckBookingAndDurationLimitsService } from "@calcom/features/di/containers/BookingLimits";

import { getCacheService } from "@calcom/features/di/containers/Cache";

-import { getLuckyUserService } from "@calcom/features/di/containers/LuckyUser";

@pallava-joshi pallava-joshi marked this pull request as draft October 17, 2025 11:54
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 11 files

@dhairyashiil dhairyashiil marked this pull request as ready for review October 18, 2025 19:21
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 11 files

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.ts">

<violation number="1" location="packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.ts:89">
The fallback guest no-show payload leaves subscriberUrl empty, but the task schema enforces subscriberUrl to be a valid URL; this will throw at runtime and block automatic guest no-show processing.</violation>
</file>

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

@github-actions
Copy link
Contributor

This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active.

@github-actions github-actions bot added the Stale label Oct 27, 2025
@dhairyashiil dhairyashiil marked this pull request as draft November 27, 2025 07:30
- Add enableAutomaticNoShowTrackingForHosts/Guests to form validation schema
- Add fields to eventTypeRepository select queries for proper data fetching
- Add fields to tRPC input validation schema
- Add fields to CalVideoSettings repository type definitions
- Add fields to create handler for new event types

The issue was caused by missing fields in multiple validation layers,
particularly in the form-level Zod schema which was stripping the fields
before submission. Now the settings properly save to DB and persist after reload.
- Add comprehensive authorization checks before accessing webhook data
- Verify user has access to event type (direct ownership, assigned, or team member)
- Optimize query by checking direct ownership first (fastest path)
- Prevent unauthorized users from probing other teams' webhook configurations

Addresses security review comment about missing authorization validation.
Replace fake webhook URLs with isAutomaticTrackingOnly flag for clarity.
When Cal Video automatic tracking is enabled without webhooks, we now:
- Set isAutomaticTrackingOnly: true flag
- Use empty subscriberUrl instead of fake URL
- Check the flag instead of URL patterns

This makes the code more maintainable and addresses review feedback
about confusing dummy webhook objects.
Add comprehensive tests for automatic no-show tracking feature:
- Test enableAutomaticNoShowTrackingForHosts toggle
- Test enableAutomaticNoShowTrackingForGuests toggle
- Test both toggles enabled/disabled scenarios
- Test webhook precedence over automatic tracking
- Test location validation (Cal Video only)
- Test edge cases (null settings, isDryRun, empty location)

10 essential tests with full coverage of the toggle feature.
@dhairyashiil dhairyashiil marked this pull request as ready for review November 27, 2025 09:09
@dhairyashiil dhairyashiil requested a review from a team November 27, 2025 09:09
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 23 files

Prompt for AI agents (all 2 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/trpc/server/routers/viewer/bookings/confirm.handler.ts">

<violation number="1" location="packages/trpc/server/routers/viewer/bookings/confirm.handler.ts:93">
Consider selecting only the needed `calVideoSettings` fields rather than all columns. Based on `scheduleNoShowTriggers` usage, only `enableAutomaticNoShowTrackingForHosts` and `enableAutomaticNoShowTrackingForGuests` appear to be required:
```typescript
calVideoSettings: {
  select: {
    enableAutomaticNoShowTrackingForHosts: true,
    enableAutomaticNoShowTrackingForGuests: true,
    redirectUrlOnExit: true,
  }
}

This follows the project guideline of selecting only required fields to reduce performance overhead.

Bug: `isAutomaticTrackingOnly` will always be `false` because it's not included in `ZSendNoShowWebhookPayloadSchema` (gets stripped during Zod parsing) and not returned from `prepareNoShowTrigger`. The property needs to be added to the schema and included in the function's return value for this feature to work. ```

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

…ettings select

Address reviewer comments:

1. Add isAutomaticTrackingOnly to ZSendNoShowWebhookPayloadSchema
   - Update schema.ts to include the optional boolean field
   - Update common.ts prepareNoShowTrigger to extract and return the field
   - Update triggerHostNoShow.ts and triggerGuestNoShow.ts to use properly typed field
   - Remove unsafe 'as any' type assertions

2. Optimize calVideoSettings select in confirm.handler.ts
   - Only select required fields (enableAutomaticNoShowTrackingForHosts, enableAutomaticNoShowTrackingForGuests)
   - Remove unnecessary spreading and redirectUrlOnExit handling
   - Follows project guideline to reduce performance overhead

Fixes type check errors and improves code quality.
@dhairyashiil
Copy link
Member Author

  1. Authorization check - Fixed
    Added authorization checks. Includes user, team, and org checks.

  2. Cal video settings not saved - Fixed
    The issue was that the new fields were missing from multiple Zod schemas, causing them to be stripped during validation. Added enableAutomaticNoShowTrackingForHosts and enableAutomaticNoShowTrackingForGuests to all required schemas.

  3. Unit tests - Added
    Created scheduleNoShowTriggers.test.ts.

  4. "Internal webhook" / dummy webhooks confusion - Refactored
    Changed to use an explicit isAutomaticTrackingOnly flag instead:
    Added isAutomaticTrackingOnly: z.boolean().optional() to the Zod schema
    Tasks check this flag and skip webhook sending while still marking no-shows in DB

@Udit-takkar please check 🙏🏼

Udit-takkar
Udit-takkar previously approved these changes Dec 2, 2025
Copy link
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 25 files

Prompt for AI agents (all 1 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts">

<violation number="1" location="packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts:121">
P2: Consider using a nested `select` to fetch only the specific `CalVideoSettings` fields needed for the no-show feature, rather than selecting all fields from the relation. This aligns with the project&#39;s Prisma guidelines to only select data you need.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

@github-actions
Copy link
Contributor

Devin AI is resolving merge conflicts

This PR has merge conflicts with the main branch. A Devin session has been created to automatically resolve them.

View Devin Session

Devin will:

  1. Merge the latest main into this branch
  2. Resolve any conflicts intelligently
  3. Run lint/type checks to ensure validity
  4. Push the resolved changes

If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself.

Resolved merge conflicts in:
- apps/web/modules/event-types/components/locations/CalVideoSettings.tsx
- apps/web/modules/event-types/components/locations/Locations.tsx
- packages/features/bookings/lib/service/RegularBookingService.ts
- packages/features/eventtypes/lib/types.ts
- packages/features/tasker/tasks/triggerNoShow/triggerGuestNoShow.ts
- packages/features/tasker/tasks/triggerNoShow/triggerHostNoShow.ts

Also updated:
- packages/features/eventtypes/lib/schemas.ts (added no-show tracking fields)
- packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.ts (added version field)

Co-Authored-By: unknown <>
@github-actions
Copy link
Contributor

Devin AI is resolving merge conflicts

This PR has merge conflicts with the main branch. A Devin session has been created to automatically resolve them.

View Devin Session

Devin will:

  1. Merge the latest main into this branch
  2. Resolve any conflicts intelligently
  3. Run lint/type checks to ensure validity
  4. Push the resolved changes

If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself.

@github-actions
Copy link
Contributor

Devin AI is resolving merge conflicts

This PR has merge conflicts with the main branch. The existing Devin session has been notified to resolve them.

View Devin Session

Devin will:

  1. Merge the latest main into this branch
  2. Resolve any conflicts intelligently
  3. Run lint/type checks to ensure validity
  4. Push the resolved changes

If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself.

@github-actions
Copy link
Contributor

Devin AI is resolving merge conflicts

This PR has merge conflicts with the main branch. The existing Devin session has been notified to resolve them.

View Devin Session

Devin will:

  1. Merge the latest main into this branch
  2. Resolve any conflicts intelligently
  3. Run lint/type checks to ensure validity
  4. Push the resolved changes

If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself.

1 similar comment
@github-actions
Copy link
Contributor

Devin AI is resolving merge conflicts

This PR has merge conflicts with the main branch. The existing Devin session has been notified to resolve them.

View Devin Session

Devin will:

  1. Merge the latest main into this branch
  2. Resolve any conflicts intelligently
  3. Run lint/type checks to ensure validity
  4. Push the resolved changes

If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself.

Copy link
Contributor

@eunjae-lee eunjae-lee left a comment

Choose a reason for hiding this comment

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

CI fails

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community Created by Linear-GitHub Sync community-interns The team responsible for reviewing, testing and shipping low/medium community PRs consumer enterprise area: enterprise, audit log, organisation, SAML, SSO ✨ feature New feature or request Medium priority Created by Linear-GitHub Sync ❗️ migrations contains migration files ready-for-e2e size/XL Stale webhooks area: webhooks, callback, webhook payload

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cal Video no show setting

7 participants