Code review comments

impactupgrade · Jun 21, 2024 · 62452a9 · 62452a9
1 parent 3d52be0
commit 62452a9
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 26 deletions.
diff --git a/src/main/java/com/impactupgrade/nucleus/client/MailchimpClient.java b/src/main/java/com/impactupgrade/nucleus/client/MailchimpClient.java
@@ -115,17 +115,15 @@ public List<MemberInfo> getListMembers(String listId) throws IOException, Mailch
   }
 
   public List<MemberInfo> getListMembers(String listId, String status, Calendar sinceLastChanged) throws IOException, MailchimpException {
-    return getListMembers(listId, status, null, sinceLastChanged);
+    return getListMembers(listId, status,
+            "members.email_address,members.tags,total_items", // HUGE performance improvement -- limit to only what we need
+            sinceLastChanged);
   }
 
   public List<MemberInfo> getListMembers(String listId, String status, String fields, Calendar sinceLastChanged) throws IOException, MailchimpException {
     GetMembersMethod getMembersMethod = new GetMembersMethod(listId);
     getMembersMethod.status = status;
-    if (!Strings.isNullOrEmpty(fields)) {
-      getMembersMethod.fields = fields;
-    } else {
-      getMembersMethod.fields = "members.email_address,members.tags,total_items"; // HUGE performance improvement -- limit to only what we need
-    }
+    getMembersMethod.fields = fields;
     getMembersMethod.count = 1000; // subjective, but this is timing out periodically -- may need to dial it back further
     env.logJobInfo("retrieving list {} contacts", listId);
     if (sinceLastChanged != null) {

diff --git a/src/main/java/com/impactupgrade/nucleus/environment/Environment.java b/src/main/java/com/impactupgrade/nucleus/environment/Environment.java
@@ -200,6 +200,10 @@ public List<CommunicationService> allCommunicationServices() {
     return segmentServices(CommunicationService.class);
   }
 
+  public CommunicationService communicationService(String name) {
+    return segmentService(name, CommunicationService.class);
+  }
+
   public EnrichmentService enrichmentService(String name) {
     return segmentService(name, EnrichmentService.class);
   }

diff --git a/src/test/java/com/impactupgrade/nucleus/it/MailchimpCommunicationServiceIT.java b/src/test/java/com/impactupgrade/nucleus/it/MailchimpCommunicationServiceIT.java
@@ -62,17 +62,13 @@ public void syncContacts() throws Exception {
     assertTrue(crmContactsEmails.containsAll(emails));
 
     // Sync contacts to MC
-    for (CommunicationService communicationService : env.allCommunicationServices()) {
-      if (communicationService instanceof MailchimpCommunicationService) {
-        communicationService.syncContacts(beforeBulkImport);
-      }
-    }
+    env.communicationService("mailchimp").syncContacts(beforeBulkImport);
 
     EnvironmentConfig.Mailchimp mailchimp = env.getConfig().mailchimp.get(0);
     String listId = mailchimp.lists.get(0).id;
     MailchimpClient mailchimpClient = new MailchimpClient(mailchimp, env);
 
-    assertEmailsStatus(emails, "unsubscribed", listId, mailchimpClient);
+    assertEmailsStatus(emails, "subscribed", listId, mailchimpClient);
   }
 
   @Test
@@ -125,21 +121,20 @@ public void syncUnsubscribes() throws Exception {
     assertEmailsStatus(cleanEmails, "cleaned", listId, mailchimpClient);
 
     // Sync unsubscribes MC >> SF
-    for (CommunicationService communicationService : env.allCommunicationServices()) {
-      if (communicationService instanceof MailchimpCommunicationService) {
-        communicationService.syncUnsubscribes(beforeBulkImport);
-      }
-    }
+    env.communicationService("mailchimp").syncContacts(beforeBulkImport);
 
     crmContacts = env.primaryCrmService().getContactsByEmails(unsubscribeEmails);
+    assertTrue(!crmContacts.isEmpty());
+
     for (CrmContact crmContact : crmContacts) {
       assertTrue(Boolean.TRUE == crmContact.emailOptOut);
     }
 
     crmContacts = env.primaryCrmService().getContactsByEmails(cleanEmails);
+    assertTrue(!crmContacts.isEmpty());
+
     for (CrmContact crmContact : crmContacts) {
-      //TODO: add custom field definition to json config
-      //assertTrue(Boolean.TRUE == crmContact.emailBounced);
+      assertTrue(Boolean.TRUE == crmContact.emailBounced);
     }
   }
 
@@ -164,7 +159,6 @@ private void updateEmailsStatus(List<String> emails, String status, String listI
             .collect(Collectors.toMap(
                     m -> m.email_address, m -> m
             ));
-    // TODO: replace with bulk update?
     for (String email : emails) {
       MemberInfo memberInfo = memberInfoMap.get(email);
       if (memberInfo == null) {

diff --git a/src/test/resources/environment-it-sfdc-mailchimp.json b/src/test/resources/environment-it-sfdc-mailchimp.json
@@ -9,16 +9,12 @@
 
     "fieldDefinitions": {
       "emailOptOut": "HasOptedOutOfEmail",
-      "emailBounced": "",
-      "paymentGatewayName": "Payment_Gateway_Name__c",
-      "paymentGatewayTransactionId": "Payment_Gateway_Transaction_Id__c",
-      "paymentGatewayCustomerId": "Payment_Gateway_Customer_Id__c",
-      "paymentGatewaySubscriptionId": "Payment_Gateway_Subscription_Id__c"
+      "emailBounced": "Email_Bounced__c"
     },
     "customQueryFields": {
       "account": ["External_Reference__c"],
       "campaign": [],
-      "contact": ["External_Reference__c", "HasOptedOutOfEmail"],
+      "contact": ["External_Reference__c", "HasOptedOutOfEmail", "Email_Bounced__c"],
       "donation": ["Payment_Gateway_Name__c", "Payment_Gateway_Transaction_Id__c", "Payment_Gateway_Customer_Id__c"],
       "recurringDonation": ["Payment_Gateway_Name__c", "Payment_Gateway_Subscription_Id__c", "Payment_Gateway_Customer_Id__c"],
       "user": []