21677: Mailchimp SMS - Integration Update #249
Conversation
| } catch (MailchimpException e) { | ||
| env.logJobWarn("Mailchimp syncContacts failed: {}", mailchimpClient.exceptionToString(e)); | ||
| } catch (Exception e) { | ||
| env.logJobWarn("Mailchimp syncContacts failed", e); | ||
| } | ||
| } | ||
|
|
||
| protected void backfillContacts(List<CrmContact> contacts, List<MemberInfo> memberInfos) {} |
There was a problem hiding this comment.
@VSydor We can remove this method and no need to call it from syncContacts. For the backfill, let's take the code you put together in dr-hub (the implementation of this method) and we'll instead do a one-time script in ManualUtil. The backfill is just a one-time thing to catch MC contacts up. After that, it's just the normal daily sync as contacts update in SFDC.
| @@ -421,6 +424,12 @@ protected MemberInfo toMcMemberInfo(CrmContact crmContact, Map<String, Object> c | |||
| mcContact.merge_fields.mapping.put(FIRST_NAME, crmContact.firstName); | |||
| mcContact.merge_fields.mapping.put(LAST_NAME, crmContact.lastName); | |||
| mcContact.merge_fields.mapping.put(PHONE_NUMBER, crmContact.mobilePhone); | |||
|
|
|||
| mcContact.consents_to_one_to_one_messaging = true; //? | |||
There was a problem hiding this comment.
@VSydor Do the docs mention what this field is, how it differs from sms_subscription_status, and if this is required? No concerns, just curious.
There was a problem hiding this comment.
I'm afraid the filed has a very brief description in the API docs (and is only listed in response examples):
consents_to_one_to_one_messaging
boolean
Indicates whether a contact consents to 1:1 messaging.
(https://mailchimp.com/developer/marketing/api/list-members/add-member-to-list/)
| import static com.impactupgrade.nucleus.client.MailchimpClient.LAST_NAME; | ||
| import static com.impactupgrade.nucleus.client.MailchimpClient.PHONE_NUMBER; | ||
| import static com.impactupgrade.nucleus.client.MailchimpClient.SUBSCRIBED; | ||
| import static com.impactupgrade.nucleus.client.MailchimpClient.*; |
There was a problem hiding this comment.
Picky: kill wildcard imports in your IDE?
There was a problem hiding this comment.
Snap - must have defaulted my settings for this after re-installing/updating IDE
will remove wildcards
|
|
||
| if (!Strings.isNullOrEmpty(crmFields.smsOptIn) && !crmFields.listFilterOverridesOptIn) { | ||
| clauses.add(crmFields.smsOptIn + "=TRUE"); | ||
| } |
There was a problem hiding this comment.
@VSydor I think we should probably remove this block for now. IIUC, DR wants SMS to be additive to contacts that are already in MC due to an email subscription, and no contacts in MC purely for SMS (and no email). And if you look at the uses of the CrmContact.canReceiveEmail() method, you'll see we're using it to decide multiple pieces of the sync, including who to archive. Maybe in the future they'll want us to sync SMS-only contacts. But for now, kill this block and we'll still sync only contacts that are email-ready? And if they happen to also be SMS opted in, that gets set too.
There was a problem hiding this comment.
You're right
I was overthinking this (thought should be very separate filter)
| @@ -421,6 +424,12 @@ protected MemberInfo toMcMemberInfo(CrmContact crmContact, Map<String, Object> c | |||
| mcContact.merge_fields.mapping.put(FIRST_NAME, crmContact.firstName); | |||
| mcContact.merge_fields.mapping.put(LAST_NAME, crmContact.lastName); | |||
| mcContact.merge_fields.mapping.put(PHONE_NUMBER, crmContact.mobilePhone); | |||
|
|
|||
| mcContact.consents_to_one_to_one_messaging = true; //? | |||
| mcContact.sms_phone_number = crmContact.mobilePhone; | |||
There was a problem hiding this comment.
@VSydor Update this to use crmContact.phoneNumberForSMS() instead -- it attempts Home Phone if Mobile doesn't exist.
There was a problem hiding this comment.
But is it possible to send sms to a home phone?
There was a problem hiding this comment.
Clients aren't all that consistent when they're entering phone numbers, so a cell number will often land in Home Phone instead of Mobile Phone. Ditto for web form integrations. So, we just try both...
| mcContact.consents_to_one_to_one_messaging = true; //? | ||
| mcContact.sms_phone_number = crmContact.mobilePhone; | ||
| mcContact.merge_fields.mapping.put(SMS_PHONE_NUMBER, crmContact.mobilePhone); | ||
| mcContact.sms_subscription_status = SUBSCRIBED; |
There was a problem hiding this comment.
We shouldn't set this (and shouldn't set the phone number or consents either) unless CrmContact has smsOptIn=true and smsOptOut=false
There was a problem hiding this comment.
Also, as mentioned in Slack, add a countryCode configuration in env.json's Mailchimp account section. If the contact's number starts with that countryCode, OR if the contact's number does NOT start with a country code at all, only then allow it to be opted it into SMS here -- it appears MC only allows numbers from the country the account's organization is within.
VSydor
force-pushed
the
feature/21677-mailchimp-sms-integration-update
branch
from
fd87910
to
ca08cf2
Compare
| smsAllowed = true; | ||
| } else if (!phoneNumber.startsWith("+") && !Strings.isNullOrEmpty(mailchimpConfig.country) | ||
| && crmContact.account.billingAddress != null && !Strings.isNullOrEmpty(crmContact.account.billingAddress.country) | ||
| && crmContact.account.billingAddress.country.equalsIgnoreCase(mailchimpConfig.country)) { |
There was a problem hiding this comment.
Solid! One suggestion: let's also check crmContact.account.mailingAddress and crmContact.mailingAddress? Just in case. BillingAddress is more typical, but we do periodically have situations where the others are used. If Australia is in either of the 3, smsAllowed.
…gument ordering match
Depends on:
impactupgrade/mailchimp-jvm-client#4