Skip to content

feat: add usernameInOrg field to webhook organizer payload for organization users#23246

Merged
hariombalhara merged 6 commits intomainfrom
devin/1755764679-add-profile-username-webhook
Oct 6, 2025
Merged

feat: add usernameInOrg field to webhook organizer payload for organization users#23246
hariombalhara merged 6 commits intomainfrom
devin/1755764679-add-profile-username-webhook

Conversation

@hariombalhara
Copy link
Member

@hariombalhara hariombalhara commented Aug 21, 2025

What does this PR do?

Corresponding update to help document - calcom/help#40

This PR adds a new usernameInOrg field to webhook organizer payloads for organization users across all webhook sending locations in the Cal.com codebase, while maintaining backward compatibility with the existing username field.

Problem: Currently, webhooks send organizer.username which comes from the User table. However, for organization users, the actual username stored in the organization profile (profile.username) is different and more relevant for practical purposes.

Solution: Added a new usernameInOrg field that contains the organization profile username for organization users, while preserving the existing username field for backward compatibility.

Field Behavior

  • For organization users: usernameInOrg contains the profile username
  • For regular users: usernameInOrg is not sent
  • All users: username field remains unchanged for backward compatibility

How should this be tested?

Prerequisites

  • Set up a local Cal.com development environment
  • Create webhook endpoints to capture payloads
  • Set up organization with users having profile usernames

Test Cases

  1. Organization User Booking: Create a booking with an organization user and verify webhook payload contains both username and usernameInOrg fields
  2. Regular User Booking: Create a booking with a regular user and verify usernameInOrg is undefined while username is present
  3. Webhook Types: Test across different webhook events (created, cancelled, confirmed, rescheduled)

Expected Webhook Payload Example

{
  "organizer": {
    "id": 1644281,
    "name": "John Doe", 
    "username": "john-doe-123",
    "usernameInOrg": "john.doe",
    "email": "john.doe@example.com",
    "timeZone": "America/Chicago"
  }
}

Visual Demo

No UI changes - this is a backend webhook payload enhancement. Testing requires webhook payload inspection.

Human Review Checklist

⚠️ High Priority Items to Verify:

  • Completeness: Verify all webhook sending locations have been updated (search for any missed locations)
  • Performance: Check that new organizerOrganizationProfile database queries don't cause performance issues
  • Payload Testing: Test actual webhook payloads to ensure usernameInOrg appears correctly for organization users
  • Organization Profile Resolution: Verify the organization profile fetching logic correctly identifies the right profile for users
  • Backward Compatibility: Confirm existing username field behavior is unchanged

Mandatory Tasks

  • I have self-reviewed the code
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. N/A - This is an additive webhook field that doesn't require documentation changes.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

Link to Devin run: https://app.devin.ai/sessions/2e5b9c296aff41c2aebe17f02d0afc27
Requested by: @hariombalhara

⚠️ Note: Local Playwright tests failed due to database schema issues (allowReschedulingCancelledBookings column missing), but this appears to be an environment setup issue unrelated to the webhook changes. The new test case was added but couldn't complete due to the schema mismatch.

…zation users

- Add usernameInOrg field to CalendarEventBuilder organizer interface
- Update handleNewBooking to pass organizerOrganizationProfile.username as usernameInOrg
- Include usernameInOrg in webhook payload generation (sendPayload.ts)
- Add webhook form variable for usernameInOrg with translation
- Update Person type to include usernameInOrg field
- Add test case for organization user webhook verification
- Maintain backward compatibility by keeping existing username field unchanged

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 21, 2025

Walkthrough

Adds a new optional organizer field usernameInOrg across multiple layers: event builders, booking handlers (new and cancel), webhook payloads (including Zapier), viewer confirm handler, and the Person type declaration. Webhook UI/variables and English localization gain organizer.username and organizer.usernameInOrg entries. Tests and test utilities are updated to fetch and use organization profile usernames and to assert the new field. Test utility typings were tightened (event metadata schema, credential key as Prisma.JsonValue, new PaymentData type, and more structured event-type inputs).

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly states the central feature of this pull request by describing the addition of the usernameInOrg field to webhook organizer payloads for organization users. It directly reflects the primary change without extraneous detail and follows a concise, conventional commit style. This makes it easy for reviewers and future readers to understand the purpose at a glance.
Description Check ✅ Passed The pull request description clearly outlines the addition of the new usernameInOrg field to webhook organizer payloads for organization users, explains the motivation, and matches the extensive code changes across webhook handlers, types, and tests as shown in the diff summaries. It directly relates to the changeset and is neither off-topic nor overly vague. The level of detail is sufficient to understand the intent and scope of the updates.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch devin/1755764679-add-profile-username-webhook

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

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Aug 21, 2025
- Update handleCancelBooking.ts to include usernameInOrg in organizer payload
- Update confirm.handler.ts to include usernameInOrg for booking confirmations
- Update getBooking.ts to include usernameInOrg in payment-related webhooks
- Maintain backward compatibility with existing username field
- All webhook sending locations now include organization profile username

Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
@vercel
Copy link

vercel bot commented Aug 21, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Oct 6, 2025 1:26pm
cal-eu Ignored Ignored Oct 6, 2025 1:26pm

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2025

This PR is being marked as stale due to inactivity.

@github-actions github-actions bot added the Stale label Sep 5, 2025
@hariombalhara hariombalhara added the 🚧 wip / in the making This is currently being worked on label Sep 26, 2025
@hariombalhara hariombalhara reopened this Oct 4, 2025
Comment on lines 1621 to 1641
@@ -1617,11 +1632,13 @@ export function getScenarioData(
if (!orgId) {
throw new Error("If org is specified org.id is required");
}

users.forEach((user) => {
const profileUsername = org.profileUsername ?? user.username ?? "";
user.profiles = [
{
organizationId: orgId,
username: user.username || "",
username: profileUsername,
Copy link
Member Author

Choose a reason for hiding this comment

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

Main change is here.

Others are to make angry eslint less angry

@hariombalhara hariombalhara force-pushed the devin/1755764679-add-profile-username-webhook branch from 4996cbf to ec2e1c2 Compare October 6, 2025 10:22
const organizerOrganizationProfile = await prisma.profile.findFirst({
where: {
userId: organizerUser.id,
username: dynamicUserList[0],
Copy link
Member Author

Choose a reason for hiding this comment

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

Important change. For team events, organizerOrganizationProfile would always be null because dynamicUserList[0] would be teamSlug.

This never caused any problem because, for team events we were using different way to determine bookingUrl.

Now I need to use organizerOrganizationProfile to detemine the username correctly for team as well as user events.

Image

Comment on lines +168 to +171
name: "organizer.username",
variable: "{{organizer.username}}",
type: "String",
description: t("webhook_organizer_username"),
Copy link
Member Author

Choose a reason for hiding this comment

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

documented this existing variable too cc @alishaz-polymath

@hariombalhara hariombalhara marked this pull request as ready for review October 6, 2025 10:28
@dosubot dosubot bot added organizations area: organizations, orgs webhooks area: webhooks, callback, webhook payload ✨ feature New feature or request labels Oct 6, 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: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/features/bookings/lib/handleNewBooking.ts (1)

1208-1215: Limit Prisma profile fetch to required fields.

We only need username and organizationId here, but findFirst currently pulls the full row. Please add a select to keep the query lean and aligned with our Prisma usage guideline.

-  const organizerOrganizationProfile = await prisma.profile.findFirst({
-    where: {
-      userId: organizerUser.id,
-    },
-  });
+  const organizerOrganizationProfile = await prisma.profile.findFirst({
+    where: {
+      userId: organizerUser.id,
+    },
+    select: {
+      username: true,
+      organizationId: true,
+    },
+  });

Also applies to: 1266-1274

packages/trpc/server/routers/viewer/bookings/confirm.handler.ts (1)

190-198: Scope down the organizer profile query.

findFirst is currently returning the entire profile, even though we only use username (and the existing code path already has the organization ID). Please add a select to keep the query tight and avoid pulling unused columns.

-  const organizerOrganizationProfile = await prisma.profile.findFirst({
-    where: {
-      userId: user.id,
-    },
-  });
+  const organizerOrganizationProfile = await prisma.profile.findFirst({
+    where: {
+      userId: user.id,
+    },
+    select: {
+      username: true,
+    },
+  });

Also applies to: 219-227

🧹 Nitpick comments (5)
packages/features/bookings/lib/handleNewBooking/test/fresh-booking.test.ts (1)

152-166: Skip profile lookup when no org context.

When org is undefined we still call prismaMock.profile.findMany, which ends up querying without filters and can return an unrelated profile; the first hit then leaks into user and the webhook expectations. Guard the lookup (and orgProfile usage) so we only touch profiles when org?.organization exists.

-        const orgProfiles = await prismaMock.profile.findMany({
-          where: {
-            organizationId: org?.organization.id,
-          },
-        });
-        const orgProfile = orgProfiles[0];
+        const orgProfile = org?.organization
+          ? await prismaMock.profile.findFirst({
+              where: {
+                organizationId: org.organization.id,
+                userId: organizer.id,
+              },
+            })
+          : null;
@@
-        const mockBookingData = getMockRequestDataForBooking({
-          data: {
-            user: orgProfile ? orgProfile.username : organizer.username,
+        const mockBookingData = getMockRequestDataForBooking({
+          data: {
+            user: orgProfile?.username ?? organizer.username,
@@
-        expectBookingCreatedWebhookToHaveBeenFired({
+        expectBookingCreatedWebhookToHaveBeenFired({
           booker,
           organizer: {
             ...organizer,
-            ...(orgProfile?.username ?
-              { usernameInOrg: orgProfile?.username } : null),
+            ...(orgProfile?.username ? { usernameInOrg: orgProfile.username } : null),
           },

Also applies to: 181-190, 258-266

packages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.ts (2)

130-130: Consider maintaining consistency with app references.

The change from a dynamic metadataLookupKey to hardcoded "dailyvideo" works correctly but introduces a slight inconsistency. The test setup uses TestData.apps["daily-video"] elsewhere, suggesting a preference for centralized app references.

For better maintainability, consider either:

  1. Using a constant like const DAILY_VIDEO_KEY = "dailyvideo" if hardcoding is preferred
  2. Or deriving the key from the app metadata: TestData.apps["daily-video"].type

Also applies to: 322-322, 431-431, 580-580


918-1088: Well-structured organization webhook test with a minor documentation issue.

The test comprehensively verifies webhook payloads for organization team events, including:

  • Organization profile setup
  • Webhook configuration
  • Booking creation
  • Webhook payload verification with usernameInOrg

However, the comment on line 1075 states "This test will initially fail due to the bug," which is misleading since this PR implements the fix. This comment should be updated or removed.

Additionally, consider adding an assertion to verify that organizerUserId is indeed one of the team members before accessing the profile:

 const organizerUserId = createdBooking.userId;
 const organizerTeamMember = teamMembers.find((member) => member.id === organizerUserId);
+expect(organizerTeamMember).toBeDefined();
 const organizerProfile = organizerTeamMember?.profiles?.[0];
apps/web/test/utils/bookingScenario/bookingScenario.ts (2)

577-578: Acceptable type assertion with acknowledged complexity.

The type cast to any is properly documented and uses the recommended as unknown as any pattern. However, this represents type system technical debt that could be addressed in the future by refining the types involved in addEventTypesToDb.


2135-2135: Optional: Remove unused variable.

The _appStoreLookupKey variable is defined but never used. Consider removing it unless it's reserved for future functionality.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7799b19 and ec2e1c2.

📒 Files selected for processing (12)
  • apps/web/public/static/locales/en/common.json (1 hunks)
  • apps/web/test/utils/bookingScenario/bookingScenario.ts (17 hunks)
  • apps/web/test/utils/bookingScenario/expects.ts (6 hunks)
  • packages/features/CalendarEventBuilder.ts (2 hunks)
  • packages/features/bookings/lib/handleCancelBooking.ts (1 hunks)
  • packages/features/bookings/lib/handleNewBooking.ts (1 hunks)
  • packages/features/bookings/lib/handleNewBooking/test/fresh-booking.test.ts (21 hunks)
  • packages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.ts (9 hunks)
  • packages/features/webhooks/components/WebhookForm.tsx (1 hunks)
  • packages/features/webhooks/lib/sendPayload.ts (1 hunks)
  • packages/trpc/server/routers/viewer/bookings/confirm.handler.ts (1 hunks)
  • packages/types/Calendar.d.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts

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

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

Files:

  • packages/types/Calendar.d.ts
  • packages/trpc/server/routers/viewer/bookings/confirm.handler.ts
  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/features/CalendarEventBuilder.ts
  • packages/features/webhooks/lib/sendPayload.ts
  • packages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.ts
  • packages/features/bookings/lib/handleNewBooking/test/fresh-booking.test.ts
  • apps/web/test/utils/bookingScenario/expects.ts
  • packages/features/bookings/lib/handleCancelBooking.ts
  • apps/web/test/utils/bookingScenario/bookingScenario.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/types/Calendar.d.ts
  • packages/trpc/server/routers/viewer/bookings/confirm.handler.ts
  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/features/CalendarEventBuilder.ts
  • packages/features/webhooks/lib/sendPayload.ts
  • packages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.ts
  • packages/features/bookings/lib/handleNewBooking/test/fresh-booking.test.ts
  • apps/web/test/utils/bookingScenario/expects.ts
  • packages/features/webhooks/components/WebhookForm.tsx
  • packages/features/bookings/lib/handleCancelBooking.ts
  • apps/web/test/utils/bookingScenario/bookingScenario.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/types/Calendar.d.ts
  • packages/trpc/server/routers/viewer/bookings/confirm.handler.ts
  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/features/CalendarEventBuilder.ts
  • packages/features/webhooks/lib/sendPayload.ts
  • packages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.ts
  • packages/features/bookings/lib/handleNewBooking/test/fresh-booking.test.ts
  • apps/web/test/utils/bookingScenario/expects.ts
  • packages/features/webhooks/components/WebhookForm.tsx
  • packages/features/bookings/lib/handleCancelBooking.ts
  • apps/web/test/utils/bookingScenario/bookingScenario.ts
**/*.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/webhooks/components/WebhookForm.tsx
🧠 Learnings (3)
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.

Applied to files:

  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/features/bookings/lib/handleNewBooking/test/fresh-booking.test.ts
  • apps/web/test/utils/bookingScenario/expects.ts
📚 Learning: 2025-09-12T07:15:58.056Z
Learnt from: hbjORbj
PR: calcom/cal.com#23475
File: packages/features/credentials/handleDeleteCredential.ts:5-10
Timestamp: 2025-09-12T07:15:58.056Z
Learning: When EventTypeAppMetadataSchema is used only in z.infer<typeof EventTypeAppMetadataSchema> type annotations or type casts, it can be imported as a type-only import since this usage is purely at the TypeScript type level and doesn't require the runtime value.

Applied to files:

  • apps/web/test/utils/bookingScenario/bookingScenario.ts
📚 Learning: 2025-09-01T08:56:14.071Z
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzon-space/lib/tokenManager.ts:25-31
Timestamp: 2025-09-01T08:56:14.071Z
Learning: In token refresh utilities like tokenManager.ts, accessing credential.key from Prisma is legitimate and necessary for OAuth token refresh flows. These internal utilities need stored credentials to refresh tokens and don't expose them in API responses.

Applied to files:

  • apps/web/test/utils/bookingScenario/bookingScenario.ts
🧬 Code graph analysis (3)
packages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.ts (2)
apps/web/test/utils/bookingScenario/bookingScenario.ts (10)
  • mockCalendarToHaveNoBusySlots (1979-1992)
  • getBooker (2239-2253)
  • Timezones (321-325)
  • TestData (1254-1526)
  • getGoogleCalendarCredential (1207-1215)
  • getOrganizer (1535-1594)
  • createBookingScenario (993-1024)
  • getScenarioData (1596-1684)
  • mockSuccessfulVideoMeetingCreation (2094-2112)
  • getDate (1108-1155)
apps/web/test/utils/bookingScenario/expects.ts (1)
  • expectBookingCreatedWebhookToHaveBeenFired (1033-1117)
packages/features/bookings/lib/handleNewBooking/test/fresh-booking.test.ts (1)
scripts/prepare-local-for-delegation-credentials-testing.js (1)
  • org (42-44)
apps/web/test/utils/bookingScenario/bookingScenario.ts (1)
packages/types/Calendar.d.ts (1)
  • IntegrationCalendar (246-255)
⏰ 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 (15)
apps/web/public/static/locales/en/common.json (1)

3680-3682: Localization entries look good.

Thanks for adding the explicit copy for both organizer username variants—this keeps the webhook variable list self-explanatory.

packages/features/CalendarEventBuilder.ts (1)

84-100: Builder update keeps organizer payload in sync.

The optional usernameInOrg passthrough slots neatly into the existing shape—nice touch keeping the defaults intact.

packages/types/Calendar.d.ts (1)

38-40: Type surface extended cleanly.

Adding usernameInOrg to Person aligns the public contract with the new organizer field—no issues spotted.

packages/features/webhooks/components/WebhookForm.tsx (1)

167-178: Webhook variables table reads clearly.

Great job surfacing both organizer.username and organizer.usernameInOrg in the variable picker with localized descriptions—it keeps template authors aware of the new field.

packages/features/webhooks/lib/sendPayload.ts (1)

143-146: Zapier payload enrichment confirmed.

Including usernameInOrg alongside the existing organizer fields ensures third-party consumers can differentiate org-scoped usernames without breaking compatibility.

packages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.ts (2)

15-15: LGTM: Necessary imports for webhook testing.

The added imports support the new organization team webhook test and are correctly used in the test suite.

Also applies to: 24-24


695-696: LGTM: Valid edge case testing.

The test correctly differentiates between groupId: null and an absent groupId property to verify proper handling of both cases.

apps/web/test/utils/bookingScenario/bookingScenario.ts (8)

32-32: LGTM: Correct type-only import.

The EventTypeMetaDataSchema is properly imported as a type-only import and used for type annotations on lines 271 and 394. Based on learnings.


54-54: LGTM: Type safety improvements.

The changes from any to unknown and adding explicit type annotations improve type safety without changing functionality.

Also applies to: 76-76


118-127: LGTM: Well-defined PaymentData type.

The new PaymentData type properly structures common payment fields while maintaining flexibility for provider-specific fields through the index signature with unknown type.


139-139: LGTM: Proper use of PaymentData type.

The change from Record<string, any> to PaymentData provides better type safety for payment data.


271-271: LGTM: Improved metadata type safety.

The metadata fields are now properly typed using z.infer<typeof EventTypeMetaDataSchema>, ensuring type consistency with the schema definition.

Also applies to: 383-394


1080-1080: LGTM: Correct Prisma JSON type.

The change from any to Prisma.JsonValue properly types the credential key field according to Prisma's type system. Based on learnings.


1621-1621: LGTM: Organization profile username support added.

The profileUsername parameter enables test scenarios to simulate organization-scoped usernames. The fallback logic (org.profileUsername ?? user.username ?? "") provides sensible defaults.

Note: When org.profileUsername is provided, all users receive the same organization username. Tests requiring different usernames per user should set the profiles array directly on each user object, as demonstrated in the round-robin organization test.

Also applies to: 1635-1645


1820-1820: LGTM: Improved mock function type annotations.

The explicit type annotations on mock function parameters improve type safety and code clarity without affecting functionality.

Also applies to: 1943-1948, 2033-2033

Copy link
Contributor

@volnei volnei left a comment

Choose a reason for hiding this comment

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

small comments

@github-actions github-actions bot marked this pull request as draft October 6, 2025 11:33
@hariombalhara hariombalhara requested a review from volnei October 6, 2025 12:48
@pull-request-size pull-request-size bot added size/XL and removed size/L labels Oct 6, 2025
@hariombalhara hariombalhara marked this pull request as ready for review October 6, 2025 12:48
volnei
volnei previously approved these changes Oct 6, 2025
@hariombalhara hariombalhara force-pushed the devin/1755764679-add-profile-username-webhook branch from 5008d4c to fa5c0ea Compare October 6, 2025 13:25
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)
apps/web/test/utils/bookingScenario/bookingScenario.ts (1)

1640-1649: Do not assign the same org profile username to every user

Inside getScenarioData, the provided org.profileUsername is copied onto every user’s profile. In multi-user scenarios this makes every profile share the same username, which clashes with the (organizationId, username) uniqueness constraint and produces incorrect usernameInOrg values for non-organizer users. Please apply the override only to the intended organizer (or leave other users on their own user.username), for example:

-    users.forEach((user) => {
-      const profileUsername = org.profileUsername ?? user.username ?? "";
-      user.profiles = [
-        {
-          organizationId: orgId,
-          username: profileUsername,
-          uid: ProfileRepository.generateProfileUid(),
-        },
-      ];
-    });
+    const organizerId = organizer?.id;
+    users.forEach((user) => {
+      if (user.profiles?.length) {
+        return;
+      }
+      const profileUsername =
+        organizerId && user.id === organizerId
+          ? org.profileUsername ?? user.username ?? ""
+          : user.username ?? "";
+      user.profiles = [
+        {
+          organizationId: orgId,
+          username: profileUsername,
+          uid: ProfileRepository.generateProfileUid(),
+        },
+      ];
+    });
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5008d4c and fa5c0ea.

📒 Files selected for processing (3)
  • apps/web/test/utils/bookingScenario/bookingScenario.ts (21 hunks)
  • apps/web/test/utils/bookingScenario/expects.ts (7 hunks)
  • packages/features/bookings/lib/handleCancelBooking/test/handleCancelBooking.test.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/features/bookings/lib/handleCancelBooking/test/handleCancelBooking.test.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/test/utils/bookingScenario/expects.ts
  • apps/web/test/utils/bookingScenario/bookingScenario.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/test/utils/bookingScenario/expects.ts
  • apps/web/test/utils/bookingScenario/bookingScenario.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/test/utils/bookingScenario/expects.ts
  • apps/web/test/utils/bookingScenario/bookingScenario.ts
🧠 Learnings (4)
📓 Common learnings
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.
📚 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/web/test/utils/bookingScenario/bookingScenario.ts
📚 Learning: 2025-09-12T07:15:58.056Z
Learnt from: hbjORbj
PR: calcom/cal.com#23475
File: packages/features/credentials/handleDeleteCredential.ts:5-10
Timestamp: 2025-09-12T07:15:58.056Z
Learning: When EventTypeAppMetadataSchema is used only in z.infer<typeof EventTypeAppMetadataSchema> type annotations or type casts, it can be imported as a type-only import since this usage is purely at the TypeScript type level and doesn't require the runtime value.

Applied to files:

  • apps/web/test/utils/bookingScenario/bookingScenario.ts
📚 Learning: 2025-09-01T08:56:14.071Z
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzon-space/lib/tokenManager.ts:25-31
Timestamp: 2025-09-01T08:56:14.071Z
Learning: In token refresh utilities like tokenManager.ts, accessing credential.key from Prisma is legitimate and necessary for OAuth token refresh flows. These internal utilities need stored credentials to refresh tokens and don't expose them in API responses.

Applied to files:

  • apps/web/test/utils/bookingScenario/bookingScenario.ts
🧬 Code graph analysis (1)
apps/web/test/utils/bookingScenario/bookingScenario.ts (2)
packages/types/VideoApiAdapter.d.ts (1)
  • VideoApiAdapter (19-49)
packages/types/Calendar.d.ts (3)
  • CalendarEvent (164-227)
  • NewCalendarEventType (72-85)
  • IntegrationCalendar (246-255)
⏰ 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

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2025

E2E results are ready!

@hariombalhara hariombalhara merged commit 9c97b2a into main Oct 6, 2025
41 checks passed
@hariombalhara hariombalhara deleted the devin/1755764679-add-profile-username-webhook branch October 6, 2025 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO ✨ feature New feature or request High priority Created by Linear-GitHub Sync organizations area: organizations, orgs ready-for-e2e size/XL webhooks area: webhooks, callback, webhook payload

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants