-
-
Notifications
You must be signed in to change notification settings - Fork 441
[FEAT] Redirect single search result to product page #4697
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
Conversation
this last version is a copy of MahoCommerce/maho#131 again without crediting the source |
I have waited for it ... @fballiano do you think i am not able to put that into core myself? I took some year old extensions and added it ... w/o moving observer-stuff to core. Did it came to your mind, to add that functionality at all? Nope. But it tooks you only a few hours to merge that - as every OM PR. Instead of making noise again, better think about to contribute back in some way. Leave a thanks - a PR - , "i have some improvements". That would be nice. (PS: i dont want to hear you cry when i port all of the no-prototype changes back - o/c /w leaving credits) Fab, you cannot answer here (to my issues/PRs) anymore. I have finally blocked you - same you did to me long time ago. Please head up to discussions if there is something to tell. |
Fab, You'd better made a suggestion before merge, but .... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add @copyright Maho
For the lazy, here's code from each PR: Maho: // Redirect to product if there's only one result
if (Mage::getStoreConfigFlag('catalog/search/redirect_to_product_if_one_result')) {
$searchResultBlock = Mage::app()->getLayout()->getBlock('search_result_list');
if ($searchResultBlock) {
/** @var Mage_CatalogSearch_Model_Resource_Fulltext_Collection $productCollection */
$productCollection = $searchResultBlock->getLoadedProductCollection();
if ($productCollection && $productCollection->getSize() === 1) {
$product = $productCollection->getFirstItem();
$this->_redirectUrl($product->getProductUrl());
}
}
} OM: // Redirect to product if there's only one result
if ($helper->isRedirectSingleSearchResult()) {
$searchResultBlock = $this->getLayout()->getBlock('search_result_list');
if ($searchResultBlock) {
/** @var Mage_CatalogSearch_Model_Resource_Fulltext_Collection $productCollection */
$productCollection = $searchResultBlock->getLoadedProductCollection();
if ($productCollection && $productCollection->getSize() === 1) {
$product = $productCollection->getFirstItem();
$this->_redirectUrl($product->getProductUrl());
}
}
} |
You took my code from this unreleased PR and opened MahoCommerce/maho#131 two days later. Only difference you added the observer code directly to the controller. No link to this PR, but adding your copyright? I'll rename the variables back later. |
I tried to extract some timestamp data from the github commits to have that in perspective:
I would say yes, @fballiano used parts of the code to make his own PR (without linking the original PR, like they usally do) |
Since I don't believe @fballiano can respond here, I will add a few more details: The original Maho commit did indeed have the link to this PR as Fabrizio always does. Then, Fabrizio and I discussed the introduction of the new observer interface and decided against it, so he rewrote the feature. We already have |
but isn't this the wrong issue? commit links to #4668 to disable it, not to add the Result redirect? |
You're right, I was mixed up because that was the commit that first introduced the However, my last point still stands, no? |
In my opinion, it should be discussed how much own work justifies an extra when i was comparing this f7e70cc with that MahoCommerce/maho@d65d8c7 the similarity was 79% (if changed the variable names, 83%) But that's not something I want to decide. |
@justinbeaty @empiricompany who copied who? |
Nope. Not for renaming some vars and move code from A to B. In german we have something like "Wertschöpfungstiefe" ... |
Add @copyright Maho |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Holding due to concerns regarding the potential need to include Maho copyright notice/credit. This is not a decision or indication of opinion, just putting it on hold while it is discussed internally.
Reverted back to observer ... Think its better to not put everything into core methods. If you dont need it - or want to change the behavior, disable the observer via XML and add your own. |
|
This PR has created disputes which in my opinion are not beneficial. Whoever is interested in having this feature is free to implement it separately. |
.... this code is 10y old .... https://packagist.org/packages/mse-sv3n/m1-single-search-result ... |
Description (*)
When product search has only one result, redirect to product page.
Manual testing scenarios (*)