feat: Salesforce skip ownership check if attendee email is a free domain #17916

joeauyeung · 2024-11-29T19:09:37Z

What does this PR do?

Introduces a new checkIfFreeEmailDomain function
Adds the option to skip returning the OwnerId of a record if it's a free email domain
Fixes #XXXX (GitHub issue number)
Fixes CAL-4810(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).
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.

…n check is enabled

linear · 2024-11-29T19:10:00Z

CAL-4810 Salesforce: If free mail domain, ignore routing

graphite-app · 2024-11-29T19:10:11Z

Graphite Automations

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

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

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

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

joeauyeung · 2024-11-29T19:25:50Z

packages/lib/freeEmailDomainCheck/checkIfFreeEmailDomain.ts

+
+  if (firstDigit) {
+    const arrayOfEmailDomains =
+      freeEmailDomainsThatStartWithANumberObject[


Since the number of email domains were so small. Rather than going through the whole binary search algo, just straight look for the domain in the array.

Udit-takkar

The new option "If attendee has a free email domain, skip the ownership check and round robin as normal" is only display when "If attendee exists in Salesforce, book directly with the owner" is enabled. is this expected behaviour?

Screen.Recording.2024-11-30.at.1.24.41.AM.mov

hariombalhara · 2024-12-02T10:25:00Z

packages/app-store/salesforce/lib/CrmService.ts

@@ -1073,4 +1074,12 @@ export default class SalesforceCRMService implements CRM {
    if (companyValue === onBookingWriteToRecordFields[companyFieldName]) return;
    return companyValue;
  }
+
+  private attendeeHasFreeEmailDomain(attendeeEmail: string) {


nit: Better name would be something like shouldSkipAttendee or shouldConsiderAttendee (and then use negation of it)

Right now it gives a sense that attendee has free email domain if it is true but it could be that he has free email domain but the organizer still wants him to be considered.

packages/lib/freeEmailDomainCheck/freeEmailDomains.ts

hariombalhara · 2024-12-02T10:35:36Z

packages/app-store/salesforce/lib/CrmService.ts

@@ -392,7 +393,7 @@ export default class SalesforceCRMService implements CRM {

    if (!records.length) records = results.records as ContactRecord[];

-    if (includeOwner || forRoundRobinSkip) {
+    if ((includeOwner || forRoundRobinSkip) && !this.attendeeHasFreeEmailDomain(emails[0])) {


nit: I think instead of doing emails[0]. We should have it stored as booker or similar and then use that variable to make code more readable and consistent

Emails come from the CalendarEvent.attendees type which is used throughout the code base. If we were to make this change I'd want to do it in a separate PR.

Oh I meant, storing it in a variable, so at the top of the fn, we should do
const booker = attendees[0] and then we use booker everywhere which is much more logical then using attendees[0]

packages/lib/freeEmailDomainCheck/checkIfFreeEmailDomain.ts

hariombalhara

Overall looks good, have some nit comments but one requirement to move the list to DB.

* Add requiresConfirmationForFreeEmail to db * Add option to event type settings * Get requiresConfirmationForFreeEmail for event type page * Include requiresConfirmationForFreeEmail in fetching event type * Pass bookerEmail to `getRequiresConfirmationFlags` * Add free email domain check to `determineRequiresConfirmation` * Add `requiresConfirmationForFreeEmail` to types

vercel · 2024-12-02T18:54:46Z

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		Dec 16, 2024 7:50pm
calcom-web-canary	⬜️ Ignored (Inspect)	Visit Preview		Dec 16, 2024 7:50pm

github-actions · 2024-12-07T06:00:09Z

E2E results are ready!

Workflow #41544.2 latest results

emrysal

Looks good to me, in the future as an improvement we can implement more caching but I reckon that's not mission critical rn.

Changes addressed

…ain (calcom#17916) * Add email domain array * Create numbered email domain object * Check email domain * Rename function * Add tests * Frontend enable skip ownership check if free email domain * Backend ignore adding ownership to return records if free email domain check is enabled * feat: Only require confirmation for free email domains (calcom#17917) * Add requiresConfirmationForFreeEmail to db * Add option to event type settings * Get requiresConfirmationForFreeEmail for event type page * Include requiresConfirmationForFreeEmail in fetching event type * Pass bookerEmail to `getRequiresConfirmationFlags` * Add free email domain check to `determineRequiresConfirmation` * Add `requiresConfirmationForFreeEmail` to types * Add severity to Watchlist table * Add migration for watchlist severity * Add `getEmailDomainInWatchlist` method to watchlist repository * Use watchlist repository to check for free email domain * Mock watchlist repository in test * Update test * Rename method * Add severity to blocked list * Move check free email domain to async * Type checks * Adjust for promise returned * Fix tests * Fix * Fix tests

joeauyeung added 7 commits November 27, 2024 23:29

Add email domain array

c13c67c

Create numbered email domain object

99e29c2

Check email domain

4e09a2a

Rename function

3a9bb3c

Add tests

8cb5fbb

Frontend enable skip ownership check if free email domain

d3c6f24

Backend ignore adding ownership to return records if free email domai…

43af906

…n check is enabled

graphite-app bot requested a review from a team November 29, 2024 19:09

keithwillcode added consumer core area: core, team members only labels Nov 29, 2024

dosubot bot added crm-apps area: crm apps, salesforce, hubspot, close.com, sendgrid ✨ feature New feature or request labels Nov 29, 2024

vercel bot deployed to Preview – dev November 29, 2024 19:16 View deployment

joeauyeung commented Nov 29, 2024

View reviewed changes

Udit-takkar reviewed Nov 29, 2024

View reviewed changes

hariombalhara reviewed Dec 2, 2024

View reviewed changes

packages/lib/freeEmailDomainCheck/freeEmailDomains.ts Outdated Show resolved Hide resolved

hariombalhara reviewed Dec 2, 2024

View reviewed changes

packages/lib/freeEmailDomainCheck/checkIfFreeEmailDomain.ts Outdated Show resolved Hide resolved

hariombalhara reviewed Dec 2, 2024

View reviewed changes

packages/lib/freeEmailDomainCheck/checkIfFreeEmailDomain.ts Outdated Show resolved Hide resolved

hariombalhara reviewed Dec 2, 2024

View reviewed changes

packages/lib/freeEmailDomainCheck/checkIfFreeEmailDomain.ts Outdated Show resolved Hide resolved

hariombalhara previously requested changes Dec 2, 2024

View reviewed changes

github-actions bot marked this pull request as draft December 2, 2024 10:53

github-actions bot added the ❗️ migrations contains migration files label Dec 2, 2024

vercel bot had a problem deploying to Preview – api December 2, 2024 19:04 Failure

vercel bot deployed to Preview – dev December 2, 2024 19:34 View deployment

joeauyeung requested a review from CarinaWolli December 5, 2024 17:38

Adjust for promise returned

cd971fe

vercel bot deployed to Preview – api December 5, 2024 19:59 View deployment

vercel bot deployed to Preview – dev December 5, 2024 20:07 View deployment

Fix tests

f515d82

vercel bot had a problem deploying to Preview – api December 5, 2024 20:21 Failure

joeauyeung marked this pull request as draft December 5, 2024 20:26

Fix

8d89577

vercel bot deployed to Preview – api December 5, 2024 20:38 View deployment

vercel bot deployed to Preview – dev December 5, 2024 20:49 View deployment

Fix tests

71d23ed

vercel bot deployed to Preview – api December 5, 2024 21:55 View deployment

vercel bot deployed to Preview – dev December 5, 2024 22:03 View deployment

joeauyeung marked this pull request as ready for review December 6, 2024 14:28

Merge branch 'main' into salesforce-skip-routing-if-free-email

4331a19

vercel bot deployed to Preview – api December 6, 2024 14:36 View deployment

vercel bot deployed to Preview – dev December 6, 2024 14:45 View deployment

joeauyeung added the ready-for-e2e label Dec 7, 2024

Merge branch 'main' into salesforce-skip-routing-if-free-email

03aaaf5

vercel bot deployed to Preview – api December 16, 2024 19:57 View deployment

joeauyeung requested a review from zomars December 16, 2024 20:03

vercel bot deployed to Preview – dev December 16, 2024 20:06 View deployment

emrysal approved these changes Dec 18, 2024

View reviewed changes

emrysal merged commit 8dda418 into main Dec 18, 2024
38 of 39 checks passed

emrysal deleted the salesforce-skip-routing-if-free-email branch December 18, 2024 15:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Salesforce skip ownership check if attendee email is a free domain #17916

feat: Salesforce skip ownership check if attendee email is a free domain #17916

joeauyeung commented Nov 29, 2024 •

edited

Loading

linear bot commented Nov 29, 2024

graphite-app bot commented Nov 29, 2024 •

edited

Loading

joeauyeung Nov 29, 2024

Udit-takkar left a comment

hariombalhara Dec 2, 2024 •

edited

Loading

hariombalhara Dec 2, 2024

joeauyeung Dec 4, 2024

hariombalhara Dec 4, 2024

hariombalhara left a comment

vercel bot commented Dec 2, 2024 •

edited

Loading

github-actions bot commented Dec 7, 2024 •

edited

Loading

emrysal left a comment

feat: Salesforce skip ownership check if attendee email is a free domain #17916

feat: Salesforce skip ownership check if attendee email is a free domain #17916

Conversation

joeauyeung commented Nov 29, 2024 • edited Loading

What does this PR do?

Mandatory Tasks (DO NOT REMOVE)

linear bot commented Nov 29, 2024

graphite-app bot commented Nov 29, 2024 • edited Loading

Graphite Automations

joeauyeung Nov 29, 2024

Choose a reason for hiding this comment

Udit-takkar left a comment

Choose a reason for hiding this comment

hariombalhara Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

hariombalhara Dec 2, 2024

Choose a reason for hiding this comment

joeauyeung Dec 4, 2024

Choose a reason for hiding this comment

hariombalhara Dec 4, 2024

Choose a reason for hiding this comment

hariombalhara left a comment

Choose a reason for hiding this comment

vercel bot commented Dec 2, 2024 • edited Loading

github-actions bot commented Dec 7, 2024 • edited Loading

E2E results are ready!

emrysal left a comment

Choose a reason for hiding this comment

joeauyeung commented Nov 29, 2024 •

edited

Loading

graphite-app bot commented Nov 29, 2024 •

edited

Loading

hariombalhara Dec 2, 2024 •

edited

Loading

vercel bot commented Dec 2, 2024 •

edited

Loading

github-actions bot commented Dec 7, 2024 •

edited

Loading