-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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
fix: No available users found #13738
Conversation
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 4 Ignored Deployments
|
@@ -339,15 +339,25 @@ export async function ensureAvailableUsers( | |||
}); | |||
} | |||
|
|||
const getStartTime = (startTimeInput: string, timeZone?: string) => { |
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.
This code was taken from packages/trpc/server/routers/viewers/slots/util.ts
so that we mirror how the getUserAvailability
function is called.
I noticed that the .utc()
function was being called for the endTime
but not the startTime
. Have added that here and will look to see if util.ts
needs it to be added as well.
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.
Is the startTimeInput in Zulu format or something like 2024-10-01T05:00
?
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.
If it isn't in Zulu format or ISO format. It reads the time in local tz. So if you run dayjs("2024-10-01T05:00")
in India.
- It will be
2024-10-01T05:00:00+05:30
. - Then we convert it into the specified timezone.
- And then to UTC.
Which I guess isn't what we want.
I think here we are thinking of reading 2024-10-01T05:00
in the specified timezone.
For that you need to do something like dayjs.tz(2024-10-01T05:00, timeZone)
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.
And I guess github actions are run in UTC tz, so It won't show any errors. And the productions server will be fine to as long as it is in UTC too.
Just my 2 cents, as I have been working with dayjs alot in recent days and have gone through many gotchas.
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.
It's in ISO format with the timezone information included. So, for example, if on the booking page you select New York for the time zone, it will pass 2024-02-17T12:00:00-05:00
.
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.
The main problem is that we were calling stringToDayjs
with this string and it was never being converted to UTC before doing all the appropriate checks and so the time slots weren't lining up properly, resulting in "No available users found" errors.
📦 Next.js Bundle Analysis for @calcom/webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
Current Playwright Test Results Summary✅ 81 Passing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 02/20/2024 03:04:43pm UTC) Run DetailsRunning Workflow PR Update on Github Actions Commit: 6997ab4 Started: 02/20/2024 03:01:17pm UTC
|
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
user can login & logout succesfully -- future login flow user & logout using dashboard
Retry 1 • Initial Attempt |
0.42% (1)1 / 239 runfailed over last 7 days |
43.51% (104)104 / 239 runsflaked over last 7 days |
📄 apps/web/playwright/event-types.e2e.ts • 2 Flakes
Top 1 Common Error Messages
|
2 Test Cases Affected |
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Event Types tests -- future user can add multiple organizer address
Retry 1 • Initial Attempt |
0% (0)0 / 245 runsfailed over last 7 days |
18.78% (46)46 / 245 runsflaked over last 7 days |
Event Types tests -- legacy user Different Locations Tests can add single organizer address location without display location public option
Retry 1 • Initial Attempt |
0.41% (1)1 / 244 runfailed over last 7 days |
2.87% (7)7 / 244 runsflaked over last 7 days |
📄 apps/web/playwright/impersonation.e2e.ts • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Users can impersonate App Admin can impersonate users with impersonation enabled
Retry 1 • Initial Attempt |
0% (0)0 / 244 runsfailed over last 7 days |
10.66% (26)26 / 244 runsflaked over last 7 days |
|
||
describe("Booking for slot only available by date override:", () => { | ||
test( | ||
`should be able to create a booking for the exact slot overridden`, |
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.
First wrote this test against main
and saw it fail. Then made it pass in this branch.
Co-authored-by: Omar López <zomars@me.com>
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.
Very nice!
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.
Fix looks good, did a lot of testing and everything worked for me. When testing I found one more case were this error is thrown, I left a comment
…o fix/cannot-book-ny-tz
What does this PR do?
Fixes #13311, #13747, #13709
Type of change
How should this be tested?
Mandatory Tasks