Skip to content

Commit

Permalink
[sc-19301] prevent duplicate contact creation using donation history …
Browse files Browse the repository at this point in the history
…as a last resort
  • Loading branch information
brmeyer committed Jun 17, 2024
1 parent 06f1e27 commit 7bc4a21
Show file tree
Hide file tree
Showing 16 changed files with 163 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ public Optional<SObject> getContactById(String contactId, String... extraFields)
String query = "select " + getFieldsList(CONTACT_FIELDS, env.getConfig().salesforce.customQueryFields.contact, extraFields) + " from contact where id = '" + contactId + "' ORDER BY name";
return querySingle(query);
}
// TODO: needs additional filters, like the opt-out fields, from queryEmailContacts -- DRY it up?
public Optional<SObject> getFilteredContactById(String contactId, String filter, String... extraFields) throws ConnectionException, InterruptedException {
String query = "select " + getFieldsList(CONTACT_FIELDS, env.getConfig().salesforce.customQueryFields.contact, extraFields) + " from contact where id = '" + contactId + "' and " + filter + " ORDER BY name";
return querySingle(query);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@
import com.impactupgrade.nucleus.model.ContactSearch;
import com.impactupgrade.nucleus.model.CrmAccount;
import com.impactupgrade.nucleus.model.CrmContact;
import com.impactupgrade.nucleus.model.CrmDonation;
import com.impactupgrade.nucleus.model.CrmRecurringDonation;
import com.impactupgrade.nucleus.model.PaymentGatewayEvent;
import com.impactupgrade.nucleus.service.segment.CrmService;

import java.util.List;
import java.util.Optional;
import java.util.Set;

Expand Down Expand Up @@ -59,7 +62,7 @@ protected void fetchAndSetDonorData(PaymentGatewayEvent paymentGatewayEvent) thr
}
}
} else {
env.logJobInfo("event included CRM contact {}, but the contact didn't exist; trying through the contact email...",
env.logJobInfo("event included CRM contact {}, but the contact didn't exist; trying through the contact...",
paymentGatewayEvent.getCrmContact().id);
// IMPORTANT: If this was the case, clear out the existingAccount and use the one discovered by the proceeding contact search!
existingAccount = Optional.empty();
Expand Down Expand Up @@ -94,6 +97,24 @@ protected void fetchAndSetDonorData(PaymentGatewayEvent paymentGatewayEvent) thr
contactSearch.keywords = Set.of(paymentGatewayEvent.getCrmAccount().billingAddress.street);
existingContact = crmService.searchContacts(contactSearch).getSingleResult();
}

// As a last resort, attempt to look up existing donations using the donor's customer or subscription. If
// donations are found, retrieve the contact/account from the latest. This prevents duplicate contacts
// when donations come in with nothing more than a first/last name.
if (existingContact.isEmpty() && !Strings.isNullOrEmpty(paymentGatewayEvent.getCrmRecurringDonation().subscriptionId)) {
Optional<CrmRecurringDonation> crmRecurringDonation = crmService.getRecurringDonationBySubscriptionId(
paymentGatewayEvent.getCrmRecurringDonation().subscriptionId);
if (crmRecurringDonation.isPresent()) {
existingContact = crmService.getContactById(crmRecurringDonation.get().contact.id);
}
}
if (existingContact.isEmpty() && !Strings.isNullOrEmpty(paymentGatewayEvent.getCrmDonation().customerId)) {
List<CrmDonation> crmDonations = crmService.getDonationsByCustomerId(
paymentGatewayEvent.getCrmDonation().customerId);
if (!crmDonations.isEmpty()) {
existingContact = crmService.getContactById(crmDonations.get(0).contact.id);
}
}
}

if (existingContact.isPresent()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,12 @@ public List<CrmDonation> getDonationsByTransactionIds(List<String> transactionId
}

@Override
public Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId, String accountId, String contactId) throws Exception {
public List<CrmDonation> getDonationsByCustomerId(String customerId) throws Exception {
return List.of();
}

@Override
public Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId) throws Exception {
return Optional.empty();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ default String insertRecurringDonation(CrmRecurringDonation crmRecurringDonation
return null;
}

default Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId, String accountId, String contactId) throws Exception {
default Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId) throws Exception {
return Optional.empty();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,12 @@ public List<CrmDonation> getDonationsByTransactionIds(List<String> transactionId
return Collections.emptyList();
}

// Not able to retrieve donations purely by customerId -- must have the Constituent.
@Override
public List<CrmDonation> getDonationsByCustomerId(String customerId) throws Exception {
return List.of();
}

@Override
public void updateDonation(CrmDonation crmDonation) throws Exception {
// Retrieving donations by Stripe IDs are not possible.
Expand Down Expand Up @@ -452,6 +458,11 @@ public void removeContactFromList(CrmContact crmContact, String listId) throws E
// SMS opt out
}

@Override
public Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId) throws Exception {
return Optional.empty(); // not possible without the contactId
}

@Override
public Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId, String accountId, String contactId) throws Exception {
return getDonation(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ default Optional<CrmDonation> getDonationByTransactionId(String transactionId) t
// Others, like SFDC's SOQL, may allow clauses like "WHERE IN (<list>)" in queries, allowing us to retrieve large
// batches all at once. This is SUPER important, especially for SFDC, where monthly API limits are in play...
List<CrmDonation> getDonationsByTransactionIds(List<String> transactionIds) throws Exception;
List<CrmDonation> getDonationsByCustomerId(String customerId) throws Exception;
String insertDonation(CrmDonation crmDonation) throws Exception;
void updateDonation(CrmDonation crmDonation) throws Exception;
void refundDonation(CrmDonation crmDonation) throws Exception;
Expand All @@ -139,7 +140,14 @@ default Optional<CrmRecurringDonation> getRecurringDonation(String id, String su
return crmRecurringDonation;
}
Optional<CrmRecurringDonation> getRecurringDonationById(String id) throws Exception;
Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId, String accountId, String contactId) throws Exception;
Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId) throws Exception;
default Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId, String accountId,
String contactId) throws Exception {
// Most CRMs do not need the accountId/contactId to retrieve RDs, with notable exceptions like Virtuous.
// We don't even always have the accountId/contactId available, like when we're attempting to figure out who a donor
// is using past donations as a last resort in ContactService.
return getRecurringDonationBySubscriptionId(subscriptionId);
}
List<CrmRecurringDonation> searchAllRecurringDonations(Optional<String> name, Optional<String> email, Optional<String> phone) throws Exception;
String insertRecurringDonation(CrmRecurringDonation crmRecurringDonation) throws Exception;
// void updateRecurringDonation(CrmRecurringDonation crmRecurringDonation) throws Exception;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ public List<CrmDonation> getDonationsByTransactionIds(List<String> transactionId
return Collections.emptyList();
}

@Override
public List<CrmDonation> getDonationsByCustomerId(String customerId) throws Exception {
return List.of();
}

@Override
public String insertDonation(CrmDonation crmDonation) throws Exception {
return dwClient.insertDonation(crmDonation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,11 @@ public List<CrmDonation> getDonationsByTransactionIds(List<String> transactionId
.collect(Collectors.toList());
}

@Override
public List<CrmDonation> getDonationsByCustomerId(String customerId) throws Exception {
return List.of();
}

@Override
public String insertDonation(CrmDonation crmDonation) throws Exception {
DynamicsCrmClient.Opportunity insertedOpportunity = dynamicsCrmClient.insertOpportunity(toOpportunity(crmDonation));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ public List<CrmDonation> getDonationsByTransactionIds(List<String> transactionId
return Collections.emptyList();
}

@Override
public List<CrmDonation> getDonationsByCustomerId(String customerId) throws Exception {
return List.of();
}

@Override
public String insertDonation(CrmDonation crmDonation) throws Exception {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,15 @@ public List<CrmDonation> getDonationsByTransactionIds(List<String> transactionId
return toCrmDonation(results.getResults());
}

@Override
public List<CrmDonation> getDonationsByCustomerId(String customerId) throws Exception {
Filter filter = new Filter(env.getConfig().hubspot.fieldDefinitions.paymentGatewayCustomerId, "EQ", customerId);
List<FilterGroup> filterGroups = List.of(new FilterGroup(List.of(filter)));

DealResults results = hsClient.deal().search(filterGroups, dealFields);
return toCrmDonation(results.getResults());
}

@Override
public List<CrmRecurringDonation> searchAllRecurringDonations(Optional<String> name, Optional<String> email, Optional<String> phone) throws Exception {
// TODO
Expand Down Expand Up @@ -305,7 +314,7 @@ protected void setTaskFields(EngagementRequest engagementRequest, CrmActivity cr
}

@Override
public Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId, String accountId, String contactId) throws Exception {
public Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId) throws Exception {
Filter filter = new Filter(env.getConfig().hubspot.fieldDefinitions.paymentGatewaySubscriptionId, "EQ", subscriptionId);
List<FilterGroup> filterGroups = List.of(new FilterGroup(List.of(filter)));
DealResults results = hsClient.deal().search(filterGroups, dealFields);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,12 @@ public List<CrmDonation> getDonationsByTransactionIds(List<String> transactionId
return toCrmDonation(sfdcClient.getDonationsByTransactionIds(transactionIds));
}

@Override
public List<CrmDonation> getDonationsByCustomerId(String customerId) throws Exception {
return toCrmDonation(sfdcClient.getDonationsByUniqueField(
env.getConfig().salesforce.fieldDefinitions.paymentGatewayCustomerId, List.of(customerId)));
}

@Override
public List<CrmRecurringDonation> searchAllRecurringDonations(Optional<String> name, Optional<String> email, Optional<String> phone) throws InterruptedException, ConnectionException {
List<String> clauses = searchRecurringDonations(name, email, phone);
Expand Down Expand Up @@ -385,7 +391,7 @@ protected void setActivityFields(SObject task, CrmActivity crmActivity) {
}

@Override
public Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId, String accountId, String contactId) throws Exception {
public Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId) throws Exception {
return toCrmRecurringDonation(sfdcClient.getRecurringDonationBySubscriptionId(subscriptionId));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,12 @@ public List<CrmDonation> getDonationsByTransactionIds(List<String> transactionId
}

@Override
public Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId, String accountId, String contactId) throws Exception {
public List<CrmDonation> getDonationsByCustomerId(String customerId) throws Exception {
return List.of();
}

@Override
public Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId) throws Exception {
return Optional.empty();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,12 @@ public List<CrmDonation> getDonationsByTransactionIds(List<String> transactionId
return donations;
}

@Override
public List<CrmDonation> getDonationsByCustomerId(String customerId) throws Exception {
// TODO: Might be possible?
return List.of();
}

@Override
public String insertDonation(CrmDonation crmDonation) throws Exception {
VirtuousClient.Gift gift = asGift(crmDonation);
Expand Down Expand Up @@ -441,6 +447,11 @@ public Optional<CrmRecurringDonation> getRecurringDonationById(String id) throws
return Optional.ofNullable(asCrmRecurringDonation(recurringGift));
}

@Override
public Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId) throws Exception {
return Optional.empty(); // not possible without the contactId
}

@Override
public Optional<CrmRecurringDonation> getRecurringDonationBySubscriptionId(String subscriptionId, String accountId, String contactId) throws Exception {
VirtuousClient.RecurringGifts recurringGifts = virtuousClient.getRecurringGiftsByContact(Integer.parseInt(contactId));
Expand Down
6 changes: 3 additions & 3 deletions src/main/resources/environment-default.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"metadataKeys": {
"account": ["account", "account_id", "sf_account", "sf_account_id"],
"campaign": ["campaign", "campaign_id", "sf_campaign", "sf_campaign_id"],
"account": ["account", "account_id", "sf_account", "sf_account_id", "crm_account", "crm_account_id"],
"campaign": ["campaign", "campaign_id", "sf_campaign", "sf_campaign_id", "crm_campaign", "crm_campaign_id"],
"fund": ["fund"],
"contact": ["contact", "contact_id", "sf_contact", "sf_contact_id"],
"contact": ["contact", "contact_id", "sf_contact", "sf_contact_id", "crm_contact", "crm_contact_id"],
"recordType": ["sf_opp_record_type", "sf_opp_record_type_id"]
},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public void coreSubscription() throws Exception {
assertEquals(customer.getEmail(), contact.email);
assertEquals(customer.getPhone(), contact.mobilePhone);

Optional<CrmRecurringDonation> _rd = hsCrmService.getRecurringDonationBySubscriptionId(subscription.getId(), null, null);
Optional<CrmRecurringDonation> _rd = hsCrmService.getRecurringDonationBySubscriptionId(subscription.getId());
assertTrue(_rd.isPresent());
CrmRecurringDonation rd = _rd.get();
Deal rdDeal = (Deal) rd.crmRawObject;
Expand Down
60 changes: 60 additions & 0 deletions src/test/java/com/impactupgrade/nucleus/it/StripeToSfdcIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@

import com.impactupgrade.nucleus.App;
import com.impactupgrade.nucleus.client.SfdcClient;
import com.impactupgrade.nucleus.client.StripeClient;
import com.impactupgrade.nucleus.it.util.StripeUtil;
import com.impactupgrade.nucleus.model.ContactSearch;
import com.sforce.soap.partner.sobject.SObject;
import com.stripe.model.Charge;
import com.stripe.model.Customer;
import com.stripe.model.PaymentIntent;
import com.stripe.model.Subscription;
import com.stripe.param.CustomerCreateParams;
import com.stripe.param.PlanCreateParams;
import org.apache.commons.lang3.RandomStringUtils;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -436,4 +438,62 @@ public void preserveData() throws Exception {
// only delete if the test passed -- keep failures in SFDC for analysis
clearSfdc(customer.getName());
}

/**
* In Stripe, if a Customer only has a name and no email, no phone, no street, and no street on the payment method,
* Nucleus was creating duplicate Contacts every time in the CRM. That was expected -- nothing to match against.
* However, one improvement:
* As a last resort, attempt to look up existing donations using the donor's customer or subscription. If
* donations are found, retrieve the contact/account from the latest.
*/
@Test
public void stripeCustomerWithNoDetailsPreventDuplicateContacts() throws Exception {
StripeClient stripeClient = env.stripeClient();
SfdcClient sfdcClient = env.sfdcClient();

// name only, no email, no phone, no address
String randomFirstName = RandomStringUtils.randomAlphabetic(8);
String randomLastName = RandomStringUtils.randomAlphabetic(8);
CustomerCreateParams.Builder customerBuilder = stripeClient.defaultCustomerBuilder(
randomFirstName + " " + randomLastName,
null,
"tok_visa"
);
Customer customer = stripeClient.createCustomer(customerBuilder);

Charge charge1 = StripeUtil.createCharge(customer, env);
String json1 = StripeUtil.createEventJson("charge.succeeded", charge1.getRawJsonObject(), charge1.getCreated());
Response response1 = target("/api/stripe/webhook").request().post(Entity.json(json1));
assertEquals(Response.Status.OK.getStatusCode(), response1.getStatus());
Thread.sleep(10000); // wait to allow processing to finish and metadata to be written back
Charge charge2 = StripeUtil.createCharge(customer, env);
String json2 = StripeUtil.createEventJson("charge.succeeded", charge2.getRawJsonObject(), charge2.getCreated());
Response response2 = target("/api/stripe/webhook").request().post(Entity.json(json2));
assertEquals(Response.Status.OK.getStatusCode(), response2.getStatus());

// verify ContactService -> SfdcCrmService
List<SObject> contacts = sfdcClient.getContactsByNames(List.of(randomFirstName + " " + randomLastName));
assertEquals(1, contacts.size()); // original bug
SObject contact = contacts.get(0);
String accountId = contact.getField("AccountId").toString();
Optional<SObject> accountO = sfdcClient.getAccountById(accountId);
assertTrue(accountO.isPresent());
SObject account = accountO.get();
assertEquals(customer.getName(), account.getField("Name"));
assertEquals(randomFirstName, contact.getField("FirstName"));
assertEquals(randomLastName, contact.getField("LastName"));

// verify DonationService -> SfdcCrmService
List<SObject> opps = sfdcClient.getDonationsByAccountId(accountId);
assertEquals(2, opps.size()); // original bug (1 opp per 2 dup contacts)
SObject opp1 = opps.get(0);
assertEquals("Stripe", opp1.getField("Payment_Gateway_Name__c"));
assertEquals(customer.getId(), opp1.getField("Payment_Gateway_Customer_Id__c"));
SObject opp2 = opps.get(1);
assertEquals("Stripe", opp2.getField("Payment_Gateway_Name__c"));
assertEquals(customer.getId(), opp2.getField("Payment_Gateway_Customer_Id__c"));

// only delete if the test passed -- keep failures in SFDC for analysis
clearSfdc(customer.getName());
}
}

0 comments on commit 7bc4a21

Please sign in to comment.