Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix store id filter in catalogProductList SOAP method #2243

Conversation

elidrissidev
Copy link
Member

@elidrissidev elidrissidev commented Jun 21, 2022

Description (*)

_applyProductLimitations (called by addStoreFilter) in product collection does not apply store_id filter if category_id and visibility filters are not set, so changing it with setStoreId which works fine and is also used in REST Api.

$filters = $this->_productLimitationFilters;
if (!isset($filters['category_id']) && !isset($filters['visibility'])) {
return $this;
}

Fixed Issues (if relevant)

  1. Fixes Can't get products by store view from catalogProductList API #1576

Manual testing scenarios (*)

You can use the following snippet assuming product name is different from the one in default store:

$client = new SoapClient('http://<magentohost>/index.php/api/v2_soap/?wsdl');
$session = $client->login('<username>', '<password>');

$product = $client->catalogProductList($session, [
    'complex_filter' => [
        [
            'key' => 'entity_id', 
            'value' => ['key' => 'in', 'value' => '<product_id>']
        ]
    ]
], <store_id>);
echo $product[0]->name . PHP_EOL;

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

`addStoreFilter` in product collection does not apply store id filter if current store is admin, so changing it with `setStoreId` which works fine and it also used in REST Api.
@github-actions github-actions bot added the Component: Catalog Relates to Mage_Catalog label Jun 21, 2022
@fballiano
Copy link
Contributor

I was checkin a little bit and run a snippet:

        $collection = Mage::getModel('catalog/product')->getCollection()
            ->addStoreFilter(3)
            ->addAttributeToSelect('name');
        echo $collection->getSelect();die();

generates this query:

SELECT `e`.* FROM `catalog_product_entity` AS `e`
 INNER JOIN `catalog_product_website` AS `product_website` ON product_website.product_id = e.entity_id AND product_website.website_id = '2'

while this code:

        $collection = Mage::getModel('catalog/product')->getCollection()
            ->setStoreId(3)
            ->addAttributeToSelect('name');
        echo $collection->getSelect();die();

generates this query:
SELECT e.* FROM catalog_product_entityASe``

So I'm not sure shat's going on here...

@elidrissidev
Copy link
Member Author

Woah, I have no idea either... I'll have to look into that when I get the time.

@elidrissidev
Copy link
Member Author

@fballiano I did some digging to find what's going on. So the query that loads attributes is completely separate from the one you're getting from $collection->getSelect() and is executed only after loading products from the main table, so you'll only be able to print it by adding an echo $select . "\n"; to app/code/core/Mage/Eav/Model/Entity/Collection/Abstract.php (line 1133).

Here's what I get with original code (addStoreFilter):

SELECT `catalog_product_entity_varchar`.`entity_id`, `catalog_product_entity_varchar`.`attribute_id`, `catalog_product_entity_varchar`.`value` 
FROM `catalog_product_entity_varchar` 
WHERE (entity_type_id =4) AND (entity_id IN (231...905)) AND (attribute_id IN (71)) AND (store_id = 0)

Here's with my change (setStoreId):

SELECT `t_d`.`entity_id`, `t_d`.`attribute_id`, `t_d`.`value` AS `default_value`, `t_s`.`value` AS `store_value`, IF(t_s.value_id IS NULL, t_d.value, t_s.value) AS `value` 
FROM `catalog_product_entity_varchar` AS `t_d` 
LEFT JOIN `catalog_product_entity_varchar` AS `t_s` ON t_s.attribute_id = t_d.attribute_id AND t_s.entity_id = t_d.entity_id AND t_s.store_id = 2 
WHERE (t_d.entity_type_id = 4) AND (t_d.entity_id IN (231...905)) AND (t_d.attribute_id IN (71)) AND (t_d.store_id = 0)

Notice the LEFT JOIN with the extra store condition.

@tmotyl
Copy link
Contributor

tmotyl commented Jul 4, 2022

@elidrissidev are you saying your change is adding a JOIN? this would have a performance impact. arent there a way to fix it without additional join?

@elidrissidev
Copy link
Member Author

@tmotyl That's how it works when filtering with setStoreId, because we still want to get attribute value from admin store if the provided store doesn't have one. I can't really find an alternate way as the addStoreFilter returns like mentioned in OP.

@tmotyl
Copy link
Contributor

tmotyl commented Jul 4, 2022

@elidrissidev understood

@fballiano fballiano merged commit 9343518 into OpenMage:1.9.4.x Jul 7, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2022

Unit Test Results

1 files  ±0  1 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
7 runs  ±0  5 ✔️ ±0  2 💤 ±0  0 ❌ ±0 

Results for commit 9343518. ± Comparison against base commit 2740724.

@elidrissidev elidrissidev deleted the fix/catalogProductList-storeid-filter branch July 7, 2022 13:03
elidrissidev added a commit to elidrissidev/magento-lts that referenced this pull request Jul 9, 2022
elidrissidev added a commit to elidrissidev/magento-lts that referenced this pull request Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Catalog Relates to Mage_Catalog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't get products by store view from catalogProductList API
6 participants