Fix customer group extension attributes for lists

[avstudnitz]

Description

The plugin \Magento\Customer\Model\Plugin\GetListCustomerGroupExcludedWebsite always created a new extension object for customer group lists, regardless of if one already exists. So it overwrote other extension attributes. This change checks if an extension object already exists.

Manual testing scenarios (*)

Create your own module, introducing another extension attribute for the customer group entity. In this example, an extension attribute named "discount_percentage" is used.

etc/db_schema.xml:

<?xml version="1.0"?>
<schema xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:noNamespaceSchemaLocation="urn:magento:framework:Setup/Declaration/Schema/etc/schema.xsd">
    <table name="customer_group">
        <column xsi:type="decimal" name="discount_percentage" scale="2" precision="4" unsigned="true"
                nullable="true" default="0" comment="Discount Percentage"/>
    </table>
</schema>

etc/extension_attributes.xml:

<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Api/etc/extension_attributes.xsd">
    <extension_attributes for="Magento\Customer\Api\Data\GroupInterface">
        <attribute code="discount_percentage" type="float" />
    </extension_attributes>
</config>

etc/di.xml:

<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
    <type name="Magento\Framework\Api\ExtensionAttribute\JoinProcessor">
        <plugin name="customer_group_discount_extension_attribute"
                type="Custom\CustomerGroupDiscount\Plugin\ExtensionAttributeJoinProcessorPlugin"/>
    </type>
</config>

Plugin/ExtensionAttributeJoinProcessorPlugin.php:

<?php
declare(strict_types=1);

namespace Custom\CustomerGroupDiscount\Plugin;

use Magento\Customer\Api\Data\GroupExtensionInterfaceFactory;
use Magento\Customer\Api\Data\GroupInterface;
use Magento\Framework\Api\ExtensibleDataInterface;
use Magento\Framework\Api\ExtensionAttribute\JoinProcessor;

class ExtensionAttributeJoinProcessorPlugin
{
    public function __construct(
        private readonly GroupExtensionInterfaceFactory $groupExtensionInterfaceFactory
    ) {
    }

    /**
     * Set "discount_percentage" extension attribute based on raw data from database
     */
    public function afterExtractExtensionAttributes(
        JoinProcessor $subject,
        array $result,
        string $extensibleEntityClass,
        array $data
    ): array {
        if ($extensibleEntityClass != ltrim(GroupInterface::class, '\\')) {
            return $result;
        }
        if (!isset($result[ExtensibleDataInterface::EXTENSION_ATTRIBUTES_KEY])) {
            $extensionAttributes = $this->groupExtensionInterfaceFactory->create();
            $result[ExtensibleDataInterface::EXTENSION_ATTRIBUTES_KEY] = $extensionAttributes;
        }
        $result[ExtensibleDataInterface::EXTENSION_ATTRIBUTES_KEY]->setDiscountPercentage($data['discount_percentage']);

        return $result;
    }
}

Without this PR, the customer group list doesn't contain the extension attribute discount_percentage.

Contribution checklist

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
• All automated tests passed successfully (all builds are green)

[ihor-sviziev]

Hi @avstudnitz,
Your changes look good to me.
Do you feel it's realistic to cover this case with some unit test?

[Den4ik]

getExtensionAttributes comment block contain information that function could return null, but in fact it never be null, see https://github.com/mage-os/mageos-magento2/blob/2.4-develop/lib/internal/Magento/Framework/Api/AbstractExtensibleObject.php#L169-L171
I believe that object creation could be removed.

[ihor-sviziev]

@Den4ik Good catch!
Having that in mind, I would replace this if block to following:

                    if (array_key_exists($customerGroupId, $allExcludedWebsites)) {
                        $excludedWebsites = $allExcludedWebsites[$customerGroupId];

                        // extension attributes are always there
                        /** @var GroupExtensionInterface $customerGroupExtensionAttributes */
                        $customerGroupExtensionAttributes = $customerGroup->getExtensionAttributes();
                        $customerGroupExtensionAttributes->setExcludeWebsiteIds($excludedWebsites);
                    }

Also, it would be great to fix the return type in \Magento\Customer\Api\Data\GroupInterface::getExtensionAttributes and \Magento\Customer\Model\Data\Group::getExtensionAttributes, but I'm not sure if we can do that as a part of mage-os.

[ihor-sviziev]

Additional idea - since the interface declares it can be null, there might be a case when the custom extension can override the value to null for some reason, and this code will start failing, so maybe having this fallback to object creation might be not that bad thing.
So now I prefer keeping the original fix

[Den4ik]

@ihor-sviziev I'm totally agree that interface method declaration should be fixed as weel. Good point for Magento Core PR, but it's backward incompatible changes that will produce long acceptance process by the Core Team.

Additional idea - since the interface declares it can be null, there might be a case when the custom extension can override the value to null for some reason, and this code will start failing, so maybe having this fallback to object creation might be not that bad thing.

I have seen many modules but don't faced with situation where it replace extension attributes to null. So I think we can stay with $customerGroupExtensionAttributes = $customerGroup->getExtensionAttributes();