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: view team bookings #14079

Merged
merged 54 commits into from Apr 10, 2024
Merged

Conversation

SomayChauhan
Copy link
Member

@SomayChauhan SomayChauhan commented Mar 14, 2024

What does this PR do?

re-doing #13591 - that was reverted due to DB load issues introduced.

Fixes #13582

Copy link

graphite-app bot commented Mar 14, 2024

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (03/14/24)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add community label" took an action on this PR • (03/14/24)

1 label was added to this PR based on Keith Williams's automation.

"Add foundation team as reviewer" took an action on this PR • (03/14/24)

1 reviewer was added to this PR based on Keith Williams's automation.

Copy link
Contributor

github-actions bot commented Mar 14, 2024

📦 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! 🙌

@Ryukemeister Ryukemeister changed the title Fix: view team bookings fix: view team bookings Mar 14, 2024
@Udit-takkar Udit-takkar added the high-risk Requires approval by Foundation team label Mar 14, 2024
@graphite-app graphite-app bot requested a review from a team March 14, 2024 09:37
@keithwillcode keithwillcode added this to the v4.0 milestone Mar 27, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

From this comment, I'm not seeing any select fields in the queries. #13591 (review)

Let's figure out the minimum amount of data we need from the DB for this endpoint.

Copy link
Member Author

Choose a reason for hiding this comment

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

@joeauyeung how about now?

joeauyeung
joeauyeung previously approved these changes Apr 3, 2024
Copy link
Contributor

@joeauyeung joeauyeung left a comment

Choose a reason for hiding this comment

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

@SomayChauhan I like this commit: 0aa47bd (#14079)

I'll approve for now, but let's see what @keithwillcode and @zomars have to say.

As a follow up I found two things that we can improve on:

  • getUniqueBookings filters duplicate bookings based on uid but when getting the nested fields from the bookings we filter on id. We should change getUniqueBookings to filter on just ids to reduce the amount of data we need to initially query the database for.
  • Optimize getUniqueBookings
    • Right now it loops through the array of bookings theoretically two times but I think we can cut it down to once
const getUniqueBookings = <T extends { uid: string }>(arr: T[]) => {
	const  unique = new Set();
    arr.forEach((booking) => {
    set.add(booking.uid);
  	});
  return [ ...unique.values() ];
};

@zomars
Copy link
Member

zomars commented Apr 4, 2024

For some reason e2e tests are broken. Will revisit after release.

@emrysal emrysal enabled auto-merge (squash) April 10, 2024 15:56
Copy link
Contributor

@emrysal emrysal left a comment

Choose a reason for hiding this comment

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

Tests passing, looks ready to go! :)

@emrysal emrysal merged commit f86c448 into calcom:main Apr 10, 2024
31 of 39 checks passed
keithwillcode added a commit that referenced this pull request Apr 13, 2024
keithwillcode added a commit that referenced this pull request Apr 13, 2024
@dosubot dosubot bot modified the milestones: v4.0, v4.1 Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bookings area: bookings, availability, timezones, double booking community Created by Linear-GitHub Sync foundation High priority Created by Linear-GitHub Sync high-risk Requires approval by Foundation team teams area: teams, round robin, collective, managed event-types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CAL-3089] View Bookings of Team
6 participants