21368: Code review comments, part 2

impactupgrade · Sep 18, 2024 · eace60d · eace60d
1 parent 6d3e6fe
commit eace60d
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 14 deletions.
diff --git a/src/main/java/com/impactupgrade/nucleus/client/SfdcClient.java b/src/main/java/com/impactupgrade/nucleus/client/SfdcClient.java
@@ -619,12 +619,12 @@ public List<QueryResult> getDonorContacts(Calendar updatedSince)
     if (updatedSince != null) {
       updatedSinceClause = "SystemModStamp >= " + new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'").format(updatedSince.getTime());
     }
-    queryResults.add(queryDonatingContacts(updatedSinceClause));
+    queryResults.add(queryDonorContacts(updatedSinceClause));
 
     return queryResults;
   }
 
-  protected QueryResult queryDonatingContacts(String updatedSinceClause) throws ConnectionException, InterruptedException {
+  protected QueryResult queryDonorContacts(String updatedSinceClause) throws ConnectionException, InterruptedException {
     if (Strings.isNullOrEmpty(updatedSinceClause)) {
       env.logJobWarn("no filter provided; out of caution, skipping the query to protect API limits");
       return new QueryResult();
@@ -808,7 +808,7 @@ public List<SObject> getDonationsByAccountId(String accountId, String... extraFi
   public List<SObject> getDonationsUpdatedAfter(Calendar updatedSince, String... extraFields) throws ConnectionException, InterruptedException {
     String updatedSinceClause = "SystemModStamp >= " + new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'").format(updatedSince.getTime());
     String query = "select " + getFieldsList(DONATION_FIELDS, env.getConfig().salesforce.customQueryFields.donation, extraFields) + " from Opportunity " +
-            "where " + updatedSinceClause + " ORDER BY CloseDate DESC";
+        "where " + updatedSinceClause + " AND stageName = 'Closed Won' ORDER BY CloseDate DESC ";
     return queryListAutoPaged(query);
   }
 

diff --git a/src/main/java/com/impactupgrade/nucleus/service/segment/AccountingPlatformService.java b/src/main/java/com/impactupgrade/nucleus/service/segment/AccountingPlatformService.java
@@ -15,8 +15,6 @@ public interface AccountingPlatformService extends SegmentService {
 
     Optional<AccountingTransaction> getTransaction(CrmDonation crmDonation) throws Exception;
 
-    //String updateOrCreateContact(CrmContact crmContact) throws Exception;
-
     List<String> updateOrCreateContacts(List<CrmContact> crmContacts) throws Exception;
 
     String createTransaction(AccountingTransaction accountingTransaction) throws Exception;

diff --git a/src/main/java/com/impactupgrade/nucleus/service/segment/XeroAccountingPlatformService.java b/src/main/java/com/impactupgrade/nucleus/service/segment/XeroAccountingPlatformService.java
@@ -480,8 +480,16 @@ protected Contact toContact(CrmContact crmContact) {
             return null;
         }
 
+        String supporterId;
+        if (crmContact.crmRawObject instanceof SObject sObject) {
+            supporterId = (String) sObject.getField(SUPPORTER_ID_FIELD_NAME);
+        } else {
+            //Should be unreachable
+            supporterId = crmContact.account.id;
+        }
+
         Contact contact = new Contact();
-        contact.accountNumber(crmContact.account.id);
+        contact.accountNumber(supporterId);
         Phone mobilePhone = new Phone();
         mobilePhone.setPhoneType(Phone.PhoneTypeEnum.MOBILE);
         mobilePhone.setPhoneNumber(crmContact.mobilePhone);
@@ -494,13 +502,13 @@ protected Contact toContact(CrmContact crmContact) {
 
         if (crmContact.account.recordType == EnvironmentConfig.AccountType.HOUSEHOLD) {
             // Household
-            contact.setName(crmContact.getFullName() + " " + crmContact.account.id);
+            contact.setName(crmContact.getFullName() + " " + supporterId);
             contact.setFirstName(crmContact.firstName);
             contact.setLastName(crmContact.lastName);
         } else {
             // Organization
             //TODO: Three different record types to include: AU ORGANISATION, AU CHURCH, AU SCHOOL?
-            contact.setName(crmContact.account.name + " " + crmContact.account.id);
+            contact.setName(crmContact.account.name + " " + supporterId);
             ContactPerson primaryContactPerson = new ContactPerson();
             primaryContactPerson.setFirstName(crmContact.firstName);
             primaryContactPerson.setLastName(crmContact.lastName);

diff --git a/src/main/java/com/impactupgrade/nucleus/service/segment/XeroDataSyncService.java b/src/main/java/com/impactupgrade/nucleus/service/segment/XeroDataSyncService.java
@@ -40,7 +40,7 @@ public void syncContacts(Calendar updatedAfter) throws Exception {
         try {
           env.accountingPlatformService().get().updateOrCreateContacts(resultSet.getRecords());
         } catch (Exception e) {
-          //TODO: process errors
+          env.logJobError("{}/syncContacts failed: {}", this.name(), e);
         }
       }
     }
@@ -51,18 +51,18 @@ public void syncTransactions(Calendar updatedAfter) throws Exception {
     List<CrmDonation> crmDonations = env.primaryCrmService().getDonations(updatedAfter);
     if (env.accountingPlatformService().isPresent()) {
       //TODO: get all contacts ids for crm contacts and do bulk update instead?
-      for (CrmDonation crmDonation : crmDonations) {
-        try {
+      try {
+        for (CrmDonation crmDonation : crmDonations) {
           CrmContact crmContact = crmDonation.contact;
+          //TODO: replace with 'getContactIdForTransaction?'
           String contactId = env.accountingPlatformService().get().updateOrCreateContacts(List.of(crmContact))
               .stream()
               .findFirst().orElse(null);
           env.accountingPlatformService().get().createTransaction(toAccountingTransaction(contactId, crmContact, crmDonation));
-        } catch (Exception e) {
-          //TODO: process errors
         }
+      } catch (Exception e) {
+        env.logJobError("{}/syncTransactions failed: {}", this.name(), e);
       }
-
     }
   }