Skip to content

fix: eventtype null cant be booked error#18975

Merged
keithwillcode merged 13 commits intomainfrom
fix/eventtype-null-cant-be-booked-err
Jan 29, 2025
Merged

fix: eventtype null cant be booked error#18975
keithwillcode merged 13 commits intomainfrom
fix/eventtype-null-cant-be-booked-err

Conversation

@alishaz-polymath
Copy link
Copy Markdown
Member

@alishaz-polymath alishaz-polymath commented Jan 29, 2025

What does this PR do?

  • Fixes #XXXX (GitHub issue number)
  • Fixes CAL-XXXX (Linear issue number - should be visible at the bottom of the GitHub issue description)

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • N/A I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Are there environment variables that should be set?
  • What are the minimal test data to have?
  • What is expected (happy path) to have (input and output)?
  • Any other important info that could help to test that PR

Checklist

@vercel
Copy link
Copy Markdown

vercel Bot commented Jan 29, 2025

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 Jan 29, 2025 0:27am
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Jan 29, 2025 0:27am

@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Jan 29, 2025
Comment thread packages/lib/isOutOfBounds.tsx Outdated
Comment on lines +253 to +258
if (isAfterRollingEndDay)
log.warn("Booking is out of bounds due to rolling period end day.", {
formattedDate: dateInSystemTz.format(),
isAfterRollingEndDay,
endOfRollingPeriodEndDayInBookerTz: periodLimits.endOfRollingPeriodEndDayInBookerTz.format(),
});
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Add detailed logs when booking fails due to attempted booking after the rolling period

Comment thread packages/lib/isOutOfBounds.tsx Outdated
Comment on lines +272 to +279
if (isBeforeRangeStart || isAfterRangeEnd)
log.warn("Booking is out of bounds due to range start and end.", {
formattedDate: dateInSystemTz.format(),
isBeforeRangeStart,
isAfterRangeEnd,
startOfRangeStartDayInEventTz: periodLimits.startOfRangeStartDayInEventTz.format(),
endOfRangeEndDayInEventTz: periodLimits.endOfRangeEndDayInEventTz.format(),
});
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Add detailed logs when booking fails due to attempted booking outside of period Range

Comment thread packages/lib/isOutOfBounds.tsx Outdated
Comment on lines +303 to +339
): { isOutOfBounds: boolean; cause: string | null } {
const isOutOfBoundsByTime = isTimeOutOfBounds({ time, minimumBookingNotice });
const periodLimits = calculatePeriodLimits({
periodType,
periodDays,
periodCountCalendarDays,
periodStartDate,
periodEndDate,
allDatesWithBookabilityStatusInBookerTz: null, // Temporary workaround
_skipRollingWindowCheck: true,
eventUtcOffset,
bookerUtcOffset,
});

const isOutOfBoundsByPeriod = isTimeViolatingFutureLimit({
time,
periodLimits,
});

if (isOutOfBoundsByTime)
return {
isOutOfBounds: true,
cause: "Booking is out of bounds due to minimum booking notice.",
};

if (isOutOfBoundsByPeriod) {
log.warn(`Booking is out of bounds due to period restrictions - ${periodLimits}`);
return {
isOutOfBounds: true,
cause: "Booking is out of bounds due to period restrictions.",
};
}

return {
isOutOfBounds: false,
cause: null,
};
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Return cause along with isOutOfBounds to have a distinct reason on why a booking failed


logger.warn({
message: `NewBooking: EventType '${eventType.eventName}' cannot be booked at this time.`,
message: `NewBooking: EventType '${eventType.title}' cannot be booked at this time. Reason: ${cause}`,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We also add cause here to have a clearer picture of what the cause of this failure is.

@alishaz-polymath alishaz-polymath marked this pull request as ready for review January 29, 2025 09:01
@dosubot dosubot Bot added event-types area: event types, event-types 🐛 bug Something isn't working labels Jan 29, 2025
Comment on lines +254 to +261
log.warn(
"Booking is out of bounds due to rolling period end day.",
safeStringify({
formattedDate: dateInSystemTz.format(),
isAfterRollingEndDay,
endOfRollingPeriodEndDayInBookerTz: periodLimits.endOfRollingPeriodEndDayInBookerTz.format(),
})
);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We log all necessary details when user attempts to book after the rolling period

Comment on lines +276 to +285
log.warn(
"Booking is out of bounds due to range start and end.",
safeStringify({
formattedDate: dateInSystemTz.format(),
isBeforeRangeStart,
isAfterRangeEnd,
startOfRangeStartDayInEventTz: periodLimits.startOfRangeStartDayInEventTz.format(),
endOfRangeEndDayInEventTz: periodLimits.endOfRangeEndDayInEventTz.format(),
})
);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We log all necessary details when user attempts to book outside the period range

Comment thread packages/lib/isOutOfBounds.tsx Outdated
Udit-takkar

This comment was marked as resolved.

@graphite-app
Copy link
Copy Markdown

graphite-app Bot commented Jan 29, 2025

Graphite Automations

"Add ready-for-e2e label" took an action on this PR • (01/29/25)

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

@keithwillcode keithwillcode enabled auto-merge (squash) January 29, 2025 12:43
@keithwillcode keithwillcode merged commit 92678dc into main Jan 29, 2025
@keithwillcode keithwillcode deleted the fix/eventtype-null-cant-be-booked-err branch January 29, 2025 13:00
@github-actions
Copy link
Copy Markdown
Contributor

E2E results are ready!

keithwillcode pushed a commit that referenced this pull request Jan 29, 2025
* fix err msg and add more description

* improvements

* fix cause

* add warn logs for when failing to specific limit checks

* typefix

* safestringify log

* improve error

* test fix and log min booking notice

* adds out of bounds error code

* test fix

* fix
@sentry
Copy link
Copy Markdown

sentry Bot commented Jan 29, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ HttpError: booking_time_out_of_bounds_error POST /api/bookings View Issue

Did you find this useful? React with a 👍 or 👎

emrysal added a commit that referenced this pull request Feb 4, 2025
MuhammadAimanSulaiman pushed a commit to hit-pay/cal.com that referenced this pull request Feb 25, 2025
* fix err msg and add more description

* improvements

* fix cause

* add warn logs for when failing to specific limit checks

* typefix

* safestringify log

* improve error

* test fix and log min booking notice

* adds out of bounds error code

* test fix

* fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO event-types area: event types, event-types ready-for-e2e

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants