Conversation
|
@romitg2 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a new GET /v2/bookings/{bookingUid}/session-details endpoint (2024-08-13) guarded by BookingUidGuard. The controller method returns GetBookingSessionDetailsOutput and delegates to CalVideoService.getSessionDetails(bookingUid). CalVideoService.getSessionDetails loads the booking, extracts a daily_video roomName from booking.references, and calls getSessionDetailsFromRoomName(roomName). A new helper getSessionDetailsFromRoomName was added to packages/lib/videoClient and re-exported from packages/platform/libraries/conferencing. OpenAPI/Swagger documentation, platform DTO types, and a re-export in the bookings outputs barrel were added. Tests were updated with new cases and a PermissionsGuard override. Assessment against linked issues
Out-of-scope changes
✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
github-actions
bot
added
api
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
dosubot
bot
added
the
bookings
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/14/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (08/14/25)1 label was added to this PR based on Keith Williams's automation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (14)
apps/api/v2/swagger/documentation.json (14)
22264-22266: Same concern: conditional schema needed for cal_ai_phone_call in UpdateEmailAddressWorkflowStepDto.Please extend the update DTO schema with oneOf/discriminator rules and examples for this new action.
22350-22352: Same concern repeated for UpdateEmailAttendeeWorkflowStepDto.Mirror the action-specific payload schema and examples for cal_ai_phone_call.
22427-22429: Same concern repeated for UpdateEmailHostWorkflowStepDto.Add conditional schema and examples.
22523-22525: Same concern repeated for WorkflowPhoneWhatsAppAttendeeStepDto.Define payload requirements for cal_ai_phone_call and add examples.
22593-22595: Same concern repeated for WorkflowPhoneWhatsAppNumberStepDto.Define payload requirements and examples for cal_ai_phone_call.
22672-22674: Same concern repeated for WorkflowPhoneNumberStepDto.Please document fields required for AI phone call action.
22751-22753: Same concern repeated for WorkflowPhoneAttendeeStepDto.Extend with oneOf/discriminator rules for cal_ai_phone_call.
22941-22943: Same concern repeated for UpdateEmailAddressWorkflowStepDto.Document cal_ai_phone_call semantics and constraints.
23032-23034: Same concern repeated for UpdateEmailAttendeeWorkflowStepDto.Add conditional schema and example.
23114-23116: Same concern repeated for UpdateEmailHostWorkflowStepDto.Document payload for cal_ai_phone_call action.
23196-23198: Same concern repeated for UpdatePhoneAttendeeWorkflowStepDto.Define fields and constraints for AI phone call action.
23275-23277: Same concern repeated for UpdatePhoneWhatsAppNumberWorkflowStepDto.Add conditional schema and examples.
23359-23361: Same concern repeated for UpdateWhatsAppAttendeePhoneWorkflowStepDto.Document cal_ai_phone_call payload.
23434-23436: Same concern repeated for UpdatePhoneNumberWorkflowStepDto.Add conditional schema and examples.
🧹 Nitpick comments (4)
packages/lib/videoClient.ts (1)
421-444: Type the return value, add input guard, and use the standard logger instead of console.error
- Add an explicit return type to make the contract clear and safer for downstream usage.
- Guard against empty roomName to avoid unnecessary adapter calls and confusing logs.
- Prefer the existing sub-logger (log) and include the caught error for diagnostics (consistent with the rest of this file using safeStringify).
Apply this diff within the function:
-const getSessionDetailsFromRoomName = async (roomName: string) => { +const getSessionDetailsFromRoomName = async (roomName: string): Promise<MeetingInformation | undefined> => { + if (!roomName?.trim()) { + log.error("getSessionDetailsFromRoomName called with empty roomName"); + return; + } let dailyAppKeys: Awaited<ReturnType<typeof getDailyAppKeys>>; try { dailyAppKeys = await getDailyAppKeys(); } catch (e) { - console.error("Error: Cal video provider is not installed."); + log.error("Error: Cal video provider is not installed.", safeStringify(e)); return; }And add this type alias once at the top-level (outside the selected range), right below the existing VideoApiAdapter import:
// Derived return type of the adapter's getMeetingInformation method type MeetingInformation = Awaited<ReturnType<NonNullable<VideoApiAdapter["getMeetingInformation"]>>>;Optional follow-up: the “get daily adapter” boilerplate is duplicated across multiple helpers in this file. Consider extracting a small
getDailyVideoAdapter()utility to DRY these call-sites.apps/api/v2/swagger/documentation.json (3)
7944-7962: Tighten parameter documentation (reuse shared header param, add descriptions and examples).
- Line 7948: For consistency, prefer reusing the shared header parameter via $ref if it exists (many endpoints use a common
cal-api-versiondefinition).- Lines 7956-7962: Add a clear description and example for
bookingUidto improve DX and client generation.Apply this diff to enrich param docs (adjust $ref if you have a shared component):
"parameters": [ { - "name": "cal-api-version", - "in": "header", - "description": "Must be set to 2024-08-13", - "required": true, - "schema": { - "type": "string", - "default": "2024-08-13" - } + "name": "cal-api-version", + "in": "header", + "description": "Must be set to 2024-08-13", + "required": true, + "schema": { + "type": "string", + "enum": ["2024-08-13"] + }, + "example": "2024-08-13" }, { "name": "bookingUid", "required": true, "in": "path", - "schema": { - "type": "string" - } + "description": "Public UID of the booking whose video session details will be returned", + "schema": { + "type": "string" + }, + "example": "abcdEF12" } ],
7941-7943: Wording nit: tighten summary/description.Minor copy edits to improve clarity.
Apply this small tweak:
- "summary": "Get the session details for the booking", - "description": "Fetches the session details for the booking `:bookingUid`", + "summary": "Get session details for a booking", + "description": "Returns provider-specific video session details for the booking identified by `bookingUid`."
21901-21903: Add a brief enum item description for cal_ai_phone_call across all schemas.To improve generated SDK docs, add a short description for the new enum value (e.g., “Initiate an AI-powered outbound phone call”).
Example change (apply similarly where applicable):
- "whatsapp_number", - "cal_ai_phone_call" + "whatsapp_number", + "cal_ai_phone_call" ], + "x-enumDescriptions": { + "cal_ai_phone_call": "Initiate an AI-powered outbound phone call" + }Note: If you don’t use x-enumDescriptions, add this to the surrounding schema’s description or provide per-action examples.
Also applies to: 22264-22266, 22350-22352, 22427-22429, 22523-22525, 22593-22595, 22672-22674, 22751-22753, 22941-22943, 23032-23034, 23114-23116, 23196-23198, 23275-23277, 23359-23361, 23434-23436
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts(1 hunks)apps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.ts(2 hunks)apps/api/v2/swagger/documentation.json(16 hunks)packages/lib/videoClient.ts(2 hunks)packages/platform/libraries/conferencing.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 useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/platform/libraries/conferencing.tsapps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.tspackages/lib/videoClient.tsapps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.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/platform/libraries/conferencing.tsapps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.tspackages/lib/videoClient.tsapps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts
**/*.{service,repository}.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Avoid dot-suffixes like
.service.tsor.repository.tsfor new files; reserve.test.ts,.spec.ts,.types.tsfor their specific purposes
Files:
apps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.ts
🧬 Code Graph Analysis (3)
apps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.ts (2)
packages/platform/libraries/conferencing.ts (1)
getSessionDetailsFromRoomName(5-5)packages/lib/videoClient.ts (1)
getSessionDetailsFromRoomName(458-458)
packages/lib/videoClient.ts (1)
packages/platform/libraries/conferencing.ts (1)
getSessionDetailsFromRoomName(5-5)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts (1)
packages/platform/constants/api.ts (1)
SUCCESS_STATUS(9-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Detect changes
🔇 Additional comments (3)
packages/lib/videoClient.ts (1)
446-459: LGTM on exportExporting getSessionDetailsFromRoomName here is consistent with how other helpers are surfaced and aligns with the PR’s new API usage via the platform re-export.
packages/platform/libraries/conferencing.ts (1)
5-5: LGTM: clean re-export for session details helperThe new re-export of getSessionDetailsFromRoomName aligns with existing exports and keeps API surface consistent.
apps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.ts (1)
9-10: LGTM: importing the re-exported helperImporting getSessionDetailsFromRoomName from @calcom/platform-libraries/conferencing matches the established pattern for the other video helpers.
| @Get("/:bookingUid/session-details") | ||
| @UseGuards(BookingUidGuard) | ||
| @ApiOperation({ | ||
| summary: "Get the session details for the booking", | ||
| description: `Fetches the session details for the booking \`:bookingUid\``, | ||
| }) | ||
| async getBookingSessionDetails(@Param("bookingUid") bookingUid: string): Promise<any> { | ||
| const sessionDetails = await this.calVideoService.getSessionDetails(bookingUid); | ||
| return { | ||
| status: SUCCESS_STATUS, | ||
| data: sessionDetails, | ||
| }; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Type the response and handle empty session details explicitly
Avoid Promise for public APIs and normalize undefined responses from the service to a predictable shape for clients.
Apply this diff to improve typing and normalize the data field:
- async getBookingSessionDetails(@Param("bookingUid") bookingUid: string): Promise<any> {
+ async getBookingSessionDetails(
+ @Param("bookingUid") bookingUid: string
+ ): Promise<GetBookingSessionDetailsOutput> {
const sessionDetails = await this.calVideoService.getSessionDetails(bookingUid);
return {
status: SUCCESS_STATUS,
- data: sessionDetails,
+ data: sessionDetails ?? null,
};
}Additionally (outside this hunk):
- Import GetBookingSessionDetailsOutput from @calcom/platform-types alongside other outputs.
- Define GetBookingSessionDetailsOutput in @calcom/platform-types to match the existing pattern:
// @calcom/platform-types (proposed)
export type GetBookingSessionDetailsOutput = {
status: "success";
data: SessionDetails | null;
};
export type SessionDetails = {
// shape returned by video client (e.g., roomName, createdAt, participants, etc.)
// ensure no sensitive tokens are included
};To verify whether a suitable output type already exists and to avoid duplicating types, run:
🏁 Script executed:
#!/bin/bash
# Look for an existing type for session details output
rg -n --hidden --glob '!**/node_modules/**' 'GetBookingSessionDetailsOutput|SessionDetails' @calcom
# Check if the controller already imports such a type
rg -n 'GetBookingSessionDetailsOutput' apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.tsLength of output: 262
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== controller file content ==="
if [[ -f "apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts" ]]; then
sed -n '1,240p' apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts || true
else
echo "controller file not found at the given path"
rg -n --hidden --glob '!**/node_modules/**' 'bookings.controller.ts' || true
fi
echo
echo "=== search for types / platform-types across repo ==="
rg -n --hidden --glob '!**/node_modules/**' 'GetBookingSessionDetailsOutput|SessionDetails|GetBookingSessionDetails|@calcom/platform-types|platform-types' || true
echo
echo "=== search for calVideoService/getSessionDetails occurrences ==="
rg -n --hidden --glob '!**/node_modules/**' 'calVideoService|getSessionDetails|sessionDetails' || true
echo
echo "=== repo root listing ==="
ls -la || true
echo
echo "=== package.json workspaces (if present) ==="
if [[ -f package.json ]]; then
grep -n '"workspaces"' package.json || true
fiLength of output: 68026
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== cal-video.service.ts ==="
sed -n '1,240p' apps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.ts || true
echo
echo "=== packages/lib/videoClient.ts (around getSessionDetailsFromRoomName) ==="
sed -n '380,520p' packages/lib/videoClient.ts || true
echo
echo "=== MeetingSessionDetailsDialog.tsx ==="
sed -n '1,320p' packages/features/ee/video/MeetingSessionDetailsDialog.tsx || true
echo
echo "=== search platform types for session / meeting related types ==="
rg -n --hidden --glob '!**/node_modules/**' 'SessionDetails|MeetingSessionDetails|MeetingSession|sessionDetails|meetingSession' packages/platform/types || true
echo
echo "=== list platform types bookings outputs dir ==="
ls -la packages/platform/types/bookings/2024-08-13/outputs || trueLength of output: 14788
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== outputs/index.ts ==="
sed -n '1,240p' packages/platform/types/bookings/2024-08-13/outputs/index.ts || true
echo
echo "=== get-booking-recordings.output.ts ==="
sed -n '1,240p' packages/platform/types/bookings/2024-08-13/outputs/get-booking-recordings.output.ts || true
echo
echo "=== get-booking-transcripts.output.ts ==="
sed -n '1,240p' packages/platform/types/bookings/2024-08-13/outputs/get-booking-transcripts.output.ts || true
echo
echo "=== packages/platform/types/index.ts (root) ==="
sed -n '1,240p' packages/platform/types/index.ts || trueLength of output: 3500
Type the response and normalize empty session details (fix required)
Avoid Promise; return a typed Api output and normalize undefined service responses to a predictable value.
Files to update:
- apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts — change signature, import type, normalize data to null.
- packages/platform/types/bookings/2024-08-13/outputs/get-booking-session-details.output.ts — add new output type.
- packages/platform/types/bookings/2024-08-13/outputs/index.ts — export the new output.
Apply this controller diff:
- async getBookingSessionDetails(@Param("bookingUid") bookingUid: string): Promise<any> {
+ async getBookingSessionDetails(
+ @Param("bookingUid") bookingUid: string
+ ): Promise<GetBookingSessionDetailsOutput> {
const sessionDetails = await this.calVideoService.getSessionDetails(bookingUid);
return {
status: SUCCESS_STATUS,
- data: sessionDetails,
+ data: sessionDetails ?? null,
};
}Also add GetBookingSessionDetailsOutput to the existing import from "@calcom/platform-types".
Suggested new platform-types file (follow existing class-validator / swagger patterns):
// packages/platform/types/bookings/2024-08-13/outputs/get-booking-session-details.output.ts
import { ApiProperty } from "@nestjs/swagger";
import { Type } from "class-transformer";
import {
ValidateNested,
IsEnum,
IsString,
IsNumber,
IsOptional,
IsArray,
IsBoolean,
} from "class-validator";
import { SUCCESS_STATUS, ERROR_STATUS } from "@calcom/platform-constants";
export class MeetingParticipant {
@ApiProperty({ example: "participant-id-123" }) @IsString() participant_id!: string;
@ApiProperty({ example: "user-id-123", required: false }) @IsString() @IsOptional() user_id?: string | null;
@ApiProperty({ example: "Jane Doe", required: false }) @IsString() @IsOptional() user_name?: string | null;
@ApiProperty({ example: 1678901234, required: false }) @IsNumber() @IsOptional() join_time?: number | null;
@ApiProperty({ example: 120, required: false }) @IsNumber() @IsOptional() duration?: number | null;
}
export class SessionDetailsItem {
@ApiProperty({ example: "session-id-123" }) @IsString() id!: string;
@ApiProperty({ example: 1678901234, required: false }) @IsNumber() @IsOptional() start_time?: number | null;
@ApiProperty({ example: 3600, required: false }) @IsNumber() @IsOptional() duration?: number | null;
@ApiProperty({ example: 10, required: false }) @IsNumber() @IsOptional() max_participants?: number | null;
@ApiProperty({ example: true, required: false }) @IsBoolean() @IsOptional() ongoing?: boolean;
@ApiProperty({ type: [MeetingParticipant], required: false })
@ValidateNested({ each: true })
@Type(() => MeetingParticipant)
@IsArray()
@IsOptional()
participants?: MeetingParticipant[] | null;
}
export class GetBookingSessionDetailsOutput {
@ApiProperty({ example: SUCCESS_STATUS, enum: [SUCCESS_STATUS, ERROR_STATUS] })
@IsEnum([SUCCESS_STATUS, ERROR_STATUS])
status!: typeof SUCCESS_STATUS | typeof ERROR_STATUS;
error?: Error;
@ValidateNested({ each: true })
@Type(() => SessionDetailsItem)
data!: SessionDetailsItem[] | null;
}Finally, export the new file from:
- packages/platform/types/bookings/2024-08-13/outputs/index.ts
(add: export * from "./get-booking-session-details.output";)
Reason: no existing session-details output type was found in @calcom/platform-types; keeping the API response typed and normalizing undefined -> null avoids leaking undefined and makes clients' handling explicit.
| async getSessionDetails(bookingUid: string) { | ||
| const booking = await this.bookingsRepository.getByUidWithBookingReference(bookingUid); | ||
| if (!booking) { | ||
| throw new NotFoundException(`Booking with uid=${bookingUid} was not found in the database`); | ||
| } | ||
|
|
||
| const roomName = | ||
| booking?.references?.filter((reference) => reference.type === "daily_video")?.pop()?.meetingId ?? | ||
| undefined; | ||
|
|
||
| if (!roomName) { | ||
| throw new NotFoundException(`No Cal Video reference found with booking uid ${bookingUid}`); | ||
| } | ||
|
|
||
| const sessionDetails = await getSessionDetailsFromRoomName(roomName); | ||
|
|
||
| return sessionDetails; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Handle missing session details gracefully and DRY the roomName extraction
- The helper getSessionDetailsFromRoomName may return undefined (e.g., provider not installed). Return a normalized value and log a warning to aid observability.
- The roomName extraction logic is duplicated across getRecordings, getTranscripts, and getSessionDetails. Extract a private helper to reduce duplication and potential drift.
Apply this diff to improve resilience of getSessionDetails:
async getSessionDetails(bookingUid: string) {
const booking = await this.bookingsRepository.getByUidWithBookingReference(bookingUid);
if (!booking) {
throw new NotFoundException(`Booking with uid=${bookingUid} was not found in the database`);
}
const roomName =
booking?.references?.filter((reference) => reference.type === "daily_video")?.pop()?.meetingId ??
undefined;
if (!roomName) {
throw new NotFoundException(`No Cal Video reference found with booking uid ${bookingUid}`);
}
- const sessionDetails = await getSessionDetailsFromRoomName(roomName);
-
- return sessionDetails;
+ const sessionDetails = await getSessionDetailsFromRoomName(roomName);
+ if (!sessionDetails) {
+ this.logger.warn(
+ `No session details returned for roomName=${roomName} (bookingUid=${bookingUid})`
+ );
+ return null;
+ }
+ return sessionDetails;
}And add this private helper (outside this hunk) to avoid repetition across methods:
// inside CalVideoService class
private getLatestCalVideoRoomName(booking: { references?: Array<{ type: string; meetingId?: string }> }) {
return booking?.references?.filter((r) => r.type === "daily_video")?.pop()?.meetingId ?? undefined;
}Then refactor getRecordings/getTranscripts/getSessionDetails to call this.getLatestCalVideoRoomName(booking) instead of repeating the filter/pop chain.
Security note: Ensure the session details object does not include sensitive fields (e.g., API keys, JWTs, share tokens). Map the returned details to a safe DTO before returning to the controller if needed.
To validate the shape and sensitivity of the data returned by the video client, and the authorization semantics of the guard:
If you want, I can follow through with a PR-wide refactor to add the helper and a typed, sanitized DTO for session details.
🏁 Script executed:
#!/bin/bash
# Inspect the video client implementation of getSessionDetailsFromRoomName
fd -a 'videoClient.ts'
rg -n -A 40 -B 10 'getSessionDetailsFromRoomName' packages/lib/videoClient.ts
# Check for any tokens/keys in the returned meeting information fields
rg -n -A 5 -B 5 'getMeetingInformation' packages/lib/videoClient.ts
# Review the BookingUidGuard to confirm access control for these endpoints
fd -a 'booking-uid.guard.ts'
rg -n -A 40 -B 10 '' apps/api/v2/src/ee/bookings/2024-08-13/guards/booking-uid.guard.tsLength of output: 2653
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- show cal-video.service.ts (first 300 lines) ----"
sed -n '1,300p' apps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.ts || true
echo
echo "---- locate the three methods in cal-video.service.ts ----"
rg -n -A 10 -B 10 'getRecordings|getTranscripts|getSessionDetails' apps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.ts || true
echo
echo "---- logger / Logger occurrences in cal-video.service.ts ----"
rg -n 'logger|Logger' apps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.ts || true
echo
echo "---- occurrences of daily_video across repo ----"
rg -n 'daily_video' || true
echo
echo "---- occurrences of getMeetingInformation across repo ----"
rg -n 'getMeetingInformation' -S || trueLength of output: 16563
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- locate getMeetingInformation in adapter ----"
rg -n "getMeetingInformationResponseSchema|getMeetingInformation:" packages/app-store/dailyvideo/lib/VideoApiAdapter.ts -n -S || true
echo
echo "---- show surrounding lines for schema + implementation ----"
sed -n '1,260p' packages/app-store/dailyvideo/lib/VideoApiAdapter.ts || true
sed -n '500,760p' packages/app-store/dailyvideo/lib/VideoApiAdapter.ts || true
echo
echo "---- VideoApiAdapter types ----"
sed -n '1,240p' packages/types/VideoApiAdapter.d.ts || true
echo
echo "---- videoClient getSessionDetailsFromRoomName (context) ----"
sed -n '400,460p' packages/lib/videoClient.ts || true
echo
echo "---- search for sensitive field names in adapter + videoClient ----"
rg -n --hidden -S 'api|key|token|secret|jwt|password|share_token|access_token|delegationCredentialId' packages/app-store/dailyvideo/lib/VideoApiAdapter.ts packages/lib/videoClient.ts || trueLength of output: 17819
Handle missing session details gracefully and DRY the roomName extraction — verified
Quick summary (verified):
- getSessionDetailsFromRoomName can return undefined when the video provider isn't installed (packages/lib/videoClient.ts — getDailyAppKeys catch returns without a value).
- CalVideoService already exposes a Logger (private readonly logger = new Logger("CalVideoService")).
- The roomName extraction is duplicated in getRecordings, getTranscripts and getSessionDetails in apps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.ts.
Files to change
- apps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.ts
- Refactor getRecordings, getTranscripts, getSessionDetails to use a single helper for roomName extraction.
- Make getSessionDetails resilient to missing/empty session details and errors from the video client.
Suggested patch for getSessionDetails (apply similar roomName helper use in the other two methods):
async getSessionDetails(bookingUid: string) {
const booking = await this.bookingsRepository.getByUidWithBookingReference(bookingUid);
if (!booking) {
throw new NotFoundException(`Booking with uid=${bookingUid} was not found in the database`);
}
- const roomName =
- booking?.references?.filter((reference) => reference.type === "daily_video")?.pop()?.meetingId ??
- undefined;
-
- if (!roomName) {
- throw new NotFoundException(`No Cal Video reference found with booking uid ${bookingUid}`);
- }
-
- const sessionDetails = await getSessionDetailsFromRoomName(roomName);
-
- return sessionDetails;
+ const roomName =
+ booking?.references?.filter((reference) => reference.type === "daily_video")?.pop()?.meetingId ??
+ undefined;
+
+ if (!roomName) {
+ throw new NotFoundException(`No Cal Video reference found with booking uid ${bookingUid}`);
+ }
+
+ try {
+ const sessionDetails = await getSessionDetailsFromRoomName(roomName);
+ if (!sessionDetails) {
+ this.logger.warn(`No session details returned for roomName=${roomName} (bookingUid=${bookingUid})`);
+ return null;
+ }
+ return sessionDetails;
+ } catch (err) {
+ this.logger.warn(
+ `Error fetching session details for roomName=${roomName} (bookingUid=${bookingUid}): ${err?.message ?? err}`
+ );
+ return null;
+ }
}Add this private helper (inside CalVideoService) and use it in all three methods to remove duplication:
private getLatestCalVideoRoomName(booking: { references?: Array<{ type: string; meetingId?: string }> }) {
return booking?.references?.filter((r) => r.type === "daily_video")?.pop()?.meetingId ?? undefined;
}Security note (verified):
- The Daily adapter’s getMeetingInformation response (packages/app-store/dailyvideo/lib/VideoApiAdapter.ts — getMeetingInformationResponseSchema) contains meeting/session metadata (id, room, start_time, duration, participants, etc.) and does not include API keys or tokens. Still, map the returned value to a safe DTO before returning from the service if this endpoint is exposed externally — defense-in-depth.
Recommendation: apply the helper + defensive handling (try/catch + null-return + logger.warn) across the three methods. I confirmed the conditions that motivated these changes, so the suggested refactor is actionable.
🤖 Prompt for AI Agents
In apps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.ts around
lines 85-102, extract the duplicated roomName logic into a private helper named
getLatestCalVideoRoomName(booking) that returns the latest daily_video
meetingId, replace the inline extraction in getRecordings, getTranscripts and
getSessionDetails with that helper, and make getSessionDetails resilient: keep
the NotFoundException when the roomName is missing, but wrap the call to
getSessionDetailsFromRoomName in a try/catch, log a warning via this.logger.warn
on errors or when the returned sessionDetails is undefined/empty, and return
null (or undefined consistently) instead of propagating the video-client error;
apply the same defensive pattern (try/catch + this.logger.warn + graceful
null-return) to getRecordings and getTranscripts where they call the video
client.
| "responses": { | ||
| "200": { | ||
| "description": "", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "object" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Document error responses (401/403/404) surfaced by the controller/service.
Per the implementation, a missing or mismatched video reference throws NotFound. Auth is typically required for v2 bookings. Please expose these in the contract to prevent client-side surprises.
Proposed additive responses:
"responses": {
"200": {
"description": "Successfully retrieved session details for the booking.",
"content": {
"application/json": {
"schema": {
"type": "object",
"required": ["status", "data"],
"properties": {
"status": {
"type": "string",
"enum": ["success"],
"description": "Indicates the request succeeded"
},
"data": {
"type": "object",
"description": "Provider-specific session details (e.g., Daily meeting information)",
"additionalProperties": true
}
},
"additionalProperties": false
},
"examples": {
"success": {
"summary": "Example response",
"value": {
"status": "success",
"data": {
"roomName": "example-room",
"start_time": "2024-08-13T10:00:00Z"
}
}
}
}
}
}
+ },
+ "401": {
+ "description": "Unauthorized – missing or invalid credentials"
+ },
+ "403": {
+ "description": "Forbidden – authenticated but not permitted to view this booking"
+ },
+ "404": {
+ "description": "Not Found – booking not found or no associated session details"
}
},Follow-up:
- Align the security requirement with adjacent bookings endpoints (e.g., transcripts). If those use a security scheme, add the same here. Can you confirm the scheme name used in this spec (e.g., BearerAuth) so we can add it?
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "----- Displaying lines ~7950-7990 from apps/api/v2/swagger/documentation.json -----"
if [ -f apps/api/v2/swagger/documentation.json ]; then
sed -n '7950,7990p' apps/api/v2/swagger/documentation.json || true
else
echo "File not found: apps/api/v2/swagger/documentation.json"
fi
echo
echo "----- Search for security occurrences in the spec file -----"
rg -n '"security"' apps/api/v2/swagger/documentation.json || true
echo
echo "----- Search for Bearer / BearerAuth / bearerAuth in repo -----"
rg -n 'BearerAuth|bearerAuth|\"bearer' -S || true
echo
echo "----- Search for '/bookings' paths in the spec file -----"
rg -n '\"/bookings' apps/api/v2/swagger/documentation.json || true
echo
echo "----- Show contexts for paths that include '/bookings' (±5 lines) -----"
rg -n '\"/bookings' apps/api/v2/swagger/documentation.json -n -A5 -B5 || true
echo
echo "----- Search for 'transcript' or 'transcripts' in repo -----"
rg -n 'transcript' -S || true
echo
echo "----- Search for explicit 401/403/404 response entries in the spec file -----"
rg -n '"401"|"403"|"404"' apps/api/v2/swagger/documentation.json || trueLength of output: 136279
Document 401/403/404 responses + add security for the booking session endpoint
Verified: apps/api/v2/swagger/documentation.json (near lines 7964–7975) only declares a 200 response for the bookings endpoint and contains no explicit "401"/"403"/"404" or "security" entries. The v2 controller/service for bookings/transcripts exist (api/v2/src/ee/bookings/... controllers/bookings.controller.ts and services/cal-video.service.ts), so surface errors and auth in the spec to avoid client surprises.
Files to update:
- apps/api/v2/swagger/documentation.json — lines ~7964-7975 (the responses block for the booking session endpoint)
- Confirm security alignment with adjacent bookings endpoints (e.g., /v2/bookings/{bookingUid}/transcripts in the same spec)
Proposed additive responses (apply to the responses block shown in the PR):
"responses": {
"200": {
"description": "Successfully retrieved session details for the booking.",
"content": {
"application/json": {
"schema": {
"type": "object",
"required": ["status", "data"],
"properties": {
"status": {
"type": "string",
"enum": ["success"],
"description": "Indicates the request succeeded"
},
"data": {
"type": "object",
"description": "Provider-specific session details (e.g., Daily meeting information)",
"additionalProperties": true
}
},
"additionalProperties": false
},
"examples": {
"success": {
"summary": "Example response",
"value": {
"status": "success",
"data": {
"roomName": "example-room",
"start_time": "2024-08-13T10:00:00Z"
}
}
}
}
}
}
+ },
+ "401": {
+ "description": "Unauthorized – missing or invalid credentials"
+ },
+ "403": {
+ "description": "Forbidden – authenticated but not permitted to view this booking"
+ },
+ "404": {
+ "description": "Not Found – booking not found or no associated session details"
}
},Follow-ups:
- Add the same security requirement used elsewhere for bookings. I couldn't locate a "security" or "securitySchemes" declaration in apps/api/v2/swagger/documentation.json during verification — can you confirm the scheme name in this spec (e.g., BearerAuth / bearerAuth) so we can add it here?
- While updating, verify the controller/service error shapes (NotFound/Unauthorized) and add example error bodies if you want richer client contracts.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "responses": { | |
| "200": { | |
| "description": "", | |
| "content": { | |
| "application/json": { | |
| "schema": { | |
| "type": "object" | |
| } | |
| } | |
| } | |
| } | |
| }, | |
| "responses": { | |
| "200": { | |
| "description": "Successfully retrieved session details for the booking.", | |
| "content": { | |
| "application/json": { | |
| "schema": { | |
| "type": "object", | |
| "required": ["status", "data"], | |
| "properties": { | |
| "status": { | |
| "type": "string", | |
| "enum": ["success"], | |
| "description": "Indicates the request succeeded" | |
| }, | |
| "data": { | |
| "type": "object", | |
| "description": "Provider-specific session details (e.g., Daily meeting information)", | |
| "additionalProperties": true | |
| } | |
| }, | |
| "additionalProperties": false | |
| }, | |
| "examples": { | |
| "success": { | |
| "summary": "Example response", | |
| "value": { | |
| "status": "success", | |
| "data": { | |
| "roomName": "example-room", | |
| "start_time": "2024-08-13T10:00:00Z" | |
| } | |
| } | |
| } | |
| } | |
| } | |
| } | |
| }, | |
| "401": { | |
| "description": "Unauthorized – missing or invalid credentials" | |
| }, | |
| "403": { | |
| "description": "Forbidden – authenticated but not permitted to view this booking" | |
| }, | |
| "404": { | |
| "description": "Not Found – booking not found or no associated session details" | |
| } | |
| }, |
🤖 Prompt for AI Agents
In apps/api/v2/swagger/documentation.json around lines 7964–7975, the booking
session endpoint only declares a 200 response and lacks 401/403/404 responses
and a security requirement; add explicit 401 (Unauthorized), 403 (Forbidden) and
404 (Not Found) response entries under the "responses" block with brief
descriptions and reference the existing error schema (or a new standard error
schema used elsewhere in the spec) for each response, and add a "security" array
for the endpoint that matches the other bookings endpoints (e.g., [ {
"BearerAuth": [] } ] or [ { "bearerAuth": [] } ] depending on the scheme name
used in this spec); confirm the exact security scheme name by inspecting the
top-level "components/securitySchemes" section and use that exact name so the
endpoint aligns with /v2/bookings/{bookingUid}/transcripts and the rest of the
spec.
| "whatsapp_number", | ||
| "cal_ai_phone_call" | ||
| ] |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add action semantics and payload constraints for cal_ai_phone_call.
New enum value added without documenting what fields are required/used when this action is selected (e.g., target phone number, locale, TTS voice). Without a discriminator or conditional schema, client SDKs won’t know how to construct a valid payload.
Recommendations:
- Introduce a discriminator mapping (oneOf) for workflow steps keyed by
action, defining the shape for"cal_ai_phone_call"alongside existing actions. - Document required fields and add an example for this action.
- Ensure server-side validation mirrors the spec.
If you want, I can draft the OpenAPI oneOf for this component once you confirm the exact fields expected for the AI phone call step.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Searching repository for 'cal_ai_phone_call' ==="
rg -n "cal_ai_phone_call" --hidden || true
echo
echo "=== Searching swagger doc for 'cal_ai_phone_call' ==="
rg -n "cal_ai_phone_call" apps/api/v2/swagger/documentation.json -n -S || true
echo
echo "=== Searching for workflow schema files and related symbols (workflow, action, agentId) ==="
rg -n "workflows" --hidden || true
rg -n "agentId" --hidden || true
rg -n "workflow" --hidden || true
echo
echo "=== Show packages/features/ee/workflows/lib/schema.ts (if present) ==="
if [ -f packages/features/ee/workflows/lib/schema.ts ]; then
sed -n '1,240p' packages/features/ee/workflows/lib/schema.ts
else
echo "packages/features/ee/workflows/lib/schema.ts not found"
fi
echo
echo "=== Show snippet around lines 21900-21910 in swagger doc ==="
if [ -f apps/api/v2/swagger/documentation.json ]; then
sed -n '21890,21910p' apps/api/v2/swagger/documentation.json || true
else
echo "apps/api/v2/swagger/documentation.json not found"
fiLength of output: 271089
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Show swagger snippet around first occurrence (21890-21910) ==="
sed -n '21880,21920p' apps/api/v2/swagger/documentation.json || true
echo
echo "=== Show swagger snippets around other occurrences (22260-22280, 22340-22360) ==="
sed -n '22260,22280p' apps/api/v2/swagger/documentation.json || true
sed -n '22340,22360p' apps/api/v2/swagger/documentation.json || true
echo
echo "=== Show workflow step input file ==="
sed -n '1,240p' apps/api/v2/src/modules/workflows/inputs/workflow-step.input.ts || true
echo
echo "=== Show workflow update schema (might influence openapi) ==="
sed -n '1,320p' apps/api/v2/src/modules/workflows/inputs/update-workflow.input.ts || true
echo
echo "=== Show workflows types in features (types file) ==="
rg -n "type WorkflowStep" packages --hidden || true
sed -n '1,240p' packages/features/ee/workflows/lib/types.ts || true
echo
echo "=== Show calAIPhone workspace files that mention required fields (search for 'phone' 'locale' 'agentId' near cal_ai_phone_call) ==="
rg -n "cal_ai_phone_call" -n -S apps packages || true
rg -n "phoneNumber|phone_number|locale|tts|voice|agentId" apps packages -S || trueLength of output: 263779
Action needed: add OpenAPI schema + discriminator for cal_ai_phone_call workflow step
The new enum value "cal_ai_phone_call" is present in apps/api/v2/swagger/documentation.json but there is no corresponding DTO/oneOf mapping, so generated OpenAPI clients won’t know the payload shape or required fields.
Please address (definitive locations to change):
- apps/api/v2/src/modules/workflows/inputs/workflow-step.input.ts
- Add a WorkflowCalAiPhoneCallStepDto (extend BaseWorkflowStepDto) and declare action = CAL_AI_PHONE_CALL. Include the AI-call-specific fields (e.g., phone resource / verifiedPhoneId or sendTo, optional agentId, locale and voice/tts/model options as applicable). Mark agentId as optional/nullable.
- apps/api/v2/src/modules/workflows/inputs/update-workflow.input.ts
- Register the UpdateCalAiPhoneCallWorkflowStepDto in ApiExtraModels and add it to the steps discriminator subTypes / oneOf so the OpenAPI output includes the concrete schema for action = "cal_ai_phone_call".
- apps/api/v2/src/modules/workflows/inputs/create-workflow.input.ts (if present) — mirror the same create-step DTO and mappings.
- trpc/server/routers/viewer/workflows/update.handler.ts (and related validation/handlers) — ensure server-side validation accepts the new step shape and treats agentId as nullable (see note below), and implements any linking logic that creates/associates an agent after the step is saved.
- Regenerate the swagger JSON (apps/api/v2/swagger/documentation.json) and add a clear example for the cal_ai_phone_call step.
Note: per internal guidance (Udit‑takkar) agentId can legitimately be null during initial setup (workflow step saved first, agent created/linked later). Ensure the DTO allows agentId: string | null to support this two‑step flow.
If you confirm the exact fields you want exposed for the AI phone call step, I can draft the OpenAPI oneOf / example DTO for you.
🤖 Prompt for AI Agents
In apps/api/v2/swagger/documentation.json around lines 21901-21903 the enum
gained "cal_ai_phone_call" but there is no corresponding DTO/discriminator
mapping; add a new WorkflowCalAiPhoneCallStepDto (extends BaseWorkflowStepDto)
in apps/api/v2/src/modules/workflows/inputs/workflow-step.input.ts with action =
CAL_AI_PHONE_CALL and fields: phone resource reference (e.g., verifiedPhoneId or
sendTo), optional/nullable agentId: string | null, locale, and voice/tts/model
options as applicable; then register the UpdateCalAiPhoneCallWorkflowStepDto in
apps/api/v2/src/modules/workflows/inputs/update-workflow.input.ts via
ApiExtraModels and include it in the steps discriminator subTypes/oneOf (and
mirror in create-workflow.input.ts if present) so OpenAPI emits the concrete
schema; update server-side validation/handlers in
trpc/server/routers/viewer/workflows/update.handler.ts to accept the new shape
and treat agentId as nullable and implement/linking logic to associate/create
agent after save; finally regenerate apps/api/v2/swagger/documentation.json and
add an explicit example for the cal_ai_phone_call step.
| const getSessionDetailsFromRoomName = async (roomName: string) => { | ||
| let dailyAppKeys: Awaited<ReturnType<typeof getDailyAppKeys>>; | ||
| try { | ||
| dailyAppKeys = await getDailyAppKeys(); | ||
| } catch (e) { | ||
| console.error("Error: Cal video provider is not installed."); | ||
| return; | ||
| } | ||
| const [videoAdapter] = await getVideoAdapters([ | ||
| { | ||
| id: 0, | ||
| appId: "daily-video", | ||
| type: "daily_video", | ||
| userId: null, | ||
| user: { email: "" }, | ||
| teamId: null, | ||
| key: dailyAppKeys, | ||
| invalid: false, | ||
| delegationCredentialId: null, | ||
| }, | ||
| ]); | ||
|
|
||
| return videoAdapter?.getMeetingInformation?.(roomName); | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify downstream API doesn’t leak secrets or PII
This helper returns the adapter’s getMeetingInformation output verbatim. Please verify the v2 endpoint that consumes this does not expose credential.key or any sensitive tokens in its response, per our guidelines.
Run the following script to:
- Confirm the re-export is used by the v2 service.
- Inspect service code returning session details.
- Surface any suspicious “key”/“token” fields around the new endpoint code.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Locate usages of getSessionDetailsFromRoomName:"
rg -n --hidden --glob '!**/node_modules/**' 'getSessionDetailsFromRoomName' -A 5 -B 3
echo
echo "2) Inspect the v2 cal-video service that calls it:"
rg -n --hidden --glob '!**/node_modules/**' 'getSessionDetails\(|session-details' -A 12 -B 6
echo
echo "3) Check that the function is re-exported from the platform libraries:"
rg -n --hidden --glob '!**/node_modules/**' 'packages/platform/libraries/conferencing\.ts' -A 50 -B 2 | rg -n 'getSessionDetailsFromRoomName'
echo
echo "4) Scan the v2 bookings API code for potential secret fields in returned payloads (heuristic):"
rg -n --hidden --glob '!**/node_modules/**' 'apps/api/v2' -e '\b(key|secret|token|apiKey|accessToken)\b' -A 3 -B 3 || true
echo
echo "5) Try to find the VideoApiAdapter type to confirm getMeetingInformation is typed:"
fd -t f 'VideoApiAdapter' | sed -n '1,20p'
if fd -t f 'VideoApiAdapter' | head -n 1 | grep -q '.'; then
FILE=$(fd -t f 'VideoApiAdapter' | head -n 1)
echo "--- Showing excerpts from ${FILE} ---"
rg -n 'getMeetingInformation' "$FILE" -A 5 -B 5 || true
fiLength of output: 10183
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) All occurrences of getMeetingInformation:"
rg -n --hidden --glob '!**/node_modules/**' 'getMeetingInformation' -A 3 -B 3 || true
echo
echo "2) All occurrences of getVideoAdapters:"
rg -n --hidden --glob '!**/node_modules/**' 'getVideoAdapters' -A 5 -B 5 || true
echo
echo "3) All occurrences of getDailyAppKeys:"
rg -n --hidden --glob '!**/node_modules/**' 'getDailyAppKeys' -A 5 -B 5 || true
# If found, show the file contents around its definition (first match)
FIRST_DAILY=$(rg -n --hidden --glob '!**/node_modules/**' 'getDailyAppKeys' | head -n1 || true)
if [ -n "$FIRST_DAILY" ]; then
FILE=$(echo "$FIRST_DAILY" | cut -d: -f1)
echo "--- Showing excerpt from $FILE ---"
rg -n --hidden --glob '!**/node_modules/**' 'getDailyAppKeys' -n "$FILE" -A 10 -B 10 || true
sed -n '1,240p' "$FILE" || true
fi
echo
echo "4) Show packages/lib/videoClient.ts around getSessionDetailsFromRoomName (lines ~400-480):"
sed -n '390,480p' packages/lib/videoClient.ts || true
echo
echo "5) Show cal-video.service.ts getSessionDetails (lines ~1-220):"
sed -n '1,220p' apps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.ts || true
echo
echo "6) Show controller that returns session-details:"
sed -n '180,210p' apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts || true
echo
echo "7) Show the re-export file packages/platform/libraries/conferencing.ts:"
sed -n '1,240p' packages/platform/libraries/conferencing.ts || true
echo
echo "8) Check swagger docs for /v2/bookings/{bookingUid}/session-details response schema (around earlier match):"
sed -n '7928,7960p' apps/api/v2/swagger/documentation.json || true
echo
echo "9) Search repo for 'credential'/'credentials' and 'credential.key' usages:"
rg -n --hidden --glob '!**/node_modules/**' '\bcredential\b|\bcredentials\b|credential.key' -A 3 -B 3 || true
echo
echo "10) Search for 'daily_video' / 'daily-video' occurrences to locate adapters:"
rg -n --hidden --glob '!**/node_modules/**' 'daily_video|daily-video' -A 4 -B 4 || trueLength of output: 281087
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='packages/app-store/dailyvideo/lib/VideoApiAdapter.ts'
echo "---- Showing ${FILE} (lines 1-600) ----"
sed -n '1,600p' "$FILE" || true
echo
echo "---- Matching schema identifiers ----"
rg -n --hidden --glob '!**/node_modules/**' 'meetingSessionSchema|getMeetingInformationResponseSchema|meetingParticipantSchema' "$FILE" -A 5 -B 5 || trueLength of output: 19649
No credential/token leak found — adapter response is safe, but it exposes participant PII (user_name / user_id); please redact or confirm
Short summary:
- getSessionDetailsFromRoomName is re-exported and used by the v2 service (apps/api/v2/.../cal-video.service.ts) and returned directly from the controller.
- Daily adapter (packages/app-store/dailyvideo/lib/VideoApiAdapter.ts) parses /meetings with getMeetingInformationResponseSchema = { data: meetingSessionSchema } — meetingSessionSchema contains participants with user_id and user_name, but no credential/key/token fields.
- fetcher uses the server-side Daily api_key for Authorization (not returned to clients).
Files to review / change:
- apps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.ts — getSessionDetails currently returns adapter output verbatim.
- packages/lib/videoClient.ts — getSessionDetailsFromRoomName (returns adapter.getMeetingInformation result).
- packages/app-store/dailyvideo/lib/VideoApiAdapter.ts — getMeetingInformationResponseSchema (defines returned fields including participants.user_name/user_id).
Recommended action (one of):
- Redact PII before returning from CalVideoService.getSessionDetails (map to allowed fields or remove participants.user_name and user_id).
- Or, if returning participants is intended, add explicit docs and ensure clients are allowed to receive that PII.
Example fix (minimal, apply in CalVideoService.getSessionDetails):
- Map sessionDetails -> safeSessionDetails selecting only non-sensitive fields (e.g., id, room, start_time, duration, ongoing, max_participants, participants_count) and return that instead of the raw adapter response.
Tag:
🤖 Prompt for AI Agents
In packages/lib/videoClient.ts around lines 421 to 444,
getSessionDetailsFromRoomName currently returns the
adapter.getMeetingInformation result which includes participant PII (user_name
and user_id); before returning, strip or redact participant PII by mapping the
session object to a safeSessionDetails shape (e.g., keep id, room, start_time,
duration, ongoing, max_participants, participants_count and replace participants
array with items containing only non-PII like an anonymized id or count), then
return that safeSessionDetails instead of the raw adapter response so no
user_name or user_id leaves the server.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/api/v2/swagger/documentation.json (1)
8251-8261: Document 200 response shape + example; add standard error responses (401/403/404).The 200 response has an empty description and no example, and there are no explicit 401/403/404 responses even though the controller/service can surface them. Please make the contract clear and client-friendly.
Apply this diff to improve the responses block:
"responses": { "200": { - "description": "", + "description": "Successfully retrieved session details for the booking.", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/GetBookingSessionDetailsOutput" - } + }, + "examples": { + "success": { + "summary": "Example response", + "value": { + "status": "success", + "data": { + "data": [ + { + "id": "50455614-55b3-4e66-835d-102f562a5070", + "room": "k6ahwPBbTu2Qr5ZuCvn0", + "start_time": 1755161937, + "duration": 31, + "ongoing": false, + "max_participants": 1, + "participants": [ + { + "user_id": "8", + "participant_id": "f269d7f3-afce-4d39-8df6-69d134fe6af7", + "user_name": "Team Free Example", + "join_time": 1755161937, + "duration": 31 + } + ] + } + ] + } + } + } + } } } + }, + "401": { + "description": "Unauthorized – missing or invalid credentials" + }, + "403": { + "description": "Forbidden – authenticated but not permitted to view this booking" + }, + "404": { + "description": "Not Found – booking not found or no associated session details" } },
🧹 Nitpick comments (17)
packages/platform/types/bookings/2024-08-13/outputs/get-booking-session-details.output.ts (4)
69-73: Prefer IsIn over IsEnum for string literal unions; tighten status validationClass-validator’s IsEnum is intended for real enums/enum-like objects. For a string literal union, IsIn communicates intent better and avoids oddities with arrays. Swagger enum metadata can remain as-is.
Apply:
-import { IsArray, IsBoolean, IsEnum, IsNumber, IsOptional, IsString, ValidateNested } from "class-validator"; +import { IsArray, IsBoolean, IsIn, IsNumber, IsOptional, IsString, ValidateNested } from "class-validator";- @IsEnum([SUCCESS_STATUS, ERROR_STATUS]) + @IsIn([SUCCESS_STATUS, ERROR_STATUS])Also applies to: 3-3
12-15: Use UUID validation for IDs where applicableBoth participant_id and id look like UUIDs (per examples). Enforce with IsUUID to catch invalid data earlier.
-import { IsArray, IsBoolean, IsEnum, IsNumber, IsOptional, IsString, ValidateNested } from "class-validator"; +import { IsArray, IsBoolean, IsEnum, IsNumber, IsOptional, IsString, ValidateNested, IsUUID } from "class-validator";- @ApiProperty({ example: "f269d7f3-afce-4d39-8df6-69d134fe6af7" }) - @IsString() - participant_id!: string; + @ApiProperty({ example: "f269d7f3-afce-4d39-8df6-69d134fe6af7" }) + @IsUUID() + participant_id!: string;- @ApiProperty({ example: "50455614-55b3-4e66-835d-102f562a5070" }) - @IsString() - id!: string; + @ApiProperty({ example: "50455614-55b3-4e66-835d-102f562a5070" }) + @IsUUID() + id!: string;Also applies to: 30-33, 3-3
20-27: Clarify units in Swagger for time/duration fieldsMake it explicit these are Unix epoch seconds and durations in seconds to avoid misinterpretation by API consumers.
- @ApiProperty({ example: 1755161937 }) + @ApiProperty({ example: 1755161937, description: "Unix timestamp in seconds when the participant joined" }) @IsNumber() join_time!: number; - @ApiProperty({ example: 31 }) + @ApiProperty({ example: 31, description: "Duration in seconds the participant has been in the session" }) @IsNumber() duration!: number;- @ApiProperty({ example: 1755161937 }) + @ApiProperty({ example: 1755161937, description: "Unix timestamp in seconds when the session started" }) @IsNumber() start_time!: number; - @ApiProperty({ example: 31 }) + @ApiProperty({ example: 31, description: "Session duration in seconds" }) @IsNumber() duration!: number;Also applies to: 38-45
74-76: Document error shape or remove it; current type is opaque for consumerserror?: Error isn’t represented in Swagger and isn’t very useful to clients. Either remove it (prefer standard HTTP errors) or document a minimal wire shape (e.g., { message: string }).
Example (if you want to keep a documented error payload):
-import { ApiProperty } from "@nestjs/swagger"; +import { ApiProperty, ApiPropertyOptional } from "@nestjs/swagger";- @IsOptional() - error?: Error; + @ApiPropertyOptional({ + description: "Error details when status is 'error'", + example: { message: "Booking not found" }, + }) + @IsOptional() + error?: { message: string };If other outputs use a shared error DTO, align with that instead.
Also applies to: 1-1
docs/api-reference/v2/openapi.json (7)
7876-7877: Tighten the summary per guidelinesThe current summary is fine, but can be more concise. Suggest: “Get session details for a booking” (no period, American English).
Apply this diff:
- "summary": "Get the session details for the booking", + "summary": "Get session details for a booking",
7890-7896: Add a description for bookingUid path parameterImproves docs usability and consistency with other endpoints.
Apply this diff:
{ "name": "bookingUid", "required": true, "in": "path", + "description": "The unique UID of the booking", "schema": { "type": "string" } }
7898-7909: Fill 200 response description and consider standard error responses
- Empty description for 200 → use “OK” or “Successful response”.
- Consider adding 400 (invalid UID), 404 (booking not found), and 401/403 if auth applies, to mirror other Bookings endpoints.
Apply this diff to improve 200 description:
"200": { - "description": "", + "description": "OK", "content": {Optionally extend with standardized errors (adjust to your project’s common error schema if available):
"responses": { "200": { "description": "OK", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/GetBookingSessionDetailsOutput" } } } }, + "400": { + "description": "Invalid bookingUid" + }, + "404": { + "description": "Booking not found" + } },
25795-25820: Refine types and formats for participant fieldsUse integer types for timestamps/durations and add UUID format where applicable. Also clarifying units helps consumers.
Apply this diff:
"SessionParticipantItem": { "type": "object", "properties": { "user_id": { "type": "string", "example": "8" }, "participant_id": { - "type": "string", + "type": "string", + "format": "uuid", "example": "f269d7f3-afce-4d39-8df6-69d134fe6af7" }, "user_name": { "type": "string", "example": "Team Free Example" }, "join_time": { - "type": "number", + "type": "integer", + "format": "int64", + "description": "Unix timestamp in seconds when the participant joined", "example": 1755161937 }, "duration": { - "type": "number", + "type": "integer", + "format": "int32", + "description": "Duration in seconds the participant stayed in the session", "example": 31 } }, "required": ["user_id", "participant_id", "user_name", "join_time", "duration"] },
25821-25856: Refine types/formats and add minor constraints to session details
- Use integer for timestamps/duration and UUID format for id.
- max_participants is an integer.
- Adding minItems helps clarify array behavior.
Apply this diff:
"SessionDetailsItem": { "type": "object", "properties": { "id": { - "type": "string", + "type": "string", + "format": "uuid", "example": "50455614-55b3-4e66-835d-102f562a5070" }, "room": { "type": "string", "example": "k6ahwPBbTu2Qr5ZuCvn0" }, "start_time": { - "type": "number", + "type": "integer", + "format": "int64", + "description": "Unix timestamp in seconds when the session started", "example": 1755161937 }, "duration": { - "type": "number", + "type": "integer", + "format": "int32", + "description": "Duration in seconds for the session", "example": 31 }, "ongoing": { "type": "boolean", "example": false }, "max_participants": { - "type": "number", + "type": "integer", + "format": "int32", "example": 1 }, "participants": { "type": "array", + "minItems": 0, "items": { "$ref": "#/components/schemas/SessionParticipantItem" } } }, "required": ["id", "room", "start_time", "duration", "ongoing", "max_participants", "participants"] },
25857-25868: Optional: Clarify array wrapper semanticsIf consistent with other v2 endpoints, keep as is. Otherwise, consider flattening to return the array directly under data or add a short description. Also consider minItems to be explicit.
Apply this diff if you want to document/clarify:
"SessionDetailsDataWrapper": { "type": "object", "properties": { "data": { "type": "array", + "minItems": 0, + "description": "List of session details for the booking", "items": { "$ref": "#/components/schemas/SessionDetailsItem" } } }, "required": ["data"] },Please confirm whether other responses (e.g., transcripts) also embed arrays in a data wrapper to ensure consistency.
25869-25885: Consider documenting the error object shape or referencing a shared Error schemaIf the API has a standard error shape, reference it here for consistency and better DX.
If available, apply a change like:
"GetBookingSessionDetailsOutput": { "type": "object", "properties": { "status": { "type": "string", "example": "success", "enum": ["success", "error"] }, "data": { "$ref": "#/components/schemas/SessionDetailsDataWrapper" }, "error": { - "type": "object" + "$ref": "#/components/schemas/Error" } }, "required": ["status", "data"] },apps/api/v2/swagger/documentation.json (6)
8231-8240: Constrain header to the fixed API version and provide an example.Make the version contract self-documenting by constraining the header value to the known version and adding an example.
{ "name": "cal-api-version", "in": "header", - "description": "Must be set to 2024-08-13", + "description": "API version; must be set to 2024-08-13", "required": true, "schema": { - "type": "string", - "default": "2024-08-13" - } + "type": "string", + "enum": ["2024-08-13"] + }, + "example": "2024-08-13" },
8241-8248: Enrich bookingUid param with description and example.Helps client SDKs and readers.
{ "name": "bookingUid", "required": true, "in": "path", "schema": { "type": "string" - } + }, + "description": "Unique identifier of the booking", + "example": "abCDeFgH123" }
28247-28278: Use integer types with formats for time/duration fields; consider additionalProperties=false.
- join_time and duration should be integers (seconds), not generic numbers. Add format "int64" to avoid float clients.
- Consider
"additionalProperties": falseto lock the schema."SessionParticipantItem": { "type": "object", "properties": { "user_id": { "type": "string", "example": "8" }, "participant_id": { "type": "string", "example": "f269d7f3-afce-4d39-8df6-69d134fe6af7" }, "user_name": { "type": "string", "example": "Team Free Example" }, "join_time": { - "type": "number", + "type": "integer", + "format": "int64", "example": 1755161937 }, "duration": { - "type": "number", + "type": "integer", + "format": "int64", "example": 31 } }, "required": [ "user_id", "participant_id", "user_name", "join_time", "duration" - ] + ], + "additionalProperties": false },
28280-28322: Tighten types and add formats; avoid floats for counts/epochs.
- start_time/duration are epochs and lengths in seconds: use integer int64.
- max_participants should be integer.
- Consider
format: uuidfor id if always a UUID; if provider IDs may vary, skip this.- Add
"additionalProperties": false."SessionDetailsItem": { "type": "object", "properties": { "id": { "type": "string", "example": "50455614-55b3-4e66-835d-102f562a5070" + /* Optionally add: "format": "uuid" */ }, "room": { "type": "string", "example": "k6ahwPBbTu2Qr5ZuCvn0" }, "start_time": { - "type": "number", + "type": "integer", + "format": "int64", "example": 1755161937 }, "duration": { - "type": "number", + "type": "integer", + "format": "int64", "example": 31 }, "ongoing": { "type": "boolean", "example": false }, "max_participants": { - "type": "number", + "type": "integer", + "format": "int32", "example": 1 }, "participants": { "type": "array", "items": { "$ref": "#/components/schemas/SessionParticipantItem" } } }, "required": [ "id", "room", "start_time", "duration", "ongoing", "max_participants", "participants" - ] + ], + "additionalProperties": false },If "id" might not always be a UUID across providers, keep it as string without the format. Otherwise, add
"format": "uuid".
28323-28336: Lock down wrapper schema and consider naming/shape consistency with other v2 outputs.
- Add
"additionalProperties": false.- Optional: If other v2 outputs avoid nested data.data, consider aligning; otherwise, keep as-is for consistency with transcripts.
"SessionDetailsDataWrapper": { "type": "object", "properties": { "data": { "type": "array", "items": { "$ref": "#/components/schemas/SessionDetailsItem" } } }, "required": [ "data" - ] + ], + "additionalProperties": false },If transcripts use
{ "data": { "data": [...] } }, this is consistent; otherwise, consider flattening to avoid data.data nesting.
28337-28359: Avoid mixing success and error shapes in one schema; prefer oneOf or 4xx for errors.
GetBookingSessionDetailsOutputincludesstatus: "success" | "error"and an optionalerror, but the 200 response only documents success. To prevent ambiguous clients:
- Either remove
errorfrom the success schema and document 4xx responses (preferred), or- Use
oneOfto define separate success and error variants.Option A (keep 200 = success-only; remove error, add additionalProperties and example):
"GetBookingSessionDetailsOutput": { "type": "object", "properties": { "status": { "type": "string", "example": "success", "enum": [ "success", - "error" + "success" ] }, "data": { "$ref": "#/components/schemas/SessionDetailsDataWrapper" - }, - "error": { - "type": "object" } }, "required": [ "status", "data" - ] + ], + "additionalProperties": false, + "example": { + "status": "success", + "data": { + "data": [] + } + } },Option B (if you need to keep a combined type, define oneOf with explicit error shape and mark data/error appropriately).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts(2 hunks)apps/api/v2/swagger/documentation.json(2 hunks)docs/api-reference/v2/openapi.json(2 hunks)packages/platform/types/bookings/2024-08-13/outputs/get-booking-session-details.output.ts(1 hunks)packages/platform/types/bookings/2024-08-13/outputs/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts
🧰 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 useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/platform/types/bookings/2024-08-13/outputs/index.tspackages/platform/types/bookings/2024-08-13/outputs/get-booking-session-details.output.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/platform/types/bookings/2024-08-13/outputs/index.tspackages/platform/types/bookings/2024-08-13/outputs/get-booking-session-details.output.ts
docs/api-reference/v2/openapi.json
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
When docs changes are made in
/docs/api-reference/v2/openapi.json, ensure 'summary' fields are short, concise, do not end with a period, and are written in proper American English
Files:
docs/api-reference/v2/openapi.json
🧠 Learnings (8)
📚 Learning: 2025-08-14T10:30:23.029Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/lib/schema.ts:41-43
Timestamp: 2025-08-14T10:30:23.029Z
Learning: In calcom/cal.com workflows with CAL_AI_PHONE_CALL actions, agentId can legitimately be null during initial setup. The workflow is: 1) save workflow to get step ID, 2) use step ID to create and link agent. Therefore, agentId should remain nullable in schemas to support this multi-step setup process (per maintainer Udit-takkar).
Applied to files:
apps/api/v2/swagger/documentation.json
📚 Learning: 2025-08-15T00:07:30.027Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/workflows/update.handler.ts:738-763
Timestamp: 2025-08-15T00:07:30.027Z
Learning: In calcom/cal.com workflows, Cal AI phone call actions (CAL_AI_PHONE_CALL) intentionally always require the phone number field when the action is present, unlike SMS/WhatsApp actions which respect the step.numberRequired flag. This is the intended behavior per maintainer Udit-takkar in PR #22995.
Applied to files:
apps/api/v2/swagger/documentation.json
📚 Learning: 2025-08-08T09:29:11.681Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.681Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Applied to files:
apps/api/v2/swagger/documentation.json
📚 Learning: 2025-08-08T10:26:13.362Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
Applied to files:
apps/api/v2/swagger/documentation.json
📚 Learning: 2025-08-14T10:48:52.556Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/ai/_router.ts:46-84
Timestamp: 2025-08-14T10:48:52.556Z
Learning: In calcom/cal.com PR #22995, packages/trpc/server/routers/viewer/ai/_router.ts, the voiceId input parameter in the create endpoint is intentionally not forwarded to aiService.createAgent() as voice customization is not required at the moment (per maintainer Udit-takkar). Future reviews should not flag this as missing functionality unless requirements change.
Applied to files:
apps/api/v2/swagger/documentation.json
📚 Learning: 2025-08-08T09:27:23.896Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.896Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Applied to files:
apps/api/v2/swagger/documentation.json
📚 Learning: 2025-08-15T00:27:33.099Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.099Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
Applied to files:
apps/api/v2/swagger/documentation.json
📚 Learning: 2025-08-14T23:07:45.240Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:312-319
Timestamp: 2025-08-14T23:07:45.240Z
Learning: In calcom/cal.com phone number import flow, an agent must always be present when importing a phone number. The validateAgentPermissions function in PhoneNumberService should throw a 400 error when agentId is missing, as this is a business requirement rather than optional functionality (per maintainer Udit-takkar).
Applied to files:
apps/api/v2/swagger/documentation.json
🧬 Code Graph Analysis (1)
packages/platform/types/bookings/2024-08-13/outputs/get-booking-session-details.output.ts (1)
packages/platform/constants/api.ts (2)
SUCCESS_STATUS(9-9)ERROR_STATUS(10-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
packages/platform/types/bookings/2024-08-13/outputs/get-booking-session-details.output.ts (1)
54-59: Nested array validation and transformation are correctGood use of IsArray + ValidateNested({ each: true }) + Type(() => SessionParticipantItem) to ensure participants are properly validated and transformed.
packages/platform/types/bookings/2024-08-13/outputs/index.ts (1)
6-6: Barrel export added correctlyRe-exporting get-booking-session-details.output ensures consumers can import the new DTO from the existing outputs barrel.
docs/api-reference/v2/openapi.json (1)
7873-7912: Endpoint addition looks good; aligns with PR objectiveThe new GET /v2/bookings/{bookingUid}/session-details path, parameters, and response wiring are consistent with the described controller/service changes. Tagging and operationId match conventions.
apps/api/v2/swagger/documentation.json (1)
8251-8265: Incorrect — spec doesn't use OpenAPI "security" arrays; it uses Authorization header paramsI inspected apps/api/v2/swagger/documentation.json: there are no components.securitySchemes and no operation-level "security" arrays. Many endpoints instead mention or include an "Authorization" header parameter. Adding an operation-level security entry here would be inconsistent unless you also add a components.securitySchemes and update other endpoints.
Paths to note:
- apps/api/v2/swagger/documentation.json — GET /v2/bookings/{bookingUid}/session-details (lines ~8225-8265) — currently no "security".
- apps/api/v2/swagger/documentation.json — GET /v2/bookings/{bookingUid}/transcripts (lines ~8267-8307) — also no "security".
Recommended options (pick one for consistency):
- Add a components.securitySchemes entry (e.g., a bearer scheme) and then add the matching "security": [{ "": [] }] to all bookings operations.
- Or keep the existing pattern and add the Authorization header parameter to this operation (or $ref the shared Authorization parameter) so it matches other endpoints.
Likely an incorrect or invalid review comment.
| "SessionParticipantItem": { | ||
| "type": "object", | ||
| "properties": { | ||
| "user_id": { | ||
| "type": "string", | ||
| "example": "8" | ||
| }, | ||
| "participant_id": { | ||
| "type": "string", | ||
| "example": "f269d7f3-afce-4d39-8df6-69d134fe6af7" | ||
| }, | ||
| "user_name": { | ||
| "type": "string", | ||
| "example": "Team Free Example" | ||
| }, | ||
| "join_time": { | ||
| "type": "number", | ||
| "example": 1755161937 | ||
| }, | ||
| "duration": { | ||
| "type": "number", | ||
| "example": 31 | ||
| } | ||
| }, | ||
| "required": ["user_id", "participant_id", "user_name", "join_time", "duration"] | ||
| }, | ||
| "SessionDetailsItem": { | ||
| "type": "object", | ||
| "properties": { | ||
| "id": { | ||
| "type": "string", | ||
| "example": "50455614-55b3-4e66-835d-102f562a5070" | ||
| }, | ||
| "room": { | ||
| "type": "string", | ||
| "example": "k6ahwPBbTu2Qr5ZuCvn0" | ||
| }, | ||
| "start_time": { | ||
| "type": "number", | ||
| "example": 1755161937 | ||
| }, | ||
| "duration": { | ||
| "type": "number", | ||
| "example": 31 | ||
| }, | ||
| "ongoing": { | ||
| "type": "boolean", | ||
| "example": false | ||
| }, | ||
| "max_participants": { | ||
| "type": "number", | ||
| "example": 1 | ||
| }, | ||
| "participants": { | ||
| "type": "array", | ||
| "items": { | ||
| "$ref": "#/components/schemas/SessionParticipantItem" | ||
| } | ||
| } | ||
| }, | ||
| "required": ["id", "room", "start_time", "duration", "ongoing", "max_participants", "participants"] | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify field naming convention (snake_case vs camelCase) across v2
This schema uses snake_case (e.g., start_time). Ensure it matches the existing v2 style to avoid inconsistencies for API consumers.
🏁 Script executed:
#!/bin/bash
# Check for both snake_case and camelCase timestamp fields in v2 OpenAPI to gauge consistency
set -euo pipefail
echo "Occurrences of snake_case time fields:"
rg -n -C1 --json '"start_time"| "join_time"| "max_participants"' docs/api-reference/v2/openapi.json | jq -r '.data.path.text,.data.lines.text' | sed '/^{/d'
echo
echo "Occurrences of camelCase time fields:"
rg -n -C1 --json '"startTime"| "joinTime"| "maxParticipants"' docs/api-reference/v2/openapi.json | jq -r '.data.path.text,.data.lines.text' | sed '/^{/d'Length of output: 5103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Scanning docs/api-reference/v2/openapi.json for targeted time/participant field names..."
jq -r '
["start_time","join_time","max_participants","startTime","joinTime","maxParticipants","endTime","end_time"] as $targets |
.components.schemas | to_entries[] |
{schema: .key, properties: (.value.properties // {}), required: (.value.required // [])} |
.schema as $s |
(.properties | keys[]?) as $k |
select($targets | index($k)) |
"\($s): \($k)\n properties keys: \(.properties | keys | join(", "))\n required: \(.required | join(", "))\n---"
' docs/api-reference/v2/openapi.jsonLength of output: 2597
Inconsistent field naming in v2 OpenAPI — please standardize
docs/api-reference/v2/openapi.json mixes snake_case and camelCase for the same kinds of fields (e.g., start_time / startTime, join_time / joinTime, max_participants / maxParticipants). This will confuse API consumers — please confirm the canonical v2 convention and make the OpenAPI consistent (or document deliberate exceptions).
Locations to address:
- docs/api-reference/v2/openapi.json
- SessionParticipantItem: join_time (snake_case; required)
- SessionDetailsItem: start_time, max_participants (snake_case; required)
- RecordingItem: maxParticipants, startTs (camelCase)
- ScheduleAvailabilityInput_2024_06_11, ScheduleOverrideInput_2024_06_11, WorkingHours, AvailabilityModel: startTime, endTime (camelCase)
Recommended action:
- Choose one naming convention for v2 (camelCase or snake_case) and update the OpenAPI schemas (and any server responses) accordingly, or explicitly document any exceptions and versioning/breaking changes.
🤖 Prompt for AI Agents
docs/api-reference/v2/openapi.json lines 25795-25856: the OpenAPI mixes
snake_case and camelCase for equivalent fields; pick a single v2 convention
(e.g., camelCase) and make the schemas consistent. Update SessionParticipantItem
(join_time → joinTime) and its required array, SessionDetailsItem (start_time →
startTime, max_participants → maxParticipants) and its required array,
RecordingItem (maxParticipants/startTs → max_participants/start_ts if you choose
snake_case or the inverse if camelCase) and the
ScheduleAvailabilityInput_2024_06_11, ScheduleOverrideInput_2024_06_11,
WorkingHours, AvailabilityModel startTime/endTime fields to the chosen
convention; update all "example" values and "$ref"/usage sites and required
lists to match the new names, and ensure server responses/validators match the
schema or document deliberate exceptions.
| class SessionDetailsDataWrapper { | ||
| @ApiProperty({ type: [SessionDetailsItem] }) | ||
| @IsArray() | ||
| @ValidateNested({ each: true }) | ||
| @Type(() => SessionDetailsItem) | ||
| data!: SessionDetailsItem[]; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Double “data.data” nesting — confirm if intentional; consider renaming inner field
Returning { status, data: { data: SessionDetailsItem[] } } is a bit awkward. If not required for consistency with other outputs, consider renaming the inner property to sessions (or items) to avoid data.data.
Example change (breaking API if consumers already rely on current shape):
class SessionDetailsDataWrapper {
- @ApiProperty({ type: [SessionDetailsItem] })
- @IsArray()
- @ValidateNested({ each: true })
- @Type(() => SessionDetailsItem)
- data!: SessionDetailsItem[];
+ @ApiProperty({ type: [SessionDetailsItem] })
+ @IsArray()
+ @ValidateNested({ each: true })
+ @Type(() => SessionDetailsItem)
+ sessions!: SessionDetailsItem[];
}To verify current usage/expectations across the repo, run:
🏁 Script executed:
#!/bin/bash
# Inspect controller/service return shapes and DTO usage around session-details.
set -euo pipefail
echo "== References to GetBookingSessionDetailsOutput =="
rg -n -C3 '\bGetBookingSessionDetailsOutput\b'
echo "== Controllers/Routes mentioning session-details =="
rg -n -C5 'session-details'
echo "== getSessionDetails implementation(s) and returns =="
rg -n -C5 '\bgetSessionDetails\b'
echo "== Look for return shapes near session-details =="
rg -n -C3 'return\s+\{[^}]*status:\s*SUCCESS_STATUS' | sed -n '1,200p'Length of output: 32073
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== get-booking-session-details.output.ts =="
sed -n '1,240p' packages/platform/types/bookings/2024-08-13/outputs/get-booking-session-details.output.ts || true
echo
echo "== cal-video.service.ts (getSessionDetails) =="
sed -n '1,240p' apps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.ts || true
echo
echo "== Other booking outputs in same folder =="
ls -1 packages/platform/types/bookings/2024-08-13/outputs || true
echo
echo "== Search for other *DataWrapper classes in types =="
rg -n --hidden 'class\s+\w+DataWrapper' packages/platform || true
echo
echo "== Examples of outputs that use inner 'data' property =="
rg -n --hidden 'class\s+\w+DataWrapper' -S packages/platform | sed -n '1,200p' || trueLength of output: 7204
Double data.data nesting — flatten or rename the inner field
Short: The DTO and controller produce { status, data: { data: SessionDetailsItem[] } } (likely unintended). Please choose whether to flatten the response or rename the inner property and update callers/docs.
Locations to change:
- packages/platform/types/bookings/2024-08-13/outputs/get-booking-session-details.output.ts
- SessionDetailsDataWrapper defines
data!: SessionDetailsItem[] - GetBookingSessionDetailsOutput.data is typed as SessionDetailsDataWrapper
- SessionDetailsDataWrapper defines
- apps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.ts
- getSessionDetails() returns the raw sessionDetails from getSessionDetailsFromRoomName()
- apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts
- returns
{ status: SUCCESS_STATUS, data: sessionDetails }→ results indata.data
- returns
- Update generated docs/schemas:
- docs/api-reference/v2/openapi.json
- apps/api/v2/swagger/documentation.json
Suggested changes (examples — will be breaking; pick one and update controller/service + docs):
Option A — Rename inner field (breaking):
class SessionDetailsDataWrapper {
- @ApiProperty({ type: [SessionDetailsItem] })
- @IsArray()
- @ValidateNested({ each: true })
- @Type(() => SessionDetailsItem)
- data!: SessionDetailsItem[];
+ @ApiProperty({ type: [SessionDetailsItem] })
+ @IsArray()
+ @ValidateNested({ each: true })
+ @Type(() => SessionDetailsItem)
+ sessions!: SessionDetailsItem[];
}Option B — Flatten the output (breaking):
- Change GetBookingSessionDetailsOutput to expose an array directly:
export class GetBookingSessionDetailsOutput {
...
- @ApiProperty({ type: SessionDetailsDataWrapper })
- @ValidateNested()
- @Type(() => SessionDetailsDataWrapper)
- data!: SessionDetailsDataWrapper;
+ @ApiProperty({ type: [SessionDetailsItem] })
+ @IsArray()
+ @ValidateNested({ each: true })
+ @Type(() => SessionDetailsItem)
+ data!: SessionDetailsItem[];
}- And update controller/service to return
data: sessionDetails.data(or have service return the array).
Reasoning/evidence: get-booking-session-details.output.ts defines the nested wrapper and the bookings controller returns data: sessionDetails, so current runtime shape will be data.data (confirmed in repository files).
Please pick which shape you want (flattened array or named inner field like sessions) and I can provide the minimal diffs/changes to controller, service, types, and docs.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class SessionDetailsDataWrapper { | |
| @ApiProperty({ type: [SessionDetailsItem] }) | |
| @IsArray() | |
| @ValidateNested({ each: true }) | |
| @Type(() => SessionDetailsItem) | |
| data!: SessionDetailsItem[]; | |
| } | |
| class SessionDetailsDataWrapper { | |
| @ApiProperty({ type: [SessionDetailsItem] }) | |
| @IsArray() | |
| @ValidateNested({ each: true }) | |
| @Type(() => SessionDetailsItem) | |
| sessions!: SessionDetailsItem[]; | |
| } |
🤖 Prompt for AI Agents
In
packages/platform/types/bookings/2024-08-13/outputs/get-booking-session-details.output.ts
lines 61-67 the SessionDetailsDataWrapper creates a nested response shape {
status, data: { data: SessionDetailsItem[] } }; flatten the response by changing
GetBookingSessionDetailsOutput.data to be SessionDetailsItem[] (or update
SessionDetailsDataWrapper to be removed) and update callers: in
apps/api/v2/src/ee/bookings/2024-08-13/services/cal-video.service.ts have
getSessionDetails() return the array (sessionDetails.data) instead of the
wrapper, and in
apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts return
{ status: SUCCESS_STATUS, data: sessionDetailsArray } (or adjust to renamed
inner field if you prefer renaming); finally regenerate/update the OpenAPI/docs
files docs/api-reference/v2/openapi.json and
apps/api/v2/swagger/documentation.json to reflect the flattened array schema.
|
This PR is being marked as stale due to inactivity. |
Devanshusharma2005
left a comment
Devanshusharma2005
left a comment
There was a problem hiding this comment.
Looks good. But let's wait for the @supalarry review.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/api-reference/v2/openapi.json (1)
26030-26091: Standardize field naming to match v2 convention (avoid mixed snake_case/camelCase)Rename snake_case fields to camelCase (or vice versa, per v2 policy) and update required lists accordingly. This prevents DX inconsistencies flagged earlier.
- "SessionParticipantItem": { + "SessionParticipantItem": { "type": "object", "properties": { - "user_id": { "type": "string", "example": "8" }, - "participant_id": { "type": "string", "example": "f269d7f3-afce-4d39-8df6-69d134fe6af7" }, - "user_name": { "type": "string", "example": "Team Free Example" }, - "join_time": { "type": "number", "example": 1755161937 }, + "userId": { "type": "string", "example": "8" }, + "participantId": { "type": "string", "example": "f269d7f3-afce-4d39-8df6-69d134fe6af7" }, + "userName": { "type": "string", "example": "Team Free Example" }, + "joinTime": { "type": "integer", "format": "int64", "description": "Unix timestamp (seconds)", "example": 1755161937 }, "duration": { - "type": "number", - "example": 31 + "type": "integer", + "format": "int32", + "minimum": 0, + "description": "Duration in seconds", + "example": 31 } }, - "required": ["user_id", "participant_id", "user_name", "join_time", "duration"] + "required": ["userId", "participantId", "userName", "joinTime", "duration"] }, - "SessionDetailsItem": { + "SessionDetailsItem": { "type": "object", "properties": { "id": { "type": "string", "example": "50455614-55b3-4e66-835d-102f562a5070" }, "room": { "type": "string", "example": "k6ahwPBbTu2Qr5ZuCvn0" }, - "start_time": { "type": "number", "example": 1755161937 }, + "startTime": { "type": "integer", "format": "int64", "description": "Unix timestamp (seconds)", "example": 1755161937 }, "duration": { - "type": "number", - "example": 31 + "type": "integer", + "format": "int32", + "minimum": 0, + "description": "Duration in seconds", + "example": 31 }, "ongoing": { "type": "boolean", "example": false }, - "max_participants": { "type": "number", "example": 1 }, + "maxParticipants": { "type": "integer", "format": "int32", "minimum": 1, "example": 1 }, "participants": { "type": "array", + "minItems": 0, "items": { "$ref": "#/components/schemas/SessionParticipantItem" } } }, - "required": ["id", "room", "start_time", "duration", "ongoing", "max_participants", "participants"] + "required": ["id", "room", "startTime", "duration", "ongoing", "maxParticipants", "participants"] },
🧹 Nitpick comments (3)
docs/api-reference/v2/openapi.json (3)
7985-8014: Tighten header schema, enrich param metadata, and document non-200 responses
- Use enum+example for cal-api-version instead of default.
- Add description/example for bookingUid.
- Provide a non-empty 200 description.
- Consider documenting common error responses (400/401/404/429/5xx).
Apply:
{ "name": "cal-api-version", "in": "header", "description": "Must be set to 2024-08-13", "required": true, "schema": { - "type": "string", - "default": "2024-08-13" + "type": "string", + "enum": ["2024-08-13"] - } + }, + "example": "2024-08-13" }, { "name": "bookingUid", "required": true, "in": "path", - "schema": { - "type": "string" - } + "description": "UID of the booking", + "schema": { "type": "string" }, + "example": "a1B2c3D4" } ], "responses": { "200": { - "description": "", + "description": "Session details retrieved", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/GetBookingSessionDetailsOutput" } } } - } + }, + "400": { "description": "Invalid header or path parameter" }, + "401": { "description": "Unauthorized" }, + "404": { "description": "Booking not found or session details unavailable" }, + "429": { "description": "Rate limit exceeded" }, + "500": { "description": "Internal server error" } },
26104-26119: Make error nullable and describe envelope semanticsClarify that error is null/omitted on success; preserve the existing status enum.
"GetBookingSessionDetailsOutput": { "type": "object", "properties": { "status": { "type": "string", "example": "success", "enum": ["success", "error"] }, "data": { "$ref": "#/components/schemas/SessionDetailsDataWrapper" }, - "error": { - "type": "object" - } + "error": { + "type": "object", + "nullable": true, + "description": "Present on error responses; null or omitted on success" + } }, "required": ["status", "data"] },
26045-26051: Prefer integer types for timestamps, duration, and participant countsUse integer + format for epoch seconds; add minimums for durations and counts.
Also applies to: 26067-26074, 26079-26082
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts(2 hunks)apps/api/v2/swagger/documentation.json(2 hunks)docs/api-reference/v2/openapi.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api/v2/swagger/documentation.json
- apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts
🧰 Additional context used
📓 Path-based instructions (1)
docs/api-reference/v2/openapi.json
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
When docs changes are made in
/docs/api-reference/v2/openapi.json, ensure 'summary' fields are short, concise, do not end with a period, and are written in proper American English
Files:
docs/api-reference/v2/openapi.json
⏰ 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). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
docs/api-reference/v2/openapi.json (2)
7981-7981: Summary looks compliantShort, clear, no trailing period; aligns with v2 docs guideline.
7980-8016: Confirm security inheritanceIf this path doesn’t inherit global security, add the appropriate security requirement (e.g., bearer/api-key) here.
|
Hey @romitg2 pls add test to verify the fix |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/user-bookings.e2e-spec.ts(2 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 useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/user-bookings.e2e-spec.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/v2/src/ee/bookings/2024-08-13/controllers/e2e/user-bookings.e2e-spec.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/v2/src/ee/bookings/2024-08-13/controllers/e2e/user-bookings.e2e-spec.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:
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/user-bookings.e2e-spec.ts
🧬 Code graph analysis (1)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/user-bookings.e2e-spec.ts (1)
packages/platform/constants/api.ts (2)
CAL_API_VERSION_HEADER(72-72)VERSION_2024_08_13(59-59)
🔇 Additional comments (1)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/user-bookings.e2e-spec.ts (1)
97-101: PermissionsGuard override is fine for this suiteKeeps auth out of the way for endpoint behavior verification. No action needed.
| describe("get booking session details", () => { | ||
| it("should return 404 for non-existent booking", async () => { | ||
| return request(app.getHttpServer()) | ||
| .get(`/v2/bookings/non-existent-booking-uid/session-details`) | ||
| .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13) | ||
| .expect(404); | ||
| }); | ||
|
|
||
| it("should return 404 for booking without cal video reference", async () => { | ||
| // Create a simple booking without Cal Video reference | ||
| const regularBooking = await bookingsRepositoryFixture.create({ | ||
| user: { | ||
| connect: { | ||
| id: user.id, | ||
| }, | ||
| }, | ||
| startTime: new Date(Date.UTC(2040, 0, 9, 15, 0, 0)), | ||
| endTime: new Date(Date.UTC(2040, 0, 9, 16, 0, 0)), | ||
| title: "Regular Booking Test", | ||
| uid: `regular-booking-${randomString()}`, | ||
| eventType: { | ||
| connect: { | ||
| id: eventTypeId, | ||
| }, | ||
| }, | ||
| location: "https://meet.google.com/abc-def-ghi", | ||
| customInputs: {}, | ||
| metadata: {}, | ||
| responses: { | ||
| name: "Regular User", | ||
| email: "regular-test@gmail.com", | ||
| }, | ||
| attendees: { | ||
| create: { | ||
| email: "regular-test@gmail.com", | ||
| name: "Regular User", | ||
| locale: "en", | ||
| timeZone: "Europe/Rome", | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| try { | ||
| return request(app.getHttpServer()) | ||
| .get(`/v2/bookings/${regularBooking.uid}/session-details`) | ||
| .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13) | ||
| .expect(404); | ||
| } finally { | ||
| // Clean up test booking | ||
| await bookingsRepositoryFixture.deleteById(regularBooking.id); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a success-path test for session-details (mock the video client)
Only negative cases are covered. Add a 200 case to prove the new endpoint returns details when a Cal Video reference exists, while avoiding external calls by stubbing the video client.
Suggested addition right below this describe:
+ it("should return 200 with session details for a Cal Video booking", async () => {
+ // Stub provider call to avoid network
+ const { getSessionDetailsFromRoomName } = require("@calcom/platform-libraries/conferencing");
+ const mockDetails = { roomName: "stub-room", roomUrl: "https://video.cal.com/stub-room" };
+ const spy = jest.spyOn(getSessionDetailsFromRoomName, "call" in getSessionDetailsFromRoomName ? "call" : "bind").mockResolvedValue(mockDetails);
+ // Create a booking via API with cal-video integration so references are created
+ const createBody = {
+ start: new Date(Date.UTC(2040, 0, 9, 17, 0, 0)).toISOString(),
+ eventTypeId,
+ attendee: { name: "Cal Video User", email: "cv@example.com", timeZone: "Europe/Rome", language: "en" },
+ location: { type: "integration", integration: "cal-video" },
+ };
+ const createRes = await request(app.getHttpServer())
+ .post("/v2/bookings")
+ .send(createBody)
+ .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13)
+ .expect(201);
+ const booking = createRes.body.data;
+ try {
+ await request(app.getHttpServer())
+ .get(`/v2/bookings/${booking.uid}/session-details`)
+ .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13)
+ .expect(200)
+ .expect((res) => {
+ expect(res.body.status).toBe("success");
+ expect(res.body.data).toEqual(mockDetails);
+ });
+ } finally {
+ spy.mockRestore();
+ await bookingsRepositoryFixture.deleteById(booking.id);
+ }
+ });Note: If the import path differs in your service, spy on the exact module used there.
Committable suggestion skipped: line range outside the PR's diff.
| try { | ||
| return request(app.getHttpServer()) | ||
| .get(`/v2/bookings/${regularBooking.uid}/session-details`) | ||
| .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13) | ||
| .expect(404); | ||
| } finally { | ||
| // Clean up test booking | ||
| await bookingsRepositoryFixture.deleteById(regularBooking.id); | ||
| } |
There was a problem hiding this comment.
Fix false-positive risk: cleanup runs before the request finishes
Returning the Supertest promise inside try causes finally to run immediately, deleting the booking before the GET resolves. The test may pass 404 because the booking was deleted, not because it lacks Cal Video references.
Apply:
- try {
- return request(app.getHttpServer())
- .get(`/v2/bookings/${regularBooking.uid}/session-details`)
- .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13)
- .expect(404);
- } finally {
- // Clean up test booking
- await bookingsRepositoryFixture.deleteById(regularBooking.id);
- }
+ try {
+ await request(app.getHttpServer())
+ .get(`/v2/bookings/${regularBooking.uid}/session-details`)
+ .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13)
+ .expect(404);
+ } finally {
+ // Clean up test booking
+ await bookingsRepositoryFixture.deleteById(regularBooking.id);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| return request(app.getHttpServer()) | |
| .get(`/v2/bookings/${regularBooking.uid}/session-details`) | |
| .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13) | |
| .expect(404); | |
| } finally { | |
| // Clean up test booking | |
| await bookingsRepositoryFixture.deleteById(regularBooking.id); | |
| } | |
| try { | |
| await request(app.getHttpServer()) | |
| .get(`/v2/bookings/${regularBooking.uid}/session-details`) | |
| .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13) | |
| .expect(404); | |
| } finally { | |
| // Clean up test booking | |
| await bookingsRepositoryFixture.deleteById(regularBooking.id); | |
| } |
🤖 Prompt for AI Agents
In
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/user-bookings.e2e-spec.ts
around lines 899 to 907, the Supertest request is returned directly from the try
which causes the finally block to run before the request completes and the
booking gets deleted prematurely; change the code to await the request promise
inside the try (e.g., const res = await request(...).expect(404); or simply
await request(...).expect(404);) so the HTTP call completes before the finally
cleanup runs, ensuring the test result isn’t influenced by early deletion.
|
This PR is being marked as stale due to inactivity. |
3 participants
What does this PR do?
Screen.Recording.2025-08-15.at.9.43.32.AM.mov
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist