Code review comments

src/main/java/com/impactupgrade/nucleus/client/SfdcClient.java

@@ -11,7 +11,6 @@
 import com.google.common.base.Strings;
 import com.impactupgrade.integration.sfdc.SFDCPartnerAPIClient;
 import com.impactupgrade.nucleus.environment.Environment;
-import com.impactupgrade.nucleus.environment.EnvironmentConfig;
 import com.impactupgrade.nucleus.model.AccountSearch;
 import com.impactupgrade.nucleus.model.ContactSearch;
 import com.impactupgrade.nucleus.util.HttpClient;
@@ -527,25 +526,19 @@ protected QueryResult queryEmailContacts(String updatedSinceClause, String filte
     // IMPORTANT: If env.json defines emailOptOut/emailBounced, also include those contacts in this query! This might seem backwards,
     // but we need them in the results so that we can archive them in Mailchimp.
     List<String> clauses = new ArrayList<>();
-    EnvironmentConfig.CRMFieldDefinitions crmFields = env.getConfig().salesforce.fieldDefinitions;
-
-    if (!Strings.isNullOrEmpty(crmFields.emailOptIn) && !crmFields.listFilterOverridesOptIn) {
-      clauses.add(crmFields.emailOptIn + "=TRUE");
+    if (!Strings.isNullOrEmpty(env.getConfig().salesforce.fieldDefinitions.emailOptIn)
+        && !env.getConfig().salesforce.fieldDefinitions.listFilterOverridesOptIn) {
+      clauses.add(env.getConfig().salesforce.fieldDefinitions.emailOptIn + "=TRUE");
 
       // ONLY ADD THESE IF WE'RE INCLUDING THE ABOVE OPT-IN FILTER! Otherwise, some orgs only have opt-out defined,
       // and we'd effectively be syncing ONLY unsubscribes.
-      if (!Strings.isNullOrEmpty(crmFields.emailOptOut)) {
-        clauses.add(crmFields.emailOptOut + "=TRUE");
+      if (!Strings.isNullOrEmpty(env.getConfig().salesforce.fieldDefinitions.emailOptOut)) {
+        clauses.add(env.getConfig().salesforce.fieldDefinitions.emailOptOut + "=TRUE");
       }
-      if (!Strings.isNullOrEmpty(crmFields.emailBounced)) {
-        clauses.add(crmFields.emailBounced + "=TRUE");
+      if (!Strings.isNullOrEmpty(env.getConfig().salesforce.fieldDefinitions.emailBounced)) {
+        clauses.add(env.getConfig().salesforce.fieldDefinitions.emailBounced + "=TRUE");
       }
     }
-
-    if (!Strings.isNullOrEmpty(crmFields.smsOptIn) && !crmFields.listFilterOverridesOptIn) {
-      clauses.add(crmFields.smsOptIn + "=TRUE");
-    }
-
     String optInOutFilters = clauses.isEmpty() ? "" : " AND (" + String.join(" OR ", clauses) + ")";
 
     // IMPORTANT: Order by CreatedDate ASC, ensuring this is FIFO for contacts sharing the same email address.

src/main/java/com/impactupgrade/nucleus/environment/EnvironmentConfig.java

@@ -327,6 +327,8 @@ public static class CommunicationPlatform extends Platform {
     // Transactional email (donation receipts, notifications, etc.) need one of the email platforms to be
     // designated as the conduit!
     public boolean transactionalSender = false;
+    public String country;
+    public String countryCode;
   }
 
   public List<CommunicationPlatform> mailchimp = new ArrayList<>();

src/main/java/com/impactupgrade/nucleus/service/segment/MailchimpCommunicationService.java

@@ -15,9 +15,9 @@
 import com.impactupgrade.nucleus.model.CrmAccount;
 import com.impactupgrade.nucleus.model.CrmContact;
 import com.impactupgrade.nucleus.model.PagedResults;
+import com.impactupgrade.nucleus.util.CountryCallingCode;
 import org.apache.commons.collections.CollectionUtils;
 
-import java.io.IOException;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Calendar;
@@ -31,7 +31,11 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import static com.impactupgrade.nucleus.client.MailchimpClient.*;
+import static com.impactupgrade.nucleus.client.MailchimpClient.FIRST_NAME;
+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.SMS_PHONE_NUMBER;
+import static com.impactupgrade.nucleus.client.MailchimpClient.SUBSCRIBED;
 
 public class MailchimpCommunicationService extends AbstractCommunicationService {
 
@@ -142,7 +146,7 @@ protected void syncContacts(PagedResults.ResultSet<CrmContact> resultSet, Enviro
           communicationList);
 
       // run the actual contact upserts
-      List<MemberInfo> upsertMemberInfos = toMcMemberInfos(communicationList, contactsToUpsert, contactsCustomFields);
+      List<MemberInfo> upsertMemberInfos = toMcMemberInfos(communicationList, mailchimpConfig, contactsToUpsert, contactsCustomFields);
       String upsertBatchId = mailchimpClient.upsertContactsBatch(communicationList.id, upsertMemberInfos);
       mailchimpClient.runBatchOperations(mailchimpConfig, upsertBatchId, 0);
 
@@ -160,18 +164,13 @@ protected void syncContacts(PagedResults.ResultSet<CrmContact> resultSet, Enviro
 
       String archiveBatchId = mailchimpClient.archiveContactsBatch(communicationList.id, emailsToArchive);
       mailchimpClient.runBatchOperations(mailchimpConfig, archiveBatchId, 0);
-
-      List<MemberInfo> memberInfos = mailchimpClient.getListMembers(communicationList.id);
-      backfillContacts(contactsToUpsert, memberInfos);
     } 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) {}
-
   @Override
   public void massArchive() throws Exception {
     for (EnvironmentConfig.CommunicationPlatform mailchimpConfig : env.getConfig().mailchimp) {
@@ -308,7 +307,7 @@ protected void upsertContact(EnvironmentConfig.CommunicationPlatform mailchimpCo
           communicationList);
 
       // run the actual contact upserts
-      MemberInfo upsertMemberInfo = toMcMemberInfo(crmContact, customFields, communicationList.groups);
+      MemberInfo upsertMemberInfo = toMcMemberInfo(crmContact, customFields, communicationList.groups, mailchimpConfig);
       mailchimpClient.upsertContact(communicationList.id, upsertMemberInfo);
 
       // update all contacts' tags
@@ -403,14 +402,14 @@ protected String updateTagsBatch(String listId, List<MailchimpClient.EmailContac
     }
   }
 
-  protected List<MemberInfo> toMcMemberInfos(EnvironmentConfig.CommunicationList communicationList, List<CrmContact> crmContacts,
+  protected List<MemberInfo> toMcMemberInfos(EnvironmentConfig.CommunicationList communicationList, EnvironmentConfig.CommunicationPlatform mailchimpConfig, List<CrmContact> crmContacts,
       Map<String, Map<String, Object>> customFieldsMap) {
     return crmContacts.stream()
-        .map(crmContact -> toMcMemberInfo(crmContact, customFieldsMap.get(crmContact.email), communicationList.groups))
+        .map(crmContact -> toMcMemberInfo(crmContact, customFieldsMap.get(crmContact.email), communicationList.groups, mailchimpConfig))
         .collect(Collectors.toList());
   }
 
-  protected MemberInfo toMcMemberInfo(CrmContact crmContact, Map<String, Object> customFields, Map<String, String> groups) {
+  protected MemberInfo toMcMemberInfo(CrmContact crmContact, Map<String, Object> customFields, Map<String, String> groups, EnvironmentConfig.CommunicationPlatform mailchimpConfig) {
     if (crmContact == null) {
       return null;
     }
@@ -425,10 +424,17 @@ protected MemberInfo toMcMemberInfo(CrmContact crmContact, Map<String, Object> c
     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;
-    mcContact.merge_fields.mapping.put(SMS_PHONE_NUMBER, crmContact.mobilePhone);
-    mcContact.sms_subscription_status = SUBSCRIBED;
+    boolean smsOptIn = Boolean.TRUE == crmContact.smsOptIn && Boolean.FALSE == crmContact.smsOptOut;
+    String phoneNumber = crmContact.phoneNumberForSMS();
+    boolean phoneNumberAllowed =
+        phoneNumber.startsWith(mailchimpConfig.countryCode)
+        || ((!phoneNumber.startsWith("+") && crmContact.account.billingAddress.country.equalsIgnoreCase(mailchimpConfig.country)));
+    if (smsOptIn && phoneNumberAllowed) {
+      mcContact.consents_to_one_to_one_messaging = true;
+      mcContact.sms_subscription_status = SUBSCRIBED;
+      mcContact.sms_phone_number = phoneNumber;
+      mcContact.merge_fields.mapping.put(SMS_PHONE_NUMBER, phoneNumber);
+    }
 
     mcContact.merge_fields.mapping.putAll(customFields);
     mcContact.status = SUBSCRIBED;

src/main/resources/environment-sample.json

@@ -138,7 +138,9 @@
           "groups": {},
           "crmFilter": ""
         }
-      ]
+      ],
+      "country": "Australia",
+      "countryCode" : "+61"
     }
   ],