🔃 [Magento Community Engineering] Community Contributions - GraphQL d…

…aily delivery

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


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

 - magento/magento2#977: Recurring Install Issue (reported by @mattisbusycom) has been fixed in #996 by @atwixfirster in 2.3-develop branch
   Related commits:
     1. 90557f9
     2. 5bf67e5
     3. f49fe71

app/code/Magento/CustomerGraphQl/Model/Customer/Address/CreateCustomerAddress.php

@@ -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 */

app/code/Magento/CustomerGraphQl/Model/Customer/Address/ExtractCustomerAddressData.php

@@ -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;
     }
 }

app/code/Magento/CustomerGraphQl/Model/Customer/Address/UpdateCustomerAddress.php

@@ -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));

app/code/Magento/CustomerGraphQl/etc/schema.graphqls

@@ -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")
@@ -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")

app/code/Magento/QuoteGraphQl/Model/Cart/AssignBillingAddressToCart.php

@@ -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;
@@ -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);
         }
     }

app/code/Magento/QuoteGraphQl/Model/Cart/AssignShippingAddressToCart.php

@@ -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;
@@ -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);
         }
     }

app/code/Magento/SalesGraphQl/etc/schema.graphqls

@@ -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

dev/tests/api-functional/testsuite/Magento/GraphQl/Customer/CreateCustomerAddressTest.php

@@ -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',
@@ -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']}"
@@ -98,7 +98,7 @@ public function testCreateCustomerAddress()
       region_id
       region_code
     }
-    country_id
+    country_code
     street
     company
     telephone
@@ -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.
@@ -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
@@ -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"
@@ -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',
@@ -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']}"
@@ -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()],

dev/tests/api-functional/testsuite/Magento/GraphQl/Customer/UpdateCustomerAddressTest.php

@@ -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.
@@ -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()],
@@ -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']}"
@@ -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']}"
@@ -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',
@@ -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']}"
@@ -446,7 +499,7 @@ private function getMutation(int $addressId): string
       region_id
       region_code
     }
-    country_id
+    country_code
     street
     company
     telephone