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

Conversation

@fooman
Copy link
Contributor

@fooman fooman commented Sep 4, 2019

Description (*)

A lot of behaviour in the checkout in particular depends on if the quote is virtual or not, ie if shipping methods need to be offered or if a shipping address should be entered. This PR makes the is_virtual property of the quote available.

Fixed Issues (if relevant)

  1. couldn't find any

Manual testing scenarios (*)

  1. Add a virtual product to the cart
  2. query graphql with something like this
{
  cart(cart_id:"$maskedQuoteId") {    
    is_virtual
  }
}

the result should be a boolean true.
3. Add a simple product to the cart
4. Run the same graphql query again - the result should now be false.

Questions or comments

I did duplicate the _security tests here as well. I believe they could be cut if preferred as the boolean provided by this resolver by itself is not sensitive.

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)

@fooman
Copy link
Contributor Author

fooman commented Sep 4, 2019

I currently am unable to find what failed in the test:
image

Happy to look into it once I know what failed.

@lenaorobei
Copy link
Contributor

@magento run Functional Tests build, WebAPI Tests build

@fooman fooman requested a review from paliarush as a code owner September 4, 2019 22:56
@fooman fooman removed the request for review from paliarush September 4, 2019 23:16
@fooman
Copy link
Contributor Author

fooman commented Sep 4, 2019

@lenaorobei found the culprit. I had a wrong fixture path reference that caused
Uncaught PHPUnit\Framework\Exception: Warning: require(/var/www/html/dev/tests/integration/testsuite/Magento/GraphQl/Catalog/_files/product_virtual.php): failed to open stream: No such file or directory which doesn't seem to get displayed in the report.

@magento-engcom-team
Copy link
Contributor

Hi @lenaorobei, thank you for the review.
ENGCOM-5795 has been created to process this Pull Request
✳️ @lenaorobei, could you please add one of the following labels to the Pull Request?

Label Description
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests

@ghost
Copy link

ghost commented Sep 10, 2019

Hi @fooman, 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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants