Skip to content
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

StoreBasedCartContext: Wrong Address Type if Adress Identifier is available #1627

Open
solverat opened this issue May 6, 2021 · 5 comments
Labels

Comments

@solverat
Copy link
Contributor

solverat commented May 6, 2021

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no

$defaultAddress = $customer->getDefaultAddress();
if (null !== $defaultAddress) {
$cart->setShippingAddress($defaultAddress);
$cart->setInvoiceAddress($defaultAddress);
}

Here the StoreBasedCartContext class assumes, that the default address always fits as shipping / invoice address, which is not always true. In some setups we need to follow EU laws (invoice => allow all countries, shipping => allow specific countries) and therefor we don't have "simple" addresses.

In this case, the default address identifier could be either shipping or invoice but never null. This needs to be checked first!

@solverat solverat added the bug label May 6, 2021
@solverat
Copy link
Contributor Author

solverat commented May 7, 2021

Same goes or CartBlamerListener:

if (null === $cart->getShippingAddress()) {
$cart->setShippingAddress($user->getDefaultAddress());
}
if (null === $cart->getInvoiceAddress()) {
$cart->setInvoiceAddress($user->getDefaultAddress());
}

@dpfaffenbauer
Copy link
Member

any idea how to solve this?

@solverat
Copy link
Contributor Author

solverat commented May 7, 2021

Overwrites in my project:

# CartBlameListener.php

private function blame(CustomerInterface $user)
{
    /**
     * @todo remove this class after #1627 has been fixed
     * @see  https://github.com/coreshop/CoreShop/issues/1627
     */

    $cart = $this->getCart();

    if ($cart === null) {
        return;
    }

    $cart->setCustomer($user);

    $defaultAddress = $user->getDefaultAddress();
    $shippingAddress = $user->getDefaultAddress();
    $invoiceAddress = $user->getDefaultAddress();

    if ($defaultAddress instanceof AddressInterface) {
        if ($defaultAddress->getAddressIdentifier() instanceof AddressIdentifierInterface) {
            if ($defaultAddress->getAddressIdentifier()->getName() === 'invoice') {
                $shippingAddress = null;
                $invoiceAddress = $user->getDefaultAddress();
            } elseif ($defaultAddress->getAddressIdentifier()->getName() === 'shipping') {
                $shippingAddress = $user->getDefaultAddress();
                $invoiceAddress = null;
            }
        }
    }

    if ($cart->getShippingAddress() === null) {
        $cart->setShippingAddress($shippingAddress);
    }

    if ($cart->getInvoiceAddress() === null) {
        $cart->setInvoiceAddress($invoiceAddress);
    }

    $this->cartProcessor->process($cart);

    if ($cart->getId()) {
        $this->cartManager->persistCart($cart);

        return;
    }

    $this->cartProcessor->process($cart);
}
# StoreBasedCartContext.php

public function getCart()
{
    /**
     * @todo remove this class after #1627 has been fixed
     * @see https://github.com/coreshop/CoreShop/issues/1627
     */

    $cart = $this->cartContext->getCart();

    if (!$cart instanceof CartInterface) {
        throw new CartNotFoundException();
    }

    if ($cart->getInvoiceAddress() instanceof AddressInterface) {
        if ($cart->getInvoiceAddress()->getAddressIdentifier() instanceof AddressIdentifierInterface) {
            if ($cart->getInvoiceAddress()->getAddressIdentifier()->getName() !== 'invoice') {
                $cart->setInvoiceAddress(null);
            }
        }
    }

    if ($cart->getShippingAddress() instanceof AddressInterface) {
        if ($cart->getShippingAddress()->getAddressIdentifier() instanceof AddressIdentifierInterface) {
            if ($cart->getShippingAddress()->getAddressIdentifier()->getName() !== 'shipping') {
                $cart->setShippingAddress(null);
            }
        }
    }

    return $cart;
}

@dpfaffenbauer
Copy link
Member

I think we should separate the address allocation into a separate service and call that from the Blamer and the Context, WDYT?

@benwalch
Copy link
Contributor

that would be a indeed a better way, especially if one wants to override this 'default' behaviour of initial cart address assignments.

Btw, the overrides of the cart context turn out to be more difficult than it could be, bc of final class declarations

final class StoreBasedCartContext implements CartContextInterface


and private methods
private function setCustomerAndAddressOnCart(CartInterface $cart, CustomerInterface $customer)

Maybe I should address this in another issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants