-
Notifications
You must be signed in to change notification settings - Fork 0
Advanced date override handling and timezone compatibility improvements #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: date-algorithm-base
Are you sure you want to change the base?
Conversation
* fix date override for fixed round robin + time zone in date override * check if slot is within working hours of fixed hosts * add test for date override in different time zone * fix date overrides for not fixed hosts (round robin) * code clean up * fix added test * use the correct timezone of user for date overrides --------- Co-authored-by: CarinaWolli <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Greptile OverviewConfidence Score: 1/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Client
participant getSchedule
participant getUserAvailability
participant getSlots
participant checkIfIsAvailable
Client->>getSchedule: request slots (eventTypeId, timeZone, dateRange)
getSchedule->>getUserAvailability: get user availability
getUserAvailability-->>getSchedule: return (busy, workingHours, dateOverrides, timeZone)
Note over getSchedule: Flatten dateOverrides from all users<br/>and add timeZone to each override
getSchedule->>getSlots: generate time slots
Note over getSlots: Calculate timezone offset between<br/>organizer and invitee
getSlots->>getSlots: Apply date overrides with<br/>timezone adjustments
getSlots-->>getSchedule: return raw time slots
loop For each time slot
getSchedule->>checkIfIsAvailable: check slot availability
Note over checkIfIsAvailable: 1. Check if date override exists<br/>2. Validate against working hours<br/>3. Check busy times
checkIfIsAvailable-->>getSchedule: return isAvailable
end
getSchedule-->>Client: return available slots
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 files reviewed, 2 comments
| const start = slotStartTime.hour() * 60 + slotStartTime.minute(); | ||
| const end = slotStartTime.hour() * 60 + slotStartTime.minute(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: end is incorrectly set to the same value as start, causing slotEndTime to never be checked
| const start = slotStartTime.hour() * 60 + slotStartTime.minute(); | |
| const end = slotStartTime.hour() * 60 + slotStartTime.minute(); | |
| const start = slotStartTime.hour() * 60 + slotStartTime.minute(); | |
| const end = slotEndTime.hour() * 60 + slotEndTime.minute(); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/trpc/server/routers/viewer/slots.ts
Line: 141:142
Comment:
**logic:** `end` is incorrectly set to the same value as `start`, causing `slotEndTime` to never be checked
```suggestion
const start = slotStartTime.hour() * 60 + slotStartTime.minute();
const end = slotEndTime.hour() * 60 + slotEndTime.minute();
```
How can I resolve this? If you propose a fix, please make it concise.| slotStartTime.format("YYYY MM DD") | ||
| ) { | ||
| dateOverrideExist = true; | ||
| if (dayjs(date.start).add(utcOffset, "minutes") === dayjs(date.end).add(utcOffset, "minutes")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: using === to compare dayjs objects will always return false since it compares object references, not values
| if (dayjs(date.start).add(utcOffset, "minutes") === dayjs(date.end).add(utcOffset, "minutes")) { | |
| if (dayjs(date.start).add(utcOffset, "minutes").isSame(dayjs(date.end).add(utcOffset, "minutes"))) { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/trpc/server/routers/viewer/slots.ts
Line: 114:114
Comment:
**logic:** using `===` to compare dayjs objects will always return false since it compares object references, not values
```suggestion
if (dayjs(date.start).add(utcOffset, "minutes").isSame(dayjs(date.end).add(utcOffset, "minutes"))) {
```
How can I resolve this? If you propose a fix, please make it concise.
Test 7
Replicated from ai-code-review-evaluation/cal.com-greptile#7