feat: Skip RR Assignment if Contact Exists In Salesforce #14556
Conversation
|
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link. |
joeauyeung
changed the title
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Ignored Deployments
|
| @@ -113,37 +112,14 @@ export default class SalesforceCRMService implements CRM { | |||
| }); | |||
| }; | |||
|
|
|||
| private salesforceContactCreate = async (attendees: Person[]) => { | |||
There was a problem hiding this comment.
Wasn't being used anywhere
| @@ -113,37 +112,14 @@ export default class SalesforceCRMService implements CRM { | |||
| }); | |||
| }; | |||
|
|
|||
| private salesforceContactCreate = async (attendees: Person[]) => { | |||
| private getSalesforceUserFromEmail = async (email: string) => { | |||
There was a problem hiding this comment.
Get the Salesfoce user info from the email provided. Assuming that the email on Cal.com is the same as Salesforce
| }; | ||
|
|
||
| private salesforceContactSearch = async (event: CalendarEvent) => { | ||
| private getSalesforceUserFromOwnerId = async (ownerId: string) => { |
There was a problem hiding this comment.
When querying contacts, it only returns the ownerId. So need to make another query to Salesfoce for the user email.
|
|
||
| if (!results || !results.records.length) return []; | ||
|
|
||
| if (includeOwner) { |
There was a problem hiding this comment.
We have to make different calls to Salesfoce for the owner information. Let's hide this logic behind a param to only make those calls if we need to.
| let organizerId: string; | ||
| if (organizerEmail) { | ||
| const userQuery = await this.getSalesforceUserFromEmail(organizerEmail); | ||
| if (userQuery) { | ||
| organizerId = (userQuery.records[0] as { Email: string; Id: string }).Id; | ||
| } | ||
| } |
There was a problem hiding this comment.
See if the organizing user exists in Salesforce. If so then connect the new contact to that organizer. If they don't exist then fallback to the account that iniated the Salesforce OAuth.
| @@ -60,6 +60,7 @@ export const useScheduleForEvent = ({ | |||
| dayCount, | |||
| selectedDate, | |||
| orgSlug, | |||
| bookerEmail, | |||
There was a problem hiding this comment.
This is the email that is taken from the URL param
| if (reqBody.teamMemberEmail) { | ||
| const teamMember = eventTypeWithUsers.users.find((user) => user.email === reqBody.teamMemberEmail); | ||
| if (teamMember) { | ||
| luckyUsers.push(teamMember); | ||
| } | ||
| } |
There was a problem hiding this comment.
If the teamMemberEmail is passed then let's skip the logic of finding the lucky user.
| users = [ | ||
| ...availableUsers.filter((user) => user.isFixed && user.email !== reqBody.teamMemberEmail), | ||
| ...luckyUsers, | ||
| ]; |
There was a problem hiding this comment.
If the team member is a fixed host then remove them from this array so they aren't double booked.
There was a problem hiding this comment.
if we use luckyUserPool above, then I think we wouldn't need that
| }); | ||
| if (crmCredential) { | ||
| const crm = new CrmManager(crmCredential); | ||
| const contact = await crm.getContacts(input.bookerEmail, true); |
There was a problem hiding this comment.
I was scared about making one more call when loading slots but if we narrow down the host we only need to get slots for that one host vs all hosts under an event type.
| @@ -655,6 +712,7 @@ export async function getAvailableSlots({ input, ctx }: GetScheduleOptions) { | |||
|
|
|||
| return { | |||
| slots: computedAvailableSlots, | |||
| teamMember, | |||
There was a problem hiding this comment.
Return the team member email to eventually send to handleNewBooking
There was a problem hiding this comment.
can we make that name clearer, something like crmContactOwner?
There was a problem hiding this comment.
Found a bug
Settings:
- teampro is contact owner of test3@test.com
Fixed host was not assigned but teampro as assigned twice:
| .filter((host) => host.user.email !== contactOwner.user.email) | ||
| .map(({ isFixed, user }) => ({ isFixed, ...user })); | ||
|
|
||
| usersWithCredentials = [{ ...contactOwner.user, isFixed: true }, ...otherHosts]; |
There was a problem hiding this comment.
otherHosts are all fixed hosts (without contactOwner) and if contactOwner is a fixed host then it also includes the round robin hosts
There was a problem hiding this comment.
Seeing from the back and forth. Code looks fine so far and tests are passing as well. I do have some observations tho:
- We got a huge amount of nested if/else statements, there a lot of conditionals that could easily be a single-purpose function.
- We're lacking both unit and integration tests to prove that this works and won't break in future updates.
Since this is not being merged to main yet and it is going to the crm-manager branch. I'll approve and merge this but in order to get crm-manager into main we need to address those concerns.
Nice work @joeauyeung and @CarinaWolli with the development and thorough testing so far. 🙏
* fix timezone display on booking page to reflect event availability timezone * migrate fetching event owner's schedule to server side * migrate fetching event owner's schedule to server side * fix e2e test errors * Add WEBAPP_URL_FOR_OAUTH to salesforce auth * In event manager constructor include "_crm" credentials as calendar creds * Change crm apps to type to end with `_crm` * Move sendgrid out of CRM * Add zoho bigin to CRM apps * When getting apps, use slug * Add `crm` variants * Hubspot Oauth use `WEBAPP_URL_FOR_OAUTH` * Refactor creating credentials * Fix empty CRM page * Use credentials with `_crm` * Abstract getAppCategoryTitle * Add integration.handler changes * Init crmManager * Change salesforce to CrmService * Create crmManager * Create contact on new event * Create event * Create new CRM reference * - Fix create new contact for salesforce - Add reschedule to crmManager * Create deleteAllCRMEvents * When searching for credential, look for current credentials in class * On cancel, delete 3rd party events * Add delete method * Type fix * Type fix * Convert Close.com to CrmService * Convert Close.com to CrmService * Move hubspot to CrmService * Convert Pipedrive to CrmService * Rename classes to CrmService * Move ZohoCrm to CrmService * Move Bigin to CrmService * Type return for CrmServices * Fix type errors * Close.com create leads and contacts * Fix tests * Type fix * Zoho bug fixes * Clean up * Type fixes * Remove apiDeletes * Type fixes * Specific typing * Type fix * Type fix * Type fix * Type fix * Type fix * feat: Enable CRM apps on a per event type basis (#14450) * Add Salesforce to be an event type app * Handle new booking, only get enabled CRM credentials * Abstract generating search params * Add close.com to event type * Clean up * Move hubspot to event type * Add pipedrive to event type * Add zoho bigin to event type * Add zoho crm to event type * Remove console.log * Add deleting CRM apps from event type * Delete event type apps * Fix deleting credentials * Add CRM app data to event type metadata * Backwards compatibility: add CRM credential if doesn't exist on event type * Don't include user CRM credentials for backwards comp * Backwards compatibility show CRM app is enabled and dirty field * Clean up * Type fixes * Type fixes * Type fix * Type fix * Remove console.log * Test fix * Upgrade embed-react vite version - dev * Change build can't find error message * Add back omni install prop * Clean up * Refactor `writeAppDataToEventType` * Use eventType repository in writeAppDataToEventType * Clean up old comments * Add error logging * createCRMEvents pass event uid as created event uid * Use `getUid` * Clean up props in create crm event * Small changes to `crmManager` * Fix zoho CRM * refactor crmManager * Undo vite config change * Fix teamId query * Fix bigin error * Remove need for `writeAppDataToEventType` * Add `getAllCredentials` test * Add crmManager tests * Type fixes * Fix type errors * Fix getAllCredentials test * Fix tests * Skip CRM manager tests for now * feat: Skip RR Assignment if Contact Exists In Salesforce (#14556) Co-authored-by: CarinaWolli <wollencarina@gmail.com> * Update yarn.lock * @zomars feedback - use new URL for state params * fix: update hook to not produce enabled === undefined * fix: update app card interfaces to use the new enabled from useIsAppEnabled * fix: feedback for crm RR skip (#15160) * code clean up * fix type any * test setup for RR lead skip * code clean up * simplify code * type error * finshed first test for RR lead skip * add seconds test * add test for handleNewBooking * test if teamMember is set * fix missing enabled key * fix tests * fix type error * use setSystemTime instead of getDate * remove nested if --------- Co-authored-by: CarinaWolli <wollencarina@gmail.com> * fix type error * fix: remove app metadata from all eventTypes on deleting the app * fix: update hook to not produce enabled === undefined (default to false) --------- Co-authored-by: Shaik-Sirajuddin <sirajuddinshaik30gmail.com> Co-authored-by: Shaik-Sirajuddin <sirajudddinshaik30@gmail.com> Co-authored-by: Shaik-Sirajuddin <89742297+Shaik-Sirajuddin@users.noreply.github.com> Co-authored-by: sean-brydon <55134778+sean-brydon@users.noreply.github.com> Co-authored-by: Omar López <zomars@me.com> Co-authored-by: CarinaWolli <wollencarina@gmail.com> Co-authored-by: sean-brydon <sean@cal.com> Co-authored-by: Carina Wollendorfer <30310907+CarinaWolli@users.noreply.github.com> Co-authored-by: Somay Chauhan <somaychauhan98@gmail.com>
This PR is stacked on #14450
What does this PR do?
Fixes CAL-3370
https://www.loom.com/share/2a3d126c82d94997ab957469536e2930?sid=ebeaf6a3-2e2c-41e7-8519-6a8c4ea4f312
Requirement/Documentation
Type of change
How should this be tested?
Mandatory Tasks
Checklist