Skip to content

Commit

Permalink
🔃 [Magento Community Engineering] Community Contributions - GraphQL d…
Browse files Browse the repository at this point in the history
…aily delivery

Accepted Community Pull Requests:
 - magento/graphql-ce#1013: magento graphql-ce#970  Cannot return several errors for one GraphQL request (by @lenaorobei)
 - magento/graphql-ce#1012: magento/graphql-ce#: Editorial. Fix CustomerOrder.increment_id description (by @atwixfirster)
 - magento/graphql-ce#996: magento/graphql-ce#977: [Test coverage] Cover exceptions in AssignShippingAddressToCart, AssignBillingAddressToCart (by @atwixfirster)
 - magento/graphql-ce#958: graphQl-914: [Customer] Improve consistency of country field in custo… (by @kisroman)


Fixed GitHub Issues:
 - #970: Product reviews list template paging issue. (reported by @tzyganu) has been fixed in magento/graphql-ce#1013 by @lenaorobei in 2.3-develop branch
   Related commits:
     1. a18b0e1
     2. 39042ac

 - #977: Recurring Install Issue (reported by @mattisbusycom) has been fixed in magento/graphql-ce#996 by @atwixfirster in 2.3-develop branch
   Related commits:
     1. 90557f9
     2. 5bf67e5
     3. f49fe71
  • Loading branch information
