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

feat: checkbox to count managed events for team-wide limits #16923

Merged
merged 10 commits into from
Oct 7, 2024

Conversation

CarinaWolli
Copy link
Member

@CarinaWolli CarinaWolli commented Oct 3, 2024

What does this PR do?

Adds a checkbox to team-wide bookings limits so admins can choose if bookings from managed event types are counted for the time-wide booking limits.

Screenshot 2024-10-03 at 11 48 58 AM

Also fixes the following bug:
Screenshot 2024-10-04 at 8 54 32 PM
How to reproduce bug?

  • Set schedule timezone to Singapore (any + timezone)
  • Add event type limit, 1 per year
  • book one meeting
  • load availability again and see that it's still shows slots for this year, when trying to confirm the booking you get an error

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have added a Docs issue: Global booking limits for teams docs#139
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

Copy link

vercel bot commented Oct 3, 2024

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

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Oct 7, 2024 2:53am
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Oct 7, 2024 2:53am

@CarinaWolli CarinaWolli added High priority Created by Linear-GitHub Sync bookings area: bookings, availability, timezones, double booking and removed core area: core, team members only labels Oct 3, 2024
@@ -369,10 +429,10 @@ describe(

const { req: req2 } = createMockNextJsRequest({
method: "POST",
body: mockBookingData1,
body: mockBookingData2,
Copy link
Member Author

Choose a reason for hiding this comment

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

the wrong mockBookingData was used here

Copy link
Contributor

Choose a reason for hiding this comment

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

can we rename the variable to some self-explanatory to prevent this from happening again?

@@ -468,10 +557,9 @@ describe(

const { req: req2 } = createMockNextJsRequest({
method: "POST",
body: mockBookingData1,
body: mockBookingData2,
Copy link
Member Author

Choose a reason for hiding this comment

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

same, wrong mockBookingData was used

@@ -123,6 +129,8 @@ const _getBusyTimesFromBookingLimits = async (params: {
teamId,
user,
rescheduleUid,
includeManagedEvents,
timeZone,
Copy link
Member Author

Choose a reason for hiding this comment

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

not passing timeZone here caused this error with yearly booking limits:
Screenshot 2024-10-04 at 9 06 34 PM

@CarinaWolli CarinaWolli marked this pull request as ready for review October 4, 2024 21:06
@CarinaWolli CarinaWolli requested a review from a team October 4, 2024 21:07
@dosubot dosubot bot added teams area: teams, round robin, collective, managed event-types ✨ feature New feature or request 🐛 bug Something isn't working labels Oct 4, 2024
Copy link

graphite-app bot commented Oct 4, 2024

Graphite Automations

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

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

zomars
zomars previously approved these changes Oct 4, 2024
Copy link
Member

@zomars zomars left a comment

Choose a reason for hiding this comment

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

Code LGTM. Waiting on Checks. 🙏🏽

@zomars zomars enabled auto-merge (squash) October 4, 2024 21:13
apps/web/public/static/locales/en/common.json Outdated Show resolved Hide resolved
packages/core/bookingLimits/getBusyTimesFromLimits.ts Outdated Show resolved Hide resolved
@@ -369,10 +429,10 @@ describe(

const { req: req2 } = createMockNextJsRequest({
method: "POST",
body: mockBookingData1,
body: mockBookingData2,
Copy link
Contributor

Choose a reason for hiding this comment

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

can we rename the variable to some self-explanatory to prevent this from happening again?

packages/lib/server/repository/booking.ts Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Oct 7, 2024

E2E results are ready!

@zomars zomars merged commit b5fa43f into main Oct 7, 2024
39 of 40 checks passed
@zomars zomars deleted the feat/managed-event-booking-limits branch October 7, 2024 15:36
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 🐛 bug Something isn't working consumer core area: core, team members only ✨ feature New feature or request High priority Created by Linear-GitHub Sync ❗️ migrations contains migration files ready-for-e2e teams area: teams, round robin, collective, managed event-types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants