Skip to content

Comments

fix: recurring event isn't working#23346

Merged
anikdhabal merged 7 commits intocalcom:mainfrom
anikdhabal:recurring-event
Aug 26, 2025
Merged

fix: recurring event isn't working#23346
anikdhabal merged 7 commits intocalcom:mainfrom
anikdhabal:recurring-event

Conversation

@anikdhabal
Copy link
Contributor

@anikdhabal anikdhabal commented Aug 25, 2025

What does this PR do?

Regression from this:- #22925

@vercel
Copy link

vercel bot commented Aug 25, 2025

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

A member of the Team first needs to authorize it.

@graphite-app graphite-app bot requested a review from a team August 25, 2025 19:08
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 25, 2025

Walkthrough

Replaces direct useBookerStore usage in EventOccurences with the context hook useBookerStoreContext from BookerStoreProvider; selectors and component API remain unchanged. Updates Playwright test helper bookTimeSlot to accept isRecurringEvent?: boolean, choose the booking endpoint accordingly, and submit by clicking data-testid="confirm-book-button" (instead of pressing Enter). Adds a Playwright E2E test that creates a recurring event with 3 occurrences, books across months, and asserts a success page showing "3 occurrences" (the test appears duplicated in the diff).

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

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 @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai 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:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai 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 @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @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.

@anikdhabal anikdhabal enabled auto-merge (squash) August 25, 2025 19:09
@keithwillcode keithwillcode added the core area: core, team members only label Aug 25, 2025
@dosubot dosubot bot added bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working labels Aug 25, 2025
@graphite-app
Copy link

graphite-app bot commented Aug 25, 2025

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (08/25/25)

1 reviewer was added to this PR based on Keith Williams's automation.

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/features/bookings/components/event-meta/Occurences.tsx (1)

77-92: Avoid putting NaN into the store when the input is cleared; drop the regex.

When the number input is cleared, parseInt("") yields NaN, and setOccurenceCount is called with NaN. That can propagate invalid state to other consumers. Since this is a numeric input, the regex is redundant; validate with numeric checks and early-return on invalid values.

Apply this diff to harden the handler:

-          const pattern = /^(?=.*[0-9])\S+$/;
-          const inputValue = parseInt(event.target.value);
-          setOccurenceCount(inputValue);
-          if (
-            !pattern.test(event.target.value) ||
-            inputValue < 1 ||
-            (maxOccurences && inputValue > maxOccurences)
-          ) {
-            setWarning(true);
-            setRecurringEventCount(maxOccurences);
-          } else {
-            setWarning(false);
-            setRecurringEventCount(inputValue);
-          }
+          const raw = event.target.value;
+          const inputValue = raw === "" ? NaN : Number(raw);
+          if (Number.isNaN(inputValue)) {
+            // Empty/invalid entry — show warning and clamp display count to max if available.
+            setWarning(true);
+            setRecurringEventCount(maxOccurences);
+            return;
+          }
+          setOccurenceCount(inputValue);
+          if (inputValue < 1 || (maxOccurences && inputValue > maxOccurences)) {
+            setWarning(true);
+            setRecurringEventCount(maxOccurences);
+          } else {
+            setWarning(false);
+            setRecurringEventCount(inputValue);
+          }
🧹 Nitpick comments (4)
packages/features/bookings/components/event-meta/Occurences.tsx (4)

24-25: Optional: consolidate selectors or add equality for consistency.

Two separate calls are fine. If you prefer fewer hooks, you can pull both via a single selector with shallow (as above), or leave as-is and optionally pass an equality fn here too for consistency.


29-35: Clamp initial values into [1, max] on mount to avoid out-of-range state.

If occurenceCount or recurringEventCount arrive out of bounds, we only set a warning but we don’t clamp them, which can leak invalid values to downstream consumers (e.g., parsing or UI). Clamp to [1, event.recurringEvent.count] when initializing.

