Skip to content
This repository has been archived by the owner on Dec 19, 2019. It is now read-only.

Resolve coupling between objects in \Magento\QuoteGraphQl\Model\Cart\SetBillingAddressOnCart #488

Merged
merged 4 commits into from
Apr 5, 2019
Merged

Resolve coupling between objects in \Magento\QuoteGraphQl\Model\Cart\SetBillingAddressOnCart #488

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b3280d0
magento/graphql-ce:#468 - Resolve coupling between objects in `\Mage…
Mar 16, 2019
799f37e
magento/graphql-ce:#468 - Fix codestyle issue
Mar 21, 2019
ace616f
Merge remote-tracking branch 'origin/2.3-develop' into issue/468
naydav Apr 3, 2019
5b03488
GraphQL-468: Resolve coupling between objects in `\Magento\QuoteGraph…
naydav Apr 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions app/code/Magento/QuoteGraphQl/Model/Cart/CreateQuoteAddressByCustomerAddress.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\QuoteGraphQl\Model\Cart;

use Magento\CustomerGraphQl\Model\Customer\Address\GetCustomerAddress;
use Magento\CustomerGraphQl\Model\Customer\GetCustomer;
use Magento\Framework\GraphQl\Exception\GraphQlAuthenticationException;
use Magento\Framework\GraphQl\Exception\GraphQlAuthorizationException;
use Magento\Framework\GraphQl\Exception\GraphQlInputException;
use Magento\Framework\GraphQl\Exception\GraphQlNoSuchEntityException;
use Magento\Quote\Model\Quote\Address;
use Magento\Framework\GraphQl\Query\Resolver\ContextInterface;

/**
* Creates a quote address based on given context, customer address ID and customer address
*/
class CreateQuoteAddressByCustomerAddress
{
/**
* @var QuoteAddressFactory
*/
private $quoteAddressFactory;

/**
* @var GetCustomer
*/
private $getCustomer;

/**
* @var GetCustomerAddress
*/
private $getCustomerAddress;

/**
* @param QuoteAddressFactory $quoteAddressFactory
* @param GetCustomer $getCustomer
* @param GetCustomerAddress $getCustomerAddress
*/
public function __construct(
QuoteAddressFactory $quoteAddressFactory,
GetCustomer $getCustomer,
GetCustomerAddress $getCustomerAddress
) {
$this->quoteAddressFactory = $quoteAddressFactory;
$this->getCustomer = $getCustomer;
$this->getCustomerAddress = $getCustomerAddress;
}

/**
* @param ContextInterface $context
* @param int|string|null $customerAddressId
* @param array|null $customerAddress
* @return Address
* @throws GraphQlAuthenticationException
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually @throws annotation tags are added only when exception is thrown explicitly in the function. Any other exceptions from parent methods are implicit and not annotated. "Missing @throws tag(s)" can be turned off in PHPStorm Inspections->PHP->PHPDoc

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for mentioning, @TomashKhamlai. I've added requested changes. Thank you!

Copy link
Contributor

@naydav naydav Apr 3, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Usually @throws annotation tags are added only when exception is thrown explicitly in the function

I little disagree with this statement. If I work with some API I will prefer to know which of exception could be thrown without submerging in implementation (does not matter will be thrown in current method or in some child classes)

Generally, it's about readability.

* @throws GraphQlAuthorizationException
* @throws GraphQlInputException
* @throws GraphQlNoSuchEntityException
*/
public function execute(
ContextInterface $context,
$customerAddressId,
$customerAddress
): Address {
if (null === $customerAddressId) {
return $this->quoteAddressFactory->createBasedOnInputData($customerAddress);
}

$customer = $this->getCustomer->execute($context);
$customerAddress = $this->getCustomerAddress->execute((int)$customerAddressId, (int)$customer->getId());

return $this->quoteAddressFactory->createBasedOnCustomerAddress($customerAddress);
}
}
50 changes: 17 additions & 33 deletions app/code/Magento/QuoteGraphQl/Model/Cart/SetBillingAddressOnCart.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@

namespace Magento\QuoteGraphQl\Model\Cart;

use Magento\CustomerGraphQl\Model\Customer\Address\GetCustomerAddress;
use Magento\CustomerGraphQl\Model\Customer\GetCustomer;
use Magento\Framework\GraphQl\Exception\GraphQlAuthenticationException;
use Magento\Framework\GraphQl\Exception\GraphQlAuthorizationException;
use Magento\Framework\GraphQl\Exception\GraphQlInputException;
use Magento\Framework\GraphQl\Exception\GraphQlNoSuchEntityException;
use Magento\Framework\GraphQl\Query\Resolver\ContextInterface;
Expand All @@ -20,41 +20,25 @@
class SetBillingAddressOnCart
{
/**
* @var QuoteAddressFactory
*/
private $quoteAddressFactory;

/**
* @var GetCustomer
*/
private $getCustomer;

/**
* @var GetCustomerAddress
* @var AssignBillingAddressToCart
*/
private $getCustomerAddress;
private $assignBillingAddressToCart;

/**
* @var AssignBillingAddressToCart
* @var CreateQuoteAddressByCustomerAddress
*/
private $assignBillingAddressToCart;
private $createQuoteAddressByCustomerAddress;

/**
* @param QuoteAddressFactory $quoteAddressFactory
* @param GetCustomer $getCustomer
* @param GetCustomerAddress $getCustomerAddress
* @param AssignBillingAddressToCart $assignBillingAddressToCart
* @param CreateQuoteAddressByCustomerAddress $createQuoteAddressByCustomerAddress
*/
public function __construct(
QuoteAddressFactory $quoteAddressFactory,
GetCustomer $getCustomer,
GetCustomerAddress $getCustomerAddress,
AssignBillingAddressToCart $assignBillingAddressToCart
AssignBillingAddressToCart $assignBillingAddressToCart,
CreateQuoteAddressByCustomerAddress $createQuoteAddressByCustomerAddress
) {
$this->quoteAddressFactory = $quoteAddressFactory;
$this->getCustomer = $getCustomer;
$this->getCustomerAddress = $getCustomerAddress;
$this->assignBillingAddressToCart = $assignBillingAddressToCart;
$this->createQuoteAddressByCustomerAddress = $createQuoteAddressByCustomerAddress;
}

/**
Expand All @@ -66,6 +50,8 @@ public function __construct(
* @return void
* @throws GraphQlInputException
* @throws GraphQlNoSuchEntityException
* @throws GraphQlAuthorizationException
* @throws GraphQlAuthenticationException
*/
public function execute(ContextInterface $context, CartInterface $cart, array $billingAddressInput): void
{
Expand Down Expand Up @@ -93,13 +79,11 @@ public function execute(ContextInterface $context, CartInterface $cart, array $b
);
}

if (null === $customerAddressId) {
$billingAddress = $this->quoteAddressFactory->createBasedOnInputData($addressInput);
} else {
$customer = $this->getCustomer->execute($context);
$customerAddress = $this->getCustomerAddress->execute((int)$customerAddressId, (int)$customer->getId());
$billingAddress = $this->quoteAddressFactory->createBasedOnCustomerAddress($customerAddress);
}
$billingAddress = $this->createQuoteAddressByCustomerAddress->execute(
$context,
$customerAddressId,
$addressInput
);

$this->assignBillingAddressToCart->execute($cart, $billingAddress, $useForShipping);
}
Expand Down