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

Absolute image paths for the ProductInterface #183

Merged

Conversation

eugene-shab
Copy link
Contributor

@eugene-shab eugene-shab commented Sep 14, 2018

Description

This PR adds full product image url, path info and image label.

It should looks like:

small_image { url, label }
thumbnail { url, label }
image { url, label }

Regarding issue #174

Contribution checklist

  • [x ] All automated tests passed successfully (all builds on Travis CI are green)

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Sep 14, 2018

CLA assistant check
All committers have signed the CLA.

@eugene-shab eugene-shab requested a review from naydav September 14, 2018 08:05
@eugene-shab eugene-shab self-assigned this Sep 14, 2018
@naydav
Copy link
Contributor

naydav commented Sep 17, 2018

Pls, check tests


CategoryTest testCategoryProducts History | REST EE | < 1 sec
-- | -- | --
Magento\GraphQl\Catalog\CategoryTest::testCategoryProducts Exception: GraphQL response contains errors: Field "image" of type "ProductImage" must have a sub selection. Field "small_image" of type "ProductImage" must have a sub selection. Field "thumbnail" of type "ProductImage" must have a sub selection.   /opt/bamboo/builds/MCCE23-AT1255-RE/build-1/dev/tests/api-functional/framework/Magento/TestFramework/TestCase/GraphQl/Client.php:105 /opt/bamboo/builds/MCCE23-AT1255-RE/build-1/dev/tests/api-functional/framework/Magento/TestFramework/TestCase/GraphQl/Client.php:74 (2 more lines...)
  | Failed | CategoryTest testCategoryProducts History | SOAP EE | < 1 sec
Magento\GraphQl\Catalog\CategoryTest::testCategoryProducts Exception: GraphQL response contains errors: Field "image" of type "ProductImage" must have a sub selection. Field "small_image" of type "ProductImage" must have a sub selection. Field "thumbnail" of type "ProductImage" must have a sub selection.   /opt/bamboo/builds/MCCE23-AT1255-SOE/build-1/dev/tests/api-functional/framework/Magento/TestFramework/TestCase/GraphQl/Client.php:105 /opt/bamboo/builds/MCCE23-AT1255-SOE/build-1/dev/tests/api-functional/framework/Magento/TestFramework/TestCase/GraphQl/Client.php:74 (2 more lines...)
  | Failed | ProductViewTest testQueryAllFieldsSimpleProduct History | REST EE | < 1 sec
Magento\GraphQl\Catalog\ProductViewTest::testQueryAllFieldsSimpleProduct Exception: GraphQL response contains errors: Field "image" of type "ProductImage" must have a sub selection. Field "small_image" of type "ProductImage" must have a sub selection. Field "thumbnail" of type "ProductImage" must have a sub selection.   /opt/bamboo/builds/MCCE23-AT1255-RE/build-1/dev/tests/api-functional/framework/Magento/TestFramework/TestCase/GraphQl/Client.php:105 /opt/bamboo/builds/MCCE23-AT1255-RE/build-1/dev/tests/api-functional/framework/Magento/TestFramework/TestCase/GraphQl/Client.php:74

eugene-shab and others added 5 commits September 20, 2018 11:57
…mage_paths_for_products

# Conflicts:
#	app/code/Magento/CatalogGraphQl/etc/schema.graphqls
#	dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryTest.php
#	dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductViewTest.php
/**
* Returns product's image path
*/
class Path implements ResolverInterface
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to have this resolver, its logic can be moved to the main image resolver to gain better performance.

Copy link
Contributor

Choose a reason for hiding this comment

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

Removed

Valeriy Nayda added 3 commits November 6, 2018 13:03
…mage_paths_for_products

# Conflicts:
#	dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductViewTest.php
@magento-engcom-team magento-engcom-team modified the milestones: Release: 2.3.0, Release: 2.3.1 Nov 6, 2018
Valeriy Nayda added 2 commits November 6, 2018 16:03
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