Skip to content

Conversation

anikdhabal
Copy link
Contributor

Reverts #22888

Copy link
Contributor

coderabbitai bot commented Aug 28, 2025

Walkthrough

This 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 Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch revert-22888-zoom-waiting-setting

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbit in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbit in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbit gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbit read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbit help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbit ignore or @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbit summary or @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbit or @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@graphite-app graphite-app bot requested a review from a team August 28, 2025 15:32
Copy link
Contributor

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:

No release type found in pull request title "Revert "chore: Add zoom waiting room setting"". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

@keithwillcode keithwillcode added the core area: core, team members only label Aug 28, 2025
@anikdhabal anikdhabal changed the title Revert "chore: Add zoom waiting room setting" fix: remove zoom waiting room additional setting Aug 28, 2025
@anikdhabal anikdhabal changed the title fix: remove zoom waiting room additional setting fix: remove zoom waiting room additional setting Aug 28, 2025
@anikdhabal anikdhabal enabled auto-merge (squash) August 28, 2025 15:33
@dosubot dosubot bot added the app-store area: app store, apps, calendar integrations, google calendar, outlook, lark, apple calendar label Aug 28, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

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 TZ

Accessing 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 minutes

Zoom 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 logs

Avoid 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 update

Minimize 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 59c7614 and 4d198cf.

📒 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 use include, always use select
Ensure the credential.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-revert

Deriving 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 export

No 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";
Copy link
Contributor

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.

Suggested change
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.

@anikdhabal anikdhabal merged commit 464e5f1 into main Aug 28, 2025
101 of 109 checks passed
@anikdhabal anikdhabal deleted the revert-22888-zoom-waiting-setting branch August 28, 2025 16:16
Copy link
Contributor

E2E results are ready!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app-store area: app store, apps, calendar integrations, google calendar, outlook, lark, apple calendar core area: core, team members only ready-for-e2e
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants