Skip to content

fix: Generate new idempotency key when round robin reassigning#22065

Merged
joeauyeung merged 5 commits intomainfrom
fix-reassign-idempotency
Jun 26, 2025
Merged

fix: Generate new idempotency key when round robin reassigning#22065
joeauyeung merged 5 commits intomainfrom
fix-reassign-idempotency

Conversation

@joeauyeung
Copy link
Copy Markdown
Contributor

@joeauyeung joeauyeung commented Jun 26, 2025 •
edited
Loading

What does this PR do?

Fixes round robin reassignment by generating a new idempotency key when reassigning a booking. Also introduces a IdempotencyKeyService

  • Fixes #XXXX (GitHub issue number)
  • Fixes CAL-XXXX (Linear issue number - should be visible at the bottom of the GitHub issue description)

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Create a booking
  • Reassign via RR
    • The idempotency key should change for the booking
  • Reassign the booking via manual RR
    • the idempotency key should change for the booking

@vercel
Copy link
Copy Markdown

vercel bot commented Jun 26, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal-eu ⬜️ Ignored (Inspect) Visit Preview Jun 26, 2025 5:23pm
cal ⬜️ Skipped (Inspect) Jun 26, 2025 5:23pm

@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Jun 26, 2025
@delve-auditor
Copy link
Copy Markdown

delve-auditor bot commented Jun 26, 2025 •
edited
Loading

✅ No security or compliance issues detected. Reviewed everything up to fe6c05e.

Security Overview
  • 🔎 Scanned files: 4 changed file(s)
Detected Code Changes
Change Type Relevant files
Enhancement ► roundRobinManualReassignment.ts
    Add idempotency key handling for round robin reassignment
► roundRobinReassignment.ts
    Add idempotency key handling for round robin reassignment
► idempotencyKeyService.ts
    New service for generating idempotency keys
► booking-idempotency-key.ts
    Update booking extension to use new idempotency key service

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

@joeauyeung joeauyeung marked this pull request as ready for review June 26, 2025 14:47
@joeauyeung joeauyeung requested a review from a team as a code owner June 26, 2025 14:47
@graphite-app graphite-app bot requested a review from a team June 26, 2025 14:48
@dosubot dosubot bot added teams area: teams, round robin, collective, managed event-types 🐛 bug Something isn't working labels Jun 26, 2025
@graphite-app
Copy link
Copy Markdown

graphite-app bot commented Jun 26, 2025

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (06/26/25)

1 reviewer was added to this PR based on Keith Williams's automation.

cubic-dev-ai[bot]
cubic-dev-ai bot reviewed Jun 26, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

cubic found 1 issue across 4 files. Review it in cubic.dev

React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.

packages/lib/idempotencyKey/idempotencyKeyService.ts
reassignedById?: number | null;
}) {
return uuidv5(
`${startTime.valueOf()}.${endTime.valueOf()}.${userId}${reassignedById ? `.${reassignedById}` : ""}`,
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Jun 26, 2025

Choose a reason for hiding this comment

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

Calling valueOf() on a string will not yield the intended timestamp; if startTime or endTime are strings, valueOf() returns the string itself, not a numeric timestamp. This can lead to inconsistent idempotency keys if the input is sometimes a Date and sometimes a string. Consider normalizing to a Date before calling valueOf().

Suggested change
`${startTime.valueOf()}.${endTime.valueOf()}.${userId}${reassignedById ? `.${reassignedById}` : ""}`,
`${new Date(startTime).valueOf()}.${new Date(endTime).valueOf()}.${userId}${reassignedById ? `.${reassignedById}` : ""}`,

cubic-dev-ai[bot]
cubic-dev-ai bot reviewed Jun 26, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

cubic found 1 issue across 4 files. Review it in cubic.dev

React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.

packages/lib/idempotencyKey/idempotencyKeyService.ts
reassignedById?: number | null;
}) {
return uuidv5(
`${startTime.valueOf()}.${endTime.valueOf()}.${userId}${reassignedById ? `.${reassignedById}` : ""}`,
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Jun 26, 2025

Choose a reason for hiding this comment

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

Calling valueOf() on a string will not yield the intended timestamp; if startTime or endTime are strings, valueOf() returns the string itself, not a numeric timestamp. This can lead to inconsistent idempotency keys if the input is sometimes a Date and sometimes a string. Consider normalizing to a Date before calling valueOf().

Suggested change
`${startTime.valueOf()}.${endTime.valueOf()}.${userId}${reassignedById ? `.${reassignedById}` : ""}`,
`${new Date(startTime).valueOf()}.${new Date(endTime).valueOf()}.${userId}${reassignedById ? `.${reassignedById}` : ""}`,

emrysal
emrysal approved these changes Jun 26, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@emrysal emrysal left a comment

Choose a reason for hiding this comment

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

Consistent with previous behaviour

@vercel vercel bot temporarily deployed to Preview – api June 26, 2025 17:23 Inactive
@vercel vercel bot temporarily deployed to Preview – cal June 26, 2025 17:23 Inactive
@joeauyeung joeauyeung enabled auto-merge (squash) June 26, 2025 17:38
@joeauyeung joeauyeung merged commit b0999ab into main Jun 26, 2025
37 of 38 checks passed
@joeauyeung joeauyeung deleted the fix-reassign-idempotency branch June 26, 2025 17:43
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jun 26, 2025

E2E results are ready!

emrysal pushed a commit that referenced this pull request Jun 26, 2025
@joeauyeung @emrysal
fix: Generate new idempotency key when round robin reassigning (#22065)
0aaa234 
* Create idempotencyKeyService

* Use `idempotencyKeyService` in middleware

* Use `idempotencyKeyService` in RR reassignments
zomars pushed a commit that referenced this pull request Jun 30, 2025
@joeauyeung @zomars
fix: Generate new idempotency key when round robin reassigning (#22065)
5030072 
* Create idempotencyKeyService

* Use `idempotencyKeyService` in middleware

* Use `idempotencyKeyService` in RR reassignments
@zomars zomars mentioned this pull request Jul 1, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cubic-dev-ai cubic-dev-ai[bot] cubic-dev-ai[bot] left review comments

@emrysal emrysal emrysal approved these changes

@CarinaWolli CarinaWolli CarinaWolli approved these changes

Assignees

No one assigned

Labels

🐛 bug Something isn't working core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO ready-for-e2e teams area: teams, round robin, collective, managed event-types

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@joeauyeung @emrysal @CarinaWolli @keithwillcode