Problem/Motivation
When no Salesforce ID is set, Upsert is required on push, but in some cases this is undesired.
We have a case where a Lead is converted to a Contact and then the Contact subsequently had their email address changed, in which case we can prevent the push with SalesforcePushAllowedEvent but actually we may want the push to still occur to create a new Lead in this case rather than Upsert into a converted lead (which would result in error code "CANNOT_UPDATE_CONVERTED_LEAD" from Salesforce). Yet the below code does not allow us to skip the upsert and instead go the 'create' route.
MappedObject::push contains:
if ($this->sfid() && !$mapping->alwaysUpsert()) {
$action = 'update';
$result = $this->client()->objectUpdate(
$mapping->getSalesforceObjectType(),
$this->sfid(),
$params->getParams()
);
}
elseif ($mapping->hasKey()) {
$action = 'upsert';
$result = $this->client()->objectUpsert(
$mapping->getSalesforceObjectType(),
$mapping->getKeyField(),
$mapping->getKeyValue($drupal_entity),
$params->getParams()
);
}
else {
$action = 'create';
$result = $this->client()->objectCreate(
$mapping->getSalesforceObjectType(),
$params->getParams()
);
}
Steps to reproduce
- Have a mapping for Lead <> Drupal User with upsert key as 'Email'
- Have a mapping for Contact <> Drupal User
- Register a new user with e.g. me@example.com
- Convert a Lead in SF, change its email address to me2@example.com
- Sync down Contact <> Drupal User (assuming Lead to Contact conversion is handled in your code)
- Register a new user with me@example.com, ie, the same email address of the previously converted Lead
- New Mapping is created for (6) yet the Upsert behaviour will kick in on upsert key. But the Lead with that email is already converted. We instead want a new Lead.
Proposed resolution
Add a new event
Remaining tasks
MR
User interface changes
N/A
API changes
New SalesforcePushUpsertAllowedEvent
Data model changes
N/A
Issue fork salesforce-3529230
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
scott_euser commentedAdded test coverage for the event as well now, ready for review.
Comment #4
scott_euser commentedComment #5
aaronbaumanThank you for the detailed description and the MR.
This is an interesting idea, I think we can make something work here.
I don't like having these 2 different events one after another - one event should suffice. And I'd like to see it be a little less single-purpose.
Proposed changes:
1. Build this logic into existing PushParams event, rather than adding a new event
2. Allow subscriber to override push operation and choose any of update, upsert, or create
So maybe that looks like a new method on PushParams, which can be set by a subscriber to override whatever operation would have been selected in MappedObject::push
Does this make sense? I think it would solve your issue, as well as making this more useful in more scenarios.
Comment #6
scott_euser commentedSure sounds good, thanks for the quick review!
Comment #7
scott_euser commentedOkay I believe this is now what you described, I moved the choice of action to before the push params event, then pass it as a new argument with set/get methods. Only odd thing is that push params event is also called in PullBase hence the default value being needed here as changing the action is only appropriate to the MappedObject context.
Comment #8
aaronbaumanLooks good to me.
Thanks for jumping on that feedback so quickly.
I'll set to RTBC and get it into the next release.
Probably the SalesforcePushParamsEvent in PullBase should be made into its own class.
Seems like it was shoehorned in there, and is not exactly appropriate for the use case.
That's outside the scope of this issue though.
Comment #9
scott_euser commentedYep makes sense to keep that separate
Thanks!
Comment #11
aaronbaumanMerged, thanks for your work