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

Resolved issue for Customer address is duplicated after setBillingAddressOnCart GraphQL mutation #27107

Merged
merged 3 commits into from
Mar 26, 2020

Conversation

Aniket10Nov
Copy link
Contributor

@Aniket10Nov Aniket10Nov commented Feb 29, 2020

Description (*)

Resolved issue for Customer address is duplicated after setBillingAddressOnCart GraphQL mutation. If the customer set billing address is the same as the shipping address then it won't save as a new address.

Related Pull Requests

#26884

Fixed Issues (if relevant)

  1. Customer address is duplicated after setBillingAddressOnCart GraphQL mutation. #26884 Customer address is duplicated after setBillingAddressOnCart GraphQL mutation.

Manual testing scenarios (*)

Do all steps via GQL:

  1. Authorize ( generateCustomerToken mutation)
  2. Create cart (customerCart query)
  3. Add product to cart (addSimpleProductsToCart mutation)
  4. Set billing address (setBillingAddressOnCart mutation).
Important: save_in_address_book: true
input:{ 
   cart_id:"cart_id"   
   billing_address:{ 
      address:{ 
      ...
         save_in_address_book:true
      }
      same_as_shipping:true
   }
}
  1. Set shipping method and payment method to the cart.
  2. Place order (placeOrder mutation)

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

@m2-assistant
Copy link

m2-assistant bot commented Feb 29, 2020

Hi @Aniket10Nov. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.4-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

@@ -90,7 +90,7 @@ public function execute(ContextInterface $context, CartInterface $cart, array $b
);
}

$billingAddress = $this->createBillingAddress($context, $customerAddressId, $addressInput);
$billingAddress = $this->createBillingAddress($context, $customerAddressId, $addressInput,$sameAsShipping);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, make sure you preserve the PSR code style and add whitespace before the argument

@rogyar rogyar added the Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests label Mar 4, 2020
@rogyar
Copy link
Contributor

rogyar commented Mar 4, 2020

Hi @Aniket10Nov. According to the new Definition of Done, all new changes should be covered by automated tests. Since we don't have a GraphQl query for checking customer addresses, we are not able to cover this case using an API-functional test.
But we have a possibility to perform a GraphQl request for setting the quote address and then use the repository or resource model to assert customer addresses using an integration test. Please, refer to the following test as an example.

@rogyar rogyar added Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests and removed Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests labels Mar 4, 2020
@ghost ghost unassigned lenaorobei Mar 11, 2020
@ghost
Copy link

ghost commented Mar 11, 2020

@nrkapoor unfortunately, only members of the maintainers team are allowed to assign developers to the pull request

@engcom-Charlie
Copy link
Contributor

Hi @Aniket10Nov it is M2 core issue, please see #22596.

@prabhuram93 prabhuram93 self-assigned this Mar 16, 2020
@prabhuram93
Copy link
Contributor

Hi @Aniket10Nov , @rogyar! Our internal team has started working on this in the scope of MC-31586

@magento-engcom-team magento-engcom-team merged commit 756aedf into magento:2.4-develop Mar 26, 2020
@m2-assistant
Copy link

m2-assistant bot commented Mar 26, 2020

Hi @Aniket10Nov, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@lenaorobei lenaorobei removed the Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests label Mar 26, 2020
@lenaorobei lenaorobei added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Mar 26, 2020
@lenaorobei
Copy link
Contributor

Covered by tests internally.

@nrkapoor nrkapoor requested a review from avattam06 June 10, 2020 23:55
@nrkapoor nrkapoor added this to the 2.4.1 milestone Jun 11, 2020
@sdzhepa sdzhepa mentioned this pull request May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants