-
Notifications
You must be signed in to change notification settings - Fork 10.5k
fix: remove zoom waiting room additional setting #23427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This reverts commit c64b663.
WalkthroughThis change removes handling of Zoom’s advanced waiting room settings. The zoomUserSettingsSchema no longer contains meeting_security.waiting_room_settings, and related evaluation/fetch logic is deleted. The user settings API selection omits waiting_room_settings, limiting fields to default_password_for_scheduled_meetings, auto_recording, and waiting_room. Meeting creation no longer conditionally includes waiting_room_settings; variables and spread logic for it were removed. The waiting_room flag continues to be derived from userSettings.in_meeting.waiting_room and applied to meeting settings. Exported entities updated: zoomUserSettingsSchema and ZoomUserSettings now reflect the removal of meeting_security.waiting_room_settings. Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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
|
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:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/app-store/zoomvideo/lib/VideoApiAdapter.ts (1)
114-141
: Guard against empty attendees when computing weekly/monthly recurrence TZAccessing attendees[0].timeZone can throw for empty arrays; fall back to organizer.timeZone or UTC.
- const getRecurrence = ({ - recurringEvent, - startTime, - attendees, - }: CalendarEvent): { recurrence: ZoomRecurrence } | undefined => { + const getRecurrence = ({ + recurringEvent, + startTime, + attendees, + organizer, + }: CalendarEvent): { recurrence: ZoomRecurrence } | undefined => { @@ - case Frequency.WEEKLY: - recurrence = { - type: 2, - weekly_days: dayjs(startTime).tz(attendees[0].timeZone).day() + 1, - }; + case Frequency.WEEKLY: { + const tz = attendees?.[0]?.timeZone || organizer?.timeZone || "UTC"; + recurrence = { + type: 2, + weekly_days: dayjs(startTime).tz(tz).day() + 1, + }; break; - case Frequency.MONTHLY: - recurrence = { - type: 3, - monthly_day: dayjs(startTime).tz(attendees[0].timeZone).date(), - }; + } + case Frequency.MONTHLY: { + const tz = attendees?.[0]?.timeZone || organizer?.timeZone || "UTC"; + recurrence = { + type: 3, + monthly_day: dayjs(startTime).tz(tz).date(), + }; break; + }
🧹 Nitpick comments (3)
packages/app-store/zoomvideo/lib/VideoApiAdapter.ts (3)
183-184
: Ensure duration is an integer number of minutesZoom expects integer minutes. Round and enforce a minimum of 1.
- duration: (new Date(event.endTime).getTime() - new Date(event.startTime).getTime()) / 60000, + duration: Math.max( + 1, + Math.round( + (new Date(event.endTime).getTime() - new Date(event.startTime).getTime()) / 60000 + ) + ),
221-229
: Redact and truncate raw Zoom response in logsAvoid leaking PII (emails) and overly large payloads.
- log.error( - "Error in JSON parsing Zoom API response", - safeStringify({ - error: safeStringify(error), - // Log Raw response body here. - responseBody: await clonedResponse.text(), - status: clonedResponse.status, - }) - ); + const rawBody = await clonedResponse.text(); + const redactedBody = rawBody.replace( + /[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,}/gi, + "<redacted>" + ); + const truncatedBody = redactedBody.slice(0, 4096); + log.error( + "Error in JSON parsing Zoom API response", + safeStringify({ + error: safeStringify(error), + responseBody: truncatedBody, + status: clonedResponse.status, + }) + );
314-322
: Use Prisma select to avoid fetching credential.key on updateMinimize data returned and align with repo guideline to only select what you need.
updateTokenObject: async (newTokenObject) => { await prisma.credential.update({ where: { id: credential.id, }, data: { key: newTokenObject, - }, + }, + select: { id: true }, }); },
📜 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)
packages/app-store/zoomvideo/lib/VideoApiAdapter.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.key
field is never returned from tRPC endpoints or APIs
Files:
packages/app-store/zoomvideo/lib/VideoApiAdapter.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/app-store/zoomvideo/lib/VideoApiAdapter.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/app-store/zoomvideo/lib/VideoApiAdapter.ts
🔇 Additional comments (2)
packages/app-store/zoomvideo/lib/VideoApiAdapter.ts (2)
175-205
: LGTM: waiting_room toggle mapping is consistent post-revertDeriving waiting_room from userSettings.in_meeting.waiting_room and inverting for join_before_host is correct.
97-97
: Export ZoomVideoApiAdapter as a named export-const ZoomVideoApiAdapter = (credential: CredentialPayload): VideoApiAdapter => { +export const ZoomVideoApiAdapter = (credential: CredentialPayload): VideoApiAdapter => { @@ -export default ZoomVideoApiAdapter; +// removed default exportNo default imports of ZoomVideoApiAdapter were found in the repo; safe to convert now.
}); | ||
|
||
// https://developers.zoom.us/docs/api/rest/reference/user/methods/#operation/userSettings | ||
// append comma separated settings here, to retrieve only these specific settings | ||
const settingsApiFilterResp = | ||
"default_password_for_scheduled_meetings,auto_recording,waiting_room,waiting_room_settings"; | ||
const settingsApiFilterResp = "default_password_for_scheduled_meetings,auto_recording,waiting_room"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Fix Zoom custom_query_fields paths for nested settings
Zoom expects dotted paths for nested objects. Current value risks empty sections and schema parse failures. Use fully qualified fields.
Apply this diff:
-const settingsApiFilterResp = "default_password_for_scheduled_meetings,auto_recording,waiting_room";
+const settingsApiFilterResp =
+ "schedule_meeting.default_password_for_scheduled_meetings,recording.auto_recording,in_meeting.waiting_room";
Optionally verify against docs:
🌐 Web query:
Zoom user settings API custom_query_fields: Should fields be dotted, e.g., "recording.auto_recording" and "in_meeting.waiting_room", rather than "auto_recording" and "waiting_room"?
💡 Result:
Yes — use the dotted (nested) names. For user/account settings the API exposes nested objects (recording, in_meeting, schedule_meeting, etc.), and custom_query_fields targets those nested properties (e.g. recording.auto_recording, in_meeting.waiting_room). See Zoom's settings field list that shows the nested keys and examples. [1][2]
Sources:
- Zoom API settings field list showing nested keys (recording., in_meeting.). [1]
- Third‑party docs / examples describing custom_query_fields and returned nested objects. [2]
Use fully qualified dotted paths for nested Zoom settings in custom_query_fields
Update the filter in packages/app-store/zoomvideo/lib/VideoApiAdapter.ts
at line 86:
-const settingsApiFilterResp = "default_password_for_scheduled_meetings,auto_recording,waiting_room";
+const settingsApiFilterResp =
+ "schedule_meeting.default_password_for_scheduled_meetings,recording.auto_recording,in_meeting.waiting_room";
📝 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.
const settingsApiFilterResp = "default_password_for_scheduled_meetings,auto_recording,waiting_room"; | |
const settingsApiFilterResp = | |
"schedule_meeting.default_password_for_scheduled_meetings,recording.auto_recording,in_meeting.waiting_room"; |
🤖 Prompt for AI Agents
In packages/app-store/zoomvideo/lib/VideoApiAdapter.ts around line 86, the
filter string uses top-level names for nested Zoom meeting settings; update it
to use fully qualified dotted paths for each nested field (e.g.,
settings.default_password_for_scheduled_meetings, settings.auto_recording,
settings.waiting_room) so the custom_query_fields target the correct nested
properties when querying the Zoom API.
E2E results are ready! |
Reverts #22888