Example change:

   useEffect(() => {
-    if (!event.recurringEvent?.count) return;
-    setOccurenceCount(occurenceCount || event.recurringEvent.count);
-    setRecurringEventCount(recurringEventCount || event.recurringEvent.count);
-    if (occurenceCount && (occurenceCount > event.recurringEvent.count || occurenceCount < 1))
-      setWarning(true);
+    const max = event.recurringEvent?.count;
+    if (!max) return;
+    const nextOcc = occurenceCount && occurenceCount >= 1 && occurenceCount <= max ? occurenceCount : max;
+    const nextRec = recurringEventCount && recurringEventCount >= 1 && recurringEventCount <= max ? recurringEventCount : max;
+    setOccurenceCount(nextOcc);
+    setRecurringEventCount(nextRec);
+    setWarning(nextOcc < 1 || nextOcc > max);
   }, [setRecurringEventCount, event.recurringEvent, recurringEventCount, setOccurenceCount, occurenceCount]);

59-60: Localize the literal “+” prefix (minor).

UI-visible literals should be translated. Consider folding the “+ ” into the translation string for “plus_more”.


14-16: Spelling: Occurences/occurence → Occurrences/occurrence (minor).

Public component name and local vars use a common misspelling. Renaming the exported symbol is likely out of scope here, but consider aligning new/local identifiers to the correct spelling in future changes and adding transitional aliases if/when you clean this up.

📜 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 babeb1f and 8f3f4cc.

📒 Files selected for processing (1)
  • packages/features/bookings/components/event-meta/Occurences.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx

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

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

Files:

  • packages/features/bookings/components/event-meta/Occurences.tsx
