GraphQl Product visibility for search results #36623

Vasudev-22 · 2022-12-16T04:59:17Z

Description (*)

Product Visibility handling for search feature ,

Related Pull Requests

Fixes for #36591

Fixed Issues (if relevant)

Fixes GraphQL product visibility isn't handled correctly for search results #36591

Manual testing scenarios (*)

SKU API-TEST-1, has assigned a visibility as Catalog, so it will visible only with checking category condition.
Result won't be appear if we search this sku.
Result will appear by checking the combination of assigned category and search key word

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)
README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
All automated tests passed successfully (all builds are green)

m2-assistant · 2022-12-16T04:59:19Z

Hi @Vasudev-22. Thank you for your contribution
Here are 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
Semantic Version Checker

You can find more information about the builds here

ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

Vasudev-22 · 2022-12-16T08:01:41Z

@magento run all tests

magento-automated-testing · 2022-12-16T08:01:49Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

Vasudev-22 · 2023-01-05T07:36:07Z

@magento run all unit tests

magento-automated-testing · 2023-01-05T07:36:12Z

Failed to run the builds. Please try to re-run them later.

Vasudev-22 · 2023-01-05T07:38:25Z

@magento run all tests

magento-automated-testing · 2023-01-05T07:38:32Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

paales · 2023-01-06T08:55:06Z

app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php

@@ -122,7 +122,7 @@ public function build(array $args, bool $includeAggregation): SearchCriteriaInte
        }

        $this->addEntityIdSort($searchCriteria);
-        $this->addVisibilityFilter($searchCriteria, $isSearch, !empty($args['filter']));


I think this logic doesn't make sense:

magento2/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php

Line 142 in 688a0b4

if ($isFilter && $isSearch) {

If a search query is present it should always use getVisibleInSearchIds and if it doesn't contain a search query it should use getVisibleInCatalogIds. I don't think it makes sense to not filter anything. We are either in 'search mode' or in 'catalog mode' not in both?

@paales , Which you mentioned this logic if ($isFilter && $isSearch) { , is not related to this PR changes, it was available from core standards.
This PR is related to filter category conditions for catalog visibility combination $this->addVisibilityFilter($searchCriteria, $isSearch, !empty($args['filter']['category_id']));

I don't think selecting by category_id is a valid condition:

When searching for a keyword but filtering on a category_id it should still have visibility search? Or you could argue that each item should be in Search AND Catalog, but not neither?

You only check on category_id but we've also got category_uid so, that should probably be changed.

@paales I have checked in the codebase this build method has been called from the below file

magento2/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/Query/Search.php

Line 185 in cfaeeae

private function buildSearchCriteria(array $args, ResolveInfo $info): SearchCriteriaInterface

And if from the query, someone is passing category_uid then it is converted in category_id through the below function call:

magento2/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/Query/Search.php

Line 190 in cfaeeae

$processedArgs = $this->argsSelection->process((string) $fieldName, $args);

engcom-Lima · 2023-06-07T11:23:39Z

@magento run all tests

magento-automated-testing · 2023-06-07T11:23:48Z