Conversation
|
@hemantmm is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe e2e test apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts was modified to add creation of an org-admin managed user via POST to the OAuth client users endpoint, assign that user an ADMIN membership in the organization via POST to /v2/organizations/{organization.id}/memberships, and a brief comment was added above app.init(). These steps run in the beforeAll setup after the existing Owner membership creation. Assessment against linked issues
Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
github-actions
bot
added
api
github-actions
bot
added
enterprise
dosubot
bot
added
organizations
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (1)
147-149: Assert response status before using dataSmall safety net: assert SUCCESS_STATUS to fail fast with clearer signal if the API ever regresses.
Apply this diff:
const responseBody: CreateManagedUserOutput = response.body; + expect(responseBody.status).toEqual(SUCCESS_STATUS); orgAdminManagedUser = responseBody.data;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts
🧠 Learnings (1)
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts
🧬 Code graph analysis (1)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (2)
apps/api/v2/src/modules/users/inputs/create-managed-user.input.ts (1)
CreateManagedUserInput(10-79)apps/api/v2/src/modules/oauth-clients/controllers/oauth-client-users/outputs/create-managed-user.output.ts (1)
CreateManagedUserOutput(18-31)
⏰ 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
| // Create org admin managed user and assign ADMIN membership in org | ||
| const requestBody: CreateManagedUserInput = { | ||
| email: orgAdminManagedUserEmail, | ||
| timeZone: managedUsersTimeZone, | ||
| weekStart: "Monday", | ||
| timeFormat: 24, | ||
| locale: Locales.FR, | ||
| name: "Org Admin Smith", | ||
| avatarUrl: "https://cal.com/api/avatar/2b735186-b01b-46d3-87da-019b8f61776b.png", | ||
| }; | ||
|
|
||
| const response = await request(app.getHttpServer()) | ||
| .post(`/api/v2/oauth-clients/${oAuthClient.id}/users`) | ||
| .set("x-cal-secret-key", oAuthClient.secret) | ||
| .send(requestBody) | ||
| .expect(201); | ||
|
|
||
| const responseBody: CreateManagedUserOutput = response.body; | ||
| orgAdminManagedUser = responseBody.data; | ||
|
|
||
| await request(app.getHttpServer()) | ||
| .post(`/v2/organizations/${organization.id}/memberships`) | ||
| .set("x-cal-client-id", oAuthClient.id) | ||
| .set("x-cal-secret-key", oAuthClient.secret) | ||
| .send({ | ||
| userId: orgAdminManagedUser.user.id, | ||
| role: "ADMIN", | ||
| accepted: true, | ||
| }) | ||
| .expect(201); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate org admin user creation will likely 409 and/or flake
You now create the org admin managed user here and again in test "should create org admin managed user" (Lines 290-329). Using the same email will typically return 409 (user exists) and break the test. Seed once in beforeAll and repurpose the test to assert seeding/membership rather than creating again.
Example change outside this hunk:
it(`should have org admin managed user seeded`, async () => {
expect(orgAdminManagedUser).toBeDefined();
expect(orgAdminManagedUser.user.email).toBe(
OAuthClientUsersService.getOAuthUserEmail(oAuthClient.id, orgAdminManagedUserEmail)
);
});🤖 Prompt for AI Agents
In
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts
around lines 130-159, you are creating the org admin managed user here and again
later (lines ~290-329), which causes 409 flakes; seed the org admin managed user
only once (keep creation in beforeAll or this initial setup block) and remove
the duplicate POST in the later test, then change that later test to assert the
seeded user and membership instead of creating it (e.g., expect
orgAdminManagedUser to be defined and its email to match
OAuthClientUsersService.getOAuthUserEmail(oAuthClient.id,
orgAdminManagedUserEmail), and assert membership exists), ensuring headers and
expected status assertions are only in the seeding step.
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (1)
289-327: Remove duplicate org admin managed user creation to avoid 409 flakesThis test recreates the same org admin managed user seeded in beforeAll, likely causing 409 conflicts and nondeterministic failures. Replace it with an assertion that the seeded user exists (and optionally its transformed email).
- it(`should create org admin managed user`, async () => { - const requestBody: CreateManagedUserInput = { - email: orgAdminManagedUserEmail, - timeZone: managedUsersTimeZone, - weekStart: "Monday", - timeFormat: 24, - locale: Locales.FR, - name: "Org Admin Smith", - avatarUrl: "https://cal.com/api/avatar/2b735186-b01b-46d3-87da-019b8f61776b.png", - }; - - const response = await request(app.getHttpServer()) - .post(`/api/v2/oauth-clients/${oAuthClient.id}/users`) - .set("x-cal-secret-key", oAuthClient.secret) - .send(requestBody) - .expect(201); - - const responseBody: CreateManagedUserOutput = response.body; - expect(responseBody.status).toEqual(SUCCESS_STATUS); - expect(responseBody.data).toBeDefined(); - expect(responseBody.data.user.email).toEqual( - OAuthClientUsersService.getOAuthUserEmail(oAuthClient.id, requestBody.email) - ); - expect(responseBody.data.accessToken).toBeDefined(); - expect(responseBody.data.refreshToken).toBeDefined(); - - orgAdminManagedUser = responseBody.data; - - await request(app.getHttpServer()) - .post(`/v2/organizations/${organization.id}/memberships`) - .set("x-cal-client-id", oAuthClient.id) - .set("x-cal-secret-key", oAuthClient.secret) - .send({ - userId: orgAdminManagedUser.user.id, - role: "ADMIN", - accepted: true, - }) - .expect(201); - }); + it(`should have org admin managed user seeded`, async () => { + expect(orgAdminManagedUser).toBeDefined(); + expect(orgAdminManagedUser.user.email).toBe( + OAuthClientUsersService.getOAuthUserEmail(oAuthClient.id, orgAdminManagedUserEmail) + ); + expect(orgAdminManagedUser.accessToken).toBeDefined(); + });#!/bin/bash # Verify duplicate creation attempts for the same org admin managed user rg -n -C2 'orgAdminManagedUserEmail' apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts
🧹 Nitpick comments (1)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (1)
142-147: Add x-cal-client-id for header parity and future-proofingThe create-managed-user endpoint currently authenticates with the secret, but most platform endpoints expect both client id and secret. Add the client id for consistency and to guard against future contract tightening.
- const response = await request(app.getHttpServer()) - .post(`/api/v2/oauth-clients/${oAuthClient.id}/users`) - .set("x-cal-secret-key", oAuthClient.secret) - .send(requestBody) - .expect(201); + const response = await request(app.getHttpServer()) + .post(`/api/v2/oauth-clients/${oAuthClient.id}/users`) + .set("x-cal-client-id", oAuthClient.id) + .set("x-cal-secret-key", oAuthClient.secret) + .send(requestBody) + .expect(201);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts
🧠 Learnings (1)
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts
🧬 Code graph analysis (1)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (2)
apps/api/v2/src/modules/users/inputs/create-managed-user.input.ts (1)
CreateManagedUserInput(10-79)apps/api/v2/src/modules/oauth-clients/controllers/oauth-client-users/outputs/create-managed-user.output.ts (1)
CreateManagedUserOutput(18-31)
⏰ 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). (4)
- GitHub Check: Type check / check-types
- GitHub Check: Tests / Unit
- GitHub Check: Linters / lint
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (1)
130-161: App initialization moved before HTTP calls — LGTM
await app.init()now runs before the first supertest request; the prior double-init risk is gone.
| const requestBody: CreateManagedUserInput = { | ||
| email: orgAdminManagedUserEmail, | ||
| timeZone: managedUsersTimeZone, | ||
| weekStart: "Monday", | ||
| timeFormat: 24, | ||
| locale: Locales.FR, | ||
| name: "Org Admin Smith", | ||
| avatarUrl: "https://cal.com/api/avatar/2b735186-b01b-46d3-87da-019b8f61776b.png", | ||
| }; | ||
|
|
||
| const response = await request(app.getHttpServer()) | ||
| .post(`/api/v2/oauth-clients/${oAuthClient.id}/users`) | ||
| .set("x-cal-secret-key", oAuthClient.secret) | ||
| .send(requestBody) | ||
| .expect(201); | ||
|
|
||
| const responseBody: CreateManagedUserOutput = response.body; | ||
| orgAdminManagedUser = responseBody.data; | ||
|
|
||
| await request(app.getHttpServer()) | ||
| .post(`/v2/organizations/${organization.id}/memberships`) | ||
| .set("x-cal-client-id", oAuthClient.id) |
There was a problem hiding this comment.
Where is the original fix? It only updating the test. Also, pls include evidence of the fix or we can't approve it
|
Thanks for your work. But @supalarry is working on it 🙏 |
2 participants
closes: #22513
What does this PR do?
Added the creation of an org admin-managed user and assigned them as ADMIN in the organization.
Uses the org admin-managed user's access token to create teams and event types, verifying that 401 does not occur.
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist