Skip to content

Fix bug 27490: Can't change customer group when placing an admin order #27501

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 13 commits into from
Closed
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Quote\Observer\Backend\Quote\Address;

use Magento\Framework\App\Area;
use Magento\Framework\App\State;
use Magento\Framework\Event\ObserverInterface;
use Magento\Quote\Observer\Frontend\Quote\Address\CollectTotalsObserver as FrontendCollectTotalsObserver;
use Magento\Quote\Observer\Frontend\Quote\Address\VatValidator;

/**
* Handle customer VAT number on collect_totals_before event of quote address.
*
* @SuppressWarnings(PHPMD.CookieAndSessionMisuse)
* @SuppressWarnings(PHPMD.TooManyFields)
*/
class CollectTotalsObserver extends FrontendCollectTotalsObserver implements ObserverInterface
{
/**
* @var State
*/
private $state;

/**
* Initialize dependencies.
*
* @param \Magento\Customer\Helper\Address $customerAddressHelper
* @param \Magento\Customer\Model\Vat $customerVat
* @param VatValidator $vatValidator
* @param \Magento\Customer\Api\Data\CustomerInterfaceFactory $customerDataFactory
* @param \Magento\Customer\Api\GroupManagementInterface $groupManagement
* @param \Magento\Customer\Api\AddressRepositoryInterface $addressRepository
* @param \Magento\Customer\Model\Session $customerSession
* @param State $state
*/
public function __construct(
\Magento\Customer\Helper\Address $customerAddressHelper,
\Magento\Customer\Model\Vat $customerVat,
VatValidator $vatValidator,
\Magento\Customer\Api\Data\CustomerInterfaceFactory $customerDataFactory,
\Magento\Customer\Api\GroupManagementInterface $groupManagement,
\Magento\Customer\Api\AddressRepositoryInterface $addressRepository,
\Magento\Customer\Model\Session $customerSession,
State $state
) {
parent::__construct(
$customerAddressHelper,
$customerVat,
$vatValidator,
$customerDataFactory,
$groupManagement,
$addressRepository,
$customerSession
);
$this->state = $state;
}

/**
* Conditions to change customer group
*
* @param int|null $groupId
* @return bool
*/
protected function assignCustomerGroupConditions($groupId)
{
if ($groupId !== null
&& !(
$this->state->getAreaCode() == Area::AREA_ADMINHTML
&& $groupId == $this->groupManagement->getNotLoggedInGroup()->getId()
Comment on lines +73 to +74
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use strict comparison operator

)) {
return true;
}

return false;
Comment on lines +71 to +79
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put those long difficult conditions to separate private methods with proper naming and shorten this function to just return statement:
return $groupId !== null && !($this->isAdminArea() && $this->isNotLoggedInGroup($groupId));
At least like this.

You can think even more to make this code more readable.

}
}
13 changes: 12 additions & 1 deletion app/code/Magento/Quote/Observer/Frontend/Quote/Address/CollectTotalsObserver.php
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,23 @@ public function execute(\Magento\Framework\Event\Observer $observer)
);
}

if ($groupId !== null) {
if ($this->assignCustomerGroupConditions($groupId)) {
$address->setPrevQuoteCustomerGroupId($quote->getCustomerGroupId());
$quote->setCustomerGroupId($groupId);
$this->customerSession->setCustomerGroupId($groupId);
$customer->setGroupId($groupId);
$quote->setCustomer($customer);
}
}

/**
* Conditions to change customer group
*
* @param int|null $groupId
* @return bool
*/
protected function assignCustomerGroupConditions($groupId)
{
return $groupId !== null ? true : false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need it here? Why don't you just write return $groupId !== null?

}
}
Loading