Add new GraphQL query for single product retrieval #28316

serhiyzhovnir · 2020-05-21T12:27:47Z

Description (*)

This PR adds a new GraphQL endpoint for the products fetching by IDs.
The new GraphQL single product retrieval endpoint was requested at magento/catalog-storefront#12 issue.
After the conversations in this PR, there was a request to support the fetching of several products by IDs. Comment Link

Fixed Issues (if relevant)

Fixes New GraphQL endpoint for single product retrieval catalog-storefront#12: New GraphQL endpoint for single product retrieval

Manual testing scenarios (*)

Create a product
Use a similar query to retrieve the products data by IDs

{
  productsByID(ids: [1, 2]) {
    id
    name
    sku
    categories {
      id
      name
    }
    media_gallery {
      url
    }
  }
}

The result should be like on the following screenshot:

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)

…uct retrieval

m2-assistant · 2020-05-21T12:27:49Z

Hi @serhiyzhovnir. 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

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

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

Serializator · 2020-05-21T14:28:28Z

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

+    product (
+        id: Int @doc(description: "Id of the product.")
+    ): ProductInterface
+    @resolver(class: "Magento\\CatalogGraphQl\\Model\\Resolver\\ProductId") @doc(description: "The product query searches for product that match the criteria specified in the search and filter attributes.") @cache(cacheIdentity: "Magento\\CatalogGraphQl\\Model\\Resolver\\Product\\ProductIdIdentity")


"that match the criteria specified in the search and filter attributes" doesn't seem correct if the query only has an attribute to specify the id of the product.

"The product query searches for a product that matches the specified id." feels a bit more closer to what it should be, looking at the attribute.

Thank you for your suggestion, the description has been adjusted.

rogyar · 2020-05-21T16:11:59Z

app/code/Magento/CatalogGraphQl/Model/Resolver/Product/DataProvider/ProductDataProvider.php

+     * @param array $attributeCodes
+     * @return ProductInterface|Product
+     */
+    public function getProductById(int $productId, array $attributeCodes)


Not sure, maybe this part was agreed previously. But I would use return types hinting since we use strict_types declaration.

It wasn't agreed previously, I just missed the strict type here.
I've just added it.
Thanks!

okorshenko · 2020-05-21T19:15:55Z

@magento run Unit Tests

rogyar · 2020-05-28T07:39:01Z

app/code/Magento/CatalogGraphQl/Model/Resolver/ProductId.php

+        }
+
+        /** @var \Magento\Catalog\Model\Product $product */
+        $product = $this->productDataProvider->getProductById($productId, $fields);


Have you tried the following scenario?
Pass "-1" as the product ID in the GraphQL query 😎

I have not tried this scenario before.
I've just added the additional validation rule for throwing the exception if the ID argument is negative.

Thank you!

paliarush · 2020-06-10T14:58:25Z

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

@@ -10,6 +10,10 @@ type Query {
        sort: ProductAttributeSortInput @doc(description: "Specifies which attributes to sort on, and whether to return the results in ascending or descending order.")
    ): Products
    @resolver(class: "Magento\\CatalogGraphQl\\Model\\Resolver\\Products") @doc(description: "The products query searches for products that match the criteria specified in the search and filter attributes.") @cache(cacheIdentity: "Magento\\CatalogGraphQl\\Model\\Resolver\\Product\\Identity")
