fix(api): return original email without OAuth suffix in bookings #31
Conversation
WalkthroughThe changes introduce a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 (1)
apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts (1)
411-424: Add sorting togetOutputRecurringSeatedBookingsto match other recurring methods.The similar methods
getOutputRecurringBookings(line 243) andgetOutputCreateRecurringSeatedBooking(line 448) both sort results by start time using.sort((a, b) => new Date(a.start).getTime() - new Date(b.start).getTime()). The seated variant should apply the same sort before returning.
🧹 Nitpick comments (4)
.github/oasdiff-err-ignore.txt (1)
5-10: Consider adding comments to document the rationale.Adding inline comments explaining why these particular subschemas are being ignored would help future maintainers understand the context, especially since this relates to the new
displayEmailfield additions mentioned in the PR summary.📝 Example with explanatory comments
+# Ignoring displayEmail field additions in booking responses POST /v2/bookings added 'subschema `#2`, subschema `#4`' to the 'data' response property 'oneOf' list for the response status '201' GET /v2/bookings/{bookingUid} added 'subschema `#3`, subschema `#6`' to the 'data' response property 'oneOf' list for the response status '200' POST /v2/bookings/{bookingUid}/cancel added 'subschema `#3`, subschema `#6`' to the 'data' response property 'oneOf' list for the response status '200' POST /v2/bookings/{bookingUid}/confirm added 'subschema `#3`, subschema `#6`' to the 'data' response property 'oneOf' list for the response status '200' POST /v2/bookings/{bookingUid}/decline added 'subschema `#3`, subschema `#6`' to the 'data' response property 'oneOf' list for the response status '200' POST /v2/bookings/{bookingUid}/guests added 'subschema `#3`, subschema `#6`' to the 'data' response property 'oneOf' list for the response status '200'packages/platform/types/bookings/2024-08-13/outputs/booking.output.ts (1)
59-62: Consider tightening and centralizing the email-sanitizing regex.Anchoring the suffix removal to the local part end reduces accidental stripping and keeps behavior aligned with the OAuth suffix format. Also, keep this helper aligned with
OutputBookingsService_2024_08_13.getDisplayEmailto avoid drift.🛠️ Suggested tweak
- static cleanEmailForDisplay(email: string): string { - return email.replace(/\+[a-zA-Z0-9]{25}/, ""); - } + static cleanEmailForDisplay(email: string): string { + return email.replace(/\+[A-Za-z0-9]{25}(?=@)/, ""); + }apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts (1)
96-98: Centralize email-sanitizing logic to avoid drift.
getDisplayEmailduplicates the helper inBookingAttendee.cleanEmailForDisplay. Consider reusing a single utility (e.g., import the static helper or move to a shared util) so regex updates stay consistent.docs/api-reference/v2/openapi.json (1)
31715-31719: Consider examples that illustrate the actual use case.The
displayEmailfield is intended to show a "clean" email by stripping OAuth/plus-addressing suffixes. However, the examples show identical values for bothdisplayEmail. To better communicate the field's purpose to API consumers, consider using examples that demonstrate when these values differ:"email": { "type": "string", "example": "[email protected]" }, "displayEmail": { "type": "string", "example": "[email protected]", "description": "Clean email for display purposes" }This applies to all schemas where
displayEmailis added (BookingHost, BookingAttendee, seated booking attendees, and ReassignedToDto).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.github/oasdiff-err-ignore.txtapps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/api-key-bookings.e2e-spec.tsapps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.tsapps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/reassign-bookings.e2e-spec.tsapps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/recurring-bookings.e2e-spec.tsapps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/seated-bookings.e2e-spec.tsapps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/team-bookings.e2e-spec.tsapps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/user-bookings.e2e-spec.tsapps/api/v2/src/ee/bookings/2024-08-13/services/output.service.tsapps/api/v2/src/modules/organizations/bookings/organizations-bookings.controller.e2e-spec.tsapps/api/v2/src/modules/organizations/teams/bookings/organizations-teams-bookings.controller.e2e-spec.tsapps/api/v2/src/modules/organizations/users/bookings/controllers/organizations-users-bookings.e2e-spec.tsdocs/api-reference/v2/openapi.jsonpackages/platform/types/bookings/2024-08-13/outputs/booking.output.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (2)
apps/api/index.js (1)
app(13-13)packages/platform/constants/api.ts (2)
CAL_API_VERSION_HEADER(72-72)VERSION_2024_08_13(59-59)
🔇 Additional comments (16)
.github/oasdiff-err-ignore.txt (1)
5-10: Subschema numbers verified as correct.Line 5 correctly references
subschema#2, subschema#4(POST /v2/bookings, status 201 response), and lines 6-10 correctly reference `subschema `#3`, subschema `#6(GET/POST endpoints, status 200 responses). The oneOf schemas in the OpenAPI spec confirm these indices are accurate. The difference correlates with the response types: POST /v2/bookings usesCreateSeatedBookingOutputschemas, while the other endpoints useGetSeatedBookingOutputschemas.apps/api/v2/src/modules/organizations/users/bookings/controllers/organizations-users-bookings.e2e-spec.ts (1)
233-240: LGTM!The test expectations are correctly updated to include the new
displayEmailfield. Since the test data uses regular email addresses without OAuth suffixes,displayEmailcorrectly mirrorsAlso applies to: 292-299, 349-356
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/recurring-bookings.e2e-spec.ts (1)
185-192: LGTM!Test expectations for recurring bookings are correctly updated to include
displayEmailfor each attendee across all recurrence instances. The pattern is consistent with other e2e test updates in this PR.Also applies to: 207-214, 264-271, 287-294, 308-315
apps/api/v2/src/modules/organizations/teams/bookings/organizations-teams-bookings.controller.e2e-spec.ts (1)
250-257: LGTM!Test expectation correctly includes the new
displayEmailfield for the team booking attendee.apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/api-key-bookings.e2e-spec.ts (1)
177-184: LGTM!Test expectation correctly includes the new
displayEmailfield for the API key booking scenario.apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/reassign-bookings.e2e-spec.ts (1)
491-496: LGTM!Test expectations for booking reassignment correctly include
displayEmailin thereassignedToresponse object, matching the user's email who receives the reassigned booking.Also applies to: 532-537
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/seated-bookings.e2e-spec.ts (1)
201-215: LGTM!Test expectations for seated bookings are correctly updated to include
displayEmailacross all scenarios:
- Initial seat booking
- Subsequent seat bookings (correctly references
displayEmailfrom previously stored attendee data)- Rescheduled seated bookings (uses optional chaining appropriately)
- Cancellation scenarios
Also applies to: 276-291, 293-307, 421-434, 845-859
apps/api/v2/src/modules/organizations/bookings/organizations-bookings.controller.e2e-spec.ts (1)
335-342: DisplayEmail expectations align with the new attendee shape.These assertions correctly validate the new displayEmail field alongside existing attendee attributes.
Also applies to: 389-396, 443-450, 497-504
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/user-bookings.e2e-spec.ts (4)
483-506: Create-booking expectations updated correctly for displayEmail/displayGuests.
569-576: Recurring booking attendee assertions include displayEmail as expected.Also applies to: 592-599, 614-621
2083-2106: Username-based booking expectations now match the displayEmail/displayGuests additions.
2536-2552: SMS-reminder booking responses correctly assert displayEmail fields.apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/team-bookings.e2e-spec.ts (1)
519-526: Team-booking tests now correctly assert displayEmail across attendee shapes.Also applies to: 574-582, 631-646, 694-701, 749-756, 933-941, 1017-1024
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (1)
627-709: Nice coverage for displayEmail/displayGuests with OAuth-managed users.packages/platform/types/bookings/2024-08-13/outputs/booking.output.ts (1)
32-35: DTOs correctly expose displayEmail for attendees/hosts/reassigned users.Also applies to: 108-112, 414-418
docs/api-reference/v2/openapi.json (1)
31729-31736: MakingdisplayEmailrequired is appropriate.Adding
displayEmailto the required arrays across all affected schemas (BookingHost, BookingAttendee, seated booking attendees, ReassignedToDto) is correct since the service layer will always populate this field. For API response schemas, this is not a breaking change as long as the backend always provides the field, which the implementation changes ensure.Also applies to: 31833-31839, 32267-32273, 33370-33375
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Benchmark PR from agentic-review-benchmarks#4
Summary by CodeRabbit
New Features
displayEmailfield to booking responses for attendees, hosts, and guests, providing a sanitized email address for display purposes alongside the original email.Tests
displayEmailfield in API responses.✏️ Tip: You can customize this high-level summary in your review settings.