cal.com
cal.com copied to clipboard
calcom
fix: #15833 [CAL-4055] Same first slot in different timezones
What does this PR do?
- Fixes #15833
- Fixes CAL-4055(Linear issue number - should be visible at the bottom of the GitHub issue description)
After Changes - https://www.loom.com/share/d3e067e558184cd2aa7e62ca2e48c770?sid=76ab6332-5e38-4421-8247-e76991fa4751
Root cause - The minDate, used to set the start date in getAvailableDatesInMonth was using browserDate, updated this to calculate today's date in selected timezone. https://github.com/calcom/cal.com/blob/a74522af3d667cd78daee7990d58d5610312fb8c/packages/features/calendars/lib/getAvailableDatesInMonth.ts#L10
Mandatory Tasks (DO NOT REMOVE)
- [x] I have self-reviewed the code (A decent size PR without self-review might be rejected).
- [x] N/A-I have added a Docs issue here if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
- [x] I confirm automated tests are in place that prove my fix is effective or that my feature works.
@vijayraghav-io is attempting to deploy a commit to the cal Team on Vercel.
A member of the Team first needs to authorize it.
![vercel[bot] avatar](https://avatars.githubusercontent.com/in/8329?v=4 "Published by a pub.dev verified publisher"){kind=small}
Graphite Automations
"Add community label" took an action on this PR • (07/19/24)
1 label was added to this PR based on Keith Williams's automation.
"Add consumer team as reviewer" took an action on this PR • (07/19/24)
1 reviewer was added to this PR based on Keith Williams's automation.
![graphite-app[bot] avatar](https://avatars.githubusercontent.com/in/158384?v=4 "Published by a pub.dev verified publisher"){kind=small}
@vijayraghav-io great works. Left some comment. Let me know what you think?
Thankyou! replied inline
Scenario of first day in month:
https://www.loom.com/share/f917aa77fc4f442aadb7d31b94be7435?sid=0a61481c-d67b-4154-b68f-66ee6a19c490
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Could we also fix this dot that is shown on the current day? Everything else looks good to me 🙏
Thankyou! 🙏 , Sure have fixed the dot to show as per selected timezone's 'today' and verified.
Hi @vijayraghav-io I've created a spun off PR that fixes this issue in a different manner by addressing the issue in the code responsible for generating the date string array in the first place. I've left a self-review with a detailed overview of the changes I've made. I've used the tests you've created for this PR.
https://github.com/calcom/cal.com/pull/19389
Any thoughts greatly appreciated!
Hi @vijayraghav-io I've created a spun off PR that fixes this issue in a different manner by addressing the issue in the code responsible for generating the date string array in the first place. I've left a self-review with a detailed overview of the changes I've made. I've used the tests you've created for this PR.
#19389
Any thoughts greatly appreciated!
Hi @emrysal ,
When i started with this bug fix, i din't dare to change anything inside the function getAvailableDatesInMonth(), because recently it had fix related to timezone bugs (daylight adjustments).
So, I felt changing the data type from Date to Dayjs inside this function may be riskier. As new Date() and dayjs.toDate() behaves differently when we have timezones, because of which we have the bug.
Hence took the approach (playing it safe) of changing the type of input parameter minDate passed to this function instead.
As, i had called out in my self review as well - https://github.com/calcom/cal.com/pull/15840/files#r1821905247 and in description.
Nonetheless, you would have considered all scenarios.
My PR also had the fix for this comment https://github.com/calcom/cal.com/pull/15840#pullrequestreview-2344109636 of showing the dot as per the new start date as per timezone https://github.com/calcom/cal.com/pull/15840#issuecomment-2390729279 Hope you would have considered this or can take as a reminder.
hey @vijayraghav-io is this PR still on?
yes, will resolve merge conflicts
hey @vijayraghav-io u still working on this ?
yes it was RFR, will resolve latest merge conflicts
@Devanshusharma2005 , resolved merge conflicts and all tests are passing.
@vijayraghav-io thanks for your work. But we internally talked about this and @emrysal is planning to tackle it here: - https://github.com/calcom/cal.com/pull/19389 in a diff way.
@vijayraghav-io thanks for your work. But we internally talked about this and @emrysal is planning to tackle it here: - #19389 in a diff way.
@anikdhabal , please refer this https://github.com/calcom/cal.com/pull/19389#issuecomment-2671271703