Skip to content
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/replacing bookingAttendees for bookingUid #8997

Merged
merged 1 commit into from May 22, 2023

Conversation

alannnc
Copy link
Contributor

@alannnc alannnc commented May 19, 2023

What does this PR do?

  • Replace bookingAttendees length for bookingUid, if required we fetch booking attendees on server.

Environment: Staging(main branch)

Type of change

  • Chore (refactoring code, technical debt, workflow improvements)

How should this be tested?

Checklist

  • I haven't added tests that prove my fix is effective or that my feature works

@vercel
Copy link

vercel bot commented May 19, 2023

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

Name Status Preview Comments Updated (UTC)
cal ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 19, 2023 6:59pm
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 19, 2023 6:59pm

@@ -72,7 +72,7 @@ const AvailableTimes: FC<AvailableTimesProps> = ({
slotUtcStartDate: slot.time,
eventTypeId,
slotUtcEndDate: dayjs(slot.time).utc().add(duration, "minutes").format(),
bookingAttendees: bookingAttendees || undefined,
bookingUid: slot.bookingUid,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of sending total bookingAttendees we send currentSlotBooking UID

@@ -40,8 +40,13 @@ export const reserveSlotHandler = async ({ ctx, input }: ReserveSlotOptions) =>
// If this is a seated event then don't reserve a slot
if (eventType.seatsPerTimeSlot) {
// Check to see if this is the last attendee
if (bookingAttendees) {
const seatsLeft = eventType.seatsPerTimeSlot - bookingAttendees;
const bookingWithAttendees = await prisma.booking.findFirst({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fetch current attendees on server

@@ -72,7 +72,7 @@ const AvailableTimes: FC<AvailableTimesProps> = ({
slotUtcStartDate: slot.time,
eventTypeId,
slotUtcEndDate: dayjs(slot.time).utc().add(duration, "minutes").format(),
bookingAttendees: bookingAttendees || undefined,
bookingUid: slot.bookingUid,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of sending total bookingAttendees we send currentSlotBooking UID

@roae
Copy link
Contributor

roae commented May 19, 2023

@alannnc What is the purpose of this refactor? Does it solve any bug?

@github-actions
Copy link
Contributor

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@deploysentinel
Copy link

Current Playwright Test Results Summary

✅ 114 Passing - ⚠️ 1 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 05/19/2023 07:14:28pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 0deca26

Started: 05/19/2023 07:04:09pm UTC

⚠️ Flakes

📄   apps/web/playwright/booking-seats.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Booking with Seats -- new-booker Reschedule for booking with seats -- old-booker Should reschedule booking with seats and if everyone rescheduled it should be deleted
Retry 1Initial Attempt
0% (0) 0 / 265 runs
failed over last 7 days
83.77% (222) 222 / 265 runs
flaked over last 7 days

View Detailed Build Results


Copy link
Contributor

@JeroenReumkens JeroenReumkens left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks a lot @alannnc!

@roae The main reason for this change is because I asked it. Before this change this reserve mutation expected to have the data of the bookingattendees client side. That means that the booker needs to do an additional query, only to then send it to the backend and not use it themselves. So imo it made more sense to query this data directly in the backend, which is faster as well. That way I didn't need to add an additional query to the new booker.

@PeerRich PeerRich added this pull request to the merge queue May 22, 2023
Merged via the queue into main with commit ff6c6cc May 22, 2023
22 checks passed
@PeerRich PeerRich deleted the fix/remove-booking-attendees-from-mutation branch May 22, 2023 09:52
@PeerRich PeerRich added the core area: core, team members only label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core area: core, team members only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants