Advanced date override handling and timezone compatibility improvements

[akshayutture-augment]

Test 7

---

Replicated from ai-code-review-evaluation/cal.com-greptile#7

[akshayutture-augment]

@claude review this PR

[akshayutture-augment]

@claude review this PR

[claude]

Claude finished @akshayutture-augment's task —— View job

---

Code Review

I've completed my review of this PR. Here are my findings:

---

Overview

This PR enhances date override handling and timezone compatibility in the scheduling system. The changes span across test files, slot generation logic, and type definitions.

---

Major Issues

1. Critical Logic Error in checkIfIsAvailable (packages/trpc/server/routers/viewer/slots.ts:142)

const start = slotStartTime.hour() * 60 + slotStartTime.minute();
const end = slotStartTime.hour() * 60 + slotStartTime.minute();

Problem: Both start and end are calculated using slotStartTime, making them identical. The end should be calculated using slotEndTime.

Impact: This bug causes the working hours validation to be completely broken. It will always pass the check start < workingHour.startTime || end > workingHour.endTime incorrectly.

Fix needed:

const start = slotStartTime.hour() * 60 + slotStartTime.minute();
const end = slotEndTime.hour() * 60 + slotEndTime.minute();

2. Incorrect Type Comparison in checkIfIsAvailable (packages/trpc/server/routers/viewer/slots.ts:531)

if (typeof availabilityCheckProps.currentSeats !== undefined) {

Problem: typeof returns a string, so this will ALWAYS be true (even when currentSeats is undefined). The correct check should be:

if (typeof availabilityCheckProps.currentSeats !== 'undefined') {
// OR better yet:
if (availabilityCheckProps.currentSeats !== undefined) {

Impact: This causes the code block to execute even when currentSeats is undefined, potentially causing runtime errors.

3. Similar Type Check Error (packages/trpc/server/routers/viewer/slots.ts:552)

if (occupiedSeats?.length && typeof availabilityCheckProps.currentSeats === undefined)

Problem: Same issue - typeof always returns a string, never undefined. This condition will never be true.

Fix needed:

if (occupiedSeats?.length && availabilityCheckProps.currentSeats === undefined)

---

Medium Issues

4. Inconsistent UTC Offset Handling in Date Overrides

In packages/lib/slots.ts:212-214, the offset calculation for date overrides:

const organizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset();
const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset();
const offset = inviteeUtcOffset - organizerUtcOffset;

And in packages/trpc/server/routers/viewer/slots.ts:107:

const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 : 0;

Concern: The first uses positive offset (inviteeUtcOffset - organizerUtcOffset) while the second multiplies by -1. This inconsistency could lead to timezone bugs. Need to verify both are correct for their contexts.

5. Test Coverage for Timezone Edge Cases

The new test in packages/web/test/lib/getSchedule.test.ts:788-804 tests date overrides with different timezones, which is excellent. However:

• The test expects the same UTC times for both +5:30 and +6:00 timezones
• The comment says "it should return the same as this is the utc time" but doesn't verify the display times would be different
• Missing test for DST transitions with date overrides

---

Minor Issues

6. Type Definition Enhancement Needed

In packages/types/schedule.d.ts, the TimeRange type was extended with timeZone:

export type TimeRange = {
  userId?: number | null;
  start: Date;
  end: Date;
  timeZone?: string;  // Added
};

Suggestion: This field is optional but appears to be required for proper date override handling. Consider:

• Making it required if it's always needed
• Adding JSDoc to explain when it's optional vs required

7. Potential DST Issue in buildSlots

In packages/lib/slots.ts:102-105:

const organizerDSTDiff =
  dayjs().tz(organizerTimeZone).utcOffset() - startOfInviteeDay.tz(organizerTimeZone).utcOffset();
const inviteeDSTDiff =
  dayjs().tz(inviteeTimeZone).utcOffset() - startOfInviteeDay.tz(inviteeTimeZone).utcOffset();

This calculates DST diff between "now" and the startOfInviteeDay. If slots are being generated for dates far in the future, this could be incorrect during DST transitions.

---

Positive Aspects

✅ Good test coverage with the new date override test
✅ Proper timezone handling in most places
✅ Clean separation of concerns between date override and working hours logic
✅ Test data structure is well-organized and reusable

---

Recommendations

1. CRITICAL: Fix the three logic errors identified above (feat: add calendar cache status and actions (#22532) #1, Add guest management functionality to existing bookings #2, SMS workflow reminder retry count tracking #3)
2. HIGH PRIORITY: Add integration tests for:
   • DST transitions with date overrides
   • Edge cases around midnight with date overrides
   • Multiple date overrides on the same day
3. MEDIUM PRIORITY: Review and document the timezone offset calculation strategy
4. LOW PRIORITY: Consider adding JSDoc comments explaining the date override precedence logic

---

Security Considerations

No security issues identified. The changes don't introduce new user input handling or authentication logic.

---

Performance Considerations

The date override filtering in packages/lib/slots.ts:206-225 iterates through overrides for each slot. With many overrides, this could be optimized with a Map keyed by date string.

---

[github-actions]

This PR is being marked as stale due to inactivity.