fix(api-v2): enable editing phone-only event types#28870
fix(api-v2): enable editing phone-only event types#28870bandhan-majumder wants to merge 1 commit intocalcom:mainfrom
Conversation
📝 WalkthroughWalkthroughThe PR modifies booking field validation for event types across multiple modules. A local validation method is replaced with a shared library validation function ( 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/api/v2/src/modules/teams/event-types/controllers/teams-event-types.controller.e2e-spec.ts (1)
235-239: Use unique slugs in the new tests to avoid cross-test coupling.Both new cases reuse
"phone-coding-consultation". A unique slug per test makes the validation target deterministic.Suggested fix
- slug: "phone-coding-consultation", + slug: `phone-coding-consultation-${randomString()}`, ... - slug: "phone-coding-consultation", + slug: `phone-coding-consultation-hidden-${randomString()}`,Also applies to: 283-286
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/v2/src/modules/teams/event-types/controllers/teams-event-types.controller.e2e-spec.ts` around lines 235 - 239, The two tests that create phone-only event types (the "should be able to create phone-only event type" it-block and the other it-block reusing the same slug) reuse the slug "phone-coding-consultation" which can cause cross-test coupling; update each CreateTeamEventTypeInput_2024_06_14.slug to a unique value per test (e.g., add a distinct suffix or index) so each test targets a deterministic, independent slug and avoids interference between test runs.apps/api/v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.e2e-spec.ts (1)
374-375: Prefer unique slugs for the new phone-only tests.Using the same fixed slug in both tests adds avoidable coupling and can obscure the real failure reason.
Suggested fix
- slug: "phone-coding-consultation", + slug: `phone-coding-consultation-${randomString()}`, ... - slug: "phone-coding-consultation", + slug: `phone-coding-consultation-hidden-${randomString()}`,Also applies to: 411-412
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.e2e-spec.ts` around lines 374 - 375, Replace the duplicated slug "phone-coding-consultation" used in the two phone-only test fixtures with distinct values (e.g., "phone-coding-consultation-1" and "phone-coding-consultation-2") so the tests do not share the same identifier; locate the slug occurrences (the object containing slug: "phone-coding-consultation" and description: "Our team will review your codebase.") in the event-types.controller.e2e-spec.ts phone-only tests, update both slug strings to unique names, and also update any assertions or lookups that reference that slug so they match the new unique values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/api/v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.e2e-spec.ts`:
- Around line 400-405: The test assertion for creating an event type is
expecting 200 but should expect 201; update the call in the spec that posts to
"/api/v2/event-types" (the request(...) chain using CAL_API_VERSION_HEADER,
VERSION_2024_06_14, apiKeyString, and body) to use .expect(201) instead of
.expect(200) so the create-endpoint contract matches the rest of the suite.
In
`@apps/api/v2/src/modules/teams/event-types/controllers/teams-event-types.controller.e2e-spec.ts`:
- Around line 276-280: The test is asserting a 200 on the POST to create an
event type but other tests and the API contract use 201 for successful creates;
update the assertion in the POST request to /v2/teams/${team.id}/event-types in
teams-event-types.controller.e2e-spec.ts (the request block that currently calls
.expect(200)) to .expect(201) so the test matches the create response code.
---
Nitpick comments:
In
`@apps/api/v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.e2e-spec.ts`:
- Around line 374-375: Replace the duplicated slug "phone-coding-consultation"
used in the two phone-only test fixtures with distinct values (e.g.,
"phone-coding-consultation-1" and "phone-coding-consultation-2") so the tests do
not share the same identifier; locate the slug occurrences (the object
containing slug: "phone-coding-consultation" and description: "Our team will
review your codebase.") in the event-types.controller.e2e-spec.ts phone-only
tests, update both slug strings to unique names, and also update any assertions
or lookups that reference that slug so they match the new unique values.
In
`@apps/api/v2/src/modules/teams/event-types/controllers/teams-event-types.controller.e2e-spec.ts`:
- Around line 235-239: The two tests that create phone-only event types (the
"should be able to create phone-only event type" it-block and the other it-block
reusing the same slug) reuse the slug "phone-coding-consultation" which can
cause cross-test coupling; update each CreateTeamEventTypeInput_2024_06_14.slug
to a unique value per test (e.g., add a distinct suffix or index) so each test
targets a deterministic, independent slug and avoids interference between test
runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e77a48e-87a6-47c7-858d-f15a00469674
📒 Files selected for processing (5)
apps/api/v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.e2e-spec.tsapps/api/v2/src/ee/event-types/event-types_2024_06_14/services/event-types.service.tsapps/api/v2/src/modules/teams/event-types/controllers/teams-event-types.controller.e2e-spec.tsapps/api/v2/src/modules/teams/event-types/services/teams-event-types.service.tspackages/platform/libraries/event-types.ts
.../v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.e2e-spec.ts
Show resolved
Hide resolved
apps/api/v2/src/modules/teams/event-types/controllers/teams-event-types.controller.e2e-spec.ts
Show resolved
Hide resolved
|
@Udit-takkar @supalarry can I please get a review? |
What does this PR do?
Previously only email based event types were supported to be updated via API v2. But we also have phone-only options for event types. So, this PR reuses the existing function to check whether the booking fields of the event has either phone/email or not. Includes relevant test updates as well.
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
after.mp4
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Create an event type and go to the advanced tab and disable email. Enable phone and click save.
Get a api key
Now query the endpoint
http://localhost:5555/v2/event-types/{eventTypeId}to update event type with the following headersAuthorization: api_key
cal-api-version: 2024-06-14
Content-Type: application/json
and this body:
Checklist