Changing an upsert key field value in Drupal creates a new/duplicate object on SF

Problem/Motivation

I found a difference in the upsert logic between 7.x-3.x and 8.x-3.x, and I think the 7.x-3.x logic is better. In 7.x-3.x I can change the value of an upsert key field in Drupal and see that change reflected in the related Salesforce object. When I do this in 8.x-3.x I get a new/duplicate object up on Salesforce and the original Salesforce object is disconnected from Drupal. It comes down to this:

In 7.x-3.x upsert is only attempted when our Drupal entity isn't already linked/mapped to a Salesforce object. Drupal entities that are already mapped are updated instead of upserted. This allows updates to the Drupal entity's upsert key field value to flow through to the Salesforce object.

In 8.x-3.x upsert is always used if an upsert key field is defined, period. This means when a mapped Drupal entity's upsert key field value is changed, a new object is created up on Salesforce. Following this, the mapped object in Drupal is updated to hold this new Salesforce ID, so our existing Drupal entity is re-mapped to this newly-created Salesforce object and the originally-mapped salesforce object is essentially disconnected from Drupal. Messy to say the least.

Proposed resolution

Change the push logic back to what we had in 7.x-3.x: If a mapped object with a Salesforce ID already exists for the entity, perform an objectUpdate(). If not, and an upsert key is defined, perform an objectUpsert(). Failing all of that, perform an objectCreate().

In code I'm essentially proposing we change this (modules/salesforce_mapping/src/Entity/MappedObject.php):

if ($mapping->hasKey()) {
  $action = 'upsert';
  $result = $this->client()->objectUpsert(
    $mapping->getSalesforceObjectType(),
    $mapping->getKeyField(),
    $mapping->getKeyValue($drupal_entity),
    $params->getParams()
  );
}
elseif ($this->sfid()) {
  $action = 'update';
  $result = $this->client()->objectUpdate(
    $mapping->getSalesforceObjectType(),
    $this->sfid(),
    $params->getParams()
  );
}
else {
  $action = 'create';
  $result = $this->client()->objectCreate(
    $mapping->getSalesforceObjectType(),
    $params->getParams()
  );
}

To this:

if ($this->sfid()) {
  $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()
  );
}

Remaining tasks

Patch & review.

User interface changes

None.

API changes

None.

Data model changes

None.