magento-engcom-team authored Oct 23, 2019
2 parents 4003975 + 2ed4caa commit cc599e5
Show file tree
Hide file tree
Showing 13 changed files with 311 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ public function __construct(
*/
public function execute(int $customerId, array $data): AddressInterface
{
// It is needed because AddressInterface has country_id field.
if (isset($data['country_code'])) {
$data['country_id'] = $data['country_code'];
}
$this->validateData($data);

/** @var AddressInterface $address */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ public function execute(AddressInterface $address): array

$addressData['customer_id'] = null;

if (isset($addressData['country_id'])) {
$addressData['country_code'] = $addressData['country_id'];
}

return $addressData;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ public function __construct(
*/
public function execute(AddressInterface $address, array $data): void
{
if (isset($data['country_code'])) {
$data['country_id'] = $data['country_code'];
}
$this->validateData($data);

$filteredData = array_diff_key($data, array_flip($this->restrictedKeys));
Expand Down
6 changes: 4 additions & 2 deletions app/code/Magento/CustomerGraphQl/etc/schema.graphqls
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ input CustomerAddressInput {
city: String @doc(description: "The city or town")
region: CustomerAddressRegionInput @doc(description: "An object containing the region name, region code, and region ID")
postcode: String @doc(description: "The customer's ZIP or postal code")
country_id: CountryCodeEnum @doc(description: "The customer's country")
country_id: CountryCodeEnum @doc(description: "Deprecated: use `country_code` instead.")
country_code: CountryCodeEnum @doc(description: "The customer's country")
default_shipping: Boolean @doc(description: "Indicates whether the address is the default shipping address")
default_billing: Boolean @doc(description: "Indicates whether the address is the default billing address")
fax: String @doc(description: "The fax number")
Expand Down Expand Up @@ -102,7 +103,8 @@ type CustomerAddress @doc(description: "CustomerAddress contains detailed inform
customer_id: Int @doc(description: "The customer ID") @deprecated(reason: "customer_id is not needed as part of CustomerAddress, address ID (id) is unique identifier for the addresses.")
region: CustomerAddressRegion @doc(description: "An object containing the region name, region code, and region ID")
region_id: Int @deprecated(reason: "Region ID is excessive on storefront and region code should suffice for all scenarios")
country_id: String @doc(description: "The customer's country")
country_id: String @doc(description: "The customer's country") @deprecated(reason: "Use `country_code` instead.")
country_code: CountryCodeEnum @doc(description: "The customer's country")
street: [String] @doc(description: "An array of strings that define the street number and name")
company: String @doc(description: "The customer's company")
telephone: String @doc(description: "The telephone number")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace Magento\QuoteGraphQl\Model\Cart;

use Magento\Framework\Exception\LocalizedException;
use Magento\Framework\Exception\InputException;
use Magento\Framework\Exception\NoSuchEntityException;
use Magento\Framework\GraphQl\Exception\GraphQlInputException;
use Magento\Framework\GraphQl\Exception\GraphQlNoSuchEntityException;
Expand Down Expand Up @@ -52,7 +52,7 @@ public function execute(
$this->billingAddressManagement->assign($cart->getId(), $billingAddress, $useForShipping);
} catch (NoSuchEntityException $e) {
throw new GraphQlNoSuchEntityException(__($e->getMessage()), $e);
} catch (LocalizedException $e) {
} catch (InputException $e) {
throw new GraphQlInputException(__($e->getMessage()), $e);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace Magento\QuoteGraphQl\Model\Cart;

use Magento\Framework\Exception\LocalizedException;
use Magento\Framework\Exception\InputException;
use Magento\Framework\Exception\NoSuchEntityException;
use Magento\Framework\GraphQl\Exception\GraphQlInputException;
use Magento\Framework\GraphQl\Exception\GraphQlNoSuchEntityException;
Expand Down Expand Up @@ -50,7 +50,7 @@ public function execute(
$this->shippingAddressManagement->assign($cart->getId(), $shippingAddress);
} catch (NoSuchEntityException $e) {
throw new GraphQlNoSuchEntityException(__($e->getMessage()), $e);
} catch (LocalizedException $e) {
} catch (InputException $e) {
throw new GraphQlInputException(__($e->getMessage()), $e);
}
}
Expand Down
2 changes: 1 addition & 1 deletion app/code/Magento/SalesGraphQl/etc/schema.graphqls
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ type Query {

type CustomerOrder @doc(description: "Order mapping fields") {
id: Int
increment_id: String @deprecated(reason: "Use the order_number instaed.")
increment_id: String @deprecated(reason: "Use the order_number instead.")
order_number: String! @doc(description: "The order number")
created_at: String
grand_total: Float
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public function testCreateCustomerAddress()
'region_id' => 4,
'region_code' => 'AZ'
],
'country_id' => 'US',
'country_code' => 'US',
'street' => ['Line 1 Street', 'Line 2'],
'company' => 'Company name',
'telephone' => '123456789',
Expand All @@ -75,7 +75,7 @@ public function testCreateCustomerAddress()
region_id: {$newAddress['region']['region_id']}
region_code: "{$newAddress['region']['region_code']}"
}
country_id: {$newAddress['country_id']}
country_code: {$newAddress['country_code']}
street: ["{$newAddress['street'][0]}","{$newAddress['street'][1]}"]
company: "{$newAddress['company']}"
telephone: "{$newAddress['telephone']}"
Expand All @@ -98,7 +98,7 @@ public function testCreateCustomerAddress()
region_id
region_code
}
country_id
country_code
street
company
telephone
Expand Down Expand Up @@ -133,6 +133,75 @@ public function testCreateCustomerAddress()
$this->assertCustomerAddressesFields($address, $newAddress);
}

/**
* Test case for deprecated `country_id` field.
*
* @magentoApiDataFixture Magento/Customer/_files/customer_without_addresses.php
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/
public function testCreateCustomerAddressWithCountryId()
{
$newAddress = [
'region' => [
'region' => 'Arizona',
'region_id' => 4,
'region_code' => 'AZ'
],
'country_id' => 'US',
'street' => ['Line 1 Street', 'Line 2'],
'company' => 'Company name',
'telephone' => '123456789',
'fax' => '123123123',
'postcode' => '7777',
'city' => 'City Name',
'firstname' => 'Adam',
'lastname' => 'Phillis',
'middlename' => 'A',
'prefix' => 'Mr.',
'suffix' => 'Jr.',
'vat_id' => '1',
'default_shipping' => true,
'default_billing' => false
];

$mutation
= <<<MUTATION
mutation {
createCustomerAddress(input: {
region: {
region: "{$newAddress['region']['region']}"
region_id: {$newAddress['region']['region_id']}
region_code: "{$newAddress['region']['region_code']}"
}
country_id: {$newAddress['country_id']}
street: ["{$newAddress['street'][0]}","{$newAddress['street'][1]}"]
company: "{$newAddress['company']}"
telephone: "{$newAddress['telephone']}"
fax: "{$newAddress['fax']}"
postcode: "{$newAddress['postcode']}"
city: "{$newAddress['city']}"
firstname: "{$newAddress['firstname']}"
lastname: "{$newAddress['lastname']}"
middlename: "{$newAddress['middlename']}"
prefix: "{$newAddress['prefix']}"
suffix: "{$newAddress['suffix']}"
vat_id: "{$newAddress['vat_id']}"
default_shipping: true
default_billing: false
}) {
country_id
}
}
MUTATION;

$userName = 'customer@example.com';
$password = 'password';

$response = $this->graphQlMutation($mutation, [], '', $this->getCustomerAuthHeaders($userName, $password));
$this->assertArrayHasKey('createCustomerAddress', $response);
$this->assertEquals($newAddress['country_id'], $response['createCustomerAddress']['country_id']);
}

/**
* @expectedException Exception
* @expectedExceptionMessage The current customer isn't authorized.
Expand All @@ -153,7 +222,7 @@ public function testCreateCustomerAddressIfUserIsNotAuthorized()
region: {
region_id: 1
}
country_id: US
country_code: US
postcode: "9999"
default_shipping: true
default_billing: false
Expand Down Expand Up @@ -182,7 +251,7 @@ public function testCreateCustomerAddressWithMissingAttribute()
region: {
region_id: 1
}
country_id: US
country_code: US
street: ["Line 1 Street","Line 2"]
company: "Company name"
telephone: "123456789"
Expand Down Expand Up @@ -235,7 +304,7 @@ public function testCreateCustomerAddressWithRedundantStreetLine()
'region_id' => 4,
'region_code' => 'AZ'
],
'country_id' => 'US',
'country_code' => 'US',
'street' => ['Line 1 Street', 'Line 2', 'Line 3'],
'company' => 'Company name',
'telephone' => '123456789',
Expand All @@ -261,7 +330,7 @@ public function testCreateCustomerAddressWithRedundantStreetLine()
region_id: {$newAddress['region']['region_id']}
region_code: "{$newAddress['region']['region_code']}"
}
country_id: {$newAddress['country_id']}
country_code: {$newAddress['country_code']}
street: ["{$newAddress['street'][0]}","{$newAddress['street'][1]}","{$newAddress['street'][2]}"]
company: "{$newAddress['company']}"
telephone: "{$newAddress['telephone']}"
Expand Down Expand Up @@ -334,12 +403,15 @@ public function invalidInputDataProvider()
*
* @param AddressInterface $address
* @param array $actualResponse
* @param string $countryFieldName
*/
private function assertCustomerAddressesFields(AddressInterface $address, array $actualResponse): void
{
private function assertCustomerAddressesFields(
AddressInterface $address,
array $actualResponse
): void {
/** @var $addresses */
$assertionMap = [
['response_field' => 'country_id', 'expected_value' => $address->getCountryId()],
['response_field' => 'country_code', 'expected_value' => $address->getCountryId()],
['response_field' => 'street', 'expected_value' => $address->getStreet()],
['response_field' => 'company', 'expected_value' => $address->getCompany()],
['response_field' => 'telephone', 'expected_value' => $address->getTelephone()],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,56 @@ public function testUpdateCustomerAddress()
$this->assertCustomerAddressesFields($address, $updateAddress);
}

/**
* Test case for deprecated `country_id` field.
*
* @magentoApiDataFixture Magento/Customer/_files/customer.php
* @magentoApiDataFixture Magento/Customer/_files/customer_address.php
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/
public function testUpdateCustomerAddressWithCountryId()
{
$userName = 'customer@example.com';
$password = 'password';
$addressId = 1;

$updateAddress = $this->getAddressData();

$mutation = $mutation
= <<<MUTATION
mutation {
updateCustomerAddress(id: {$addressId}, input: {
region: {
region: "{$updateAddress['region']['region']}"
region_id: {$updateAddress['region']['region_id']}
region_code: "{$updateAddress['region']['region_code']}"
}
country_id: {$updateAddress['country_code']}
street: ["{$updateAddress['street'][0]}","{$updateAddress['street'][1]}"]
company: "{$updateAddress['company']}"
telephone: "{$updateAddress['telephone']}"
fax: "{$updateAddress['fax']}"
postcode: "{$updateAddress['postcode']}"
city: "{$updateAddress['city']}"
firstname: "{$updateAddress['firstname']}"
lastname: "{$updateAddress['lastname']}"
middlename: "{$updateAddress['middlename']}"
prefix: "{$updateAddress['prefix']}"
suffix: "{$updateAddress['suffix']}"
vat_id: "{$updateAddress['vat_id']}"
default_shipping: true
default_billing: true
}) {
country_id
}
}
MUTATION;

$response = $this->graphQlMutation($mutation, [], '', $this->getCustomerAuthHeaders($userName, $password));
$this->assertArrayHasKey('updateCustomerAddress', $response);
$this->assertEquals($updateAddress['country_code'], $response['updateCustomerAddress']['country_id']);
}

/**
* @expectedException Exception
* @expectedExceptionMessage The current customer isn't authorized.
Expand Down Expand Up @@ -131,12 +181,15 @@ public function testUpdateCustomerAddressWithMissingAttribute()
*
* @param AddressInterface $address
* @param array $actualResponse
* @param string $countryFieldName
*/
private function assertCustomerAddressesFields(AddressInterface $address, $actualResponse): void
{
private function assertCustomerAddressesFields(
AddressInterface $address,
$actualResponse
): void {
/** @var $addresses */
$assertionMap = [
['response_field' => 'country_id', 'expected_value' => $address->getCountryId()],
['response_field' => 'country_code', 'expected_value' => $address->getCountryId()],
['response_field' => 'street', 'expected_value' => $address->getStreet()],
['response_field' => 'company', 'expected_value' => $address->getCompany()],
['response_field' => 'telephone', 'expected_value' => $address->getTelephone()],
Expand Down Expand Up @@ -187,7 +240,7 @@ public function testUpdateCustomerAddressWithMissingId()
region_id: {$updateAddress['region']['region_id']}
region_code: "{$updateAddress['region']['region_code']}"
}
country_id: {$updateAddress['country_id']}
country_code: {$updateAddress['country_code']}
street: ["{$updateAddress['street'][0]}","{$updateAddress['street'][1]}"]
company: "{$updateAddress['company']}"
telephone: "{$updateAddress['telephone']}"
Expand Down Expand Up @@ -243,7 +296,7 @@ public function testUpdateCustomerAddressWithInvalidIdType()
region_id: {$updateAddress['region']['region_id']}
region_code: "{$updateAddress['region']['region_code']}"
}
country_id: {$updateAddress['country_id']}
country_code: {$updateAddress['country_code']}
street: ["{$updateAddress['street'][0]}","{$updateAddress['street'][1]}"]
company: "{$updateAddress['company']}"
telephone: "{$updateAddress['telephone']}"
Expand Down Expand Up @@ -382,11 +435,11 @@ private function getAddressData(): array
{
return [
'region' => [
'region' => 'Alaska',
'region_id' => 2,
'region_code' => 'AK'
'region' => 'Alberta',
'region_id' => 66,
'region_code' => 'AB'
],
'country_id' => 'US',
'country_code' => 'CA',
'street' => ['Line 1 Street', 'Line 2'],
'company' => 'Company Name',
'telephone' => '123456789',
Expand Down Expand Up @@ -423,7 +476,7 @@ private function getMutation(int $addressId): string
region_id: {$updateAddress['region']['region_id']}
region_code: "{$updateAddress['region']['region_code']}"
}
country_id: {$updateAddress['country_id']}
country_code: {$updateAddress['country_code']}
street: ["{$updateAddress['street'][0]}","{$updateAddress['street'][1]}"]
company: "{$updateAddress['company']}"
telephone: "{$updateAddress['telephone']}"
Expand All @@ -446,7 +499,7 @@ private function getMutation(int $addressId): string
region_id
region_code
}
country_id
country_code
street
company
telephone
Expand Down
Loading

0 comments on commit cc599e5

Please sign in to comment.