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

Api2: Fixes getProductUrl #4511

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Api2: Fixes getProductUrl #4511

wants to merge 1 commit into from

Conversation

Hanmac
Copy link
Contributor

@Hanmac Hanmac commented Jan 27, 2025

Description (*)

This fixes REST API2 Product url to use the store parameter.

Related Pull Requests

  • see OpenMage/magento-lts#<issue_number>

Fixed Issues (if relevant)

Manual testing scenarios (*)

  1. ...
  2. ...

Questions or comments

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)

@Hanmac Hanmac requested review from theroch and sreichel January 27, 2025 14:43
@github-actions github-actions bot added the Component: Catalog Relates to Mage_Catalog label Jan 27, 2025
Copy link
Contributor

github-actions bot commented Jan 27, 2025

Test Results

541 tests  ±0   533 ✅ ±0   3s ⏱️ ±0s
145 suites ±0     8 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit bce2f88. ± Comparison against base commit fc9a8ca.

♻️ This comment has been updated with latest results.

@sreichel
Copy link
Contributor

Mhhh, cant reproduce atm ... my json-response has the correct frontend-url.

@Hanmac
Copy link
Contributor Author

Hanmac commented Jan 30, 2025

Use multiple stores with web/url/use_store enabled
Then the Guest Api doesn't use the Frontend Route
So it can't decide what the current store should be

then this line:

return Mage::getModel('catalog/product')->load($product)->getProductUrl();

does load the product without store id set

@sreichel
Copy link
Contributor

sreichel commented Jan 30, 2025

The fix is valid, but i still have no admin-url in response - its always a frontend-url.

  • Use multiple stores with web/url/use_store enabled (thats default in sample data)
  • enabled "add store code to url"

-> returns default store url (not admin)

After this fix i get the correct frontend-url.

@Hanmac
Copy link
Contributor Author

Hanmac commented Jan 30, 2025

What exactly is your REST url?
I tried https://myserver/api/rest/products/sku?store=5
when doing this, my current Store is 0

that's why i get admin as store name

@Hanmac
Copy link
Contributor Author

Hanmac commented Jan 30, 2025

@sreichel you can try to debug it too, but for me, when getUrlModel is called, the StoreId of the Product is still null,
and then app says the current store is admin

public function getStoreId()
{
if ($this->hasData('store_id')) {
return (int) $this->getData('store_id');
}
return Mage::app()->getStore()->getId();
}

@sreichel
Copy link
Contributor

Checked and everything is fine.

Please debug Mage_Api2_Model_Resource::_getStore().

@Hanmac
Copy link
Contributor Author

Hanmac commented Jan 31, 2025

Checked and everything is fine.

Please debug Mage_Api2_Model_Resource::_getStore().

like is said before, it doesn't matter. While the _getSore method is called and loads the store for the given parameter, it doesn't alter the default store.

So when this line later happens, it will create a new Product object with the store parameter set to null, and then it will fall back to the current store (which is admin for the api)

$productData['url'] = $productHelper->getProductUrl($product->getId());

@sreichel
Copy link
Contributor

The fix is valid, but i still have no admin-url in response - its always a frontend-url.

You got me wrong. Your fix works - but for me the behavoir was different before the fix. I had no admin url,

@Hanmac
Copy link
Contributor Author

Hanmac commented Jan 31, 2025

@sreichel makes me wonder what does this $productHelper->getProductUrl does different for you than for me, that you get a different store.

Also for later, we might need to think about deprecating the getProductUrl from the Product Helper because it only causes problems in multi store if using product_id?

The only other place that uses Mage_Catalog_Helper_Product::getProductUrl is Mage_Catalog_Block_Seo_Sitemap_Product,
and the Sitemap uses the Product Item and not the Id

sreichel added a commit to sreichel/magento-lts that referenced this pull request Feb 1, 2025
@sreichel sreichel added the bug label Feb 2, 2025
@sreichel sreichel added this to the 20.13.0 milestone Feb 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Component: Catalog Relates to Mage_Catalog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Api2] [Catalog] Product URL returned by REST Api is wrong store
2 participants