magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.51k stars 9.31k forks source link

Commit for B2B-2404 causes issues with GraphQL Product Data Provider #37405

Open pmzandbergen opened 1 year ago

pmzandbergen commented 1 year ago

Summary

Starting from version 2.4.6 the Product Data Provider filters on the Visibility, this is not always desired (depending on the resolver). See commit: https://github.com/magento/magento2/commit/8681d30d2517a47baba01be70dc08031a0ab2671#diff-f94c66fe63952d9bf9efbd6d9cb69ddcb479df56dde42b9d9327a7bf4a9446abR137

File: https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/Deferred/Product.php#L138

Filtering (in this case on Visibility) is (in my opinion) something which should be handled by the Resolver, not the Data Provider.

Examples

There are multiple resolvers in 3rd party modules using the Product Data Provider. These modules expect the products to be returned, no matter their visibility. Please note that disabled or unassigned products is another story, those are expected to be filtered.

Proposed solution

Review if the filtering is desired and if so, move it to the Resolver.

Additional Information

For further commentary on this issue, this also impacts bundled products.

If you add a simple product that is visiblity - IN_CATALOG_ONLY that is not in any category on the site, then this will cause the product node of the BundleItemOption to return null since the underlying join is done against catalog_category_product_index_storeX which will not contain products if they are not assigned to any category.

This is not expected behavior because there may be a product that you want to sell as part of a bundle that isn't a part of any specific category since there's no need to list it anywhere.

Release note

No response

Triage and priority

m2-assistant[bot] commented 1 year ago

Hi @pmzandbergen. Thank you for your report. To speed up processing of this issue, make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, Add a comment to the issue:

joanhe commented 1 year ago

@pmzandbergen This change is done for fixing bugs causing by over fetching ignoring context. Do you see any use case that has conflict with this change? If so, please add to the issue report, so we can evaluate it.

ananth-iyer commented 1 year ago

Hi @joanhe on line 139, it is causing a major issue. I am getting grouped product and its child products by GraphQL. It returns grouped product details but child products are empty.

This conflicts with all GraphQL API and grouped products that add product node.

Tagging @pmzandbergen

Issue on Magento 2.4.6 and 2.4.6-p1 PHP 8.1

https://github.com/magento/magento2/blob/ca30c47787db932b9431e42226bb918e95c932d9/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/Deferred/Product.php#L134-L141

On line 139, if you change the parameter from true to false then it returns all child product properly.

query-25 07 2023_18 42 23_REC

25 07 2023_18 40 57_REC

Please check this again.

ananth-iyer commented 1 year ago

@engcom-Hotel tagging for visibility

ananth-iyer commented 1 year ago

@engcom-Dash

m2-assistant[bot] commented 1 year ago

Hi @engcom-November. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

engcom-November commented 1 year ago

Hello @pmzandbergen, Thank you for report and collaboration! Thank you @ananth-iyer for your input!

We have tried to reproduce this issue on 2.4-develop CE and B2B. We are getting expected results. Please take a look at the screenshot below: image We are able to get the child product properly from the below query.

{
  products(filter:{
    url_key:{
      eq:"a-bag1"
    }
  }) {
    items {
      url_key
      sku
         price_range {
             minimum_price {
                    final_price {
                      value
                      currency
                    }

              discount {
                percent_off
                amount_off
              }

              regular_price {
                value
                currency
              }

                  }

                  maximum_price {
                    final_price {
                      value
                      currency
                    }
                    regular_price {
                      value
                      currency
                    }
                    discount {
                      percent_off
                      amount_off
                    }
                  }
        }
   ... on GroupedProduct {
       items {
           position
           qty
           product {
               uid
               sku
               name
               only_x_left_in_stock
               price_range {
                   maximum_price {
                       final_price {
                           currency
                           value
                       }
                       regular_price {
                           currency
                           value
                       }
                   }
                   minimum_price {
                       final_price {
                           currency
                           value
                       }
                       regular_price {
                           currency
                           value
                       }
                   }
               }
           }
       }
   }

    }
  }
}

Please let us know if we are missing something.

Thank you.

pmzandbergen commented 1 year ago

We have tried to reproduce this issue on 2.4-develop CE and B2B. We are getting expected results.

That is correct, on a native Magento installation you will get the expected results. Please see the original description: "Filtering (in this case on Visibility) is (in my opinion) something which should be handled by the Resolver, not the Data Provider."

Its more a discussion about resolvers VS providers. Changing the behavior of the provider affected some 3rd party modules, since they didn't expect the provider to apply additional filters on its own. Isn't that something which should be handled by the resolver?

engcom-November commented 1 year ago

Hello @pmzandbergen,

Thank you for your quick response!

As the issue is not present in the core-magento library, but some 3rd party modules, please let us know in which modules you are getting the issue, this will help us to dig deep into the issue.

Thank you.

pmzandbergen commented 1 year ago

@engcom-November please have a look at the comment from @ananth-iyer: https://github.com/magento/magento2/issues/37405#issuecomment-1649827128

It seems this change does cause issues in the Magento core (did you test it with the child products being invisible but enabled?). Even if it didn't the original issue shouldn't be fixed by changing the Data Provider in my opinion.

With this change its the Data Provider applying filters, thats not its role (except for, maybe, filtering disabled products).

engcom-November commented 9 months ago

Hello @pmzandbergen,

Thank you for the response, and we agree with your point https://github.com/magento/magento2/issues/37405#issuecomment-1698600535.

Resolver should add filters and not data provider, but as we can see here data provider is applying the filter. Hence this issue can be confirmed.

Thank you.

github-jira-sync-bot commented 9 months ago

:white_check_mark: Jira issue https://jira.corp.adobe.com/browse/AC-10849 is successfully created for this GitHub issue.

m2-assistant[bot] commented 9 months ago

:white_check_mark: Confirmed by @engcom-November. Thank you for verifying the issue.
Issue Available: @engcom-November, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

damienwebdev commented 4 months ago

For further commentary on this issue, this also impacts bundled products.

If you add a simple product that is visiblity - IN_CATALOG_ONLY that is not in any category on the site, then this will cause the product node of the BundleItemOption to return null since the underlying join is done against catalog_category_product_index_storeX which will not contain products if they are not assigned to any category.

This is not expected behavior because there may be a product that you want to sell as part of a bundle that isn't a part of any specific category since there's no need to list it anywhere.

engcom-Hotel commented 4 months ago

Thanks @pmzandbergen for more information on this. We have added the same in Additional Information of the main description.