+    product (
+        id: Int @doc(description: "Id of the product.")


The argument should be required.

Also, @kokoc what if we change the type to ID, it seems to correspond our strategy. But may cause the need of type casting in strictly typed languages.

Suggested change

id: Int @doc(description: "Id of the product.")

id: ID! @doc(description: "Id of the product.")

akaplya · 2020-06-10T15:05:26Z

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

@@ -10,6 +10,10 @@ type Query {
        sort: ProductAttributeSortInput @doc(description: "Specifies which attributes to sort on, and whether to return the results in ascending or descending order.")
    ): Products
    @resolver(class: "Magento\\CatalogGraphQl\\Model\\Resolver\\Products") @doc(description: "The products query searches for products that match the criteria specified in the search and filter attributes.") @cache(cacheIdentity: "Magento\\CatalogGraphQl\\Model\\Resolver\\Product\\Identity")
+    product (


why this endpoint does not support multiple IDs?

Seconding this - input should be a non-nullable list type of IDs.

Fun fact: GraphQL spec allows clients to pass a single value when the input is a list type, and it will coerce to a list length of 1 with that value:

http://spec.graphql.org/June2018/#sec-Type-System.List

In other words, a list type will work fine for both use-cases, with some nice syntax sugar from GraphQL.

One other note: if we accept a list type as an input, we'll need a different name for this field that isn't singular.

nrkapoor · 2020-10-29T15:36:09Z

@magento run all tests

cpartica · 2020-11-04T22:49:15Z

hey guys, why not do this?
#28578
This is introducing a new query, and it's not good...

DrewML · 2020-11-17T02:25:29Z

@cpartica we had a discussion recently about this one offline. I think your preference was to go the route of the Node interface we discussed instead, right?

IOW, ProductInterface and other entities implement this (rough) interface

interface Node {
   uid: ID!
}

and then we add a new root field:

type Query {
   node(id: ID!): Node
}

Is that right?

cpartica · 2021-09-07T21:26:50Z

we actually want this: magento/architecture#477

#catalog-storefront/issue-12 Add new GraphQL endpoint for single prod…

e5ae941

…uct retrieval

ghost added the Progress: pending review label May 21, 2020

magento-engcom-team added Component: CatalogGraphQl Release Line: 2.4 Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner labels May 21, 2020

Serializator reviewed May 21, 2020

View reviewed changes

rogyar reviewed May 21, 2020

View reviewed changes

ghost assigned rogyar May 21, 2020

Sergiy Zhovnir added 2 commits May 22, 2020 09:48

Add strict type

5d21f23

Adjust description

4e17fd7

serhiyzhovnir requested a review from rogyar May 26, 2020 05:21

rogyar reviewed May 28, 2020

View reviewed changes

rogyar added Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests Award: category of expertise labels May 28, 2020

#catalog-storefront/issue-12 Added validation for negative product ID

7c02282

serhiyzhovnir requested a review from rogyar June 9, 2020 13:23

nrkapoor added the Project: GraphQL label Jun 10, 2020

nrkapoor added this to the 2.4.1 milestone Jun 10, 2020

paliarush reviewed Jun 10, 2020

View reviewed changes

ghost assigned paliarush Jun 10, 2020

akaplya suggested changes Jun 10, 2020

View reviewed changes

ghost assigned akaplya Jun 10, 2020

ghost added Progress: needs update and removed Progress: pending review labels Jun 10, 2020

kandy closed this Jun 10, 2020

nrkapoor unassigned DrewML Jul 14, 2020

nrkapoor removed request for akaplya, rogyar and DrewML July 14, 2020 14:32

nrkapoor modified the milestones: 2.4.1, 2.4.2 Aug 15, 2020

ghost added Progress: review and removed Progress: pending review labels Oct 2, 2020

nrkapoor requested review from rogyar, DrewML, akaplya and Serializator November 4, 2020 19:28

gabrieldagama added the Priority: P1 Once P0 defects have been fixed, a defect having this priority is the next candidate for fixing. label Mar 23, 2021

gabrieldagama assigned cpartica Mar 23, 2021

m2-community-project bot added Progress: pending review and removed Progress: review labels Dec 7, 2022

ishakhsuvarov removed the Project: GraphQL label Dec 8, 2022

m2-community-project bot added Progress: review and removed Progress: pending review labels Dec 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new GraphQL query for single product retrieval #28316

Add new GraphQL query for single product retrieval #28316

serhiyzhovnir commented May 21, 2020 •

edited

Loading

m2-assistant bot commented May 21, 2020

Serializator May 21, 2020

serhiyzhovnir May 22, 2020

rogyar May 21, 2020

serhiyzhovnir May 22, 2020

okorshenko commented May 21, 2020

rogyar May 28, 2020

serhiyzhovnir Jun 9, 2020

paliarush Jun 10, 2020

akaplya Jun 10, 2020

DrewML Jun 15, 2020 •

edited

Loading

DrewML Jun 15, 2020

nrkapoor commented Oct 29, 2020

cpartica commented Nov 4, 2020

DrewML commented Nov 17, 2020 •

edited

Loading

cpartica commented Sep 7, 2021

	id: Int @doc(description: "Id of the product.")
	id: ID! @doc(description: "Id of the product.")

Add new GraphQL query for single product retrieval #28316

Are you sure you want to change the base?

Add new GraphQL query for single product retrieval #28316

Conversation

serhiyzhovnir commented May 21, 2020 • edited Loading

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Contribution checklist (*)

m2-assistant bot commented May 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

okorshenko commented May 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DrewML Jun 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nrkapoor commented Oct 29, 2020

cpartica commented Nov 4, 2020

DrewML commented Nov 17, 2020 • edited Loading

cpartica commented Sep 7, 2021

serhiyzhovnir commented May 21, 2020 •

edited

Loading

DrewML Jun 15, 2020 •

edited

Loading

DrewML commented Nov 17, 2020 •

edited

Loading