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: orgs trigger alert when loading a calendar and no availability is found #14796

Conversation

sean-brydon
Copy link
Member

@sean-brydon sean-brydon commented Apr 29, 2024

What does this PR do?

Fixes CAL-3480
Fixes #14609

Loom: https://share.cleanshot.com/B5yBz18n

Type of change

  • New feature (non-breaking change which adds functionality)

How should this be tested?

  • Run migrations
  • Have upstash setup
  • Enable this setting for orgs
  • Visit a booking page with 0 availability 2 times
  • Check mails
  • Wait for expiry (in dev its 60 seconds)
  • try again
  • See email again

Mandatory Tasks

  • Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

Copy link

linear bot commented Apr 29, 2024

Copy link
Contributor

github-actions bot commented Apr 29, 2024

Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link.

@github-actions github-actions bot added ❗️ migrations contains migration files booking-page area: booking page, public booking page, booker consumer High priority Created by Linear-GitHub Sync organizations area: organizations, orgs ✨ feature New feature or request 🎨 needs design Before engineering kick-off, a designer needs to submit a mockup labels Apr 29, 2024
@keithwillcode keithwillcode added the core area: core, team members only label Apr 29, 2024
Copy link

vercel bot commented Apr 29, 2024

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

Name Status Preview Comments Updated (UTC)
ai ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 3, 2024 1:25pm
platform-starter-kit ❌ Failed (Inspect) May 3, 2024 1:25pm
2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview May 3, 2024 1:25pm
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview May 3, 2024 1:25pm

Comment on lines +37 to 43
if (input.hasOwnProperty("lockEventTypeCreation")) {
data.lockEventTypeCreationForUsers = input.lockEventTypeCreation;
}

if (input.hasOwnProperty("adminGetsNoSlotsNotification")) {
data.adminGetsNoSlotsNotification = input.adminGetsNoSlotsNotification;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

We have to construct data property so we only update what has changed - allows us to pass in only the orgSetting we want to update and not have the previous state of for data before

@sean-brydon sean-brydon marked this pull request as ready for review April 29, 2024 12:56
@graphite-app graphite-app bot requested a review from a team April 29, 2024 12:56
@dosubot dosubot bot added the emails area: emails, cancellation email, reschedule email, inbox, spam folder, not getting email label Apr 29, 2024
Comment on lines 18 to 20
const NO_SLOTS_NOTIFICATION_FREQUENCY = IS_PRODUCTION ? 604_800 : 60;

const NO_SLOTS_COUNT_FOR_NOTIFICATION = 2;
Copy link
Member Author

Choose a reason for hiding this comment

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

We will make these options configureable on a perOrg basis if required.

Will allign with customer to find out what they want initally

Copy link

graphite-app bot commented Apr 29, 2024

Graphite Automations

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

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

// We may need to get more data and check the startDate occurrence of this
// Not trigger email if the start months are the same
const usersExistingNoSlots = await redis.lrange(usersUniqueKey, 0, NO_SLOTS_COUNT_FOR_NOTIFICATION - 1);
await redis.lpush(usersUniqueKey, constructDataHash(eventDetails));
Copy link
Member Author

Choose a reason for hiding this comment

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

lpush creates and pushes to key if no key is found

Comment on lines +74 to +76
if (!usersExistingNoSlots.length) {
await redis.expire(usersUniqueKey, NO_SLOTS_NOTIFICATION_FREQUENCY);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

If this is the first push to the key - we push a expiry date (to the key not each item in the array)

Copy link
Contributor

github-actions bot commented Apr 29, 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! 🙌

…er-alert-when-loading-a-calendar-and-no-availability-is' into sean/cal-3480-trigger-alert-when-loading-a-calendar-and-no-availability-is
Copy link
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

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

PR looks good to me.

What we need to see though is if the latency added to getSchedule due to this is significant or not.

@hariombalhara
Copy link
Member

Awesome work @sean-brydon 🙏

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.

Giving another reapproval for @hariombalhara

Copy link
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

LGTM.

@keithwillcode
Copy link
Contributor

@sean-brydon Great PR. One suggestion for a follow-up.

@sean-brydon
Copy link
Member Author

@sean-brydon Great PR. One suggestion for a follow-up.

Great idea - i cant find any tests currently for that file so we'd need to write them before implementing this

@keithwillcode keithwillcode merged commit c478f73 into main May 3, 2024
40 of 41 checks passed
@keithwillcode keithwillcode deleted the sean/cal-3480-trigger-alert-when-loading-a-calendar-and-no-availability-is branch May 3, 2024 13:48
@keithwillcode keithwillcode changed the title feat: orgs trigger alert when loading a calendar and no availability is feat: orgs trigger alert when loading a calendar and no availability is found May 3, 2024
@shirazdole
Copy link
Member

@sean-brydon what a banger, just to double check, if a user within an org is added to an event-type — individual or collective and they don't have a calendar connected, that triggers a notification email to all org owners or admins? Is that programmable in terms of can that be turned on or off? Is there a nice pane to see this within the app outside of the members view which has the long list of apps per user — kind of like a filterable table

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
booking-page area: booking page, public booking page, booker consumer core area: core, team members only emails area: emails, cancellation email, reschedule email, inbox, spam folder, not getting email ✨ feature New feature or request High priority Created by Linear-GitHub Sync ❗️ migrations contains migration files 🎨 needs design Before engineering kick-off, a designer needs to submit a mockup organizations area: organizations, orgs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CAL-3480] Trigger alert when loading a calendar and no availability is loaded at all
7 participants