[4.x]: BaseRelationField::normalizeValue $value can be instance of Collection

[Anubarak]

What happened?

Description

in addition to #11667, #11670, and #13421 BaseRelationField::normalizeValue $value can be instance of Collection instead of an Element Query as well.

When applying drafts in the CP and a relation field is eager loaded due to performance reasons, your craft\services\Elements::duplicateElement does the following:

// Clone any field values that are objects (without affecting the dirty fields)
$dirtyFields = $mainClone->getDirtyFields();
foreach ($mainClone->getFieldValues() as $handle => $value) {
    if (is_object($value) && (!interface_exists(UnitEnum::class) || !$value instanceof UnitEnum)) {
        $mainClone->setFieldValue($handle, clone $value);
    }
}
$mainClone->setDirtyFields($dirtyFields, false);

clone $value is an instance of craft\elements\ElementCollection however your BaseRelationField will ignore that

foreach ($mainClone->getFieldValues() as $handle => $value) {
    if (is_object($value) && (!interface_exists(UnitEnum::class) || !$value instanceof UnitEnum)) {
        // -> entry 1
        echo $element->getFieldValue('synchronousSpeed')->one()->title;
        $mainClone->setFieldValue($handle, clone $value);
        // field value not set... mainClone has still the old entry
        echo $mainClone->getFieldValue('synchronousSpeed')->one()->title;
    }
}

I was able to fix this by adding a type check

/**
 * @inheritdoc
 */
public function normalizeValue(mixed $value, ?ElementInterface $element = null): mixed
{
    if ($value instanceof ElementQueryInterface) {
        return $value;
    }

    // check if it is a collection -> grab the element ids
    if($value instanceof \craft\elements\ElementCollection){
        $value = $value->map(fn(ElementInterface $item) => $item->id)->toArray();
    }

Steps to reproduce

1. add an event to eager load certain relation fields due to performance reasons
2. apply a draft for this element
3. dirty fields where the value is in instance of Collection due to eager loading are not applied

Expected behavior

• dirty fields should be applied even if the field value is eager loaded

Actual behavior

• eager loaded fields are ignored

Craft CMS version

4.5.14

PHP version

8.2.0

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

No response

[brandonkelly]

If it’s already a collection, I’m pretty sure this would have worked too:

if ($value instanceof ElementQueryInterface || $value instanceof Collection) {
    return $value;
}

Can you verify that‘s going to work for you?

[Anubarak]

doh
yeah that works as well - I can't believe I missed that.

[brandonkelly]

Thanks for confirming! Craft 4.5.15 and 4.6.0 are out with that change.

[boboldehampsink]

@brandonkelly I'm now seeing this when trying to update to 4.6:

web-1  | Exception 'TypeError' with message 'craft\fields\BaseRelationField::craft\fields\{closure}(): Argument #1 ($element) must be of type craft\base\ElementInterface, int given'
web-1  | 
web-1  | in /app/user/vendor/craftcms/cms/src/fields/BaseRelationField.php:941
web-1  | 
web-1  | Stack trace:
web-1  | #0 [internal function]: craft\fields\BaseRelationField->craft\fields\{closure}(211, 0)
web-1  | #1 /app/user/vendor/illuminate/collections/Arr.php(560): array_map(Object(Closure), Array, Array)
web-1  | #2 /app/user/vendor/illuminate/collections/Collection.php(768): Illuminate\Support\Arr::map(Array, Object(Closure))
web-1  | #3 /app/user/vendor/craftcms/cms/src/fields/BaseRelationField.php(941): Illuminate\Support\Collection->map(Object(Closure))

Could it be related to this fix?

[boboldehampsink]

This was when programatically setting a relation field to a collection. Worked in 4.5.14. Fixed it for 4.6.0 by using toArray on the collection.

[brandonkelly]

@boboldehampsink Setting the value to a collection of integers wouldn’t have actually done anything. The only values that were supported before 4.5.15 are:

• an element query
• an array of IDs (specifically checked with in_array(), which would be false for a collection)
• an empty string (indicating that the field was edited, but set to no relations)

Anything else would cause normalizeValue() to completely discard the passed-in value, and return an element query that fetches relations from the database.

[boboldehampsink]

Check!