**/*.{ts,tsx}

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

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

Files:

  • packages/features/bookings/components/event-meta/Occurences.tsx
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

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

Files:

  • packages/features/bookings/components/event-meta/Occurences.tsx
🧬 Code graph analysis (1)
packages/features/bookings/components/event-meta/Occurences.tsx (1)
packages/features/bookings/Booker/BookerStoreProvider.tsx (1)
  • useBookerStoreContext (24-35)
🔇 Additional comments (1)
packages/features/bookings/components/event-meta/Occurences.tsx (1)

3-3: All EventOccurences usages are properly wrapped in BookerStoreProvider

I’ve verified that:

  • The only import of EventOccurences (which calls useBookerStoreContext) is in
    packages/features/bookings/components/event-meta/Occurences.tsx, and it’s used exclusively in the EventDetails component in
    packages/features/bookings/components/event-meta/Details.tsx turn…​
  • EventDetails is rendered inside EventMeta (<EventDetails event={event} />) in
    packages/features/bookings/Booker/components/EventMeta.tsx turn…​
  • All entry points for the booking UI (e.g. BookerWebWrapper in packages/platform/atoms/booker/BookerWebWrapper.tsx) wrap their children in <BookerStoreProvider>, ensuring EventMeta (and thus EventOccurences) is always under the provider turn…​

No unwrapped usage sites were found—this concern can be resolved.

@anikdhabal anikdhabal added the High priority Created by Linear-GitHub Sync label Aug 25, 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

🧹 Nitpick comments (3)
apps/web/playwright/event-types.e2e.ts (3)

462-462: Avoid brittle nth(1) selector; prefer a stable, semantic locator

Relying on .nth(1) for the numeric input is fragile if the UI reorders or adds inputs. Prefer a dedicated test id or an accessible label.

Example (if product code can expose a stable test id):

await page.getByTestId("recurring-occurrences-input").fill("3");

If adding a test id isn't feasible, target an accessible name (spinbutton with label "Occurrences") to avoid index-based selection:

await page.getByRole("spinbutton", { name: /occurrences/i }).fill("3");

I can open a follow-up to add a data-testid="recurring-occurrences-input" in the settings UI and update tests accordingly.


472-474: Strengthen validation: assert the UI shows exactly 3 generated recurring dates

Currently we only assert that the recurring dates container is visible. To better prove the regression is fixed, assert that the UI renders exactly 3 dates for the chosen rule.

Example (adjust selector to match actual markup):

await expect(page.locator('[data-testid="recurring-dates"] [data-testid="recurring-date"]')).toHaveCount(3);

If [data-testid="recurring-date"] doesn’t exist, consider adding it to each rendered date item.


478-478: Make success assertion locale-insensitive

Hard-coding "3 occurrences" can break if copy changes slightly or casing differs. Prefer a regex match or, ideally, a dedicated test id for the occurrences count.

Example:

await expect(page.locator('text=/\\b3\\s+occurrences\\b/i')).toBeVisible();

Even better, expose [data-testid="recurring-occurrences-count"] on the success page and assert its value.

📜 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 8f3f4cc and c290be7.

📒 Files selected for processing (1)
  • apps/web/playwright/event-types.e2e.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

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

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

Files:

  • apps/web/playwright/event-types.e2e.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/web/playwright/event-types.e2e.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/web/playwright/event-types.e2e.ts
🧬 Code graph analysis (1)
apps/web/playwright/event-types.e2e.ts (2)
apps/api/v2/test/utils/randomString.ts (1)
  • randomString (3-6)
apps/web/playwright/lib/testUtils.ts (5)
  • createNewUserEventType (182-194)
  • saveEventType (476-480)
  • gotoBookingPage (470-474)
  • selectFirstAvailableTimeSlotNextMonth (109-117)
  • bookTimeSlot (150-173)
⏰ 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). (1)
  • GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (1)
apps/web/playwright/event-types.e2e.ts (1)

449-479: No duplicate test definitions found

I searched the entire apps/web/playwright directory for the test title and only a single occurrence was found at apps/web/playwright/event-types.e2e.ts:449. There are no duplicate definitions of this test, so it’s safe to proceed.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
apps/web/playwright/event-types.e2e.ts (3)

453-485: Add teardown to prevent leaked test users in parallel runs

This test creates a user but doesn’t delete it. Other tests in this file rely on afterEach cleanup within describe("user"), but this test runs outside that scope. In parallel mode, leaked users can accumulate and cause flaky cross-test state. Wrap the body in try/finally and call users.deleteAll().

Apply this diff:

   test("should create recurring event and successfully book multiple occurrences", async ({
     page,
     users,
   }) => {
     const user = await users.create();
-    await user.apiLogin();
-    await page.goto("/event-types");
-    const nonce = randomString(3);
-    const eventTitle = `Recurring Event Test ${nonce}`;
-
-    await createNewUserEventType(page, { eventTitle });
-
-    await page.waitForSelector('[data-testid="event-title"]');
-    await expect(page.getByTestId("vertical-tab-basics")).toHaveAttribute("aria-current", "page");
-    await page.click("[data-testid=vertical-tab-recurring]");
-    await expect(page.locator("[data-testid=recurring-event-collapsible]")).toBeHidden();
-    await page.click("[data-testid=recurring-event-check]");
-    await expect(page.locator("[data-testid=recurring-event-collapsible]")).toBeVisible();
-
-    await page.locator("[data-testid=recurring-event-collapsible] input[type=number]").nth(1).fill("3");
-
-    await saveEventType(page);
-
-    await gotoBookingPage(page);
-
-    await expect(page.locator("[data-testid=occurrence-input]")).toHaveValue("3");
-
-    await selectFirstAvailableTimeSlotNextMonth(page);
-
-    await expect(page.locator("[data-testid=recurring-dates]")).toBeVisible();
-
-    await bookTimeSlot(page);
-
-    await expect(page.locator("[data-testid=success-page]")).toBeVisible();
-
-    await expect(page.locator("text=3 occurrences")).toBeVisible();
+    try {
+      await user.apiLogin();
+      await page.goto("/event-types");
+      const nonce = randomString(3);
+      const eventTitle = `Recurring Event Test ${nonce}`;
+
+      await createNewUserEventType(page, { eventTitle });
+
+      await page.waitForSelector('[data-testid="event-title"]');
+      await expect(page.getByTestId("vertical-tab-basics")).toHaveAttribute("aria-current", "page");
+      await page.click("[data-testid=vertical-tab-recurring]");
+      await expect(page.locator("[data-testid=recurring-event-collapsible]")).toBeHidden();
+      await page.click("[data-testid=recurring-event-check]");
+      await expect(page.locator("[data-testid=recurring-event-collapsible]")).toBeVisible();
+
+      await page.locator("[data-testid=recurring-event-collapsible] input[type=number]").nth(1).fill("3");
+
+      await saveEventType(page);
+      await gotoBookingPage(page);
+      await expect(page.locator("[data-testid=occurrence-input]")).toHaveValue("3");
+
+      await selectFirstAvailableTimeSlotNextMonth(page);
+      await expect(page.locator("[data-testid=recurring-dates]")).toBeVisible();
+
+      await bookTimeSlot(page);
+      await expect(page.locator("[data-testid=success-page]")).toBeVisible();
+      await expect(page.locator("text=3 occurrences")).toBeVisible();
+    } finally {
+      await users.deleteAll();
+    }
   });

If the users fixture already performs implicit cleanup for top-level tests, confirm and ignore; otherwise, the above will keep CI isolated.


468-468: Prefer stable, semantic locator over nth()

Using nth(1) is brittle if the form fields change order. If the input has an accessible label (e.g., “Occurrences”) or its own data-testid, prefer those. Example:

Apply this diff (adjust the name to match the actual accessible label):

-    await page.locator("[data-testid=recurring-event-collapsible] input[type=number]").nth(1).fill("3");
+    await page.getByRole("spinbutton", { name: /occurrence|occurrences/i }).fill("3");

If no accessible name exists, consider adding a dedicated test id (e.g., data-testid="occurrence-count-input") in the UI and target that instead.


484-484: Make the success assertion locale-/copy-proof

Text selectors are sensitive to copy changes and language. Anchor the check within the success container and use a case-insensitive regex. This reduces flakiness without changing semantics.

Apply this diff:

-    await expect(page.locator("text=3 occurrences")).toBeVisible();
+    await expect(
+      page.locator("[data-testid=success-page]").getByText(/3\s+occurrences/i)
+    ).toBeVisible();
📜 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 c290be7 and 028f669.

📒 Files selected for processing (1)
  • apps/web/playwright/event-types.e2e.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

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

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

Files:

  • apps/web/playwright/event-types.e2e.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/web/playwright/event-types.e2e.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/web/playwright/event-types.e2e.ts
🧬 Code graph analysis (1)
apps/web/playwright/event-types.e2e.ts (2)
apps/api/v2/test/utils/randomString.ts (1)
  • randomString (3-6)
apps/web/playwright/lib/testUtils.ts (5)
  • createNewUserEventType (182-194)
  • saveEventType (476-480)
  • gotoBookingPage (470-474)
  • selectFirstAvailableTimeSlotNextMonth (109-117)
  • bookTimeSlot (150-173)
🔇 Additional comments (1)
apps/web/playwright/event-types.e2e.ts (1)

449-485: Solid, self-contained E2E for recurring bookings

Good addition: the flow logs in, creates an event type, enables recurring, books, and asserts success. This directly covers the regression and addresses the earlier auth/context concern flagged in a previous review. Assertions on the booking page (occurrence-input and recurring-dates) give meaningful signal.

joeauyeung
joeauyeung previously approved these changes Aug 25, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Aug 25, 2025

E2E results are ready!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/web/playwright/lib/testUtils.ts (1)

169-173: Make the click more robust and consider DRY-ing with confirmBooking.

Two optional improvements:

  • Wait for the button to be enabled before clicking to avoid rare timing issues.
  • Reuse the confirmBooking helper to centralize the submit action (you may extend it to accept expectedStatusCode).

Apply this local diff to harden the click:

   await submitAndWaitForResponse(page, "/api/book/event", {
-    action: () => page.locator('[data-testid="confirm-book-button"]').click(),
+    action: async () => {
+      const confirmBtn = page.getByTestId("confirm-book-button");
+      await expect(confirmBtn).toBeEnabled();
+      await confirmBtn.click();
+    },
     expectedStatusCode: opts?.expectedStatusCode,
   });

Optionally, centralize the logic by extending confirmBooking and calling it from here (outside this hunk):

// Update helper (outside this hunk)
export async function confirmBooking(page: Page, url = "/api/book/event", expectedStatusCode = 200) {
  await submitAndWaitForResponse(page, url, {
    action: async () => {
      const btn = page.getByTestId("confirm-book-button");
      await expect(btn).toBeEnabled();
      await btn.click();
    },
    expectedStatusCode,
  });
}

// Then in bookTimeSlot (replace the submitAndWaitForResponse block):
await confirmBooking(page, "/api/book/event", opts?.expectedStatusCode ?? 200);
📜 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 028f669 and ff0b6b2.

📒 Files selected for processing (2)
  • apps/web/playwright/event-types.e2e.ts (1 hunks)
  • apps/web/playwright/lib/testUtils.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/playwright/event-types.e2e.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 use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/web/playwright/lib/testUtils.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/web/playwright/lib/testUtils.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/web/playwright/lib/testUtils.ts
⏰ 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). (1)
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
apps/web/playwright/lib/testUtils.ts (2)

169-172: Switching to an explicit click on the confirm button reduces flakiness — good change.

Using the dedicated data-testid click avoids Enter key side effects (e.g., triggering the wrong form, dialogs consuming Enter, focus issues) and aligns with other helpers like confirmBooking. Looks good.


169-172: No additional Enter-key booking submissions found

I scanned the entire apps/** directory for any page.keyboard.press("Enter") or press("Enter") calls as well as for any other submitAndWaitFor…"/api/book/event" usages. The only booking flow submission to /api/book/event is already using the confirm-book-button click helper in apps/web/playwright/lib/testUtils.ts. All other Enter presses occur in unrelated filters or event-type update tests and do not affect booking flows.
No further changes needed here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
apps/web/playwright/event-types.e2e.ts (1)

447-477: Past review concern appears resolved — test now runs inside authenticated “user” suite

Earlier feedback noted this test ran outside the authenticated context. In the current diff it’s placed inside the describe("user") block that creates a user and logs in via beforeEach. No change needed.

🧹 Nitpick comments (5)
apps/web/playwright/event-types.e2e.ts (2)

460-461: Stabilize selector: avoid positional nth(1) for the occurrences input

Relying on the second numeric input under the recurring section is brittle and may break with innocuous UI changes. Prefer a stable, semantic handle (data-testid or label).

If the occurrences input has or can have a test id, target it directly:

- await page.locator("[data-testid=recurring-event-collapsible] input[type=number]").nth(1).fill("3");
+ await page.getByTestId("recurring-occurrences-input").fill("3");

If adding a data-testid isn’t possible right now, at least anchor by a nearby label to avoid index reliance (only if label text is stable and not localized in this suite):

- await page.locator("[data-testid=recurring-event-collapsible] input[type=number]").nth(1).fill("3");
+ await page.getByLabel("Occurrences").fill("3");

453-471: Add explicit readiness checks around booking page to reduce flakiness

Right after navigating to the booking page, assert visibility of both the occurrences input and the calendar grid before interacting. This helps in parallel mode.

   await gotoBookingPage(page);
-  await expect(page.locator("[data-testid=occurrence-input]")).toHaveValue("3");
+  await expect(page.locator("[data-testid=occurrence-input]")).toBeVisible();
+  await expect(page.locator("[data-testid=occurrence-input]")).toHaveValue("3");
+  await expect(page.locator('[data-testid="day"][data-disabled="false"]')).toBeVisible();

   await selectFirstAvailableTimeSlotNextMonth(page);

-  await expect(page.locator("[data-testid=recurring-dates]")).toBeVisible();
+  await expect(page.locator("[data-testid=recurring-dates]")).toBeVisible();
apps/web/playwright/lib/testUtils.ts (3)

158-160: Option naming scales better as an explicit booking type

A boolean flag doesn’t scale if more booking kinds are added (e.g., seated, managed variations). Consider a discriminated union to avoid boolean proliferation and improve call-site clarity.

-export const bookTimeSlot = async (
-  page: Page,
-  opts?: {
-    name?: string;
-    email?: string;
-    title?: string;
-    attendeePhoneNumber?: string;
-    expectedStatusCode?: number;
-    isRecurringEvent?: boolean;
-  }
-) => {
+type BookingKind = "single" | "recurring";
+export const bookTimeSlot = async (
+  page: Page,
+  opts?: {
+    name?: string;
+    email?: string;
+    title?: string;
+    attendeePhoneNumber?: string;
+    expectedStatusCode?: number;
+    bookingKind?: BookingKind; // default: "single"
+  }
+) => {

And update the endpoint logic below accordingly.


170-174: Minor: co-locate endpoint derivation and button click with a bit more explicitness

Two small tweaks improve readability and resilience:

  • Derive endpoint from a clear “kind” (if adopting the previous suggestion), otherwise keep your boolean but name the local accordingly.
  • Use getByTestId for the action for consistency with the rest of the helpers.
-  const url = opts?.isRecurringEvent ? "/api/book/recurring-event" : "/api/book/event";
-  await submitAndWaitForResponse(page, url, {
-    action: () => page.locator('[data-testid="confirm-book-button"]').click(),
+  const url =
+    (opts as any)?.bookingKind === "recurring" || opts?.isRecurringEvent
+      ? "/api/book/recurring-event"
+      : "/api/book/event";
+  await submitAndWaitForResponse(page, url, {
+    action: () => page.getByTestId("confirm-book-button").click(),
     expectedStatusCode: opts?.expectedStatusCode,
   });

Note: The cast preserves backward compatibility if you adopt bookingKind incrementally.


170-174: Consider making submitAndWaitForResponse robust to baseURL differences

submitAndWaitForResponse currently receives a string and waits for that exact URL. If your baseURL differs across environments, using a predicate avoids brittle matches. This affects all callers, including this one.

Outside this hunk, update submitAndWaitForResponse to:

-  const submitPromise = page.waitForResponse(url);
+  const submitPromise = page.waitForResponse((res) => res.url().includes(url));

No call-site changes required.

📜 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 ff0b6b2 and c96500a.

📒 Files selected for processing (2)
  • apps/web/playwright/event-types.e2e.ts (1 hunks)
  • apps/web/playwright/lib/testUtils.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 use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/web/playwright/lib/testUtils.ts
  • apps/web/playwright/event-types.e2e.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/web/playwright/lib/testUtils.ts
  • apps/web/playwright/event-types.e2e.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/web/playwright/lib/testUtils.ts
  • apps/web/playwright/event-types.e2e.ts
🧬 Code graph analysis (1)
apps/web/playwright/event-types.e2e.ts (2)
apps/api/v2/test/utils/randomString.ts (1)
  • randomString (3-6)
apps/web/playwright/lib/testUtils.ts (5)
  • createNewUserEventType (184-196)
  • saveEventType (478-482)
  • gotoBookingPage (472-476)
  • selectFirstAvailableTimeSlotNextMonth (109-117)
  • bookTimeSlot (150-175)
🔇 Additional comments (2)
apps/web/playwright/event-types.e2e.ts (1)

447-477: Recurring booking E2E flow looks correct and aligns with helper updates

  • Uses the new isRecurringEvent option in bookTimeSlot, targets the recurring API endpoint, and validates success state. Flow matches how the UI is configured earlier in the test (enable recurring, set occurrences to 3). Good job.
apps/web/playwright/lib/testUtils.ts (1)

150-176: Overall: change is cohesive and backwards compatible

The helper now supports recurring bookings and aligns with the new confirm-book-button interaction. Existing single-booking call sites continue to work without changes.

Copy link
Contributor

@Devanshusharma2005 Devanshusharma2005 left a comment

Choose a reason for hiding this comment

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

LGTM

@anikdhabal anikdhabal merged commit f4944a0 into calcom:main Aug 26, 2025
57 of 64 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working core area: core, team members only High priority Created by Linear-GitHub Sync ready-for-e2e

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants