fix(bookings): thread recurring series id through round-robin recurri…#29767
Open
shafi-VM wants to merge 2 commits into
Open
fix(bookings): thread recurring series id through round-robin recurri…#29767shafi-VM wants to merge 2 commits into
shafi-VM wants to merge 2 commits into
Conversation
…ng slots In RecurringBookingService, the round-robin pre-loop creates the first recurring slot but extracted only luckyUsers from the result, not thirdPartyRecurringEventId. The remaining slots then started the loop with a null id and each created their own third-party recurring series (via withRecurringEventId -> existingRecurringEvent -> the calendar service's insert path), orphaning the first occurrence in a separate series. The non-round-robin path already threads the id by extracting it after each booking. Extract thirdPartyRecurringEventId from the first round-robin slot's references too, mirroring the loop below, so all occurrences join one series. Adds a unit regression test.
Contributor
|
Welcome to Cal.diy, @shafi-VM! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdated 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/features/bookings/lib/service/RecurringBookingService.ts (1)
72-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLogic is correct; consider extracting the duplicated reference-extraction pattern.
The new block at lines 75-82 duplicates the same
thirdPartyRecurringEventIdextraction logic already present at lines 130-138. Extracting a small helper would eliminate the duplication and keep both call sites in sync.♻️ Optional: extract a shared helper
+function extractThirdPartyRecurringEventId( + references: { thirdPartyRecurringEventId?: string | null }[] | undefined +): string | null { + if (references && references.length > 0) { + for (const reference of references) { + if (reference.thirdPartyRecurringEventId) { + return reference.thirdPartyRecurringEventId; + } + } + } + return null; +} + // ... in the round-robin block: - if (!thirdPartyRecurringEventId && firstBookingResult.references && firstBookingResult.references.length > 0) { - for (const reference of firstBookingResult.references) { - if (reference.thirdPartyRecurringEventId) { - thirdPartyRecurringEventId = reference.thirdPartyRecurringEventId; - break; - } - } - } + if (!thirdPartyRecurringEventId) { + thirdPartyRecurringEventId = extractThirdPartyRecurringEventId(firstBookingResult.references) ?? thirdPartyRecurringEventId; + }And similarly for the loop at lines 130-138.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/features/bookings/lib/service/RecurringBookingService.ts` around lines 72 - 82, Extract the repeated thirdPartyRecurringEventId lookup from the firstBookingResult block and the later recurring-booking loop into a shared helper, then replace both inline loops with calls to that helper while preserving the existing fallback behavior and first-match selection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/features/bookings/lib/service/RecurringBookingService.ts`:
- Around line 72-82: Extract the repeated thirdPartyRecurringEventId lookup from
the firstBookingResult block and the later recurring-booking loop into a shared
helper, then replace both inline loops with calls to that helper while
preserving the existing fallback behavior and first-match selection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f29bd3a2-ebd1-431c-9acc-0fda7a10e5b9
📒 Files selected for processing (2)
packages/features/bookings/lib/service/RecurringBookingService.test.tspackages/features/bookings/lib/service/RecurringBookingService.ts
Address CodeRabbit nitpick on calcom#29767: dedupe the thirdPartyRecurringEventId extraction used by the round-robin pre-loop and the main loop into a shared helper, so both sites stay in sync — their divergence was the root cause of the bug. Also types the accumulator as string | null.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes #29766
For round-robin recurring bookings,
RecurringBookingServicehandles the first slot in a pre-loop (to pick the lucky host) but extracted onlyluckyUsersfrom its result — notthirdPartyRecurringEventId. The remaining occurrences then started the loop with a null series id and each created their own third-party recurring series (withRecurringEventId(null)→ noexistingRecurringEvent→ the calendar service'sevents.insertpath), orphaning the first occurrence in a separate series. The non-round-robin path already threads the id (it extracts it after each booking). This PR extractsthirdPartyRecurringEventIdfrom the first round-robin slot too, mirroring the main loop, so all occurrences join one series.Visual Demo (For contributors especially)
Backend logic fix with no UI surface — demonstrated with a red→green unit test rather than a recording (see How should this be tested?).
Video Demo (if applicable):
N/A — no UI change.
Image Demo (if applicable):
N/A — no UI change.
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
RecurringBookingService.test.tsasserts the second booking is invoked with the first slot'sthirdPartyRecurringEventId— verified red (null) before the fix, green after; the existing recurring integration test in the same file still passes.Checklist