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 _addUrlRewrite() ignoring collection store scope. #510

Merged
merged 1 commit into from
Aug 21, 2020

Conversation

edannenberg
Copy link
Contributor

@edannenberg edannenberg commented Aug 25, 2018

While Mage_Catalog_Model_Resource_Product_Collection::addUrlRewrite() respects any set storeId on the collection it's helper that does the actual work is hard-coded to the current active store. So the following will not work as expected when, for example, called from a script:

Mage::getResourceModel('catalog/product_collection')
    ->setStore($someStoreId)
    ->addUrlRewrite();

Mage::getSingleton('core/app_emulation')->startEnvironmentEmulation($someStoreId); could be used to work around the problem, but this looks like a bug to me plus it's fairly annoying to debug if you are not aware.

The fix (ab)uses the fact that Mage::app()->getStore($this->getStoreId()) will fall back to the current active store if there is no storeId set on the current collection, so there should be no BC issues. Happy to amend if something less cute is prefered, i.e.:

$storeId = $this->getStoreId();
if (!$storeId)
    $storeId = Mage::app()->getStore()->getId();

$select = $this->_factory->getProductUrlRewriteHelper()
    ->getTableSelect($productIds, $this->_urlRewriteCategory, $storeId);

While Mage_Catalog_Model_Resource_Product_Collection::addUrlRewrite()
does respect any set storeId on the collection it's helper that does
the actual work is hard-coded to the current active store.
@tbaden tbaden added the review needed Problem should be verified label Jun 1, 2019
Copy link
Contributor

@sreichel sreichel left a comment

Choose a reason for hiding this comment

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

Tested 👍

@sreichel sreichel added Enough Reviews and removed review needed Problem should be verified labels Aug 21, 2020
@colinmollenhour colinmollenhour changed the base branch from 1.9.3.x to 20.0 August 21, 2020 06:13
@colinmollenhour colinmollenhour merged commit 8627073 into OpenMage:20.0 Aug 21, 2020
colinmollenhour pushed a commit that referenced this pull request Aug 21, 2020
While Mage_Catalog_Model_Resource_Product_Collection::addUrlRewrite()
does respect any set storeId on the collection it's helper that does
the actual work is hard-coded to the current active store.
@sreichel sreichel added this to the Release 19.4.7 / 20.0.3 milestone Aug 21, 2020
edannenberg added a commit to edannenberg/magento-lts that referenced this pull request Aug 24, 2020
While Mage_Catalog_Model_Resource_Product_Collection::addUrlRewrite()
does respect any set storeId on the collection it's helper that does
the actual work is hard-coded to the current active store.
sreichel added a commit that referenced this pull request Mar 11, 2021
#1302)

* bump version

* Fix _addUrlRewrite() ignoring collection store scope. (#510)

While Mage_Catalog_Model_Resource_Product_Collection::addUrlRewrite()
does respect any set storeId on the collection it's helper that does
the actual work is hard-coded to the current active store.

* Add doc comments to image related classes (#1146)

* Prevent duplicate entry when updating salesrule_coupon_usage (#1117)

* remove $timesUsed > 0 check to prevent duplicate entry

* Prevent $timesUsed from going less than 0

* Fix a bug where media upload via API are not possible anymore: #1178

* revert unwanted changes

* Update lib/Varien/Io/File.php

Co-authored-by: Flyingmana <flyingmana@googlemail.com>
Co-authored-by: Colin Mollenhour <colin@mollenhour.com>
Co-authored-by: Erik Dannenberg <ed@reshape.de>
Co-authored-by: Tymoteusz Motylewski <t.motylewski@gmail.com>
Co-authored-by: Dean Williams <me@deanwilliams.org>
Co-authored-by: sv3n <github-sr@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants