View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0006928 | OXID eShop (all versions) | 1.01. Products (product, categories, manufacturer, promotions etc.) | public | 2018-12-04 11:04 | 2022-03-30 11:45 |
Reporter | mediaworker | Assigned To | |||
Priority | normal | Severity | feature | Reproducibility | always |
Status | closed | Resolution | no change required | ||
Product Version | 6.0.2 | ||||
Summary | 0006928: Categorienames at the end of an url consisting only of digits are interpreted as page numbers | ||||
Description | Suppose to have a category structure like batterie/2032/ then clicking to this categorie the shop will make an redirect to batterie/ This happens in Controller/ArticleListController.php in function _checkRequestedPage(): ... if ($pageCount && (($pageCount - 1) < $currentPageNumber)) { \OxidEsales\Eshop\Core\Registry::getUtils()->redirect($this->getActiveCategory()->getLink(), false); } the interpretion of digits in an url only as page number happens in Core/SeoDecoder.php, function extractPageNumberFromSeoUrl($seoUrl). With this grep pattern all numbers become page numbers: preg_match('/(.*?)\/(\d+)\/(.*)/', $seoUrl, $matches) We think at this point the software has to check if the number ist an exsting category. | ||||
Tags | 404, Category, Redirect | ||||
Theme | Not defined | ||||
Browser | Not defined | ||||
PHP Version | Not defined | ||||
Database Version | Not defined | ||||
|
Hi as this is a specific use case I changed the entry to a feature request, so our PM can check it. Also because the shop is working as intended: Digits at the end of an URL are to interpreted as page numbers. In the meantime I suggest to give the category a prefix to identify them as a category and not as a page number like "https://example.com/batterie/c-665/" Kind regards, QA -MK |
|
Why is that a feature? Categories can be created with number names and create exactly this SEO address without error message. The call of this generated SEO address then no longer works. The definition of a specific case does not necessarily exclude the definition as a (rare) bug. By the way: In shop version 4.7, these SEO addresses also worked with page switches. DS |
|
The following change was sufficient for our purposes: Core/SeoDecoder.php (row 350 ss) private function extractPageNumberFromSeoUrl($seoUrl) { $oDb = \OxidEsales\Eshop\Core\DatabaseProvider::getDb(); $shopid = $this->getConfig()->getShopId(); $sSelect = "select oxobjectid from oxseo where oxseourl LIKE '%{$seoUrl}%' and oxshopid = '{$shopid}' LIMIT 1"; $sOxobjectid = $oDb->getOne($sSelect); $pageNumber = null; if (1 === preg_match('/(.*?)\/(\d+)\/(.*)/', $seoUrl, $matches) && !$sOxobjectid) { $seoUrl = $matches[1] . '/' . $matches[3]; $pageNumber = $this->convertSeoPageStringToActualPageNumber($matches[2]); } return [$seoUrl, $pageNumber]; } |
|
@rubbercut I guess your solution make problems in the rare case if subcategories are also numeric. Eg: /3000/12/ - this entry could be /category3000/category12/ or /category3000/page12/. I think the most easy way is add seo-prefix (why its called prefix?) if title ist numeric. https://github.com/OXID-eSales/oxideshop_ce/blob/461d53556be27f515692daaa9f687d5356f707a4/source/Application/Model/SeoEncoderCategory.php#L106 $sTitle = $this->_prepareTitle($oCat->oxcategories__oxtitle->value.($oCat->oxcategories__oxtitle->value === (string) intval($oCat->oxcategories__oxtitle->value) ? self::$_sSeparator.self::$_sPrefix : ''), false, $oCat->getLanguage()); Also, its possible make it as a modul - ticket is since 2018 and as "feature". |
|
https://github.com/OXID-eSales/oxideshop_ce/commit/9e2bf5226250e86d40889adecb0020ede72b71be > OXDEV-21 Use pgNr parameter for seo pagination > > Do not store paginated Seo urls in database any more. > Use url parameter pgNr for standard as well as seo urls. > Shop still can handle old style paginated seo urls. > Use 'noindex follow' for paginated pages. > Update tests. Since this version urls are generated like /name/?pgNr=%d And there is a test if exists an old seo-entry like /name/%d in case of numeric value - this entries could only exists for old installations. This test crashes and works only for deprecated db-entries. So finally @rubbercut solution is ok. As a quickfix, make the method protected, please |
|
In current versions all the methods involved here are already protected or public |
|
Does the current implementation fit your needs or would you need further tweaks? |