[4.x]: Re-saving an address can mutate the first and last name on the address

[a-solaris]

What happened?

Description

If you have an address element with the middle name stuffed in either firstName or lastName for any reason, saving the address element can yield unpredictable results.

This is because of slightly circular logic in NameTrait. For example, if you have an Address element with fullName empty, but firstName and lastName set, on save the fullName field will become populated. Then, if you save the Address instead, the fullName field gets parsed into firstName and lastName fields, losing any middle names, if they were stored in those fields.

For customers that strongly prefer avoiding using the fullName field, this can become problematic when dealing with Addresses in Commerce, especially considering that selecting billing address to be the same as shipping address will duplicate the element, in essence re-saving it and potentially mutating the data already.

Steps to reproduce

In Craft shell

$address = new \craft\elements\Address(['firstName' => 'One Two', 'lastName' => 'Three Four']);
Craft::$app->elements->saveElement($address, false);
echo sprintf('%s + %s = %s', $address->firstName, $address->lastName, $address->fullName);
Craft::$app->elements->saveElement($address, false);
echo sprintf('%s + %s = %s', $address->firstName, $address->lastName, $address->fullName);

Expected behavior

One Two + Three Four = One Two Three Four is printed twice

Actual behavior

One Two + Three Four = One Two Three Four
One + Four = One Two Three Four

is printed instead, as the simple act of saving the Address element again changed the data.

Craft CMS version

4.7.2

Installed plugins and versions

• Commerce (not affecting this bug, but useful for context)

[brandonkelly]

Yeah this is a known issue, but we don’t have a great solution for it. In Craft 4.6 we added the showFirstAndLastNameFields config setting, which you can enable to just go back to explicitly showing “First Name” and “Last Name” fields and avoid all the name parsing logic.

[a-solaris]

Right, but even with that setting on, there are scenarios with issues.

Consider having firstName and lastName fields on a Commerce site. A customer fills it out with "Firstname" and "LastName LastName", respectively. The customer also ticks a "use this as the billing address" checkbox.

NameTrait::prepareNamesForSave() doesn't check that config setting, it just checks the existence of the fullName property. And, if it doesn't exist, it gets populated from the first and last name properties. Which means that on a repeated save, regardless of settings, first name and last name become populated from the fullName property. And the logic in that block always extracts single words as first and last name, thus losing the actual last name.

Now, in Commerce, when choosing an address to be also used as a billing address, the address element gets duplicated. Which means that, in this scenario, the billing address will end up with a different name, which also is likely to be wrong.

The solution might be as simple as respecting the showFirstAndLastNameFields config setting in the NameTrait trait.

[brandonkelly]

This is now resolved for Craft 4.9 and 5.1. Going forward, NameTrait::prepareNamesForSave() will no longer do any processing of the fullName or firstName/lastName values if they are each already set. So the only way to trigger re-parsing of the full name would be to set fullName and unset firstName and lastName. (#14771)

[brandonkelly]

4.9 and 5.1 are out now with that change.